Commit d5801545 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'archytaus/gitlab-ce-26372-duplicate-issue-slash-command' into 'master'

New `/duplicate` quick action

Closes #26372

See merge request !12845
parents d95e6da0 1df696f5
...@@ -18,7 +18,8 @@ module SystemNoteHelper ...@@ -18,7 +18,8 @@ module SystemNoteHelper
'milestone' => 'icon_clock_o', 'milestone' => 'icon_clock_o',
'discussion' => 'icon_comment_o', 'discussion' => 'icon_comment_o',
'moved' => 'icon_arrow_circle_o_right', 'moved' => 'icon_arrow_circle_o_right',
'outdated' => 'icon_edit' 'outdated' => 'icon_edit',
'duplicate' => 'icon_clone'
}.freeze }.freeze
def icon_for_system_note(note) def icon_for_system_note(note)
......
class SystemNoteMetadata < ActiveRecord::Base class SystemNoteMetadata < ActiveRecord::Base
ICON_TYPES = %w[ ICON_TYPES = %w[
commit description merge confidential visible label assignee cross_reference commit description merge confidential visible label assignee cross_reference
title time_tracking branch milestone discussion task moved opened closed merged title time_tracking branch milestone discussion task moved
opened closed merged duplicate
outdated outdated
].freeze ].freeze
......
...@@ -58,6 +58,7 @@ class IssuableBaseService < BaseService ...@@ -58,6 +58,7 @@ class IssuableBaseService < BaseService
params.delete(:assignee_ids) params.delete(:assignee_ids)
params.delete(:assignee_id) params.delete(:assignee_id)
params.delete(:due_date) params.delete(:due_date)
params.delete(:canonical_issue_id)
end end
filter_assignee(issuable) filter_assignee(issuable)
......
...@@ -7,6 +7,14 @@ module Issues ...@@ -7,6 +7,14 @@ module Issues
issue_data issue_data
end end
def reopen_service
Issues::ReopenService
end
def close_service
Issues::CloseService
end
private private
def create_assignee_note(issue, old_assignees) def create_assignee_note(issue, old_assignees)
......
module Issues
class DuplicateService < Issues::BaseService
def execute(duplicate_issue, canonical_issue)
return if canonical_issue == duplicate_issue
return unless can?(current_user, :update_issue, duplicate_issue)
return unless can?(current_user, :create_note, canonical_issue)
create_issue_duplicate_note(duplicate_issue, canonical_issue)
create_issue_canonical_note(canonical_issue, duplicate_issue)
close_service.new(project, current_user, {}).execute(duplicate_issue)
end
private
def create_issue_duplicate_note(duplicate_issue, canonical_issue)
SystemNoteService.mark_duplicate_issue(duplicate_issue, duplicate_issue.project, current_user, canonical_issue)
end
def create_issue_canonical_note(canonical_issue, duplicate_issue)
SystemNoteService.mark_canonical_issue_of_duplicate(canonical_issue, canonical_issue.project, current_user, duplicate_issue)
end
end
end
...@@ -5,6 +5,7 @@ module Issues ...@@ -5,6 +5,7 @@ module Issues
def execute(issue) def execute(issue)
handle_move_between_iids(issue) handle_move_between_iids(issue)
filter_spam_check_params filter_spam_check_params
change_issue_duplicate(issue)
update(issue) update(issue)
end end
...@@ -53,14 +54,6 @@ module Issues ...@@ -53,14 +54,6 @@ module Issues
end end
end end
def reopen_service
Issues::ReopenService
end
def close_service
Issues::CloseService
end
def handle_move_between_iids(issue) def handle_move_between_iids(issue)
return unless params[:move_between_iids] return unless params[:move_between_iids]
...@@ -72,6 +65,15 @@ module Issues ...@@ -72,6 +65,15 @@ module Issues
issue.move_between(issue_before, issue_after) issue.move_between(issue_before, issue_after)
end end
def change_issue_duplicate(issue)
canonical_issue_id = params.delete(:canonical_issue_id)
canonical_issue = IssuesFinder.new(current_user).find_by(id: canonical_issue_id)
if canonical_issue
Issues::DuplicateService.new(project, current_user).execute(issue, canonical_issue)
end
end
private private
def get_issue_if_allowed(project, iid) def get_issue_if_allowed(project, iid)
......
...@@ -471,6 +471,24 @@ module QuickActions ...@@ -471,6 +471,24 @@ module QuickActions
end end
end end
desc 'Mark this issue as a duplicate of another issue'
explanation do |duplicate_reference|
"Marks this issue as a duplicate of #{duplicate_reference}."
end
params '#issue'
condition do
issuable.is_a?(Issue) &&
issuable.persisted? &&
current_user.can?(:"update_#{issuable.to_ability_name}", issuable)
end
command :duplicate do |duplicate_param|
canonical_issue = extract_references(duplicate_param, :issue).first
if canonical_issue.present?
@updates[:canonical_issue_id] = canonical_issue.id
end
end
def extract_users(params) def extract_users(params)
return [] if params.nil? return [] if params.nil?
......
...@@ -552,6 +552,44 @@ module SystemNoteService ...@@ -552,6 +552,44 @@ module SystemNoteService
create_note(NoteSummary.new(noteable, project, author, body, action: 'moved')) create_note(NoteSummary.new(noteable, project, author, body, action: 'moved'))
end end
# Called when a Noteable has been marked as a duplicate of another Issue
#
# noteable - Noteable object
# project - Project owning noteable
# author - User performing the change
# canonical_issue - Issue that this is a duplicate of
#
# Example Note text:
#
# "marked this issue as a duplicate of #1234"
#
# "marked this issue as a duplicate of other_project#5678"
#
# Returns the created Note object
def mark_duplicate_issue(noteable, project, author, canonical_issue)
body = "marked this issue as a duplicate of #{canonical_issue.to_reference(project)}"
create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate'))
end
# Called when a Noteable has been marked as the canonical Issue of a duplicate
#
# noteable - Noteable object
# project - Project owning noteable
# author - User performing the change
# duplicate_issue - Issue that was a duplicate of this
#
# Example Note text:
#
# "marked #1234 as a duplicate of this issue"
#
# "marked other_project#5678 as a duplicate of this issue"
#
# Returns the created Note object
def mark_canonical_issue_of_duplicate(noteable, project, author, duplicate_issue)
body = "marked #{duplicate_issue.to_reference(project)} as a duplicate of this issue"
create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate'))
end
private private
def notes_for_mentioner(mentioner, noteable, notes) def notes_for_mentioner(mentioner, noteable, notes)
......
<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="14" height="14" viewBox="0 0 14 14">
<path d="M13 12.75v-8.5q0-0.102-0.074-0.176t-0.176-0.074h-8.5q-0.102 0-0.176 0.074t-0.074 0.176v8.5q0 0.102 0.074 0.176t0.176 0.074h8.5q0.102 0 0.176-0.074t0.074-0.176zM14 4.25v8.5q0 0.516-0.367 0.883t-0.883 0.367h-8.5q-0.516 0-0.883-0.367t-0.367-0.883v-8.5q0-0.516 0.367-0.883t0.883-0.367h8.5q0.516 0 0.883 0.367t0.367 0.883zM11 1.25v1.25h-1v-1.25q0-0.102-0.074-0.176t-0.176-0.074h-8.5q-0.102 0-0.176 0.074t-0.074 0.176v8.5q0 0.102 0.074 0.176t0.176 0.074h1.25v1h-1.25q-0.516 0-0.883-0.367t-0.367-0.883v-8.5q0-0.516 0.367-0.883t0.883-0.367h8.5q0.516 0 0.883 0.367t0.367 0.883z"></path>
</svg>
---
title: Added /duplicate quick action to close a duplicate issue
merge_request: 12845
author: Ryan Scott
...@@ -37,3 +37,4 @@ do. ...@@ -37,3 +37,4 @@ do.
| `/target_branch <Branch Name>` | Set target branch for current merge request | | `/target_branch <Branch Name>` | Set target branch for current merge request |
| `/award :emoji:` | Toggle award for :emoji: | | `/award :emoji:` | Toggle award for :emoji: |
| `/board_move ~column` | Move issue to column on the board | | `/board_move ~column` | Move issue to column on the board |
| `/duplicate #issue` | Closes this issue and marks it as a duplicate of another issue |
...@@ -118,5 +118,42 @@ feature 'Issues > User uses quick actions', feature: true, js: true do ...@@ -118,5 +118,42 @@ feature 'Issues > User uses quick actions', feature: true, js: true do
expect(page).not_to have_content '/wip' expect(page).not_to have_content '/wip'
end end
end end
describe 'mark issue as duplicate' do
let(:issue) { create(:issue, project: project) }
let(:original_issue) { create(:issue, project: project) }
context 'when the current user can update issues' do
it 'does not create a note, and marks the issue as a duplicate' do
write_note("/duplicate ##{original_issue.to_reference}")
expect(page).not_to have_content "/duplicate #{original_issue.to_reference}"
expect(page).to have_content 'Commands applied'
expect(page).to have_content "marked this issue as a duplicate of #{original_issue.to_reference}"
expect(issue.reload).to be_closed
end
end
context 'when the current user cannot update the issue' do
let(:guest) { create(:user) }
before do
project.team << [guest, :guest]
gitlab_sign_out
sign_in(guest)
visit project_issue_path(project, issue)
end
it 'does not create a note, and does not mark the issue as a duplicate' do
write_note("/duplicate ##{original_issue.to_reference}")
expect(page).to have_content "/duplicate ##{original_issue.to_reference}"
expect(page).not_to have_content 'Commands applied'
expect(page).not_to have_content "marked this issue as a duplicate of #{original_issue.to_reference}"
expect(issue.reload).to be_open
end
end
end
end end
end end
require 'spec_helper'
describe Issues::DuplicateService, services: true do
let(:user) { create(:user) }
let(:canonical_project) { create(:empty_project) }
let(:duplicate_project) { create(:empty_project) }
let(:canonical_issue) { create(:issue, project: canonical_project) }
let(:duplicate_issue) { create(:issue, project: duplicate_project) }
subject { described_class.new(duplicate_project, user, {}) }
describe '#execute' do
context 'when the issues passed are the same' do
it 'does nothing' do
expect(subject).not_to receive(:close_service)
expect(SystemNoteService).not_to receive(:mark_duplicate_issue)
expect(SystemNoteService).not_to receive(:mark_canonical_issue_of_duplicate)
subject.execute(duplicate_issue, duplicate_issue)
end
end
context 'when the user cannot update the duplicate issue' do
before do
canonical_project.add_reporter(user)
end
it 'does nothing' do
expect(subject).not_to receive(:close_service)
expect(SystemNoteService).not_to receive(:mark_duplicate_issue)
expect(SystemNoteService).not_to receive(:mark_canonical_issue_of_duplicate)
subject.execute(duplicate_issue, canonical_issue)
end
end
context 'when the user cannot comment on the canonical issue' do
before do
duplicate_project.add_reporter(user)
end
it 'does nothing' do
expect(subject).not_to receive(:close_service)
expect(SystemNoteService).not_to receive(:mark_duplicate_issue)
expect(SystemNoteService).not_to receive(:mark_canonical_issue_of_duplicate)
subject.execute(duplicate_issue, canonical_issue)
end
end
context 'when the user can mark the issue as a duplicate' do
before do
canonical_project.add_reporter(user)
duplicate_project.add_reporter(user)
end
it 'closes the duplicate issue' do
subject.execute(duplicate_issue, canonical_issue)
expect(duplicate_issue.reload).to be_closed
expect(canonical_issue.reload).to be_open
end
it 'adds a system note to the duplicate issue' do
expect(SystemNoteService)
.to receive(:mark_duplicate_issue).with(duplicate_issue, duplicate_project, user, canonical_issue)
subject.execute(duplicate_issue, canonical_issue)
end
it 'adds a system note to the canonical issue' do
expect(SystemNoteService)
.to receive(:mark_canonical_issue_of_duplicate).with(canonical_issue, canonical_project, user, duplicate_issue)
subject.execute(duplicate_issue, canonical_issue)
end
end
end
end
...@@ -491,6 +491,27 @@ describe Issues::UpdateService, services: true do ...@@ -491,6 +491,27 @@ describe Issues::UpdateService, services: true do
include_examples 'updating mentions', Issues::UpdateService include_examples 'updating mentions', Issues::UpdateService
end end
context 'duplicate issue' do
let(:canonical_issue) { create(:issue, project: project) }
context 'invalid canonical_issue_id' do
it 'does not call the duplicate service' do
expect(Issues::DuplicateService).not_to receive(:new)
update_issue(canonical_issue_id: 123456789)
end
end
context 'valid canonical_issue_id' do
it 'calls the duplicate service with both issues' do
expect_any_instance_of(Issues::DuplicateService)
.to receive(:execute).with(issue, canonical_issue)
update_issue(canonical_issue_id: canonical_issue.id)
end
end
end
include_examples 'issuable update service' do include_examples 'issuable update service' do
let(:open_issuable) { issue } let(:open_issuable) { issue }
let(:closed_issuable) { create(:closed_issue, project: project) } let(:closed_issuable) { create(:closed_issue, project: project) }
......
...@@ -261,6 +261,15 @@ describe QuickActions::InterpretService, services: true do ...@@ -261,6 +261,15 @@ describe QuickActions::InterpretService, services: true do
end end
end end
shared_examples 'duplicate command' do
it 'fetches issue and populates canonical_issue_id if content contains /duplicate issue_reference' do
issue_duplicate # populate the issue
_, updates = service.execute(content, issuable)
expect(updates).to eq(canonical_issue_id: issue_duplicate.id)
end
end
it_behaves_like 'reopen command' do it_behaves_like 'reopen command' do
let(:content) { '/reopen' } let(:content) { '/reopen' }
let(:issuable) { issue } let(:issuable) { issue }
...@@ -644,6 +653,41 @@ describe QuickActions::InterpretService, services: true do ...@@ -644,6 +653,41 @@ describe QuickActions::InterpretService, services: true do
let(:issuable) { issue } let(:issuable) { issue }
end end
context '/duplicate command' do
it_behaves_like 'duplicate command' do
let(:issue_duplicate) { create(:issue, project: project) }
let(:content) { "/duplicate #{issue_duplicate.to_reference}" }
let(:issuable) { issue }
end
it_behaves_like 'empty command' do
let(:content) { '/duplicate' }
let(:issuable) { issue }
end
context 'cross project references' do
it_behaves_like 'duplicate command' do
let(:other_project) { create(:empty_project, :public) }
let(:issue_duplicate) { create(:issue, project: other_project) }
let(:content) { "/duplicate #{issue_duplicate.to_reference(project)}" }
let(:issuable) { issue }
end
it_behaves_like 'empty command' do
let(:content) { "/duplicate imaginary#1234" }
let(:issuable) { issue }
end
it_behaves_like 'empty command' do
let(:other_project) { create(:empty_project, :private) }
let(:issue_duplicate) { create(:issue, project: other_project) }
let(:content) { "/duplicate #{issue_duplicate.to_reference(project)}" }
let(:issuable) { issue }
end
end
end
context 'when current_user cannot :admin_issue' do context 'when current_user cannot :admin_issue' do
let(:visitor) { create(:user) } let(:visitor) { create(:user) }
let(:issue) { create(:issue, project: project, author: visitor) } let(:issue) { create(:issue, project: project, author: visitor) }
...@@ -693,6 +737,11 @@ describe QuickActions::InterpretService, services: true do ...@@ -693,6 +737,11 @@ describe QuickActions::InterpretService, services: true do
let(:content) { '/remove_due_date' } let(:content) { '/remove_due_date' }
let(:issuable) { issue } let(:issuable) { issue }
end end
it_behaves_like 'empty command' do
let(:content) { '/duplicate #{issue.to_reference}' }
let(:issuable) { issue }
end
end end
context '/award command' do context '/award command' do
......
...@@ -1101,4 +1101,54 @@ describe SystemNoteService, services: true do ...@@ -1101,4 +1101,54 @@ describe SystemNoteService, services: true do
expect(subject.note).to include(diffs_project_merge_request_url(project, merge_request, diff_id: diff_id, anchor: line_code)) expect(subject.note).to include(diffs_project_merge_request_url(project, merge_request, diff_id: diff_id, anchor: line_code))
end end
end end
describe '.mark_duplicate_issue' do
subject { described_class.mark_duplicate_issue(noteable, project, author, canonical_issue) }
context 'within the same project' do
let(:canonical_issue) { create(:issue, project: project) }
it_behaves_like 'a system note' do
let(:action) { 'duplicate' }
end
it { expect(subject.note).to eq "marked this issue as a duplicate of #{canonical_issue.to_reference}" }
end
context 'across different projects' do
let(:other_project) { create(:empty_project) }
let(:canonical_issue) { create(:issue, project: other_project) }
it_behaves_like 'a system note' do
let(:action) { 'duplicate' }
end
it { expect(subject.note).to eq "marked this issue as a duplicate of #{canonical_issue.to_reference(project)}" }
end
end
describe '.mark_canonical_issue_of_duplicate' do
subject { described_class.mark_canonical_issue_of_duplicate(noteable, project, author, duplicate_issue) }
context 'within the same project' do
let(:duplicate_issue) { create(:issue, project: project) }
it_behaves_like 'a system note' do
let(:action) { 'duplicate' }
end
it { expect(subject.note).to eq "marked #{duplicate_issue.to_reference} as a duplicate of this issue" }
end
context 'across different projects' do
let(:other_project) { create(:empty_project) }
let(:duplicate_issue) { create(:issue, project: other_project) }
it_behaves_like 'a system note' do
let(:action) { 'duplicate' }
end
it { expect(subject.note).to eq "marked #{duplicate_issue.to_reference(project)} as a duplicate of this issue" }
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