Merge branch '31034-refactor-common-system-notes-service' into 'master'

Move `no_touching` to `CommonSystemNotesService`

See merge request gitlab-org/gitlab!20388

app/services/issuable/common_system_notes_service.rb

@@ -7,20 +7,24 @@ module Issuable
     def execute(issuable, old_labels: [], is_update: true)
       @issuable = issuable
 
-      if is_update
-        if issuable.previous_changes.include?('title')
-          create_title_change_note(issuable.previous_changes['title'].first)
+      # We disable touch so that created system notes do not update
+      # the noteable's updated_at field
+      ActiveRecord::Base.no_touching do
+        if is_update
+          if issuable.previous_changes.include?('title')
+            create_title_change_note(issuable.previous_changes['title'].first)
+          end
+
+          handle_description_change_note
+
+          handle_time_tracking_note if issuable.is_a?(TimeTrackable)
+          create_discussion_lock_note if issuable.previous_changes.include?('discussion_locked')
         end
 
-        handle_description_change_note
-
-        handle_time_tracking_note if issuable.is_a?(TimeTrackable)
-        create_discussion_lock_note if issuable.previous_changes.include?('discussion_locked')
+        create_due_date_note if issuable.previous_changes.include?('due_date')
+        create_milestone_note if issuable.previous_changes.include?('milestone_id')
+        create_labels_note(old_labels) if old_labels && issuable.labels != old_labels
       end
-
-      create_due_date_note if issuable.previous_changes.include?('due_date')
-      create_milestone_note if issuable.previous_changes.include?('milestone_id')
-      create_labels_note(old_labels) if old_labels && issuable.labels != old_labels
     end
 
     private

app/services/issuable_base_service.rb

@@ -164,9 +164,7 @@ class IssuableBaseService < BaseService
     before_create(issuable)
 
     if issuable.save
-      ActiveRecord::Base.no_touching do
-        Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, is_update: false)
-      end
+      Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, is_update: false)
 
       after_create(issuable)
       execute_hooks(issuable)
@@ -227,10 +225,7 @@ class IssuableBaseService < BaseService
       ensure_milestone_available(issuable)
 
       if issuable.with_transaction_returning_status { issuable.save(touch: should_touch) }
-        # We do not touch as it will affect a update on updated_at field
-        ActiveRecord::Base.no_touching do
-          Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: old_associations[:labels])
-        end
+        Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: old_associations[:labels])
 
         handle_changes(issuable, old_associations: old_associations)
 
@@ -264,10 +259,7 @@ class IssuableBaseService < BaseService
       before_update(issuable, skip_spam_check: true)
 
       if issuable.with_transaction_returning_status { issuable.save }
-        # We do not touch as it will affect a update on updated_at field
-        ActiveRecord::Base.no_touching do
-          Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: nil)
-        end
+        Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, old_labels: nil)
 
         handle_task_changes(issuable)
         invalidate_cache_counts(issuable, users: issuable.assignees.to_a)

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

@@ -10,8 +10,10 @@ module EE
       def execute(_issuable, old_labels: [], is_update: true)
         super
 
-        handle_weight_change_note
-        handle_date_change_note if is_update
+        ActiveRecord::Base.no_touching do
+          handle_weight_change_note
+          handle_date_change_note if is_update
+        end
       end
 
       private