Commit d91c949f authored by Markus Koller's avatar Markus Koller

Merge branch 'id-fix-nil-line-codes-for-diff-positions' into 'master'

Avoid creating diff position when line-code is nil

See merge request gitlab-org/gitlab!40089
parents 86bc0e2e c5746169
...@@ -19,13 +19,16 @@ module Discussions ...@@ -19,13 +19,16 @@ module Discussions
position = result[:position] position = result[:position]
return unless position return unless position
line_code = position.line_code(project.repository)
return unless line_code
# Currently position data is copied across all notes of a discussion # Currently position data is copied across all notes of a discussion
# It makes sense to store a position only for the first note instead # It makes sense to store a position only for the first note instead
# Within the newly introduced table we can start doing just that # Within the newly introduced table we can start doing just that
DiffNotePosition.create_or_update_for(discussion.notes.first, DiffNotePosition.create_or_update_for(discussion.notes.first,
diff_type: :head, diff_type: :head,
position: position, position: position,
line_code: position.line_code(project.repository)) line_code: line_code)
end end
private private
......
---
title: Avoid creating diff position when line-code is nil
merge_request: 40089
author:
type: fixed
...@@ -29,18 +29,33 @@ RSpec.describe Discussions::CaptureDiffNotePositionService do ...@@ -29,18 +29,33 @@ RSpec.describe Discussions::CaptureDiffNotePositionService do
end end
end end
context 'when position tracer returned nil position' do context 'when position tracer returned position' do
let!(:note) { create(:diff_note_on_merge_request) } let!(:note) { create(:diff_note_on_merge_request) }
let(:paths) { ['files/any_file.txt'] } let(:paths) { ['files/any_file.txt'] }
it 'does not create diff note position' do before do
expect(note.noteable).to receive(:merge_ref_head).and_return(double.as_null_object) expect(note.noteable).to receive(:merge_ref_head).and_return(double.as_null_object)
expect_next_instance_of(Gitlab::Diff::PositionTracer) do |tracer| expect_next_instance_of(Gitlab::Diff::PositionTracer) do |tracer|
expect(tracer).to receive(:trace).and_return({ position: nil }) expect(tracer).to receive(:trace).and_return({ position: position })
end
end
context 'which is nil' do
let(:position) { nil }
it 'does not create diff note position' do
expect(subject.execute(note.discussion)).to eq(nil)
expect(note.diff_note_positions).to be_empty
end
end end
context 'which does not have a corresponding line' do
let(:position) { double(line_code: nil) }
it 'does not create diff note position' do
expect(subject.execute(note.discussion)).to eq(nil) expect(subject.execute(note.discussion)).to eq(nil)
expect(note.diff_note_positions).to be_empty expect(note.diff_note_positions).to be_empty
end end
end end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment