diff --git a/app/finders/concerns/merged_at_filter.rb b/app/finders/concerns/merged_at_filter.rb new file mode 100644 index 0000000000000000000000000000000000000000..d908e9e1724f2fe7d3dd91eb9adfdcfdc8e29c6f --- /dev/null +++ b/app/finders/concerns/merged_at_filter.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module MergedAtFilter + private + + # rubocop: disable CodeReuse/ActiveRecord + def by_merged_at(items) + return items unless merged_after || merged_before + + mr_metrics_scope = MergeRequest::Metrics + mr_metrics_scope = mr_metrics_scope.merged_after(merged_after) if merged_after.present? + mr_metrics_scope = mr_metrics_scope.merged_before(merged_before) if merged_before.present? + + items.joins(:metrics).merge(mr_metrics_scope) + end + # rubocop: enable CodeReuse/ActiveRecord + + def merged_after + params[:merged_after] + end + + def merged_before + params[:merged_before] + end +end diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index ceb015ad41facf86919622efc17898ff0e46f56e..b70d0b7a06a1d9dae02309ecdc35187b3c9c20e0 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -30,8 +30,10 @@ # updated_before: datetime # class MergeRequestsFinder < IssuableFinder + include MergedAtFilter + def self.scalar_params - @scalar_params ||= super + [:wip, :draft, :target_branch] + @scalar_params ||= super + [:wip, :draft, :target_branch, :merged_after, :merged_before] end def klass @@ -44,6 +46,7 @@ def filter_items(_items) items = by_source_branch(items) items = by_draft(items) items = by_target_branch(items) + items = by_merged_at(items) by_source_project_id(items) end diff --git a/app/graphql/resolvers/merge_requests_resolver.rb b/app/graphql/resolvers/merge_requests_resolver.rb index 3aa52341eec69c822a62457d64df6ceef676a78a..d15a1ede6fec801aae2c4d8e92aed3c0807c5343 100644 --- a/app/graphql/resolvers/merge_requests_resolver.rb +++ b/app/graphql/resolvers/merge_requests_resolver.rb @@ -28,6 +28,12 @@ class MergeRequestsResolver < BaseResolver required: false, as: :label_name, description: 'Array of label names. All resolved merge requests will have all of these labels.' + argument :merged_after, Types::TimeType, + required: false, + description: 'Merge requests merged after this date' + argument :merged_before, Types::TimeType, + required: false, + description: 'Merge requests merged before this date' def self.single ::Resolvers::MergeRequestResolver diff --git a/app/graphql/types/issue_connection_type.rb b/app/graphql/types/issuable_connection_type.rb similarity index 76% rename from app/graphql/types/issue_connection_type.rb rename to app/graphql/types/issuable_connection_type.rb index beed392f01abe6a75fc6a61a62a37712e85eb1c2..a228b44f6242af335c0592c117d60c21ffe30811 100644 --- a/app/graphql/types/issue_connection_type.rb +++ b/app/graphql/types/issuable_connection_type.rb @@ -2,7 +2,7 @@ module Types # rubocop: disable Graphql/AuthorizeTypes - class IssueConnectionType < GraphQL::Types::Relay::BaseConnection + class IssuableConnectionType < GraphQL::Types::Relay::BaseConnection field :count, Integer, null: false, description: 'Total count of collection' diff --git a/app/graphql/types/issue_type.rb b/app/graphql/types/issue_type.rb index 1dab61f6fb5a6c09577992906a9e5d4ac3a8d7c1..a5059276e8435aea4ae1bec85bb289fb64420458 100644 --- a/app/graphql/types/issue_type.rb +++ b/app/graphql/types/issue_type.rb @@ -4,7 +4,7 @@ module Types class IssueType < BaseObject graphql_name 'Issue' - connection_type_class(Types::IssueConnectionType) + connection_type_class(Types::IssuableConnectionType) implements(Types::Notes::NoteableType) diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index c194b46736330bfb4cbae300c55b8f9074a1fa29..c58f76ddb5c37484cd5101ed590b24c6f4266436 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -4,6 +4,8 @@ module Types class MergeRequestType < BaseObject graphql_name 'MergeRequest' + connection_type_class(Types::IssuableConnectionType) + implements(Types::Notes::NoteableType) authorize :read_merge_request diff --git a/app/models/merge_request/metrics.rb b/app/models/merge_request/metrics.rb index f679beabf9489e0334111bb3f35536dd991820a3..66bff3f598249ad67e70f02070fe8ec840be2c3a 100644 --- a/app/models/merge_request/metrics.rb +++ b/app/models/merge_request/metrics.rb @@ -8,6 +8,9 @@ class MergeRequest::Metrics < ApplicationRecord before_save :ensure_target_project_id + scope :merged_after, ->(date) { where(arel_table[:merged_at].gteq(date)) } + scope :merged_before, ->(date) { where(arel_table[:merged_at].lteq(date)) } + private def ensure_target_project_id diff --git a/changelogs/unreleased/graphql-count-and-filter-mrs-by-merged-at.yml b/changelogs/unreleased/graphql-count-and-filter-mrs-by-merged-at.yml new file mode 100644 index 0000000000000000000000000000000000000000..65dd6d27d8bc1fb2feeef6b4ed02def030e81611 --- /dev/null +++ b/changelogs/unreleased/graphql-count-and-filter-mrs-by-merged-at.yml @@ -0,0 +1,5 @@ +--- +title: Add attributes to filter project merge requests by merged at date in GraphQL +merge_request: 38584 +author: +type: added diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 2625807b11d6ffbe7ba4e94359fe5e850f6bcba9..eea40e5fbd474852b90af888af101b0e32196625 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -8017,6 +8017,11 @@ type MergeRequest implements Noteable { The connection type for MergeRequest. """ type MergeRequestConnection { + """ + Total count of collection + """ + count: Int! + """ A list of edges. """ @@ -10314,6 +10319,16 @@ type Project { """ last: Int + """ + Merge requests merged after this date + """ + mergedAfter: Time + + """ + Merge requests merged before this date + """ + mergedBefore: Time + """ Array of source branch names. All resolved merge requests will have one of these branches as their source. """ @@ -15187,6 +15202,16 @@ type User { """ last: Int + """ + Merge requests merged after this date + """ + mergedAfter: Time + + """ + Merge requests merged before this date + """ + mergedBefore: Time + """ The global ID of the project the authored merge requests should be in. Incompatible with projectPath. """ @@ -15247,6 +15272,16 @@ type User { """ last: Int + """ + Merge requests merged after this date + """ + mergedAfter: Time + + """ + Merge requests merged before this date + """ + mergedBefore: Time + """ The global ID of the project the authored merge requests should be in. Incompatible with projectPath. """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 88293c786de540984a36d4da6d741279d2a0df0c..0bc84c890cf328c85c744de87c5adac710774a5e 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -22361,6 +22361,24 @@ "name": "MergeRequestConnection", "description": "The connection type for MergeRequest.", "fields": [ + { + "name": "count", + "description": "Total count of collection", + "args": [ + + ], + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "edges", "description": "A list of edges.", @@ -30505,6 +30523,26 @@ }, "defaultValue": null }, + { + "name": "mergedAfter", + "description": "Merge requests merged after this date", + "type": { + "kind": "SCALAR", + "name": "Time", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "mergedBefore", + "description": "Merge requests merged before this date", + "type": { + "kind": "SCALAR", + "name": "Time", + "ofType": null + }, + "defaultValue": null + }, { "name": "after", "description": "Returns the elements in the list that come after the specified cursor.", @@ -44680,6 +44718,26 @@ }, "defaultValue": null }, + { + "name": "mergedAfter", + "description": "Merge requests merged after this date", + "type": { + "kind": "SCALAR", + "name": "Time", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "mergedBefore", + "description": "Merge requests merged before this date", + "type": { + "kind": "SCALAR", + "name": "Time", + "ofType": null + }, + "defaultValue": null + }, { "name": "projectPath", "description": "The full-path of the project the authored merge requests should be in. Incompatible with projectId.", @@ -44835,6 +44893,26 @@ }, "defaultValue": null }, + { + "name": "mergedAfter", + "description": "Merge requests merged after this date", + "type": { + "kind": "SCALAR", + "name": "Time", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "mergedBefore", + "description": "Merge requests merged before this date", + "type": { + "kind": "SCALAR", + "name": "Time", + "ofType": null + }, + "defaultValue": null + }, { "name": "projectPath", "description": "The full-path of the project the authored merge requests should be in. Incompatible with projectId.", diff --git a/ee/app/finders/productivity_analytics_finder.rb b/ee/app/finders/productivity_analytics_finder.rb index 9ea280cbd675c4fbfd28ab713b505223c578c67c..2365cc6976a15e840234c50d588aac178a09e4f3 100644 --- a/ee/app/finders/productivity_analytics_finder.rb +++ b/ee/app/finders/productivity_analytics_finder.rb @@ -1,25 +1,18 @@ # frozen_string_literal: true class ProductivityAnalyticsFinder < MergeRequestsFinder + extend ::Gitlab::Utils::Override + def self.array_params super.merge(days_to_merge: []) end - def self.scalar_params - @scalar_params ||= super + [:merged_before, :merged_after] - end - def filter_items(_items) - items = by_days_to_merge(super) - by_merged_at(items) + by_days_to_merge(super) end private - def metrics_table - MergeRequest::Metrics.arel_table.alias(MergeRequest::Metrics.table_name) - end - # rubocop: disable CodeReuse/ActiveRecord def by_days_to_merge(items) return items unless params[:days_to_merge].present? @@ -32,27 +25,9 @@ def days_to_merge_column "date_part('day',merge_request_metrics.merged_at - merge_requests.created_at)" end - # rubocop: disable CodeReuse/ActiveRecord - def by_merged_at(items) - return items unless params[:merged_after] || params[:merged_before] - - items = items.joins(:metrics) - items = items.where(metrics_table[:merged_at].gteq(merged_at_between[:from])) if merged_at_between[:from] - items = items.where(metrics_table[:merged_at].lteq(merged_at_between[:to])) if merged_at_between[:to] - - items - end - # rubocop: enable CodeReuse/ActiveRecord - - def merged_at_between - @merged_at_between ||= begin - boundaries = { from: params[:merged_after], to: params[:merged_before] } - - if ProductivityAnalytics.start_date && ProductivityAnalytics.start_date > boundaries[:from] - boundaries[:from] = ProductivityAnalytics.start_date - end - - boundaries - end + # originated from from MergedAtFilter + override :merged_after + def merged_after + @merged_after ||= [super, ProductivityAnalytics.start_date].compact.max end end diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index c84aec9460bf23f2390ca02dd48fa66526a3ed0d..5b86c891e477eb393d96b86e9650fc26f1f4029f 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -85,6 +85,31 @@ expect(merge_requests).to contain_exactly(merge_request5) end + context 'filters by merged_at date' do + before do + merge_request1.metrics.update!(merged_at: 5.days.ago) + merge_request2.metrics.update!(merged_at: 10.days.ago) + end + + describe 'merged_after' do + subject { described_class.new(user, merged_after: 6.days.ago).execute } + + it { is_expected.to eq([merge_request1]) } + end + + describe 'merged_before' do + subject { described_class.new(user, merged_before: 6.days.ago).execute } + + it { is_expected.to eq([merge_request2]) } + end + + describe 'when both merged_after and merged_before is given' do + subject { described_class.new(user, merged_after: 15.days.ago, merged_before: 6.days.ago).execute } + + it { is_expected.to eq([merge_request2]) } + end + end + context 'filtering by group' do it 'includes all merge requests when user has access excluding merge requests from projects the user does not have access to' do private_project = allow_gitaly_n_plus_1 { create(:project, :private, group: group) } diff --git a/spec/graphql/resolvers/merge_requests_resolver_spec.rb b/spec/graphql/resolvers/merge_requests_resolver_spec.rb index 0a8fd82613a14b15979036c067e5ee8e7aff110c..e939edae7799b1b5c6a3cdfc7af5f96ef2d1a5e0 100644 --- a/spec/graphql/resolvers/merge_requests_resolver_spec.rb +++ b/spec/graphql/resolvers/merge_requests_resolver_spec.rb @@ -161,6 +161,24 @@ end end + context 'by merged_after and merged_before' do + before do + merge_request_1.metrics.update!(merged_at: 10.days.ago) + end + + it 'returns merge requests merged between the given period' do + result = resolve_mr(project, merged_after: 20.days.ago, merged_before: 5.days.ago) + + expect(result).to eq([merge_request_1]) + end + + it 'does not return anything' do + result = resolve_mr(project, merged_after: 2.days.ago) + + expect(result).to be_empty + end + end + describe 'combinations' do it 'requires all filters' do create(:merge_request, :closed, source_project: project, target_project: project, source_branch: merge_request_4.source_branch) diff --git a/spec/graphql/types/issue_connection_type_spec.rb b/spec/graphql/types/issuable_connection_type_spec.rb similarity index 100% rename from spec/graphql/types/issue_connection_type_spec.rb rename to spec/graphql/types/issuable_connection_type_spec.rb diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 0054d120b9dc28a8b5cbe322f2640da8b2890d0c..87984dd002121aa09b3e98b5c0316b82d7f66037 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -69,7 +69,9 @@ :before, :after, :first, - :last + :last, + :merged_after, + :merged_before ) end end diff --git a/spec/models/merge_request/metrics_spec.rb b/spec/models/merge_request/metrics_spec.rb index 60a8fd21c44ba25573a404cdc2763a648ee0fe1c..82402b9559722b59bd3d6a5039d4248ad9ed40ba 100644 --- a/spec/models/merge_request/metrics_spec.rb +++ b/spec/models/merge_request/metrics_spec.rb @@ -19,4 +19,33 @@ expect(metrics.target_project_id).to eq(merge_request.target_project_id) end + + describe 'scopes' do + let_it_be(:metrics_1) { create(:merge_request).metrics.tap { |m| m.update!(merged_at: 10.days.ago) } } + let_it_be(:metrics_2) { create(:merge_request).metrics.tap { |m| m.update!(merged_at: 5.days.ago) } } + + describe '.merged_after' do + subject { described_class.merged_after(7.days.ago) } + + it 'finds the record' do + is_expected.to eq([metrics_2]) + end + + it "doesn't include record outside of the filter" do + is_expected.not_to include([metrics_1]) + end + end + + describe '.merged_before' do + subject { described_class.merged_before(7.days.ago) } + + it 'finds the record' do + is_expected.to eq([metrics_1]) + end + + it "doesn't include record outside of the filter" do + is_expected.not_to include([metrics_2]) + end + end + end end