diff options
-rw-r--r-- | changelogs/unreleased/dm-fix-position-tracer-for-hidden-lines.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/diff/position_tracer.rb | 9 | ||||
-rw-r--r-- | spec/lib/gitlab/diff/position_tracer_spec.rb | 9 |
3 files changed, 15 insertions, 8 deletions
diff --git a/changelogs/unreleased/dm-fix-position-tracer-for-hidden-lines.yml b/changelogs/unreleased/dm-fix-position-tracer-for-hidden-lines.yml new file mode 100644 index 00000000000..d9ba26a0657 --- /dev/null +++ b/changelogs/unreleased/dm-fix-position-tracer-for-hidden-lines.yml @@ -0,0 +1,5 @@ +--- +title: Fix commenting on an existing discussion on an unchanged line that is no longer + in the diff +merge_request: +author: diff --git a/lib/gitlab/diff/position_tracer.rb b/lib/gitlab/diff/position_tracer.rb index 4d04f867268..c7542a8fabc 100644 --- a/lib/gitlab/diff/position_tracer.rb +++ b/lib/gitlab/diff/position_tracer.rb @@ -82,7 +82,7 @@ module Gitlab file_diff, old_line, new_line = results - Position.new( + new_position = Position.new( old_path: file_diff.old_path, new_path: file_diff.new_path, head_sha: new_diff_refs.head_sha, @@ -91,6 +91,13 @@ module Gitlab old_line: old_line, new_line: new_line ) + + # If a position is found, but is not actually contained in the diff, for example + # because it was an unchanged line in the context of a change that was undone, + # we cannot return this as a successful trace. + return unless new_position.diff_line(repository) + + new_position end private diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb index c166f83664a..a10a251dc4a 100644 --- a/spec/lib/gitlab/diff/position_tracer_spec.rb +++ b/spec/lib/gitlab/diff/position_tracer_spec.rb @@ -569,13 +569,8 @@ describe Gitlab::Diff::PositionTracer, lib: true do # 1 1 BB # 2 2 A - it "returns the new position" do - expect_new_position( - old_path: file_name, - new_path: new_file_name, - old_line: old_position.new_line, - new_line: old_position.new_line - ) + it "returns nil since the line doesn't exist in the new diffs anymore" do + expect(subject).to be_nil end end |