Merge branch 'issue_299814-decrease_epics_max_page_size' into 'master'

Decrease epics, child epics and child issues max page size

See merge request gitlab-org/gitlab!68403

app/graphql/types/base_field.rb

@@ -9,6 +9,7 @@ module Types
     DEFAULT_COMPLEXITY = 1
 
     attr_reader :deprecation, :doc_reference
+    attr_writer :max_page_size # Can be removed with :performance_roadmap feature flag: https://gitlab.com/gitlab-org/gitlab/-/issues/337198
 
     def initialize(**kwargs, &block)
       @calls_gitaly = !!kwargs.delete(:calls_gitaly)

ee/app/graphql/ee/types/group_type.rb

@@ -22,7 +22,6 @@ module EE
         field :epics, ::Types::EpicType.connection_type, null: true,
               description: 'Find epics.',
               extras: [:lookahead],
-              max_page_size: 2000,
               resolver: ::Resolvers::EpicsResolver
 
         field :epic_board,

ee/app/graphql/resolvers/concerns/sets_max_page_size.rb

+# frozen_string_literal: true
+
+module SetsMaxPageSize
+  extend ActiveSupport::Concern
+
+  DEPRECATED_MAX_PAGE_SIZE = 1000
+
+  # We no longer need 1000 page size after epics roadmap pagination feature is released,
+  # after :performance_roadmap flag rollout we can safely use default max page size(100)
+  # for epics, child epics and child issues without breaking current roadmaps.
+  #
+  # When removing :performance_roadmap flag delete this file and remove its method call and
+  # the fields using the resolver will keep using default max page size.
+  # Flag rollout issue: https://gitlab.com/gitlab-org/gitlab/-/issues/337198
+
+  private
+
+  def set_temp_limit_for(actor)
+    max_page_size =
+      if Feature.enabled?(:performance_roadmap, actor, default_enabled: :yaml)
+        context.schema.default_max_page_size
+      else
+        DEPRECATED_MAX_PAGE_SIZE
+      end
+
+    field.max_page_size = max_page_size # rubocop: disable Graphql/Descriptions
+  end
+end

ee/app/graphql/resolvers/epic_issues_resolver.rb

@@ -3,6 +3,7 @@
 module Resolvers
   class EpicIssuesResolver < BaseResolver
     include CachingArrayResolver
+    include SetsMaxPageSize
 
     type Types::EpicIssueType.connection_type, null: true
 
@@ -21,6 +22,8 @@ module Resolvers
     end
 
     def query_for(id)
+      set_temp_limit_for(epic.group) # Can be removed with :performance_roadmap feature flag: https://gitlab.com/gitlab-org/gitlab/-/issues/337198
+
       ::Epic.related_issues(ids: id)
     end
 

ee/app/graphql/resolvers/epics_resolver.rb

@@ -4,6 +4,7 @@ module Resolvers
   class EpicsResolver < BaseResolver
     include TimeFrameArguments
     include LooksAhead
+    include SetsMaxPageSize
 
     argument :iid, GraphQL::Types::ID,
              required: false,
@@ -102,6 +103,8 @@ module Resolvers
     end
 
     def find_epics(args)
+      set_temp_limit_for(group) # Can be removed with :performance_roadmap feature flag: https://gitlab.com/gitlab-org/gitlab/-/issues/337198
+
       apply_lookahead(EpicsFinder.new(context[:current_user], args).execute)
     end
 

ee/app/graphql/types/epic_type.rb

@@ -84,7 +84,6 @@ module Types
 
     field :children, ::Types::EpicType.connection_type, null: true,
           description: 'Children (sub-epics) of the epic.',
-          max_page_size: 1000,
           resolver: ::Resolvers::EpicsResolver
     field :labels, Types::LabelType.connection_type, null: true,
           description: 'Labels assigned to the epic.'
@@ -132,7 +131,6 @@ module Types
           null: true,
           complexity: 5,
           description: 'A list of issues associated with the epic.',
-          max_page_size: 1000,
           resolver: Resolvers::EpicIssuesResolver
 
     field :descendant_counts, Types::EpicDescendantCountType, null: true,

