Commit 46476a18 authored by Adam Hegyi's avatar Adam Hegyi

Add total time to merge to MR connection

This change adds the total_time_to_merge aggregation to merge request
connections so later on mean time to merge could be calculated.
parent 5a9779ae
# 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
...@@ -4,7 +4,7 @@ module Types ...@@ -4,7 +4,7 @@ module Types
class MergeRequestType < BaseObject class MergeRequestType < BaseObject
graphql_name 'MergeRequest' graphql_name 'MergeRequest'
connection_type_class(Types::CountableConnectionType) connection_type_class(Types::MergeRequestConnectionType)
implements(Types::Notes::NoteableType) implements(Types::Notes::NoteableType)
implements(Types::CurrentUserTodos) implements(Types::CurrentUserTodos)
......
...@@ -339,6 +339,13 @@ class MergeRequest < ApplicationRecord ...@@ -339,6 +339,13 @@ class MergeRequest < ApplicationRecord
) )
end 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? after_save :keep_around_commit, unless: :importing?
alias_attribute :project, :target_project alias_attribute :project, :target_project
......
...@@ -10,6 +10,11 @@ class MergeRequest::Metrics < ApplicationRecord ...@@ -10,6 +10,11 @@ class MergeRequest::Metrics < ApplicationRecord
scope :merged_after, ->(date) { where(arel_table[:merged_at].gteq(date)) } scope :merged_after, ->(date) { where(arel_table[:merged_at].gteq(date)) }
scope :merged_before, ->(date) { where(arel_table[:merged_at].lteq(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 private
......
---
title: Add merge requests total time to merge field to the GraphQL API
merge_request: 46040
author:
type: added
# 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
bde71afbe34006eedbd97ac457df31b247fc89a572ca8900c60b16c4d6a8ef93
\ No newline at end of file
...@@ -21518,6 +21518,8 @@ CREATE UNIQUE INDEX index_mr_context_commits_on_merge_request_id_and_sha ON merg ...@@ -21518,6 +21518,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_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 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); CREATE INDEX index_namespace_onboarding_actions_on_namespace_id ON namespace_onboarding_actions USING btree (namespace_id);
......
...@@ -13010,6 +13010,11 @@ type MergeRequestConnection { ...@@ -13010,6 +13010,11 @@ type MergeRequestConnection {
Information to aid in pagination. Information to aid in pagination.
""" """
pageInfo: PageInfo! pageInfo: PageInfo!
"""
Total sum of time to merge, in seconds, for the collection of merge requests
"""
totalTimeToMerge: Float
} }
""" """
......
...@@ -35868,6 +35868,20 @@ ...@@ -35868,6 +35868,20 @@
}, },
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "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, "inputFields": null,
...@@ -52,7 +52,7 @@ FactoryBot.define do ...@@ -52,7 +52,7 @@ FactoryBot.define do
after(:build) do |merge_request, evaluator| after(:build) do |merge_request, evaluator|
metrics = merge_request.build_metrics 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.merged_by = evaluator.merged_by
metrics.pipeline = create(:ci_empty_pipeline) metrics.pipeline = create(:ci_empty_pipeline)
end end
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe GitlabSchema.types['MergeRequestConnection'] do RSpec.describe GitlabSchema.types['PipelineConnection'] do
it 'has the expected fields' do it 'has the expected fields' do
expected_fields = %i[count page_info edges nodes] expected_fields = %i[count page_info edges nodes]
......
# 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
...@@ -500,6 +500,77 @@ RSpec.describe MergeRequest, factory_default: :keep do ...@@ -500,6 +500,77 @@ RSpec.describe MergeRequest, factory_default: :keep do
end end
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 describe '#target_branch_sha' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment