diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb index 25e90036a539f5d90784fc604d6d5906e394ea37..7c17b04bbc3f4eb68e74b85a21bf173dd7acc83e 100644 --- a/app/models/legacy_diff_note.rb +++ b/app/models/legacy_diff_note.rb @@ -13,7 +13,7 @@ class LegacyDiffNote < Note validates :line_code, presence: true, line_code: true - before_create :set_diff + before_create :set_diff, unless: :skip_setting_st_diff? def discussion_class(*) LegacyDiffDiscussion @@ -90,6 +90,10 @@ def set_diff self.st_diff = diff.to_hash if diff end + def skip_setting_st_diff? + st_diff.present? && importing? && Feature.enabled?(:skip_legacy_diff_note_callback_on_import, default_enabled: :yaml) + end + def diff_for_line_code attributes = { noteable_type: noteable_type, diff --git a/config/feature_flags/development/skip_legacy_diff_note_callback_on_import.yml b/config/feature_flags/development/skip_legacy_diff_note_callback_on_import.yml new file mode 100644 index 0000000000000000000000000000000000000000..43dcbee8f75d49ae2cbe46f1b74beff8369fc168 --- /dev/null +++ b/config/feature_flags/development/skip_legacy_diff_note_callback_on_import.yml @@ -0,0 +1,8 @@ +--- +name: skip_legacy_diff_note_callback_on_import +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72897 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/343666 +milestone: '14.5' +type: development +group: group::import +default_enabled: false diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb index ee3bbf186b926151c9cd714e8229e700d7696003..b9e1bd62f33757aa67fe2616ac41d6222fe533e0 100644 --- a/spec/models/legacy_diff_note_spec.rb +++ b/spec/models/legacy_diff_note_spec.rb @@ -8,4 +8,58 @@ it { is_expected.to eq('note') } end + + describe 'callbacks' do + describe '#set_diff' do + let(:note) do + build(:legacy_diff_note_on_merge_request, st_diff: '_st_diff_').tap do |record| + record.instance_variable_set(:@diff, {}) + end + end + + context 'when not importing' do + it 'updates st_diff' do + note.save!(validate: false) + + expect(note.st_diff).to eq({}) + end + end + + context 'when importing' do + before do + note.importing = true + end + + it 'does not update st_diff' do + note.save!(validate: false) + + expect(note.st_diff).to eq('_st_diff_') + end + + context 'when feature flag is false' do + before do + stub_feature_flags(skip_legacy_diff_note_callback_on_import: false) + end + + it 'updates st_diff' do + note.save!(validate: false) + + expect(note.st_diff).to eq({}) + end + end + + context 'when st_diff is blank' do + before do + note.st_diff = nil + end + + it 'updates st_diff' do + note.save!(validate: false) + + expect(note.st_diff).to eq({}) + end + end + end + end + end end