Merge branch '337741-discussions-cache-diff-note-positions' into 'master'

Handle cache busting scenario where DiffNotePosition changes

See merge request gitlab-org/gitlab!67584

app/models/diff_discussion.rb

@@ -43,9 +43,13 @@ class DiffDiscussion < Discussion
   end
 
   def cache_key
+    positions_json = diff_note_positions.map { |dnp| dnp.position.to_json }
+    positions_sha = Digest::SHA1.hexdigest(positions_json.join(':')) if positions_json.any?
+
     [
       super,
-      Digest::SHA1.hexdigest(position.to_json)
+      Digest::SHA1.hexdigest(position.to_json),
+      positions_sha
     ].join(':')
   end
 

spec/models/diff_discussion_spec.rb

@@ -128,11 +128,20 @@ RSpec.describe DiffDiscussion do
   end
 
   describe '#cache_key' do
+    let(:notes_sha) { Digest::SHA1.hexdigest("#{diff_note.post_processed_cache_key}") }
+    let(:position_sha) { Digest::SHA1.hexdigest(diff_note.position.to_json) }
+
     it 'returns the cache key with the position sha' do
-      notes_sha = Digest::SHA1.hexdigest("#{diff_note.post_processed_cache_key}")
-      position_sha = Digest::SHA1.hexdigest(diff_note.position.to_json)
+      expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}::#{position_sha}:")
+    end
 
-      expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}::#{position_sha}")
+    context 'when first note of discussion has diff_note_position' do
+      let!(:diff_note_position) { create(:diff_note_position, note: diff_note) }
+      let(:positions_sha) { Digest::SHA1.hexdigest(diff_note_position.position.to_json) }
+
+      it 'includes sha of diff_note_positions position' do
+        expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{subject.id}:#{notes_sha}::#{position_sha}:#{positions_sha}")
+      end
     end
   end
 end

spec/requests/projects/merge_requests_discussions_spec.rb

@@ -182,6 +182,33 @@ RSpec.describe 'merge requests discussions' do
         end
       end
 
+      context 'when the HEAD diff note position changes' do
+        before do
+          # This replicates a DiffNotePosition change. This is the same approach
+          # being used in Discussions::CaptureDiffNotePositionService which is
+          # responsible for updating/creating DiffNotePosition of a diff discussions
+          # in relation to HEAD diff.
+          new_position = Gitlab::Diff::Position.new(
+            old_path: first_note.position.old_path,
+            new_path: first_note.position.new_path,
+            old_line: first_note.position.old_line,
+            new_line: first_note.position.new_line + 1,
+            diff_refs: first_note.position.diff_refs
+          )
+
+          DiffNotePosition.create_or_update_for(
+            first_note,
+            diff_type: :head,
+            position: new_position,
+            line_code: 'bd4b7bfff3a247ccf6e3371c41ec018a55230bcc_534_521'
+          )
+        end
+
+        it_behaves_like 'cache miss' do
+          let(:changed_notes) { [first_note, second_note] }
+        end
+      end
+
       context 'when author detail changes' do
         before do
           author.update!(name: "#{author.name} (Updated)")