Merge branch '36868-issues-epic-reorder' into 'master'

List epic related issues by relative_position

See merge request gitlab-org/gitlab!23776

ee/app/models/ee/epic.rb

@@ -202,12 +202,15 @@ module EE
         ::Gitlab::ObjectHierarchy.new(self.where(parent_id: nil)).max_descendants_depth
       end
 
-      def related_issues(ids:, preload: nil)
-        ::Issue.select('issues.*, epic_issues.id as epic_issue_id, epic_issues.relative_position, epic_issues.epic_id as epic_id')
+      def related_issues(ids: nil, preload: nil)
+        items = ::Issue.select('issues.*, epic_issues.id as epic_issue_id, epic_issues.relative_position, epic_issues.epic_id as epic_id')
           .joins(:epic_issue)
           .preload(preload)
-          .where("epic_issues.epic_id": ids)
           .order('epic_issues.relative_position, epic_issues.id')
+
+        return items unless ids
+
+        items.where("epic_issues.epic_id": ids)
       end
     end
 

ee/app/models/epic_issue.rb

@@ -2,6 +2,7 @@
 
 class EpicIssue < ApplicationRecord
   include EpicTreeSorting
+  include EachBatch
 
   validates :epic, :issue, presence: true
   validates :issue, uniqueness: true
@@ -13,4 +14,5 @@ class EpicIssue < ApplicationRecord
   alias_attribute :parent, :epic
 
   scope :in_epic, ->(epic_id) { where(epic_id: epic_id) }
+  scope :related_issues_for_batches, ->(epic_ids) { select(:id, :relative_position).where(epic_id: epic_ids) }
 end

ee/changelogs/unreleased/36868-issues-epic-reorder.yml

+---
+title: Order epic related issues by relative_position
+merge_request: 23776
+author:
+type: fixed

ee/lib/gitlab/graphql/loaders/batch_epic_issues_loader.rb

@@ -6,6 +6,7 @@ module Gitlab
       class BatchEpicIssuesLoader
         # this will assure that no more than 100 queries will be done to fetch issues
         MAX_LOADED_ISSUES = 100_000
+        BATCH_SIZE = 1_000
 
         def initialize(model_id, authorization_filter)
           @model_id = model_id
@@ -14,19 +15,23 @@ module Gitlab
 
         def find
           BatchLoader::GraphQL.for(@model_id).batch(default_value: []) do |ids, loader|
-            issues = ::Epic.related_issues(ids: ids, preload: { project: [:namespace, :project_feature] })
-            load_issues(loader, issues)
+            load_issues(loader, ids)
           end
         end
 
         private
 
-        # rubocop: disable CodeReuse/ActiveRecord
-        def load_issues(loader, issues)
-          issues.find_each(batch_size: 1000).with_index do |issue, idx|
-            if idx > MAX_LOADED_ISSUES
-              raise Gitlab::Graphql::Errors::ArgumentError, 'Too many epic issues requested.'
-            end
+        def load_issues(loader, ids)
+          issues = ::EpicIssue.related_issues_for_batches(ids)
+          issues.each_batch(of: BATCH_SIZE, column: 'relative_position') do |batch, idx|
+            process_batch(loader, batch, idx)
+          end
+        end
+
+        def process_batch(loader, batch, idx)
+          Epic.related_issues(preload: { project: [:namespace, :project_feature] })
+            .merge(batch.except(:select)).each do |issue|
+            ensure_limit_not_exceeded!(idx)
 
             loader.call(issue.epic_id) do |memo|
               unless memo.is_a?(Gitlab::Graphql::FilterableArray)
@@ -38,7 +43,12 @@ module Gitlab
             end
           end
         end
-        # rubocop: enable CodeReuse/ActiveRecord
+
+        def ensure_limit_not_exceeded!(current_index)
+          if current_index * BATCH_SIZE > MAX_LOADED_ISSUES
+            raise Gitlab::Graphql::Errors::ArgumentError, 'Too many epic issues requested.'
+          end
+        end
       end
     end
   end

ee/spec/graphql/resolvers/epic_issues_resolver_spec.rb

@@ -26,17 +26,17 @@ describe Resolvers::EpicIssuesResolver do
 
   describe '#resolve' do
     it 'finds all epic issues' do
-      result = batch_sync(max_queries: 4) { resolve_epic_issues(epic1) }
+      result = batch_sync(max_queries: 6) { resolve_epic_issues(epic1) }
 
       expect(result).to contain_exactly(issue1, issue2)
     end
 
     it 'can batch-resolve epic issues from different epics' do
-      result = batch_sync(max_queries: 4) do
+      result = batch_sync(max_queries: 6) do
         [resolve_epic_issues(epic1), resolve_epic_issues(epic2)]
       end
 
-      expect(result).to contain_exactly([issue1, issue2], [issue3])
+      expect(result).to contain_exactly([issue2, issue1], [issue3])
     end
   end
 

ee/spec/lib/gitlab/graphql/loaders/batch_epic_issues_loader_spec.rb

@@ -12,8 +12,10 @@ describe Gitlab::Graphql::Loaders::BatchEpicIssuesLoader do
     let_it_be(:epic2) { create(:epic, group: group) }
     let_it_be(:issue1) { create(:issue, project: project1) }
     let_it_be(:issue2) { create(:issue, project: project2) }
-    let_it_be(:epic_issue1) { create(:epic_issue, epic: epic1, issue: issue1) }
-    let_it_be(:epic_issue2) { create(:epic_issue, epic: epic2, issue: issue2) }
+    let_it_be(:issue3) { create(:issue, project: project2) }
+    let_it_be(:epic_issue1) { create(:epic_issue, epic: epic1, issue: issue1, relative_position: 1000) }
+    let_it_be(:epic_issue2) { create(:epic_issue, epic: epic2, issue: issue2, relative_position: 99999) }
+    let_it_be(:epic_issue3) { create(:epic_issue, epic: epic2, issue: issue3, relative_position: 1) }
     let(:filter) { proc {} }
 
     subject do
@@ -24,13 +26,13 @@ describe Gitlab::Graphql::Loaders::BatchEpicIssuesLoader do
     end
 
     it 'only queries once for epic issues' do
-      # 4 queries are done: getting issues and getting projects,
-      # project_features and groups for these issues
-      expect { subject }.not_to exceed_query_limit(4)
+      # 6 queries are done: 2 queries in EachBatch and then getting issues
+      # and getting projects, project_features and groups for these issues
+      expect { subject }.not_to exceed_query_limit(6)
     end
 
-    it 'returns all epic issues' do
-      expect(subject).to eq [[issue1], [issue2]]
+    it 'returns all epic issues ordered by relative position' do
+      expect(subject).to eq [[issue1], [issue3, issue2]]
     end
 
     it 'returns an instance of FilterableArray' do
@@ -38,7 +40,7 @@ describe Gitlab::Graphql::Loaders::BatchEpicIssuesLoader do
     end
 
     it 'raises an error if too many issues are loaded' do
-      stub_const('Gitlab::Graphql::Loaders::BatchEpicIssuesLoader::MAX_LOADED_ISSUES', 0)
+      stub_const('Gitlab::Graphql::Loaders::BatchEpicIssuesLoader::MAX_LOADED_ISSUES', 2)
 
       expect { subject }.to raise_error Gitlab::Graphql::Errors::ArgumentError, 'Too many epic issues requested.'
     end