Merge branch 'id-fix-merge-ref-comments-for-removals' into 'master'

Fix merge ref head comments for removed lines

See merge request gitlab-org/gitlab!37755

app/services/discussions/capture_diff_note_position_service.rb

@@ -50,9 +50,9 @@ module Discussions
       merge_ref_head = merge_request.merge_ref_head
       return unless merge_ref_head
 
-      start_sha, base_sha = merge_ref_head.parent_ids
+      start_sha, _ = merge_ref_head.parent_ids
       new_diff_refs = Gitlab::Diff::DiffRefs.new(
-        base_sha: base_sha,
+        base_sha: start_sha,
         start_sha: start_sha,
         head_sha: merge_ref_head.id)
 

changelogs/unreleased/id-fix-merge-ref-comments-for-removals.yml

+---
+title: Fix merge ref head comments for removed lines
+merge_request: 37755
+author:
+type: fixed

spec/services/discussions/capture_diff_note_positions_service_spec.rb

@@ -12,24 +12,24 @@ RSpec.describe Discussions::CaptureDiffNotePositionsService do
     let(:offset) { 30 }
     let(:first_new_line) { 508 }
     let(:second_new_line) { 521 }
+    let(:third_removed_line) { 1240 }
 
     let(:service) { described_class.new(merge_request) }
 
-    def build_position(new_line, diff_refs)
+    def build_position(diff_refs, new_line: nil, old_line: nil)
       path = 'files/markdown/ruby-style-guide.md'
       Gitlab::Diff::Position.new(old_path: path, new_path: path,
-        new_line: new_line, diff_refs: diff_refs)
+        new_line: new_line, old_line: old_line, diff_refs: diff_refs)
     end
 
-    def note_for(new_line)
-      position = build_position(new_line, merge_request.diff_refs)
+    def note_for(new_line: nil, old_line: nil)
+      position = build_position(merge_request.diff_refs, new_line: new_line, old_line: old_line)
       create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request)
     end
 
-    def verify_diff_note_position!(note, line)
-      id, old_line, new_line = note.line_code.split('_')
+    def verify_diff_note_position!(note, new_line: nil, old_line: nil)
+      id, removed_line, added_line = note.line_code.split('_')
 
-      expect(new_line).to eq(line.to_s)
       expect(note.diff_note_positions.size).to eq(1)
 
       diff_position = note.diff_note_positions.last
@@ -38,12 +38,13 @@ RSpec.describe Discussions::CaptureDiffNotePositionsService do
         start_sha: merge_request.target_branch_sha,
         head_sha: merge_request.merge_ref_head.sha)
 
-      expect(diff_position.line_code).to eq("#{id}_#{old_line.to_i - offset}_#{new_line}")
-      expect(diff_position.position).to eq(build_position(new_line.to_i, diff_refs))
+      expect(diff_position.line_code).to eq("#{id}_#{removed_line.to_i - offset}_#{added_line}")
+      expect(diff_position.position).to eq(build_position(diff_refs, new_line: new_line, old_line: old_line))
     end
 
-    let!(:first_discussion_note) { note_for(first_new_line) }
-    let!(:second_discussion_note) { note_for(second_new_line) }
+    let!(:first_discussion_note) { note_for(new_line: first_new_line) }
+    let!(:second_discussion_note) { note_for(new_line: second_new_line) }
+    let!(:third_discussion_note) { note_for(old_line: third_removed_line) }
     let!(:second_discussion_another_note) do
       create(:diff_note_on_merge_request,
         project: project,
@@ -57,8 +58,9 @@ RSpec.describe Discussions::CaptureDiffNotePositionsService do
         MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request)
         service.execute
 
-        verify_diff_note_position!(first_discussion_note, first_new_line)
-        verify_diff_note_position!(second_discussion_note, second_new_line)
+        verify_diff_note_position!(first_discussion_note, new_line: first_new_line)
+        verify_diff_note_position!(second_discussion_note, new_line: second_new_line)
+        verify_diff_note_position!(third_discussion_note, old_line: third_removed_line - offset)
 
         expect(second_discussion_another_note.diff_note_positions).to be_empty
       end

spec/support/helpers/test_env.rb

@@ -76,7 +76,7 @@ module TestEnv
     'png-lfs'                            => 'fe42f41',
     'sha-starting-with-large-number'     => '8426165',
     'invalid-utf8-diff-paths'            => '99e4853',
-    'compare-with-merge-head-source'     => 'b5f4399',
+    'compare-with-merge-head-source'     => 'f20a03d',
     'compare-with-merge-head-target'     => '2f1e176'
   }.freeze