ee/spec/requests/api/graphql/epics/epic_issues_resolver_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'getting epic issues information' do
+  include GraphqlHelpers
+
+  let_it_be(:user) { create(:user) }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
+  let_it_be(:project) { create(:project, group: group) }
+
+  before_all do
+    group.add_maintainer(user)
+  end
+
+  before do
+    stub_licensed_features(epics: true)
+  end
+
+  context 'when toggling performance_roadmap feature flag' do
+    let_it_be(:epic) { create(:epic, group: group, state: 'opened') }
+    let_it_be(:issue1) { create(:issue, project: project) }
+    let_it_be(:issue2) { create(:issue, project: project) }
+
+    let(:query) do
+      epic_issues_query(epic)
+    end
+
+    before_all do
+      create(:epic_issue, epic: epic, issue: issue1)
+      create(:epic_issue, epic: epic, issue: issue2)
+    end
+
+    def epic_issues_query(epic)
+      epic_issues_fragment = <<~EPIC_ISSUE
+        issues {
+          nodes {
+            id
+          }
+        }
+      EPIC_ISSUE
+
+      graphql_query_for(
+        :group,
+        { 'fullPath' => epic.group.full_path },
+        query_graphql_field(
+          'epic', { iid: epic.iid },
+          epic_issues_fragment
+        )
+      )
+    end
+
+    def epic_issues_count
+      graphql_data_at(:group, :epic, :issues, :nodes).count
+    end
+
+    it 'returns epics with max page based on feature flag status' do
+      stub_const('SetsMaxPageSize::DEPRECATED_MAX_PAGE_SIZE', 1)
+
+      post_graphql(epic_issues_query(epic), current_user: user)
+      expect(epic_issues_count).to eq(2)
+
+      stub_feature_flags(performance_roadmap: false)
+      post_graphql(epic_issues_query(epic), current_user: user)
+      expect(epic_issues_count).to eq(1)
+
+      stub_feature_flags(performance_roadmap: true)
+      post_graphql(epic_issues_query(epic), current_user: user)
+      expect(epic_issues_count).to eq(2)
+    end
+  end
+end

ee/spec/requests/api/graphql/epics/epic_resolver_spec.rb

@@ -13,6 +13,30 @@ RSpec.describe 'getting epics information' do
     stub_licensed_features(epics: true)
   end
 
+  context 'when performance_roadmap flag is disabled' do
+    let_it_be(:epic1) { create(:epic, group: group, state: 'opened') }
+    let_it_be(:epic2) { create(:epic, group: group, state: 'opened') }
+
+    def epics_count
+      graphql_data_at(:group, :epics, :nodes).count
+    end
+
+    it 'returns epics within timeframe' do
+      stub_const('SetsMaxPageSize::DEPRECATED_MAX_PAGE_SIZE', 1)
+
+      post_graphql(epics_query_by_hash(group), current_user: user)
+      expect(epics_count).to eq(2)
+
+      stub_feature_flags(performance_roadmap: false)
+      post_graphql(epics_query_by_hash(group), current_user: user)
+      expect(epics_count).to eq(1)
+
+      stub_feature_flags(performance_roadmap: true)
+      post_graphql(epics_query_by_hash(group), current_user: user)
+      expect(epics_count).to eq(2)
+    end
+  end
+
   describe 'query for epics which start with an iid' do
     let_it_be(:epic1) { create(:epic, group: group, iid: 11) }
     let_it_be(:epic2) { create(:epic, group: group, iid: 22) }
@@ -130,14 +154,15 @@ RSpec.describe 'getting epics information' do
     epics_query_by_hash(group, field => value)
   end
 
-  def epics_query_by_hash(group, args)
+  def epics_query_by_hash(group, args = {})
     field_queries = args.map { |key, value| "#{key}:\"#{value}\"" }.join(',')
+    field_queries = " ( #{field_queries} ) " if field_queries.present?
 
     <<~QUERY
       query {
         group(fullPath: "#{group.full_path}") {
           id,
-          epics(#{field_queries}) {
+          epics#{field_queries} {
             nodes {
               id
             }