Commit 4b54abfd authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Move no_touching to CommonSystemNotesService

This moves the call to ActiveRecord::Base.no_touching inside
the service so we won't forget to wrap calls to this service.

In Rails 6, touch resets dirty tracking causing updates to
multiple fields that would create multiple notes end up with
only one system note.
parent 1f651068
......@@ -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
......
......@@ -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)
......
......@@ -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
......
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