Handle cache busting scenario where DiffNotePosition changes

We create/update the `DiffNotePosition` of a discussion (via the
first note ID), whenever a MR has been updated and we check for
mergeability of a MR. This means that when a merge request changes
the `DiffNotePosition` can possibly change.

Previously, when that happens, the cache will be stale and the
`positions` property we return in `discussions.json` will be outdated
and won't match anything on the new HEAD diff so discussions that
supposed to be still displayed won't be displayed.

This fix adds the SHA of `DiffNotePosition#position` records of
each `DiffDiscussion` to the cache key. This way, if they change,
the cache for the corresponding discussion will be invalidated.

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)")