Commit 16328530 authored by Mario de la Ossa's avatar Mario de la Ossa

Implement Clone with notes quick_action

Implements a "clone_with_notes" quick-action that allows to copy an
issue to any other Project (or the same project) including all notes.
parent 90f3b69a
...@@ -4,10 +4,11 @@ module Issues ...@@ -4,10 +4,11 @@ module Issues
class CloneService < Issuable::Clone::BaseService class CloneService < Issuable::Clone::BaseService
CloneError = Class.new(StandardError) CloneError = Class.new(StandardError)
def execute(issue, target_project) def execute(issue, target_project, with_notes: false)
@target_project = target_project @target_project = target_project
@with_notes = with_notes
unless issue.can_clone?(current_user, @target_project) unless issue.can_clone?(current_user, target_project)
raise CloneError, s_('CloneIssue|Cannot clone issue due to insufficient permissions!') raise CloneError, s_('CloneIssue|Cannot clone issue due to insufficient permissions!')
end end
...@@ -17,6 +18,8 @@ module Issues ...@@ -17,6 +18,8 @@ module Issues
super(issue, target_project) super(issue, target_project)
notify_participants
queue_copy_designs queue_copy_designs
new_entity new_entity
...@@ -25,12 +28,14 @@ module Issues ...@@ -25,12 +28,14 @@ module Issues
private private
attr_reader :target_project attr_reader :target_project
attr_reader :with_notes
def update_new_entity def update_new_entity
# we don't call `super` because we want to be able to decide whether or not to copy all comments over. # we don't call `super` because we want to be able to decide whether or not to copy all comments over.
update_new_entity_description update_new_entity_description
update_new_entity_attributes update_new_entity_attributes
copy_award_emoji copy_award_emoji
copy_notes if with_notes
end end
def update_old_entity def update_old_entity
...@@ -51,7 +56,7 @@ module Issues ...@@ -51,7 +56,7 @@ module Issues
# Skip creation of system notes for existing attributes of the issue. The system notes of the old # Skip creation of system notes for existing attributes of the issue. The system notes of the old
# issue are copied over so we don't want to end up with duplicate notes. # issue are copied over so we don't want to end up with duplicate notes.
CreateService.new(@target_project, @current_user, new_params).execute(skip_system_notes: true) CreateService.new(target_project, current_user, new_params).execute(skip_system_notes: true)
end end
def queue_copy_designs def queue_copy_designs
...@@ -66,6 +71,10 @@ module Issues ...@@ -66,6 +71,10 @@ module Issues
log_error(response.message) if response.error? log_error(response.message) if response.error?
end end
def notify_participants
notification_service.async.issue_cloned(original_entity, new_entity, current_user)
end
def add_note_from def add_note_from
SystemNoteService.noteable_cloned(new_entity, target_project, SystemNoteService.noteable_cloned(new_entity, target_project,
original_entity, current_user, original_entity, current_user,
......
...@@ -129,13 +129,14 @@ module Issues ...@@ -129,13 +129,14 @@ module Issues
def clone_issue(issue) def clone_issue(issue)
target_project = params.delete(:target_clone_project) target_project = params.delete(:target_clone_project)
with_notes = params.delete(:clone_with_notes)
return unless target_project && return unless target_project &&
issue.can_clone?(current_user, target_project) issue.can_clone?(current_user, target_project)
# we've pre-empted this from running in #execute, so let's go ahead and update the Issue now. # we've pre-empted this from running in #execute, so let's go ahead and update the Issue now.
update(issue) update(issue)
Issues::CloneService.new(project, current_user).execute(issue, target_project) Issues::CloneService.new(project, current_user).execute(issue, target_project, with_notes: with_notes)
end end
def create_merge_request_from_quick_action def create_merge_request_from_quick_action
......
---
title: Implement a /clone_with_notes quick-action to quickly clone an Issue will all
its notes
merge_request: 48539
author:
type: added
...@@ -34,7 +34,7 @@ The following quick actions are applicable to descriptions, discussions and thre ...@@ -34,7 +34,7 @@ The following quick actions are applicable to descriptions, discussions and thre
| `/award :emoji:` | ✓ | ✓ | ✓ | Toggle emoji award. | | `/award :emoji:` | ✓ | ✓ | ✓ | Toggle emoji award. |
| `/child_epic <epic>` | | | ✓ | Add child epic to `<epic>`. The `<epic>` value should be in the format of `&epic`, `group&epic`, or a URL to an epic ([introduced in GitLab 12.0](https://gitlab.com/gitlab-org/gitlab/-/issues/7330)). **(ULTIMATE)** | | `/child_epic <epic>` | | | ✓ | Add child epic to `<epic>`. The `<epic>` value should be in the format of `&epic`, `group&epic`, or a URL to an epic ([introduced in GitLab 12.0](https://gitlab.com/gitlab-org/gitlab/-/issues/7330)). **(ULTIMATE)** |
| `/clear_weight` | ✓ | | | Clear weight. **(STARTER)** | | `/clear_weight` | ✓ | | | Clear weight. **(STARTER)** |
| `/clone <path/to/project>` | ✓ | | | Clone the issue to given project, or the current one if no arguments are given ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9421) in GitLab 13.7). Copies as much data as possible as long as the target project contains equivalent labels, milestones, etc. Does not copy comments or system notes. | | `/clone <path/to/project> [--with_notes]`| ✓ | | | Clone the issue to given project, or the current one if no arguments are given ([introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/9421) in GitLab 13.7). Copies as much data as possible as long as the target project contains equivalent labels, milestones, and so on. Does not copy comments or system notes unless `--with_notes` is provided as an argument. |
| `/close` | ✓ | ✓ | ✓ | Close. | | `/close` | ✓ | ✓ | ✓ | Close. |
| `/confidential` | ✓ | | | Make confidential. | | `/confidential` | ✓ | | | Make confidential. |
| `/copy_metadata <!merge_request>` | ✓ | ✓ | | Copy labels and milestone from another merge request in the project. | | `/copy_metadata <!merge_request>` | ✓ | ✓ | | Copy labels and milestone from another merge request in the project. |
......
...@@ -106,17 +106,28 @@ module Gitlab ...@@ -106,17 +106,28 @@ module Gitlab
explanation do |project = quick_action_target.project.full_path| explanation do |project = quick_action_target.project.full_path|
_("Clones this issue, without comments, to %{project}.") % { project: project } _("Clones this issue, without comments, to %{project}.") % { project: project }
end end
params 'path/to/project' params 'path/to/project [--with_notes]'
types Issue types Issue
condition do condition do
quick_action_target.persisted? && quick_action_target.persisted? &&
current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project) current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project)
end end
command :clone do |target_project_path = nil| command :clone do |params = ''|
params = params.split(' ')
with_notes = params.delete('--with_notes').present?
# If we have more than 1 param, then the user supplied too many spaces, or mistyped `--with_notes`
if params.size > 1
@execution_message[:clone] = _('Failed to clone this issue: wrong parameters.')
next
end
target_project_path = params[0]
target_project = target_project_path.present? ? Project.find_by_full_path(target_project_path) : quick_action_target.project target_project = target_project_path.present? ? Project.find_by_full_path(target_project_path) : quick_action_target.project
if target_project.present? if target_project.present?
@updates[:target_clone_project] = target_project @updates[:target_clone_project] = target_project
@updates[:clone_with_notes] = with_notes
message = _("Cloned this issue to %{path_to_project}.") % { path_to_project: target_project_path || quick_action_target.project.full_path } message = _("Cloned this issue to %{path_to_project}.") % { path_to_project: target_project_path || quick_action_target.project.full_path }
else else
......
...@@ -11469,6 +11469,9 @@ msgstr "" ...@@ -11469,6 +11469,9 @@ msgstr ""
msgid "Failed to clone this issue because target project doesn't exist." msgid "Failed to clone this issue because target project doesn't exist."
msgstr "" msgstr ""
msgid "Failed to clone this issue: wrong parameters."
msgstr ""
msgid "Failed to create Merge Request. Please try again." msgid "Failed to create Merge Request. Please try again."
msgstr "" msgstr ""
......
...@@ -15,10 +15,12 @@ RSpec.describe Issues::CloneService do ...@@ -15,10 +15,12 @@ RSpec.describe Issues::CloneService do
let_it_be(:old_project) { create(:project, namespace: sub_group_1) } let_it_be(:old_project) { create(:project, namespace: sub_group_1) }
let_it_be(:new_project) { create(:project, namespace: sub_group_2) } let_it_be(:new_project) { create(:project, namespace: sub_group_2) }
let(:old_issue) do let_it_be(:old_issue, reload: true) do
create(:issue, title: title, description: description, project: old_project, author: author) create(:issue, title: title, description: description, project: old_project, author: author)
end end
let(:with_notes) { false }
subject(:clone_service) do subject(:clone_service) do
described_class.new(old_project, user) described_class.new(old_project, user)
end end
...@@ -35,7 +37,7 @@ RSpec.describe Issues::CloneService do ...@@ -35,7 +37,7 @@ RSpec.describe Issues::CloneService do
include_context 'user can clone issue' include_context 'user can clone issue'
context 'generic issue' do context 'generic issue' do
let!(:new_issue) { clone_service.execute(old_issue, new_project) } let!(:new_issue) { clone_service.execute(old_issue, new_project, with_notes: with_notes) }
it 'creates a new issue in the selected project' do it 'creates a new issue in the selected project' do
expect do expect do
...@@ -91,6 +93,16 @@ RSpec.describe Issues::CloneService do ...@@ -91,6 +93,16 @@ RSpec.describe Issues::CloneService do
it 'does not set moved_issue' do it 'does not set moved_issue' do
expect(old_issue.moved?).to eq(false) expect(old_issue.moved?).to eq(false)
end end
context 'when copying comments' do
let(:with_notes) { true }
it 'does not create extra system notes' do
new_issue = clone_service.execute(old_issue, new_project, with_notes: with_notes)
expect(new_issue.notes.count).to eq(old_issue.notes.count)
end
end
end end
context 'issue with award emoji' do context 'issue with award emoji' do
...@@ -200,6 +212,34 @@ RSpec.describe Issues::CloneService do ...@@ -200,6 +212,34 @@ RSpec.describe Issues::CloneService do
end end
end end
# These tests verify that notes are copied. More thorough tests are in
# the unit test for Notes::CopyService.
context 'issue with notes' do
let_it_be(:notes) do
[
create(:note, noteable: old_issue, project: old_project, created_at: 2.weeks.ago, updated_at: 1.week.ago),
create(:note, noteable: old_issue, project: old_project)
]
end
let(:new_issue) { clone_service.execute(old_issue, new_project, with_notes: with_notes) }
let(:copied_notes) { new_issue.notes.limit(notes.size) } # Remove the system note added by the copy itself
it 'does not copy notes' do
# only the system note
expect(copied_notes.order('id ASC').pluck(:note).size).to eq(1)
end
context 'when copying comments' do
let(:with_notes) { true }
it 'copies existing notes in order' do
expect(copied_notes.order('id ASC').pluck(:note)).to eq(notes.map(&:note))
end
end
end
context 'issue with a design', :clean_gitlab_redis_shared_state do context 'issue with a design', :clean_gitlab_redis_shared_state do
let_it_be(:new_project) { create(:project) } let_it_be(:new_project) { create(:project) }
let!(:design) { create(:design, :with_lfs_file, issue: old_issue) } let!(:design) { create(:design, :with_lfs_file, issue: old_issue) }
......
...@@ -979,15 +979,35 @@ RSpec.describe Issues::UpdateService, :mailer do ...@@ -979,15 +979,35 @@ RSpec.describe Issues::UpdateService, :mailer do
it 'calls the move service with the proper issue and project' do it 'calls the move service with the proper issue and project' do
clone_stub = instance_double(Issues::CloneService) clone_stub = instance_double(Issues::CloneService)
allow(Issues::CloneService).to receive(:new).and_return(clone_stub) allow(Issues::CloneService).to receive(:new).and_return(clone_stub)
allow(clone_stub).to receive(:execute).with(issue, target_project).and_return(issue) allow(clone_stub).to receive(:execute).with(issue, target_project, with_notes: nil).and_return(issue)
expect(clone_stub).to receive(:execute).with(issue, target_project) expect(clone_stub).to receive(:execute).with(issue, target_project, with_notes: nil)
update_issue(target_clone_project: target_project) update_issue(target_clone_project: target_project)
end end
end end
end end
context 'clone an issue with notes' do
context 'valid project' do
let(:target_project) { create(:project) }
before do
target_project.add_maintainer(user)
end
it 'calls the move service with the proper issue and project' do
clone_stub = instance_double(Issues::CloneService)
allow(Issues::CloneService).to receive(:new).and_return(clone_stub)
allow(clone_stub).to receive(:execute).with(issue, target_project, with_notes: true).and_return(issue)
expect(clone_stub).to receive(:execute).with(issue, target_project, with_notes: true)
update_issue(target_clone_project: target_project, clone_with_notes: true)
end
end
end
context 'when moving an issue ' do context 'when moving an issue ' do
it 'raises an error for invalid move ids within a project' do it 'raises an error for invalid move ids within a project' do
opts = { move_between_ids: [9000, non_existing_record_id] } opts = { move_between_ids: [9000, non_existing_record_id] }
......
...@@ -32,6 +32,34 @@ RSpec.shared_examples 'clone quick action' do ...@@ -32,6 +32,34 @@ RSpec.shared_examples 'clone quick action' do
expect(page).to have_content 'Issues 1' expect(page).to have_content 'Issues 1'
end end
context 'when cloning with notes', :aggregate_failures do
it 'clones the issue with all notes' do
add_note("Some random note")
add_note("Another note")
add_note("/clone --with_notes #{target_project.full_path}")
expect(page).to have_content "Cloned this issue to #{target_project.full_path}."
expect(issue.reload).to be_open
visit project_issue_path(target_project, issue)
expect(page).to have_content 'Issues 1'
expect(page).to have_content 'Some random note'
expect(page).to have_content 'Another note'
end
it 'returns an error if the params are malformed' do
# Note that this is missing one `-`
add_note("/clone -with_notes #{target_project.full_path}")
wait_for_requests
expect(page).to have_content 'Failed to clone this issue: wrong parameters.'
expect(issue.reload).to be_open
end
end
end end
context 'when the project is valid but the user not authorized' do context 'when the project is valid but the user not authorized' do
......
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