Merge branch 'id-fix-diff-note-position-service' into 'master'

Fix CaptureDiffNotePositionService when position is nil

See merge request gitlab-org/gitlab!30269

app/services/discussions/capture_diff_note_position_service.rb

@@ -17,6 +17,7 @@ module Discussions
       return unless result
 
       position = result[:position]
+      return unless position
 
       # Currently position data is copied across all notes of a discussion
       # It makes sense to store a position only for the first note instead

spec/services/discussions/capture_diff_note_position_service_spec.rb

@@ -3,10 +3,11 @@
 require 'spec_helper'
 
 describe Discussions::CaptureDiffNotePositionService do
+  subject { described_class.new(note.noteable, paths) }
+
   context 'image note on diff' do
     let!(:note) { create(:image_diff_note_on_merge_request) }
-
-    subject { described_class.new(note.noteable, ['files/images/any_image.png']) }
+    let(:paths) { ['files/images/any_image.png'] }
 
     it 'is note affected by the service' do
       expect(Gitlab::Diff::PositionTracer).not_to receive(:new)
@@ -18,8 +19,7 @@ describe Discussions::CaptureDiffNotePositionService do
 
   context 'when empty paths are passed as a param' do
     let!(:note) { create(:diff_note_on_merge_request) }
-
-    subject { described_class.new(note.noteable, []) }
+    let(:paths) { [] }
 
     it 'does not calculate positons' do
       expect(Gitlab::Diff::PositionTracer).not_to receive(:new)
@@ -28,4 +28,19 @@ describe Discussions::CaptureDiffNotePositionService do
       expect(note.diff_note_positions).to be_empty
     end
   end
+
+  context 'when position tracer returned nil position' do
+    let!(:note) { create(:diff_note_on_merge_request) }
+    let(:paths) { ['files/any_file.txt'] }
+
+    it 'does not create diff note position' do
+      expect(note.noteable).to receive(:merge_ref_head).and_return(double.as_null_object)
+      expect_next_instance_of(Gitlab::Diff::PositionTracer) do |tracer|
+        expect(tracer).to receive(:trace).and_return({ position: nil })
+      end
+
+      expect(subject.execute(note.discussion)).to eq(nil)
+      expect(note.diff_note_positions).to be_empty
+    end
+  end
 end