diff options
| -rw-r--r-- | app/helpers/system_note_helper.rb | 3 | ||||
| -rw-r--r-- | app/models/system_note_metadata.rb | 3 | ||||
| -rw-r--r-- | app/services/issuable_base_service.rb | 1 | ||||
| -rw-r--r-- | app/services/issues/base_service.rb | 8 | ||||
| -rw-r--r-- | app/services/issues/duplicate_service.rb | 24 | ||||
| -rw-r--r-- | app/services/issues/update_service.rb | 18 | ||||
| -rw-r--r-- | app/services/quick_actions/interpret_service.rb | 18 | ||||
| -rw-r--r-- | app/services/system_note_service.rb | 38 | ||||
| -rw-r--r-- | app/views/shared/icons/_icon_clone.svg | 3 | ||||
| -rw-r--r-- | changelogs/unreleased/26372-duplicate-issue-slash-command.yml | 4 | ||||
| -rw-r--r-- | doc/user/project/quick_actions.md | 1 | ||||
| -rw-r--r-- | spec/features/issues/user_uses_slash_commands_spec.rb | 37 | ||||
| -rw-r--r-- | spec/services/issues/duplicate_service_spec.rb | 80 | ||||
| -rw-r--r-- | spec/services/issues/update_service_spec.rb | 21 | ||||
| -rw-r--r-- | spec/services/quick_actions/interpret_service_spec.rb | 49 | ||||
| -rw-r--r-- | spec/services/system_note_service_spec.rb | 50 |
16 files changed, 348 insertions, 10 deletions
diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb index 209bd56b78a..08fd97cd048 100644 --- a/app/helpers/system_note_helper.rb +++ b/app/helpers/system_note_helper.rb @@ -18,7 +18,8 @@ module SystemNoteHelper 'milestone' => 'icon_clock_o', 'discussion' => 'icon_comment_o', 'moved' => 'icon_arrow_circle_o_right', - 'outdated' => 'icon_edit' + 'outdated' => 'icon_edit', + 'duplicate' => 'icon_clone' }.freeze def icon_for_system_note(note) diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 414c95f7705..0b33e45473b 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -1,7 +1,8 @@ class SystemNoteMetadata < ActiveRecord::Base ICON_TYPES = %w[ 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 ].freeze diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 9078b1f0983..ea497729115 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -58,6 +58,7 @@ class IssuableBaseService < BaseService params.delete(:assignee_ids) params.delete(:assignee_id) params.delete(:due_date) + params.delete(:canonical_issue_id) end filter_assignee(issuable) diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 34199eb5d13..4c198fc96ea 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -7,6 +7,14 @@ module Issues issue_data end + def reopen_service + Issues::ReopenService + end + + def close_service + Issues::CloseService + end + private def create_assignee_note(issue, old_assignees) diff --git a/app/services/issues/duplicate_service.rb b/app/services/issues/duplicate_service.rb new file mode 100644 index 00000000000..5c0854e664d --- /dev/null +++ b/app/services/issues/duplicate_service.rb @@ -0,0 +1,24 @@ +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 diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index cd9f9a4a16e..8d918ccc635 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -5,6 +5,7 @@ module Issues def execute(issue) handle_move_between_iids(issue) filter_spam_check_params + change_issue_duplicate(issue) update(issue) end @@ -53,14 +54,6 @@ module Issues end end - def reopen_service - Issues::ReopenService - end - - def close_service - Issues::CloseService - end - def handle_move_between_iids(issue) return unless params[:move_between_iids] @@ -72,6 +65,15 @@ module Issues issue.move_between(issue_before, issue_after) 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 def get_issue_if_allowed(project, iid) diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 6f82159e6c7..5dc1b91d2c0 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -471,6 +471,24 @@ module QuickActions 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) return [] if params.nil? diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index da0f21d449a..2dbee9c246e 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -552,6 +552,44 @@ module SystemNoteService create_note(NoteSummary.new(noteable, project, author, body, action: 'moved')) 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 def notes_for_mentioner(mentioner, noteable, notes) diff --git a/app/views/shared/icons/_icon_clone.svg b/app/views/shared/icons/_icon_clone.svg new file mode 100644 index 00000000000..ccc897aa98f --- /dev/null +++ b/app/views/shared/icons/_icon_clone.svg @@ -0,0 +1,3 @@ +<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> diff --git a/changelogs/unreleased/26372-duplicate-issue-slash-command.yml b/changelogs/unreleased/26372-duplicate-issue-slash-command.yml new file mode 100644 index 00000000000..3108344e0bf --- /dev/null +++ b/changelogs/unreleased/26372-duplicate-issue-slash-command.yml @@ -0,0 +1,4 @@ +--- +title: Added /duplicate quick action to close a duplicate issue +merge_request: 12845 +author: Ryan Scott diff --git a/doc/user/project/quick_actions.md b/doc/user/project/quick_actions.md index 19b51c83222..ce4dd4e99d5 100644 --- a/doc/user/project/quick_actions.md +++ b/doc/user/project/quick_actions.md @@ -37,3 +37,4 @@ do. | `/target_branch <Branch Name>` | Set target branch for current merge request | | `/award :emoji:` | Toggle award for :emoji: | | `/board_move ~column` | Move issue to column on the board | +| `/duplicate #issue` | Closes this issue and marks it as a duplicate of another issue | diff --git a/spec/features/issues/user_uses_slash_commands_spec.rb b/spec/features/issues/user_uses_slash_commands_spec.rb index 4740402dc01..0c3c27e3e45 100644 --- a/spec/features/issues/user_uses_slash_commands_spec.rb +++ b/spec/features/issues/user_uses_slash_commands_spec.rb @@ -118,5 +118,42 @@ feature 'Issues > User uses quick actions', feature: true, js: true do expect(page).not_to have_content '/wip' 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 diff --git a/spec/services/issues/duplicate_service_spec.rb b/spec/services/issues/duplicate_service_spec.rb new file mode 100644 index 00000000000..82daf53b173 --- /dev/null +++ b/spec/services/issues/duplicate_service_spec.rb @@ -0,0 +1,80 @@ +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 diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index d0b991f19ab..064be940a1c 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -491,6 +491,27 @@ describe Issues::UpdateService, services: true do include_examples 'updating mentions', Issues::UpdateService 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 let(:open_issuable) { issue } let(:closed_issuable) { create(:closed_issue, project: project) } diff --git a/spec/services/quick_actions/interpret_service_spec.rb b/spec/services/quick_actions/interpret_service_spec.rb index a2db3f68ff7..2a2a5c38e4b 100644 --- a/spec/services/quick_actions/interpret_service_spec.rb +++ b/spec/services/quick_actions/interpret_service_spec.rb @@ -261,6 +261,15 @@ describe QuickActions::InterpretService, services: true do 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 let(:content) { '/reopen' } let(:issuable) { issue } @@ -644,6 +653,41 @@ describe QuickActions::InterpretService, services: true do let(:issuable) { issue } 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 let(:visitor) { create(:user) } let(:issue) { create(:issue, project: project, author: visitor) } @@ -693,6 +737,11 @@ describe QuickActions::InterpretService, services: true do let(:content) { '/remove_due_date' } let(:issuable) { issue } end + + it_behaves_like 'empty command' do + let(:content) { '/duplicate #{issue.to_reference}' } + let(:issuable) { issue } + end end context '/award command' do diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 60477b8e9ba..681b419aedf 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -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)) 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 |
