Skip system notes when creating moved issue

We skip the creation of system notes when creating the newly moved issue
because we are copying over the system notes from the old issue

app/services/issuable_base_service.rb

@@ -136,7 +136,7 @@ class IssuableBaseService < BaseService
     {}
   end
 
-  def create(issuable)
+  def create(issuable, skip_system_notes: false)
     handle_quick_actions(issuable)
     filter_params(issuable)
 
@@ -153,7 +153,7 @@ class IssuableBaseService < BaseService
     end
 
     if issuable_saved
-      Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, is_update: false)
+      create_system_notes(issuable, is_update: false) unless skip_system_notes
 
       after_create(issuable)
       execute_hooks(issuable)
@@ -220,8 +220,9 @@ class IssuableBaseService < BaseService
       end
 
       if issuable_saved
-        Issuable::CommonSystemNotesService.new(project, current_user).execute(
-          issuable, old_labels: old_associations[:labels], old_milestone: old_associations[:milestone])
+        create_system_notes(
+          issuable, old_labels: old_associations[:labels], old_milestone: old_associations[:milestone]
+        )
 
         handle_changes(issuable, old_associations: old_associations)
 
@@ -255,7 +256,7 @@ class IssuableBaseService < BaseService
       before_update(issuable, skip_spam_check: true)
 
       if issuable.with_transaction_returning_status { issuable.save }
-        Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: nil)
+        create_system_notes(issuable, old_labels: nil)
 
         handle_task_changes(issuable)
         invalidate_cache_counts(issuable, users: issuable.assignees.to_a)
@@ -339,6 +340,10 @@ class IssuableBaseService < BaseService
     AwardEmojis::ToggleService.new(issuable, award, current_user).execute if award
   end
 
+  def create_system_notes(issuable, **options)
+    Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, **options)
+  end
+
   def associations_before_update(issuable)
     associations =
       {

app/services/issues/create_service.rb

@@ -5,13 +5,13 @@ module Issues
     include SpamCheckMethods
     include ResolveDiscussions
 
-    def execute
+    def execute(skip_system_notes: false)
       @issue = BuildService.new(project, current_user, params).execute
 
       filter_spam_check_params
       filter_resolve_discussion_params
 
-      create(@issue)
+      create(@issue, skip_system_notes: skip_system_notes)
     end
 
     def before_create(issue)

app/services/issues/move_service.rb

@@ -52,7 +52,10 @@ module Issues
                    }
 
       new_params = original_entity.serializable_hash.symbolize_keys.merge(new_params)
-      CreateService.new(@target_project, @current_user, new_params).execute
+
+      # 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)
     end
 
     def mark_as_moved

changelogs/unreleased/fix-system-notes-when-moving-issues.yml

+---
+title: Prevent duplicate system notes and events when an issue is moved
+merge_request: 41087
+author:
+type: fixed

ee/app/services/ee/issuable/common_system_notes_service.rb

@@ -11,7 +11,7 @@ module EE
         super
 
         ActiveRecord::Base.no_touching do
-          handle_weight_change(is_update)
+          handle_weight_change
           handle_iteration_change
 
           if is_update
@@ -43,13 +43,11 @@ module EE
         end
       end
 
-      def handle_weight_change(is_update)
+      def handle_weight_change
         return unless issuable.previous_changes.include?('weight')
 
         if weight_changes_tracking_enabled?
-          # Only create a resource event here if is_update is true to exclude the move issue operation.
-          # ResourceEvents for moved issues are written within AttributesRewriter.
-          EE::ResourceEvents::ChangeWeightService.new([issuable], current_user, Time.current).execute if is_update
+          EE::ResourceEvents::ChangeWeightService.new([issuable], current_user, Time.current).execute
         else
           ::SystemNoteService.change_weight_note(issuable, issuable.project, current_user)
         end

ee/changelogs/unreleased/fix-system-notes-when-moving-issues.yml

