Remove priority sort from board list issues

When retrieving issues from an issue board, we just sort by
relative_position and ID instead of including label priority.

Label priority is expensive to compute and is not really needed because
there should be no duplicate relative positions in normal cases.

Changelog: performance

app/models/issue.rb

@@ -107,8 +107,6 @@ class Issue < ApplicationRecord
   scope :order_due_date_asc, -> { reorder(::Gitlab::Database.nulls_last_order('due_date', 'ASC')) }
   scope :order_due_date_desc, -> { reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC')) }
   scope :order_closest_future_date, -> { reorder(Arel.sql('CASE WHEN issues.due_date >= CURRENT_DATE THEN 0 ELSE 1 END ASC, ABS(CURRENT_DATE - issues.due_date) ASC')) }
-  scope :order_relative_position_asc, -> { reorder(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')) }
-  scope :order_relative_position_desc, -> { reorder(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')) }
   scope :order_closed_date_desc, -> { reorder(closed_at: :desc) }
   scope :order_created_at_desc, -> { reorder(created_at: :desc) }
   scope :order_severity_asc, -> { includes(:issuable_severity).order('issuable_severities.severity ASC NULLS FIRST') }
@@ -267,8 +265,8 @@ class Issue < ApplicationRecord
         'due_date' => -> { order_due_date_asc.with_order_id_desc },
         'due_date_asc' => -> { order_due_date_asc.with_order_id_desc },
         'due_date_desc' => -> { order_due_date_desc.with_order_id_desc },
-        'relative_position' => -> { order_relative_position_asc.with_order_id_desc },
-        'relative_position_asc' => -> { order_relative_position_asc.with_order_id_desc }
+        'relative_position' => -> { order_by_relative_position },
+        'relative_position_asc' => -> { order_by_relative_position }
       }
     )
   end
@@ -278,7 +276,7 @@ class Issue < ApplicationRecord
     when 'closest_future_date', 'closest_future_date_asc' then order_closest_future_date
     when 'due_date', 'due_date_asc'                       then order_due_date_asc.with_order_id_desc
     when 'due_date_desc'                                  then order_due_date_desc.with_order_id_desc
-    when 'relative_position', 'relative_position_asc'     then order_relative_position_asc.with_order_id_desc
+    when 'relative_position', 'relative_position_asc'     then order_by_relative_position
     when 'severity_asc'                                   then order_severity_asc.with_order_id_desc
     when 'severity_desc'                                  then order_severity_desc.with_order_id_desc
     else
@@ -286,13 +284,8 @@ class Issue < ApplicationRecord
     end
   end
 
-  # `with_cte` argument allows sorting when using CTE queries and prevents
-  # errors in postgres when using CTE search optimisation
-  def self.order_by_position_and_priority(with_cte: false)
-    order = Gitlab::Pagination::Keyset::Order.build([column_order_relative_position, column_order_highest_priority, column_order_id_desc])
-
-    order_labels_priority(with_cte: with_cte)
-      .reorder(order)
+  def self.order_by_relative_position
+    reorder(Gitlab::Pagination::Keyset::Order.build([column_order_relative_position, column_order_id_asc]))
   end
 
   def self.column_order_relative_position
@@ -307,25 +300,6 @@ class Issue < ApplicationRecord
     )
   end
 
-  def self.column_order_highest_priority
-    Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
-      attribute_name: 'highest_priority',
-      column_expression: Arel.sql('highest_priorities.label_priority'),
-      order_expression: Gitlab::Database.nulls_last_order('highest_priorities.label_priority', 'ASC'),
-      reversed_order_expression: Gitlab::Database.nulls_last_order('highest_priorities.label_priority', 'DESC'),
-      order_direction: :asc,
-      nullable: :nulls_last,
-      distinct: false
-    )
-  end
-
-  def self.column_order_id_desc
-    Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
-      attribute_name: 'id',
-      order_expression: arel_table[:id].desc
-    )
-  end
-
   def self.column_order_id_asc
     Gitlab::Pagination::Keyset::ColumnOrderDefinition.new(
       attribute_name: 'id',

app/services/boards/issues/list_service.rb

@@ -22,7 +22,7 @@ module Boards
       def order(items)
         return items.order_closed_date_desc if list&.closed?
 
-        items.order_by_position_and_priority(with_cte: params[:search].present?)
+        items.order_by_relative_position
       end
 
       def finder

app/services/issues/relative_position_rebalancing_service.rb

@@ -82,7 +82,7 @@ module Issues
       collection.each do |project|
         caching.cache_current_project_id(project.id)
         index += 1
-        scope = Issue.in_projects(project).reorder(custom_reorder).select(:id, :relative_position)
+        scope = Issue.in_projects(project).order_by_relative_position.select(:id, :relative_position)
 
         with_retry(PREFETCH_ISSUES_BATCH_SIZE, 100) do |batch_size|
           Gitlab::Pagination::Keyset::Iterator.new(scope: scope).each_batch(of: batch_size) do |batch|
@@ -166,10 +166,6 @@ module Issues
       @start_position ||= (RelativePositioning::START_POSITION - (gaps / 2) * gap_size).to_i
     end
 
-    def custom_reorder
-      ::Gitlab::Pagination::Keyset::Order.build([Issue.column_order_relative_position, Issue.column_order_id_asc])
-    end
-
     def with_retry(initial_batch_size, exit_batch_size)
       retries = 0
       batch_size = initial_batch_size

ee/spec/features/boards/boards_spec.rb

@@ -81,8 +81,8 @@ RSpec.describe 'Project issue boards', :js do
   context 'total weight' do
     let!(:label) { create(:label, project: project, name: 'Label 1') }
     let!(:list) { create(:list, board: board, label: label, position: 0) }
-    let!(:issue) { create(:issue, project: project, weight: 3) }
-    let!(:issue_2) { create(:issue, project: project, weight: 2) }
+    let!(:issue) { create(:issue, project: project, weight: 3, relative_position: 2) }
+    let!(:issue_2) { create(:issue, project: project, weight: 2, relative_position: 1) }
 
     before do
       project.add_developer(user)

ee/spec/models/issuables_analytics_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe IssuablesAnalytics do
 
     context 'when issuable relation is ordered by priority' do
       it 'generates chart data correctly' do
-        issues = project.issues.order_by_position_and_priority
+        issues = project.issues.order_labels_priority
         data = described_class.new(issuables: issues).data
 
         seed.each_pair do |months_back, issues_count|

spec/controllers/boards/issues_controller_spec.rb

@@ -116,7 +116,7 @@ RSpec.describe Boards::IssuesController do
 
         it 'does not query issues table more than once' do
           recorder = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: board, list: list1) }
