Merge branch '336054-diff-note-discussion-cache' into 'master'

Include position in the cache key of DiffDiscussion

See merge request gitlab-org/gitlab!66132

app/models/diff_discussion.rb

@@ -42,6 +42,13 @@ class DiffDiscussion < Discussion
     )
   end
 
+  def cache_key
+    [
+      super,
+      Digest::SHA1.hexdigest(position.to_json)
+    ].join(':')
+  end
+
   private
 
   def get_params

spec/models/diff_discussion_spec.rb

@@ -126,4 +126,13 @@ RSpec.describe DiffDiscussion do
       end
     end
   end
+
+  describe '#cache_key' do
+    it 'returns the cache key with the position sha' do
+      notes_sha = Digest::SHA1.hexdigest("#{diff_note.id}")
+      position_sha = Digest::SHA1.hexdigest(diff_note.position.to_json)
+
+      expect(subject.cache_key).to eq("#{described_class::CACHE_VERSION}:#{diff_note.latest_cached_markdown_version}:#{subject.id}:#{notes_sha}:#{diff_note.updated_at}::#{position_sha}")
+    end
+  end
 end

spec/requests/projects/merge_requests_discussions_spec.rb

@@ -158,6 +158,29 @@ RSpec.describe 'merge requests discussions' do
         end
       end
 
+      context 'when the diff note position changes' do
+        before do
+          # This replicates a position change wherein timestamps aren't updated
+          # which is why `Gitlab::Timeless.timeless` is utilized. This is the
+          # same approach being used in Discussions::UpdateDiffPositionService
+          # which is responsible for updating the positions of diff discussions
+          # when MR updates.
+          first_note.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
+          )
+
+          Gitlab::Timeless.timeless(first_note, &:save)
+        end
+
+        it_behaves_like 'cache miss' do
+          let(:changed_notes) { [first_note, second_note] }
+        end
+      end
+
       context 'when merge_request_discussion_cache is disabled' do
         before do
           stub_feature_flags(merge_request_discussion_cache: false)