[Issue board sorting] Specs

7 files changed, 163 insertions, 5 deletions

diff --git a/app/controllers/projects/boards/issues_controller.rb b/app/controllers/projects/boards/issues_controller.rb
index 70fcc1d60ff..28c9646910d 100644
--- a/app/controllers/projects/boards/issues_controller.rb
+++ b/app/controllers/projects/boards/issues_controller.rb
@@ -41,7 +41,7 @@ module Projects
 
       def make_sure_position_is_set(issues)
         issues.each do |issue|
-          issue.move_to_end unless issue.relative_position
+          issue.move_to_end && issue.save unless issue.relative_position
         end
       end
 
diff --git a/app/models/concerns/relative_positioning.rb b/app/models/concerns/relative_positioning.rb
index 562704a02e2..75d814b045c 100644
--- a/app/models/concerns/relative_positioning.rb
+++ b/app/models/concerns/relative_positioning.rb
@@ -103,10 +103,6 @@ module RelativePositioning
     self.relative_position = nil
   end
 
-  def move_to_front
-    self.relative_position = position_between(MIN_POSITION, min_relative_position || MAX_POSITION)
-  end
-
   def move_to_end
     self.relative_position = position_between(max_relative_position || MIN_POSITION, MAX_POSITION)
   end
diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb
index 3d0533cb516..15667e8d4b1 100644
--- a/spec/controllers/projects/boards/issues_controller_spec.rb
+++ b/spec/controllers/projects/boards/issues_controller_spec.rb
@@ -43,6 +43,7 @@ describe Projects::Boards::IssuesController do
 
           expect(response).to match_response_schema('issues')
           expect(parsed_response.length).to eq 2
+          expect(development.issues.map(&:relative_position)).not_to include(nil)
         end
       end
 
diff --git a/spec/fixtures/api/schemas/issue.json b/spec/fixtures/api/schemas/issue.json
index 8e19cee5440..21c078e0f44 100644
--- a/spec/fixtures/api/schemas/issue.json
+++ b/spec/fixtures/api/schemas/issue.json
@@ -11,6 +11,7 @@
     "title": { "type": "string" },
     "confidential": { "type": "boolean" },
     "due_date": { "type": ["date", "null"] },
