From 29e34c332687be9456578a9b5f60adb10f4e10b5 Mon Sep 17 00:00:00 2001
From: Valery Sizov <valery@gitlab.com>
Date: Tue, 7 Mar 2017 18:04:44 +0200
Subject: Preserve order by priority on issues board

---
 app/models/issue.rb                              |  7 +++++++
 app/services/boards/issues/list_service.rb       |  2 +-
 spec/models/issue_spec.rb                        | 15 ++++++++++++++
 spec/services/boards/issues/list_service_spec.rb | 26 ++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 0f7a26ee3e1..dba9398a43c 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -96,6 +96,13 @@ class Issue < ActiveRecord::Base
     end
   end
 
+  def self.order_by_position_and_priority
+    order_labels_priority.
+      reorder(Gitlab::Database.nulls_last_order('relative_position', 'ASC'),
+              Gitlab::Database.nulls_last_order('highest_priority', 'ASC'),
+              "id DESC")
+  end
+
   # `from` argument can be a Namespace or Project.
   def to_reference(from = nil, full: false)
     reference = "#{self.class.reference_prefix}#{iid}"
diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb
index 185838764c1..83f51947bd4 100644
--- a/app/services/boards/issues/list_service.rb
+++ b/app/services/boards/issues/list_service.rb
@@ -5,7 +5,7 @@ module Boards
         issues = IssuesFinder.new(current_user, filter_params).execute
         issues = without_board_labels(issues) unless movable_list?
         issues = with_list_label(issues) if movable_list?
-        issues.reorder(Gitlab::Database.nulls_last_order('relative_position', 'ASC'))
+        issues.order_by_position_and_priority
       end
 
       private
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index bba9058f394..f67fbe79bde 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -22,6 +22,21 @@ describe Issue, models: true do
     it { is_expected.to have_db_index(:deleted_at) }
   end
 
+  describe '#order_by_position_and_priority' do
+    let(:project) { create :empty_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!(:issue3) { create(:issue, project: project, relative_position: 100) }
+    let!(:issue4) { create(:issue, project: project, relative_position: 200) }
+
+    it 'returns ordered list' do
+      expect(project.issues.order_by_position_and_priority).
+        to match [issue3, issue4, issue1, issue2]
+    end
+  end
+
   describe '#to_reference' do
     let(:namespace) { build(:namespace, path: 'sample-namespace') }
     let(:project)   { build(:empty_project, name: 'sample-project', namespace: namespace) }
diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb
index 01baedc4761..22115c6566d 100644
--- a/spec/services/boards/issues/list_service_spec.rb
+++ b/spec/services/boards/issues/list_service_spec.rb
@@ -43,6 +43,32 @@ describe Boards::Issues::ListService, services: true do
       described_class.new(project, user, params).execute
     end
 
+    context 'issues are ordered by priority' do
+      it 'returns opened issues when list_id is missing' do
+        params = { board_id: board.id }
+
+        issues = described_class.new(project, user, params).execute
+
+        expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1]
+      end
+
+      it 'returns closed issues when listing issues from Done' do
+        params = { board_id: board.id, id: done.id }
+
+        issues = described_class.new(project, user, params).execute
+
+        expect(issues).to eq [closed_issue4, closed_issue2, closed_issue3, closed_issue1]
+      end
+
+      it 'returns opened issues that have label list applied when listing issues from a label list' do
+        params = { board_id: board.id, id: list1.id }
+
+        issues = described_class.new(project, user, params).execute
+
+        expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2]
+      end
+    end
+
     context 'with list that does not belong to the board' do
       it 'raises an error' do
         list = create(:list)
-- 
cgit v1.2.1