Merge branch '9421-clone_with_notes-quickaction' into 'master'

Implement Clone With Notes quick_action

See merge request gitlab-org/gitlab!48539

app/services/issues/clone_service.rb

@@ -4,10 +4,11 @@ module Issues
   class CloneService < Issuable::Clone::BaseService
     CloneError = Class.new(StandardError)
 
-    def execute(issue, target_project)
+    def execute(issue, target_project, with_notes: false)
       @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!')
       end
 
@@ -17,6 +18,8 @@ module Issues
 
       super(issue, target_project)
 
+      notify_participants
+
       queue_copy_designs
 
       new_entity
@@ -25,12 +28,14 @@ module Issues
     private
 
     attr_reader :target_project
+    attr_reader :with_notes
 
     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.
       update_new_entity_description
       update_new_entity_attributes
       copy_award_emoji
+      copy_notes if with_notes
     end
 
     def update_old_entity
@@ -51,7 +56,7 @@ module Issues
 
       # 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.
-      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
 
     def queue_copy_designs
@@ -66,6 +71,10 @@ module Issues
       log_error(response.message) if response.error?
     end
 
+    def notify_participants
+      notification_service.async.issue_cloned(original_entity, new_entity, current_user)
+    end
+
     def add_note_from
       SystemNoteService.noteable_cloned(new_entity, target_project,
         original_entity, current_user,

app/services/issues/update_service.rb

@@ -129,13 +129,14 @@ module Issues
 
     def clone_issue(issue)
       target_project = params.delete(:target_clone_project)
+      with_notes = params.delete(:clone_with_notes)
 
       return unless 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.
       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
 
     def create_merge_request_from_quick_action

changelogs/unreleased/9421-clone_with_notes-quickaction.yml

+---
+title: Implement a /clone_with_notes quick-action to quickly clone an Issue will all
+  its notes
+merge_request: 48539
+author:
+type: added

doc/user/project/quick_actions.md

@@ -34,7 +34,7 @@ The following quick actions are applicable to descriptions, discussions and thre
 | `/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)** |
 | `/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.                                                                                                                           |
 | `/confidential`                       | ✓     |               |      | Make confidential.                                                                                                               |
 | `/copy_metadata <!merge_request>`     | ✓     | ✓             |      | Copy labels and milestone from another merge request in the project.                                                             |

lib/gitlab/quick_actions/issue_actions.rb

@@ -106,17 +106,28 @@ module Gitlab
         explanation do |project = quick_action_target.project.full_path|
           _("Clones this issue, without comments, to %{project}.") % { project: project }
         end
-        params 'path/to/project'
+        params 'path/to/project [--with_notes]'
         types Issue
         condition do
           quick_action_target.persisted? &&
             current_user.can?(:"admin_#{quick_action_target.to_ability_name}", project)
         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
 
           if target_project.present?
             @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 }
           else

locale/gitlab.pot

@@ -11469,6 +11469,9 @@ msgstr ""
 msgid "Failed to clone this issue because target project doesn't exist."
 msgstr ""
 
+msgid "Failed to clone this issue: wrong parameters."
+msgstr ""
+
 msgid "Failed to create Merge Request. Please try again."
 msgstr ""
 

spec/services/issues/clone_service_spec.rb

@@ -15,10 +15,12 @@ RSpec.describe Issues::CloneService do
   let_it_be(:old_project) { create(:project, namespace: sub_group_1) }
   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)
   end
 
+  let(:with_notes) { false }
+
   subject(:clone_service) do
     described_class.new(old_project, user)
   end
@@ -35,7 +37,7 @@ RSpec.describe Issues::CloneService do
       include_context 'user can clone issue'
 
       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
           expect do
@@ -91,6 +93,16 @@ RSpec.describe Issues::CloneService do
         it 'does not set moved_issue' do
           expect(old_issue.moved?).to eq(false)
         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
 
       context 'issue with award emoji' do
@@ -200,6 +212,34 @@ RSpec.describe Issues::CloneService do
         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
         let_it_be(:new_project) { create(:project) }
         let!(:design) { create(:design, :with_lfs_file, issue: old_issue) }

spec/services/issues/update_service_spec.rb

@@ -979,15 +979,35 @@ RSpec.describe Issues::UpdateService, :mailer do
         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).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)
         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
       it 'raises an error for invalid move ids within a project' do
         opts = { move_between_ids: [9000, non_existing_record_id] }

spec/support/shared_examples/quick_actions/issue/clone_quick_action_shared_examples.rb

@@ -32,6 +32,34 @@ RSpec.shared_examples 'clone quick action' do
 
         expect(page).to have_content 'Issues 1'
       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
 
     context 'when the project is valid but the user not authorized' do