+---
+title: Create resource weight event when setting the weight during issue creation
+merge_request: 41087
+author:
+type: fixed

ee/spec/services/ee/issuable/common_system_notes_service_spec.rb

@@ -115,17 +115,13 @@ RSpec.describe Issuable::CommonSystemNotesService do
       issuable.update(weight: 5, health_status: 'at_risk')
     end
 
-    it 'does not create common system notes' do
-      expect { subject }.not_to change { issuable.notes.count }
-    end
-
     context 'when resource weight event tracking is enabled' do
       before do
         stub_feature_flags(track_issue_weight_change_events: true)
       end
 
-      it 'does not create a resource weight event' do
-        expect { subject }.not_to change { ResourceWeightEvent.count }
+      it 'creates a resource weight event' do
+        expect { subject }.to change { ResourceWeightEvent.count }
       end
 
       it 'does not create a system note' do

ee/spec/services/ee/issues/move_service_spec.rb

@@ -32,6 +32,7 @@ RSpec.describe Issues::MoveService do
     end
 
     context 'resource weight events' do
+      let(:old_issue) { create(:issue, project: old_project, author: user, weight: 5) }
       let!(:event1) { create(:resource_weight_event, issue: old_issue, weight: 1) }
       let!(:event2) { create(:resource_weight_event, issue: old_issue, weight: 42) }
       let!(:event3) { create(:resource_weight_event, issue: old_issue, weight: 5) }

spec/services/issues/create_service_spec.rb

@@ -29,6 +29,8 @@ RSpec.describe Issues::CreateService do
       end
 
       it 'creates the issue with the given params' do
+        expect(Issuable::CommonSystemNotesService).to receive_message_chain(:new, :execute)
+
         expect(issue).to be_persisted
         expect(issue.title).to eq('Awesome issue')
         expect(issue.assignees).to eq [assignee]
@@ -37,6 +39,16 @@ RSpec.describe Issues::CreateService do
         expect(issue.due_date).to eq Date.tomorrow
       end
 
+      context 'when skip_system_notes is true' do
+        let(:issue) { described_class.new(project, user, opts).execute(skip_system_notes: true) }
+
+        it 'does not call Issuable::CommonSystemNotesService' do
+          expect(Issuable::CommonSystemNotesService).not_to receive(:new)
+
+          issue
+        end
+      end
+
       it 'refreshes the number of open issues', :use_clean_rails_memory_store_caching do
         expect { issue }.to change { project.open_issues_count }.from(0).to(1)
       end

spec/services/issues/move_service_spec.rb

@@ -101,6 +101,41 @@ RSpec.describe Issues::MoveService do
         end
       end
 
+      context 'issue with milestone' do
+        let(:milestone) { create(:milestone, group: sub_group_1) }
+        let(:new_project) { create(:project, namespace: sub_group_1) }
+
+        let(:old_issue) do
+          create(:issue, title: title, description: description, project: old_project, author: author, milestone: milestone)
+        end
+
+        before do
+          create(:resource_milestone_event, issue: old_issue, milestone: milestone, action: :add)
+        end
+
+        it 'does not create extra milestone events' do
+          new_issue = move_service.execute(old_issue, new_project)
+
+          expect(new_issue.resource_milestone_events.count).to eq(old_issue.resource_milestone_events.count)
+        end
+      end
+
+      context 'issue with due date' do
+        let(:old_issue) do
+          create(:issue, title: title, description: description, project: old_project, author: author, due_date: '2020-01-10')
+        end
+
+        before do
+          SystemNoteService.change_due_date(old_issue, old_project, author, old_issue.due_date)
+        end
+
+        it 'does not create extra system notes' do
+          new_issue = move_service.execute(old_issue, new_project)
+
+          expect(new_issue.notes.count).to eq(old_issue.notes.count)
+        end
+      end
+
       context 'issue with assignee' do
         let_it_be(:assignee) { create(:user) }