Merge branch 'graphql-count-and-filter-mrs-by-merged-at' into 'master'

Graphql count and filter MRs by merged at

See merge request gitlab-org/gitlab!38584

app/finders/concerns/merged_at_filter.rb

+# 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

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 @@ class MergeRequestsFinder < IssuableFinder
     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
 

app/graphql/resolvers/merge_requests_resolver.rb

@@ -28,6 +28,12 @@ module Resolvers
              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

app/graphql/types/issue_connection_type.rb → 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'
 

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)
 

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

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

changelogs/unreleased/graphql-count-and-filter-mrs-by-merged-at.yml

+---
+title: Add attributes to filter project merge requests by merged at date in GraphQL
+merge_request: 38584
+author:
+type: added

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.
     """

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.",

ee/app/finders/productivity_analytics_finder.rb

 # 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 @@ class ProductivityAnalyticsFinder < MergeRequestsFinder
     "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

spec/finders/merge_requests_finder_spec.rb

@@ -85,6 +85,31 @@ RSpec.describe MergeRequestsFinder do
         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) }

spec/graphql/resolvers/merge_requests_resolver_spec.rb

@@ -161,6 +161,24 @@ RSpec.describe Resolvers::MergeRequestsResolver do
       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)

spec/graphql/types/project_type_spec.rb

@@ -69,7 +69,9 @@ RSpec.describe GitlabSchema.types['Project'] do
                                             :before,
                                             :after,
                                             :first,
-                                            :last
+                                            :last,
+                                            :merged_after,
+                                            :merged_before
                                            )
     end
   end

spec/models/merge_request/metrics_spec.rb

@@ -19,4 +19,33 @@ RSpec.describe MergeRequest::Metrics do
 
     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