Prevent issue move if issue has been already moved
This commit is contained in:
parent
dda7f9635f
commit
b9036ba610
|
@ -123,4 +123,16 @@ class Issue < ActiveRecord::Base
|
||||||
note.all_references(current_user).merge_requests
|
note.all_references(current_user).merge_requests
|
||||||
end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) }
|
end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def moved?
|
||||||
|
!moved_to.nil?
|
||||||
|
end
|
||||||
|
|
||||||
|
def can_move?(user, to_project = nil)
|
||||||
|
if to_project
|
||||||
|
return false unless user.can?(:admin_issue, to_project)
|
||||||
|
end
|
||||||
|
|
||||||
|
!moved? && user.can?(:admin_issue, self.project)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -33,6 +33,7 @@ module Issues
|
||||||
#
|
#
|
||||||
add_moved_to_note
|
add_moved_to_note
|
||||||
close_old_issue
|
close_old_issue
|
||||||
|
mark_as_moved
|
||||||
end
|
end
|
||||||
|
|
||||||
notify_participants
|
notify_participants
|
||||||
|
@ -47,8 +48,8 @@ module Issues
|
||||||
private
|
private
|
||||||
|
|
||||||
def can_move?
|
def can_move?
|
||||||
can?(@current_user, :admin_issue, @project_old) &&
|
@issue_old.can_move?(@current_user) &&
|
||||||
can?(@current_user, :admin_issue, @project_new)
|
@issue_old.can_move?(@current_user, @project_new)
|
||||||
end
|
end
|
||||||
|
|
||||||
def create_new_issue
|
def create_new_issue
|
||||||
|
@ -96,5 +97,9 @@ module Issues
|
||||||
def notify_participants
|
def notify_participants
|
||||||
notification_service.issue_moved(@issue_old, @issue_new, @current_user)
|
notification_service.issue_moved(@issue_old, @issue_new, @current_user)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def mark_as_moved
|
||||||
|
@issue_old.update(moved_to: @issue_new)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -67,7 +67,7 @@
|
||||||
- if can? current_user, :admin_label, issuable.project
|
- if can? current_user, :admin_label, issuable.project
|
||||||
= link_to 'Create new label', new_namespace_project_label_path(issuable.project.namespace, issuable.project), target: :blank
|
= link_to 'Create new label', new_namespace_project_label_path(issuable.project.namespace, issuable.project), target: :blank
|
||||||
|
|
||||||
- if issuable.is_a?(Issue) && can?(current_user, :admin_issue, issuable.project)
|
- if issuable.is_a?(Issue) && issuable.can_move?(current_user)
|
||||||
%hr
|
%hr
|
||||||
.form-group
|
.form-group
|
||||||
= label_tag :move_to_project_id, 'Move', class: 'control-label'
|
= label_tag :move_to_project_id, 'Move', class: 'control-label'
|
||||||
|
|
|
@ -60,6 +60,17 @@ feature 'issue move to another project' do
|
||||||
expect(page).to have_select('move_to_project_id', options: options)
|
expect(page).to have_select('move_to_project_id', options: options)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'issue has been already moved' do
|
||||||
|
let(:new_issue) { create(:issue, project: new_project) }
|
||||||
|
let(:issue) do
|
||||||
|
create(:issue, project: old_project, author: user, moved_to: new_issue)
|
||||||
|
end
|
||||||
|
|
||||||
|
scenario 'user wants to move issue that has already been moved' do
|
||||||
|
expect(page).to have_no_select('move_to_project_id')
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def edit_issue(issue)
|
def edit_issue(issue)
|
||||||
|
|
|
@ -130,6 +130,56 @@ describe Issue, models: true do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#can_move?' do
|
||||||
|
let(:user) { create(:user) }
|
||||||
|
let(:issue) { create(:issue) }
|
||||||
|
subject { issue.can_move?(user) }
|
||||||
|
|
||||||
|
context 'user is not a member of project issue belongs to' do
|
||||||
|
it { is_expected.to eq false}
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'user is reporter in project issue belongs to' do
|
||||||
|
let(:project) { create(:project) }
|
||||||
|
let(:issue) { create(:issue, project: project) }
|
||||||
|
|
||||||
|
before { project.team << [user, :reporter] }
|
||||||
|
|
||||||
|
it { is_expected.to eq true }
|
||||||
|
|
||||||
|
context 'checking destination project also' do
|
||||||
|
subject { issue.can_move?(user, to_project) }
|
||||||
|
let(:to_project) { create(:project) }
|
||||||
|
|
||||||
|
context 'destination project allowed' do
|
||||||
|
before { to_project.team << [user, :reporter] }
|
||||||
|
it { is_expected.to eq true }
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'destination project not allowed' do
|
||||||
|
before { to_project.team << [user, :guest] }
|
||||||
|
it { is_expected.to eq false }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#moved?' do
|
||||||
|
let(:issue) { create(:issue) }
|
||||||
|
subject { issue.moved? }
|
||||||
|
|
||||||
|
context 'issue not moved' do
|
||||||
|
it { is_expected.to eq false }
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'issue already moved' do
|
||||||
|
let(:moved_to_issue) { create(:issue) }
|
||||||
|
let(:issue) { create(:issue, moved_to: moved_to_issue) }
|
||||||
|
|
||||||
|
it { is_expected.to eq true }
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
it_behaves_like 'an editable mentionable' do
|
it_behaves_like 'an editable mentionable' do
|
||||||
subject { create(:issue) }
|
subject { create(:issue) }
|
||||||
|
|
||||||
|
|
|
@ -91,6 +91,11 @@ describe Issues::MoveService, services: true do
|
||||||
it 'creates a new internal id for issue' do
|
it 'creates a new internal id for issue' do
|
||||||
expect(new_issue.iid).to be 1
|
expect(new_issue.iid).to be 1
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'marks issue as moved' do
|
||||||
|
expect(old_issue.moved?).to eq true
|
||||||
|
expect(old_issue.moved_to).to eq new_issue
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'issue with notes' do
|
context 'issue with notes' do
|
||||||
|
@ -220,6 +225,19 @@ describe Issues::MoveService, services: true do
|
||||||
|
|
||||||
it { is_expected.to be_falsey }
|
it { is_expected.to be_falsey }
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'issue has already been moved' do
|
||||||
|
include_context 'user can move issue'
|
||||||
|
|
||||||
|
let(:moved_to_issue) { create(:issue) }
|
||||||
|
|
||||||
|
let(:old_issue) do
|
||||||
|
create(:issue, project: old_project, author: author,
|
||||||
|
moved_to: moved_to_issue)
|
||||||
|
end
|
||||||
|
|
||||||
|
it { is_expected.to be_falsey }
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue