Merge branch 'add-mean-time-to-merge-be' into 'master'

Add merge requests total time to merge field on project

See merge request gitlab-org/gitlab!46040

app/graphql/types/merge_request_connection_type.rb

+# frozen_string_literal: true
+
+module Types
+  # rubocop: disable Graphql/AuthorizeTypes
+  class MergeRequestConnectionType < Types::CountableConnectionType
+    field :total_time_to_merge, GraphQL::FLOAT_TYPE, null: true,
+          description: 'Total sum of time to merge, in seconds, for the collection of merge requests'
+
+    # rubocop: disable CodeReuse/ActiveRecord
+    def total_time_to_merge
+      object.items.reorder(nil).total_time_to_merge
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+  end
+end

app/graphql/types/merge_request_type.rb

@@ -4,7 +4,7 @@ module Types
   class MergeRequestType < BaseObject
     graphql_name 'MergeRequest'
 
-    connection_type_class(Types::CountableConnectionType)
+    connection_type_class(Types::MergeRequestConnectionType)
 
     implements(Types::Notes::NoteableType)
     implements(Types::CurrentUserTodos)

app/models/merge_request.rb

@@ -339,6 +339,13 @@ class MergeRequest < ApplicationRecord
     )
   end
 
+  def self.total_time_to_merge
+    join_metrics
+      .merge(MergeRequest::Metrics.with_valid_time_to_merge)
+      .pluck(MergeRequest::Metrics.time_to_merge_expression)
+      .first
+  end
+
   after_save :keep_around_commit, unless: :importing?
 
   alias_attribute :project, :target_project

app/models/merge_request/metrics.rb

@@ -10,6 +10,11 @@ class MergeRequest::Metrics < ApplicationRecord
 
   scope :merged_after, ->(date) { where(arel_table[:merged_at].gteq(date)) }
   scope :merged_before, ->(date) { where(arel_table[:merged_at].lteq(date)) }
+  scope :with_valid_time_to_merge, -> { where(arel_table[:merged_at].gt(arel_table[:created_at])) }
+
+  def self.time_to_merge_expression
+    Arel.sql('EXTRACT(epoch FROM SUM(AGE(merge_request_metrics.merged_at, merge_request_metrics.created_at)))')
+  end
 
   private
 

changelogs/unreleased/add-mean-time-to-merge-be.yml

+---
+title: Add merge requests total time to merge field to the GraphQL API
+merge_request: 46040
+author:
+type: added

db/migrate/20201104142036_add_index_to_merge_request_metrics_target_project_id.rb

+# frozen_string_literal: true
+
+class AddIndexToMergeRequestMetricsTargetProjectId < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_mr_metrics_on_target_project_id_merged_at_time_to_merge'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :merge_request_metrics, [:target_project_id, :merged_at, :created_at], where: 'merged_at > created_at', name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name(:merge_request_metrics, INDEX_NAME)
+  end
+end

db/schema_migrations/20201104142036

+bde71afbe34006eedbd97ac457df31b247fc89a572ca8900c60b16c4d6a8ef93
\ No newline at end of file

db/structure.sql

@@ -21631,6 +21631,8 @@ CREATE UNIQUE INDEX index_mr_context_commits_on_merge_request_id_and_sha ON merg
 
 CREATE INDEX index_mr_metrics_on_target_project_id_merged_at_nulls_last ON merge_request_metrics USING btree (target_project_id, merged_at DESC NULLS LAST, id DESC);
 
+CREATE INDEX index_mr_metrics_on_target_project_id_merged_at_time_to_merge ON merge_request_metrics USING btree (target_project_id, merged_at, created_at) WHERE (merged_at > created_at);
+
 CREATE UNIQUE INDEX index_namespace_aggregation_schedules_on_namespace_id ON namespace_aggregation_schedules USING btree (namespace_id);
 
 CREATE INDEX index_namespace_onboarding_actions_on_namespace_id ON namespace_onboarding_actions USING btree (namespace_id);

doc/api/graphql/reference/gitlab_schema.graphql

@@ -13050,6 +13050,11 @@ type MergeRequestConnection {
   Information to aid in pagination.
   """
   pageInfo: PageInfo!
+
+  """
+  Total sum of time to merge, in seconds, for the collection of merge requests
+  """
+  totalTimeToMerge: Float
 }
 
 """

doc/api/graphql/reference/gitlab_schema.json

@@ -36011,6 +36011,20 @@
               },
               "isDeprecated": false,
               "deprecationReason": null
+            },
+            {
+              "name": "totalTimeToMerge",
+              "description": "Total sum of time to merge, in seconds, for the collection of merge requests",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "SCALAR",
+                "name": "Float",
+                "ofType": null
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
             }
           ],
           "inputFields": null,

spec/factories/merge_requests.rb

@@ -52,7 +52,7 @@ FactoryBot.define do
 
       after(:build) do |merge_request, evaluator|
         metrics = merge_request.build_metrics
-        metrics.merged_at = 1.week.ago
+        metrics.merged_at = 1.week.from_now
         metrics.merged_by = evaluator.merged_by
         metrics.pipeline = create(:ci_empty_pipeline)
       end

spec/graphql/types/countable_connection_type_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe GitlabSchema.types['MergeRequestConnection'] do
+RSpec.describe GitlabSchema.types['PipelineConnection'] do
   it 'has the expected fields' do
     expected_fields = %i[count page_info edges nodes]
 

spec/graphql/types/merge_request_connection_type_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe GitlabSchema.types['MergeRequestConnection'] do
+  it 'has the expected fields' do
+    expected_fields = %i[count totalTimeToMerge page_info edges nodes]
+
+    expect(described_class).to have_graphql_fields(*expected_fields)
+  end
+end

spec/models/merge_request_spec.rb

@@ -500,6 +500,77 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
+  describe 'time to merge calculations' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:project) { create(:project) }
+
+    let!(:mr1) do
+      create(
+        :merge_request,
+        :with_merged_metrics,
+        source_project: project,
+        target_project: project
+      )
+    end
+
+    let!(:mr2) do
+      create(
+        :merge_request,
+        :with_merged_metrics,
+        source_project: project,
+        target_project: project
+      )
+    end
+
+    let!(:mr3) do
+      create(
+        :merge_request,
+        :with_merged_metrics,
+        source_project: project,
+        target_project: project
+      )
+    end
+
+    let!(:unmerged_mr) do
+      create(
+        :merge_request,
+        source_project: project,
+        target_project: project
+      )
+    end
+
+    before do
+      project.add_user(user, :developer)
+    end
+
+    describe '.total_time_to_merge' do
+      it 'returns the sum of the time to merge for all merged MRs' do
+        mrs = project.merge_requests
+
+        expect(mrs.total_time_to_merge).to be_within(1).of(expected_total_time(mrs))
+      end
+
+      context 'when merged_at is earlier than created_at' do
+        before do
+          mr1.metrics.update!(merged_at: mr1.metrics.created_at - 1.week)
+        end
+
+        it 'returns nil' do
+          mrs = project.merge_requests.where(id: mr1.id)
+
+          expect(mrs.total_time_to_merge).to be_nil
+        end
+      end
+
+      def expected_total_time(mrs)
+        mrs = mrs.reject { |mr| mr.merged_at.nil? }
+        mrs.reduce(0.0) do |sum, mr|
+          (mr.merged_at - mr.created_at) + sum
+        end
+      end
+    end
+  end
+
   describe '#target_branch_sha' do
     let(:project) { create(:project, :repository) }