-          query_count = recorder.occurrences.select { |query,| query.start_with?('SELECT issues.*') }.each_value.first
+          query_count = recorder.occurrences.select { |query,| query.match?(/FROM "?issues"?/) }.each_value.first
 
           expect(query_count).to eq(1)
         end

spec/graphql/resolvers/board_list_issues_resolver_spec.rb

@@ -35,7 +35,7 @@ RSpec.describe Resolvers::BoardListIssuesResolver do
         # by relative_position and then ID
         result = resolve_board_list_issues
 
-        expect(result.map(&:id)).to eq [issue3.id, issue1.id, issue2.id, issue4.id]
+        expect(result.map(&:id)).to eq [issue1.id, issue3.id, issue2.id, issue4.id]
         expect(result.map(&:relative_position)).not_to include(nil)
       end
 

spec/graphql/resolvers/issues_resolver_spec.rb

@@ -338,7 +338,7 @@ RSpec.describe Resolvers::IssuesResolver do
           let_it_be(:relative_issue4) { create(:issue, project: project, relative_position: nil) }
 
           it 'sorts issues ascending' do
-            expect(resolve_issues(sort: :relative_position_asc).to_a).to eq [relative_issue3, relative_issue1, relative_issue4, relative_issue2]
+            expect(resolve_issues(sort: :relative_position_asc).to_a).to eq [relative_issue3, relative_issue1, relative_issue2, relative_issue4]
           end
         end
 

spec/lib/gitlab/import_export/json/streaming_serializer_spec.rb

@@ -115,7 +115,7 @@ RSpec.describe Gitlab::ImportExport::Json::StreamingSerializer do
         end
 
         it 'orders exported issues by custom column(relative_position)' do
-          expected_issues = exportable.issues.order_relative_position_desc.order(id: :desc).map(&:to_json)
+          expected_issues = exportable.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :desc).map(&:to_json)
 
           expect(json_writer).to receive(:write_relation_array).with(exportable_path, :issues, expected_issues)
 

spec/lib/gitlab/pagination/keyset/iterator_spec.rb

@@ -73,7 +73,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do
 
             iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) }
 
-            expect(positions).to eq(project.issues.order_relative_position_asc.order(id: :asc).pluck(:relative_position, :id))
+            expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')).order(id: :asc).pluck(:relative_position, :id))
           end
         end
 
@@ -85,7 +85,7 @@ RSpec.describe Gitlab::Pagination::Keyset::Iterator do
 
             iterator.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) }
 
-            expect(positions).to eq(project.issues.order_relative_position_desc.order(id: :desc).pluck(:relative_position, :id))
+            expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')).order(id: :desc).pluck(:relative_position, :id))
           end
         end
 

spec/models/issue_spec.rb

@@ -222,17 +222,15 @@ RSpec.describe Issue do
     end
   end
 
-  describe '#order_by_position_and_priority' do
+  describe '#order_by_relative_position' do
     let(:project) { reusable_project }
-    let(:p1) { create(:label, title: 'P1', project: project, priority: 1) }
-    let(:p2) { create(:label, title: 'P2', project: project, priority: 2) }
-    let!(:issue1) { create(:labeled_issue, project: project, labels: [p1]) }
-    let!(:issue2) { create(:labeled_issue, project: project, labels: [p2]) }
+    let!(:issue1) { create(:issue, project: project) }
+    let!(:issue2) { create(:issue, project: project) }
     let!(:issue3) { create(:issue, project: project, relative_position: -200) }
     let!(:issue4) { create(:issue, project: project, relative_position: -100) }
 
     it 'returns ordered list' do
-      expect(project.issues.order_by_position_and_priority)
+      expect(project.issues.order_by_relative_position)
         .to match [issue3, issue4, issue1, issue2]
     end
   end

spec/requests/api/graphql/project/issues_spec.rb

@@ -233,7 +233,7 @@ RSpec.describe 'getting an issue list for a project' do
           let(:expected_results) do
             [
               relative_issue5.iid, relative_issue3.iid, relative_issue1.iid,
-              relative_issue4.iid, relative_issue2.iid
+              relative_issue2.iid, relative_issue4.iid
             ]
           end
         end

spec/workers/issue_placement_worker_spec.rb

@@ -27,7 +27,7 @@ RSpec.describe IssuePlacementWorker do
       it 'places all issues created at most 5 minutes before this one at the end, most recent last' do
         expect { run_worker }.not_to change { irrelevant.reset.relative_position }
 
-        expect(project.issues.order_relative_position_asc)
+        expect(project.issues.order_by_relative_position)
           .to eq([issue_e, issue_b, issue_a, issue, issue_c, issue_f, issue_d])
         expect(project.issues.where(relative_position: nil)).not_to exist
       end