From 800f2a722f2aac42aee85fde44311b201eaa9589 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 27 Feb 2017 17:45:55 +0200 Subject: [PATCH] [Issue board sorting] Specs --- .../projects/boards/issues_controller.rb | 2 +- app/models/concerns/relative_positioning.rb | 4 - .../projects/boards/issues_controller_spec.rb | 1 + spec/fixtures/api/schemas/issue.json | 1 + .../concerns/relative_positioning_spec.rb | 132 ++++++++++++++++++ .../boards/issues/move_service_spec.rb | 12 ++ spec/services/issues/update_service_spec.rb | 16 +++ 7 files changed, 163 insertions(+), 5 deletions(-) create mode 100644 spec/models/concerns/relative_positioning_spec.rb 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