+    "relative_position": { "type": "integer" },
     "labels": {
       "type": "array",
       "items": {
diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb
new file mode 100644
index 00000000000..989f46f1442
--- /dev/null
+++ b/spec/models/concerns/relative_positioning_spec.rb
@@ -0,0 +1,132 @@
+require 'spec_helper'
+
+describe Issue, 'RelativePositioning' do
+  let(:project) { create(:empty_project) }
+  let(:issue) { create(:issue, project: project) }
+  let(:issue1) { create(:issue, project: project) }
+
+  before do
+    [issue, issue1].each do |issue|
+      issue.move_to_end && issue.save
+    end
+  end
+
+  describe '#min_relative_position' do
+    it 'returns minimum position' do
+      expect(issue1.min_relative_position).to eq issue.relative_position
+    end
+  end
+
+  describe '#man_relative_position' do
+    it 'returns maximum position' do
+      expect(issue.max_relative_position).to eq issue1.relative_position
+    end
+  end
+
+  describe '#prev_relative_position' do
+    it 'returns previous position if there is an issue above' do
+      expect(issue1.prev_relative_position).to eq issue.relative_position
+    end
+
+    it 'returns minimum position if there is no issue above' do
+      expect(issue.prev_relative_position).to eq RelativePositioning::MIN_POSITION
+    end
+  end
+
+  describe '#next_relative_position' do
+    it 'returns next position if there is an issue below' do
+      expect(issue.next_relative_position).to eq issue1.relative_position
+    end
+
+    it 'returns next position if there is no issue below' do
+      expect(issue1.next_relative_position).to eq RelativePositioning::MAX_POSITION
+    end
+  end
+
+  describe '#move_before' do
+    it 'moves issue before' do
+      issue1.move_before(issue)
+
+      expect(issue1.relative_position).to be < issue.relative_position
+    end
+
+    it 'moves unpositioned issue before' do
+      issue.update_attribute(:relative_position, nil)
+
+      issue1.move_before(issue)
+
+      expect(issue1.relative_position).to be < issue.relative_position
+    end
+  end
+
+  describe '#move_after' do
+    it 'moves issue after' do
+      issue.move_before(issue1)
+
+      expect(issue.relative_position).to be < issue1.relative_position
+    end
+
+    it 'moves unpositioned issue after' do
+      issue1.update_attribute(:relative_position, nil)
+
+      issue.move_before(issue1)
+
+      expect(issue.relative_position).to be < issue1.relative_position
+    end
+  end
+
+  describe '#move_to_end' do
+    it 'moves issue to the end' do
+      new_issue = create :issue, project: project
+
+      new_issue.move_to_end
+
+      expect(new_issue.relative_position).to be > issue1.relative_position
+    end
+  end
+
+  describe '#move_between' do
+    it 'positions issue between two other' do
+      new_issue = create :issue, project: project
+
+      new_issue.move_between(issue, issue1)
+
+      expect(new_issue.relative_position).to be > issue.relative_position
+      expect(new_issue.relative_position).to be < issue1.relative_position
+    end
+
+    it 'positions issue between two other if position of last one is nil' do
+      new_issue = create :issue, project: project
+      issue1.relative_position = nil
+      issue1.save
+
+      new_issue.move_between(issue, issue1)
+
+      expect(new_issue.relative_position).to be > issue.relative_position
+      expect(new_issue.relative_position).to be < issue1.relative_position
+    end
+
+    it 'positions issue between two other if position of first one is nil' do
+      new_issue = create :issue, project: project
+      issue.relative_position = nil
+      issue.save
+
+      new_issue.move_between(issue, issue1)
+
+      expect(new_issue.relative_position).to be > issue.relative_position
+      expect(new_issue.relative_position).to be < issue1.relative_position
+    end
+
+    it 'calls move_after if after is nil' do
+      expect(issue).to receive(:move_after)
+
+      issue.move_between(issue1, nil)
+    end
+
+    it 'calls move_before if before is nil' do
+      expect(issue).to receive(:move_before)
+
+      issue.move_between(nil, issue1)
+    end
+  end
+end
diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb
index 77f75167b3d..5b71762e2b3 100644
--- a/spec/services/boards/issues/move_service_spec.rb
+++ b/spec/services/boards/issues/move_service_spec.rb
@@ -79,6 +79,8 @@ describe Boards::Issues::MoveService, services: true do
 
     context 'when moving to same list' do
       let(:issue)  { create(:labeled_issue, project: project, labels: [bug, development]) }
+      let(:issue1)  { create(:labeled_issue, project: project, labels: [bug, development]) }
+      let(:issue2)  { create(:labeled_issue, project: project, labels: [bug, development]) }
       let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } }
 
       it 'returns false' do
@@ -90,6 +92,16 @@ describe Boards::Issues::MoveService, services: true do
 
         expect(issue.reload.labels).to contain_exactly(bug, development)
       end
+
+      it 'sorts issues' do
+        issue.move_between(issue1, issue2)
+
+        params.merge! move_after_iid: issue1.iid, move_before_iid: issue2.iid
+
+        described_class.new(project, user, params).execute(issue1)
+
+        expect(issue1.relative_position).to be_between(issue.relative_position, issue2.relative_position)
+      end
     end
   end
 end
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index d83b09fd32c..f552252da48 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -58,6 +58,22 @@ describe Issues::UpdateService, services: true do
         expect(issue.due_date).to eq Date.tomorrow
       end
 
+      it 'sorts issues as specified by parameters' do
+        issue1 = create :issue, project: project, assignee_id: user3.id
+        issue2 = create :issue, project: project, assignee_id: user3.id
+
+        [issue, issue1, issue2].each do |issue|
+          issue.move_to_end
+          issue.save
+        end
+
+        opts.merge! move_between_iids: [issue1.iid, issue2.iid]
+
+        update_issue(opts)
+
+        expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
+      end
+
       context 'when current user cannot admin issues in the project' do
         let(:guest) { create(:user) }
         before do