summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Artur <felipefac@gmail.com>2018-03-28 16:12:56 -0300
committerFelipe Artur <felipefac@gmail.com>2018-04-03 15:51:10 -0300
commit7481fc5aa8e7d6cee3e03480a6b7dd7a7d19307e (patch)
tree5246f7ab9ea3563a2b5b6508d98ac0683c2aecfd
parent160d29b869673a8f2a3e57cf509e30b93e82a014 (diff)
downloadgitlab-ce-issue_44551.tar.gz
Fix 404 in group boards when moving issue between listsissue_44551
-rw-r--r--app/assets/javascripts/boards/services/board_service.js2
-rw-r--r--app/services/boards/issues/move_service.rb5
-rw-r--r--app/services/issues/update_service.rb17
-rw-r--r--changelogs/unreleased/issue_44551.yml5
-rw-r--r--spec/services/boards/issues/move_service_spec.rb2
-rw-r--r--spec/services/issues/update_service_spec.rb31
-rw-r--r--spec/support/shared_examples/services/boards/issues_move_service.rb15
7 files changed, 69 insertions, 8 deletions
diff --git a/app/assets/javascripts/boards/services/board_service.js b/app/assets/javascripts/boards/services/board_service.js
index d78d4701974..7c90597f77c 100644
--- a/app/assets/javascripts/boards/services/board_service.js
+++ b/app/assets/javascripts/boards/services/board_service.js
@@ -19,7 +19,7 @@ export default class BoardService {
}
static generateIssuePath(boardId, id) {
- return `${gon.relative_url_root}/-/boards/${boardId ? `/${boardId}` : ''}/issues${id ? `/${id}` : ''}`;
+ return `${gon.relative_url_root}/-/boards/${boardId ? `${boardId}` : ''}/issues${id ? `/${id}` : ''}`;
}
all() {
diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb
index 15fed7d17c1..3ceab209f3f 100644
--- a/app/services/boards/issues/move_service.rb
+++ b/app/services/boards/issues/move_service.rb
@@ -42,7 +42,10 @@ module Boards
)
end
- attrs[:move_between_ids] = move_between_ids if move_between_ids
+ if move_between_ids
+ attrs[:move_between_ids] = move_between_ids
+ attrs[:board_group_id] = board.group&.id
+ end
attrs
end
diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb
index d7aa7e2347e..4161932ad2a 100644
--- a/app/services/issues/update_service.rb
+++ b/app/services/issues/update_service.rb
@@ -55,9 +55,10 @@ module Issues
return unless params[:move_between_ids]
after_id, before_id = params.delete(:move_between_ids)
+ board_group_id = params.delete(:board_group_id)
- issue_before = get_issue_if_allowed(issue.project, before_id) if before_id
- issue_after = get_issue_if_allowed(issue.project, after_id) if after_id
+ issue_before = get_issue_if_allowed(before_id, board_group_id)
+ issue_after = get_issue_if_allowed(after_id, board_group_id)
issue.move_between(issue_before, issue_after)
end
@@ -84,8 +85,16 @@ module Issues
private
- def get_issue_if_allowed(project, id)
- issue = project.issues.find(id)
+ def get_issue_if_allowed(id, board_group_id = nil)
+ return unless id
+
+ issue =
+ if board_group_id
+ IssuesFinder.new(current_user, group_id: board_group_id).find_by(id: id)
+ else
+ project.issues.find(id)
+ end
+
issue if can?(current_user, :update_issue, issue)
end
diff --git a/changelogs/unreleased/issue_44551.yml b/changelogs/unreleased/issue_44551.yml
new file mode 100644
index 00000000000..d5265667b00
--- /dev/null
+++ b/changelogs/unreleased/issue_44551.yml
@@ -0,0 +1,5 @@
+---
+title: Fix 404 in group boards when moving issue between lists
+merge_request:
+author:
+type: fixed
diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb
index 0a6b6d880d3..dd0ad5f11bd 100644
--- a/spec/services/boards/issues/move_service_spec.rb
+++ b/spec/services/boards/issues/move_service_spec.rb
@@ -48,7 +48,7 @@ describe Boards::Issues::MoveService do
parent.add_developer(user)
end
- it_behaves_like 'issues move service'
+ it_behaves_like 'issues move service', true
end
end
end
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 41237dd7160..f95474208f3 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -97,6 +97,37 @@ describe Issues::UpdateService, :mailer do
expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
end
+ context 'when moving issue between issues from different projects' do
+ let(:group) { create(:group) }
+ let(:project_1) { create(:project, namespace: group) }
+ let(:project_2) { create(:project, namespace: group) }
+ let(:project_3) { create(:project, namespace: group) }
+
+ let(:issue_1) { create(:issue, project: project_1) }
+ let(:issue_2) { create(:issue, project: project_2) }
+ let(:issue_3) { create(:issue, project: project_3) }
+
+ before do
+ group.add_developer(user)
+ end
+
+ it 'sorts issues as specified by parameters' do
+ # Moving all issues to end here like the last example won't work since
+ # all projects only have the same issue count
+ # so their relative_position will be the same.
+ issue_1.move_to_end
+ issue_2.move_after(issue_1)
+ issue_3.move_after(issue_2)
+ [issue_1, issue_2, issue_3].map(&:save)
+
+ opts[:move_between_ids] = [issue_1.id, issue_2.id]
+ opts[:board_group_id] = group.id
+
+ described_class.new(issue_3.project, user, opts).execute(issue_3)
+ expect(issue_2.relative_position).to be_between(issue_1.relative_position, issue_2.relative_position)
+ end
+ end
+
context 'when current user cannot admin issues in the project' do
let(:guest) { create(:user) }
before do
diff --git a/spec/support/shared_examples/services/boards/issues_move_service.rb b/spec/support/shared_examples/services/boards/issues_move_service.rb
index 4a4fbaa3a0e..737863ea411 100644
--- a/spec/support/shared_examples/services/boards/issues_move_service.rb
+++ b/spec/support/shared_examples/services/boards/issues_move_service.rb
@@ -1,4 +1,4 @@
-shared_examples 'issues move service' do
+shared_examples 'issues move service' do |group|
context 'when moving an issue between lists' do
let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) }
let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list2.id } }
@@ -83,5 +83,18 @@ shared_examples 'issues move service' do
expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
end
+
+ if group
+ context 'when on a group board' do
+ it 'sends the board_group_id parameter' do
+ params.merge!(move_after_id: issue1.id, move_before_id: issue2.id)
+
+ match_params = { move_between_ids: [issue1.id, issue2.id], board_group_id: parent.id }
+ expect(Issues::UpdateService).to receive(:new).with(issue.project, user, match_params).and_return(double(execute: build(:issue)))
+
+ described_class.new(parent, user, params).execute(issue)
+ end
+ end
+ end
end
end