Commit 7412ad2e authored by Sean McGivern's avatar Sean McGivern

Merge branch 'issue_44551' into 'master'

Fix 404 in group boards when moving issue between lists

Closes #44551

See merge request gitlab-org/gitlab-ce!18064
parents b87820c1 7481fc5a
...@@ -19,7 +19,7 @@ export default class BoardService { ...@@ -19,7 +19,7 @@ export default class BoardService {
} }
static generateIssuePath(boardId, id) { 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() { all() {
......
...@@ -42,7 +42,10 @@ module Boards ...@@ -42,7 +42,10 @@ module Boards
) )
end 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 attrs
end end
......
...@@ -55,9 +55,10 @@ module Issues ...@@ -55,9 +55,10 @@ module Issues
return unless params[:move_between_ids] return unless params[:move_between_ids]
after_id, before_id = params.delete(: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_before = get_issue_if_allowed(before_id, board_group_id)
issue_after = get_issue_if_allowed(issue.project, after_id) if after_id issue_after = get_issue_if_allowed(after_id, board_group_id)
issue.move_between(issue_before, issue_after) issue.move_between(issue_before, issue_after)
end end
...@@ -84,8 +85,16 @@ module Issues ...@@ -84,8 +85,16 @@ module Issues
private private
def get_issue_if_allowed(project, id) def get_issue_if_allowed(id, board_group_id = nil)
issue = project.issues.find(id) 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) issue if can?(current_user, :update_issue, issue)
end end
......
---
title: Fix 404 in group boards when moving issue between lists
merge_request:
author:
type: fixed
...@@ -48,7 +48,7 @@ describe Boards::Issues::MoveService do ...@@ -48,7 +48,7 @@ describe Boards::Issues::MoveService do
parent.add_developer(user) parent.add_developer(user)
end end
it_behaves_like 'issues move service' it_behaves_like 'issues move service', true
end end
end end
end end
...@@ -97,6 +97,37 @@ describe Issues::UpdateService, :mailer do ...@@ -97,6 +97,37 @@ describe Issues::UpdateService, :mailer do
expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
end 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 context 'when current user cannot admin issues in the project' do
let(:guest) { create(:user) } let(:guest) { create(:user) }
before do before do
......
shared_examples 'issues move service' do shared_examples 'issues move service' do |group|
context 'when moving an issue between lists' do context 'when moving an issue between lists' do
let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } 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 } } 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 ...@@ -83,5 +83,18 @@ shared_examples 'issues move service' do
expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) expect(issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position)
end 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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment