kotovalexarian-likes-gitlab
/
gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Refactor PositionTracer to support different types 

This is to prepare for supporing image type position tracing
This commit is contained in:
Patrick Bajao 2019-07-05 11:03:47 +00:00 committed by Douwe Maan
parent a1edc5a5cb
commit 6ef6693e37
14 changed files with 2533 additions and 2059 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/services/discussions/update_diff_position_service.rb
View File

@ -3,7 +3,8 @@
module Discussions
class UpdateDiffPositionService < BaseService
def execute(discussion)
result = tracer.trace(discussion.position)
old_position = discussion.position
result = tracer.trace(old_position)
return unless result
position = result[:position]

8
app/services/system_note_service.rb
View File

@ -268,11 +268,13 @@ module SystemNoteService
merge_request = discussion.noteable
diff_refs = change_position.diff_refs
version_index = merge_request.merge_request_diffs.viewable.count
position_on_text = change_position.on_text?
text_parts = ["changed this #{position_on_text ? 'line' : 'file'} in"]
text_parts = ["changed this line in"]
if version_params = merge_request.version_params_for(diff_refs)
line_code = change_position.line_code(project.repository)
url = url_helpers.diffs_project_merge_request_path(project, merge_request, version_params.merge(anchor: line_code))
repository = project.repository
anchor = position_on_text ? change_position.line_code(repository) : change_position.file_hash
url = url_helpers.diffs_project_merge_request_path(project, merge_request, version_params.merge(anchor: anchor))
text_parts << "[version #{version_index} of the diff](#{url})"
else

5
changelogs/unreleased/57793-fix-line-age.yml Normal file
View File

@ -0,0 +1,5 @@
---
title: Support note position tracing on an image
merge_request: 30158
author:
type: fixed

4
lib/gitlab/diff/position.rb
View File

@ -134,6 +134,10 @@ module Gitlab
@line_code ||= diff_file(repository)&.line_code_for_position(self)
end
def file_hash
@file_hash ||= Digest::SHA1.hexdigest(file_path)
end
def on_image?
position_type == 'image'
end

192
lib/gitlab/diff/position_tracer.rb
View File

@ -17,187 +17,13 @@ module Gitlab
@paths = paths
end
def trace(ab_position)
def trace(old_position)
return unless old_diff_refs&.complete? && new_diff_refs&.complete?
return unless ab_position.diff_refs == old_diff_refs
return unless old_position.diff_refs == old_diff_refs
# Suppose we have an MR with source branch `feature` and target branch `master`.
# When the MR was created, the head of `master` was commit A, and the
# head of `feature` was commit B, resulting in the original diff A->B.
# Since creation, `master` was updated to C.
# Now `feature` is being updated to D, and the newly generated MR diff is C->D.
# It is possible that C and D are direct descendants of A and B respectively,
# but this isn't necessarily the case as rebases and merges come into play.
#
# Suppose we have a diff note on the original diff A->B. Now that the MR
# is updated, we need to find out what line in C->D corresponds to the
# line the note was originally created on, so that we can update the diff note's
# records and continue to display it in the right place in the diffs.
# If we cannot find this line in the new diff, this means the diff note is now
# outdated, and we will display that fact to the user.
#
# In the new diff, the file the diff note was originally created on may
# have been renamed, deleted or even created, if the file existed in A and B,
# but was removed in C, and restored in D.
#
# Every diff note stores a Position object that defines a specific location,
# identified by paths and line numbers, within a specific diff, identified
# by start, head and base commit ids.
#
# For diff notes for diff A->B, the position looks like this:
# Position
# start_sha - ID of commit A
# head_sha - ID of commit B
# base_sha - ID of base commit of A and B
# old_path - path as of A (nil if file was newly created)
# new_path - path as of B (nil if file was deleted)
# old_line - line number as of A (nil if file was newly created)
# new_line - line number as of B (nil if file was deleted)
#
# We can easily update `start_sha` and `head_sha` to hold the IDs of
# commits C and D, and can trivially determine `base_sha` based on those,
# but need to find the paths and line numbers as of C and D.
#
# If the file was unchanged or newly created in A->B, the path as of D can be found
# by generating diff B->D ("head to head"), finding the diff file with
# `diff_file.old_path == position.new_path`, and taking `diff_file.new_path`.
# The path as of C can be found by taking diff C->D, finding the diff file
# with that same `new_path` and taking `diff_file.old_path`.
# The line number as of D can be found by using the LineMapper on diff B->D
# and providing the line number as of B.
# The line number as of C can be found by using the LineMapper on diff C->D
# and providing the line number as of D.
#
# If the file was deleted in A->B, the path as of C can be found
# by generating diff A->C ("base to base"), finding the diff file with
# `diff_file.old_path == position.old_path`, and taking `diff_file.new_path`.
# The path as of D can be found by taking diff C->D, finding the diff file
# with `old_path` set to that `diff_file.new_path` and taking `diff_file.new_path`.
# The line number as of C can be found by using the LineMapper on diff A->C
# and providing the line number as of A.
# The line number as of D can be found by using the LineMapper on diff C->D
# and providing the line number as of C.
strategy = old_position.on_text? ? LineStrategy : ImageStrategy
if ab_position.added?
trace_added_line(ab_position)
elsif ab_position.removed?
trace_removed_line(ab_position)
else # unchanged
trace_unchanged_line(ab_position)
end
end
private
def trace_added_line(ab_position)
b_path = ab_position.new_path
b_line = ab_position.new_line
bd_diff = bd_diffs.diff_file_with_old_path(b_path)
d_path = bd_diff&.new_path || b_path
d_line = LineMapper.new(bd_diff).old_to_new(b_line)
if d_line
cd_diff = cd_diffs.diff_file_with_new_path(d_path)
c_path = cd_diff&.old_path || d_path
c_line = LineMapper.new(cd_diff).new_to_old(d_line)
if c_line
# If the line is still in D but also in C, it has turned from an
# added line into an unchanged one.
new_position = position(cd_diff, c_line, d_line)
if valid_position?(new_position)
# If the line is still in the MR, we don't treat this as outdated.
{ position: new_position, outdated: false }
else
# If the line is no longer in the MR, we unfortunately cannot show
# the current state on the CD diff, so we treat it as outdated.
ac_diff = ac_diffs.diff_file_with_new_path(c_path)
{ position: position(ac_diff, nil, c_line), outdated: true }
end
else
# If the line is still in D and not in C, it is still added.
{ position: position(cd_diff, nil, d_line), outdated: false }
end
else
# If the line is no longer in D, it has been removed from the MR.
{ position: position(bd_diff, b_line, nil), outdated: true }
end
end
def trace_removed_line(ab_position)
a_path = ab_position.old_path
a_line = ab_position.old_line
ac_diff = ac_diffs.diff_file_with_old_path(a_path)
c_path = ac_diff&.new_path || a_path
c_line = LineMapper.new(ac_diff).old_to_new(a_line)
if c_line
cd_diff = cd_diffs.diff_file_with_old_path(c_path)
d_path = cd_diff&.new_path || c_path
d_line = LineMapper.new(cd_diff).old_to_new(c_line)
if d_line
# If the line is still in C but also in D, it has turned from a
# removed line into an unchanged one.
bd_diff = bd_diffs.diff_file_with_new_path(d_path)
{ position: position(bd_diff, nil, d_line), outdated: true }
else
# If the line is still in C and not in D, it is still removed.
{ position: position(cd_diff, c_line, nil), outdated: false }
end
else
# If the line is no longer in C, it has been removed outside of the MR.
{ position: position(ac_diff, a_line, nil), outdated: true }
end
end
def trace_unchanged_line(ab_position)
a_path = ab_position.old_path
a_line = ab_position.old_line
b_path = ab_position.new_path
b_line = ab_position.new_line
ac_diff = ac_diffs.diff_file_with_old_path(a_path)
c_path = ac_diff&.new_path || a_path
c_line = LineMapper.new(ac_diff).old_to_new(a_line)
bd_diff = bd_diffs.diff_file_with_old_path(b_path)
d_line = LineMapper.new(bd_diff).old_to_new(b_line)
cd_diff = cd_diffs.diff_file_with_old_path(c_path)
if c_line && d_line
# If the line is still in C and D, it is still unchanged.
new_position = position(cd_diff, c_line, d_line)
if valid_position?(new_position)
# If the line is still in the MR, we don't treat this as outdated.
{ position: new_position, outdated: false }
else
# If the line is no longer in the MR, we unfortunately cannot show
# the current state on the CD diff or any change on the BD diff,
# so we treat it as outdated.
{ position: nil, outdated: true }
end
elsif d_line # && !c_line
# If the line is still in D but no longer in C, it has turned from
# an unchanged line into an added one.
# We don't treat this as outdated since the line is still in the MR.
{ position: position(cd_diff, nil, d_line), outdated: false }
else # !d_line && (c_line || !c_line)
# If the line is no longer in D, it has turned from an unchanged line
# into a removed one.
{ position: position(bd_diff, b_line, nil), outdated: true }
end
strategy.new(self).trace(old_position)
end
def ac_diffs
@ -216,18 +42,12 @@ module Gitlab
@cd_diffs ||= compare(new_diff_refs.start_sha, new_diff_refs.head_sha)
end
private
def compare(start_sha, head_sha, straight: false)
compare = CompareService.new(project, head_sha).execute(project, start_sha, straight: straight)
compare.diffs(paths: paths, expanded: true)
end
def position(diff_file, old_line, new_line)
Position.new(diff_file: diff_file, old_line: old_line, new_line: new_line)
end
def valid_position?(position)
!!position.diff_line(project.repository)
end
end
end
end

26
lib/gitlab/diff/position_tracer/base_strategy.rb Normal file
View File

@ -0,0 +1,26 @@
# frozen_string_literal: true
module Gitlab
module Diff
class PositionTracer
class BaseStrategy
attr_reader :tracer
delegate \
:project,
:ac_diffs,
:bd_diffs,
:cd_diffs,
to: :tracer
def initialize(tracer)
@tracer = tracer
end
def trace(position)
raise NotImplementedError
end
end
end
end
end

50
lib/gitlab/diff/position_tracer/image_strategy.rb Normal file
View File

@ -0,0 +1,50 @@
# frozen_string_literal: true
module Gitlab
module Diff
class PositionTracer
class ImageStrategy < BaseStrategy
def trace(position)
b_path = position.new_path
# If file exists in B->D (e.g. updated, renamed, removed), let the
# note become outdated.
bd_diff = bd_diffs.diff_file_with_old_path(b_path)
return { position: new_position(position, bd_diff), outdated: true } if bd_diff
# If file still exists in the new diff, update the position.
cd_diff = cd_diffs.diff_file_with_new_path(bd_diff&.new_path || b_path)
return { position: new_position(position, cd_diff), outdated: false } if cd_diff
# If file exists in A->C (e.g. rebased and same changes were present
# in target branch), let the note become outdated.
ac_diff = ac_diffs.diff_file_with_old_path(position.old_path)
return { position: new_position(position, ac_diff), outdated: true } if ac_diff
# If ever there's a case that the file no longer exists in any diff,
# don't set a change position and let the note become outdated.
#
# This should never happen given the file should exist in one of the
# diffs above.
{ outdated: true }
end
private
def new_position(position, diff_file)
Position.new(
diff_file: diff_file,
x: position.x,
y: position.y,
width: position.width,
height: position.height,
position_type: position.position_type
)
end
end
end
end
end

201
lib/gitlab/diff/position_tracer/line_strategy.rb Normal file
View File

@ -0,0 +1,201 @@
# frozen_string_literal: true
module Gitlab
module Diff
class PositionTracer
class LineStrategy < BaseStrategy
def trace(position)
# Suppose we have an MR with source branch `feature` and target branch `master`.
# When the MR was created, the head of `master` was commit A, and the
# head of `feature` was commit B, resulting in the original diff A->B.
# Since creation, `master` was updated to C.
# Now `feature` is being updated to D, and the newly generated MR diff is C->D.
# It is possible that C and D are direct descendants of A and B respectively,
# but this isn't necessarily the case as rebases and merges come into play.
#
# Suppose we have a diff note on the original diff A->B. Now that the MR
# is updated, we need to find out what line in C->D corresponds to the
# line the note was originally created on, so that we can update the diff note's
# records and continue to display it in the right place in the diffs.
# If we cannot find this line in the new diff, this means the diff note is now
# outdated, and we will display that fact to the user.
#
# In the new diff, the file the diff note was originally created on may
# have been renamed, deleted or even created, if the file existed in A and B,
# but was removed in C, and restored in D.
#
# Every diff note stores a Position object that defines a specific location,
# identified by paths and line numbers, within a specific diff, identified
# by start, head and base commit ids.
#
# For diff notes for diff A->B, the position looks like this:
# Position
# start_sha - ID of commit A
# head_sha - ID of commit B
# base_sha - ID of base commit of A and B
# old_path - path as of A (nil if file was newly created)
# new_path - path as of B (nil if file was deleted)
# old_line - line number as of A (nil if file was newly created)
# new_line - line number as of B (nil if file was deleted)
#
# We can easily update `start_sha` and `head_sha` to hold the IDs of
# commits C and D, and can trivially determine `base_sha` based on those,
# but need to find the paths and line numbers as of C and D.
#
# If the file was unchanged or newly created in A->B, the path as of D can be found
# by generating diff B->D ("head to head"), finding the diff file with
# `diff_file.old_path == position.new_path`, and taking `diff_file.new_path`.
# The path as of C can be found by taking diff C->D, finding the diff file
# with that same `new_path` and taking `diff_file.old_path`.
# The line number as of D can be found by using the LineMapper on diff B->D
# and providing the line number as of B.
# The line number as of C can be found by using the LineMapper on diff C->D
# and providing the line number as of D.
#
# If the file was deleted in A->B, the path as of C can be found
# by generating diff A->C ("base to base"), finding the diff file with
# `diff_file.old_path == position.old_path`, and taking `diff_file.new_path`.
# The path as of D can be found by taking diff C->D, finding the diff file
# with `old_path` set to that `diff_file.new_path` and taking `diff_file.new_path`.
# The line number as of C can be found by using the LineMapper on diff A->C
# and providing the line number as of A.
# The line number as of D can be found by using the LineMapper on diff C->D
# and providing the line number as of C.
if position.added?
trace_added_line(position)
elsif position.removed?
trace_removed_line(position)
else # unchanged
trace_unchanged_line(position)
end
end
private
def trace_added_line(position)
b_path = position.new_path
b_line = position.new_line
bd_diff = bd_diffs.diff_file_with_old_path(b_path)
d_path = bd_diff&.new_path || b_path
d_line = LineMapper.new(bd_diff).old_to_new(b_line)
if d_line
cd_diff = cd_diffs.diff_file_with_new_path(d_path)
c_path = cd_diff&.old_path || d_path
c_line = LineMapper.new(cd_diff).new_to_old(d_line)
if c_line
# If the line is still in D but also in C, it has turned from an
# added line into an unchanged one.
new_position = new_position(cd_diff, c_line, d_line)
if valid_position?(new_position)
# If the line is still in the MR, we don't treat this as outdated.
{ position: new_position, outdated: false }
else
# If the line is no longer in the MR, we unfortunately cannot show
# the current state on the CD diff, so we treat it as outdated.
ac_diff = ac_diffs.diff_file_with_new_path(c_path)
{ position: new_position(ac_diff, nil, c_line), outdated: true }
end
else
# If the line is still in D and not in C, it is still added.
{ position: new_position(cd_diff, nil, d_line), outdated: false }
end
else
# If the line is no longer in D, it has been removed from the MR.
{ position: new_position(bd_diff, b_line, nil), outdated: true }
end
end
def trace_removed_line(position)
a_path = position.old_path
a_line = position.old_line
ac_diff = ac_diffs.diff_file_with_old_path(a_path)
c_path = ac_diff&.new_path || a_path
c_line = LineMapper.new(ac_diff).old_to_new(a_line)
if c_line
cd_diff = cd_diffs.diff_file_with_old_path(c_path)
d_path = cd_diff&.new_path || c_path
d_line = LineMapper.new(cd_diff).old_to_new(c_line)
if d_line
# If the line is still in C but also in D, it has turned from a
# removed line into an unchanged one.
bd_diff = bd_diffs.diff_file_with_new_path(d_path)
{ position: new_position(bd_diff, nil, d_line), outdated: true }
else
# If the line is still in C and not in D, it is still removed.
{ position: new_position(cd_diff, c_line, nil), outdated: false }
end
else
# If the line is no longer in C, it has been removed outside of the MR.
{ position: new_position(ac_diff, a_line, nil), outdated: true }
end
end
def trace_unchanged_line(position)
a_path = position.old_path
a_line = position.old_line
b_path = position.new_path
b_line = position.new_line
ac_diff = ac_diffs.diff_file_with_old_path(a_path)
c_path = ac_diff&.new_path || a_path
c_line = LineMapper.new(ac_diff).old_to_new(a_line)
bd_diff = bd_diffs.diff_file_with_old_path(b_path)
d_line = LineMapper.new(bd_diff).old_to_new(b_line)
cd_diff = cd_diffs.diff_file_with_old_path(c_path)
if c_line && d_line
# If the line is still in C and D, it is still unchanged.
new_position = new_position(cd_diff, c_line, d_line)
if valid_position?(new_position)
# If the line is still in the MR, we don't treat this as outdated.
{ position: new_position, outdated: false }
else
# If the line is no longer in the MR, we unfortunately cannot show
# the current state on the CD diff or any change on the BD diff,
# so we treat it as outdated.
{ position: nil, outdated: true }
end
elsif d_line # && !c_line
# If the line is still in D but no longer in C, it has turned from
# an unchanged line into an added one.
# We don't treat this as outdated since the line is still in the MR.
{ position: new_position(cd_diff, nil, d_line), outdated: false }
else # !d_line && (c_line || !c_line)
# If the line is no longer in D, it has turned from an unchanged line
# into a removed one.
{ position: new_position(bd_diff, b_line, nil), outdated: true }
end
end
def new_position(diff_file, old_line, new_line)
Position.new(
diff_file: diff_file,
old_line: old_line,
new_line: new_line
)
end
def valid_position?(position)
!!position.diff_line(project.repository)
end
end
end
end
end

13
spec/lib/gitlab/diff/position_spec.rb
View File

@ -610,4 +610,17 @@ describe Gitlab::Diff::Position do
it_behaves_like "diff position json"
end
end
describe "#file_hash" do
subject do
described_class.new(
old_path: "image.jpg",
new_path: "image.jpg"
)
end
it "returns SHA1 representation of the file_path" do
expect(subject.file_hash).to eq(Digest::SHA1.hexdigest(subject.file_path))
end
end
end

238
spec/lib/gitlab/diff/position_tracer/image_strategy_spec.rb Normal file
View File

@ -0,0 +1,238 @@
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Diff::PositionTracer::ImageStrategy do
include PositionTracerHelpers
let(:project) { create(:project, :repository) }
let(:current_user) { project.owner }
let(:file_name) { 'test-file' }
let(:new_file_name) { "#{file_name}-new" }
let(:second_file_name) { "#{file_name}-2" }
let(:branch_name) { 'position-tracer-test' }
let(:old_position) { position(old_path: file_name, new_path: file_name, position_type: 'image') }
let(:tracer) do
Gitlab::Diff::PositionTracer.new(
project: project,
old_diff_refs: old_diff_refs,
new_diff_refs: new_diff_refs
)
end
let(:strategy) { described_class.new(tracer) }
subject { strategy.trace(old_position) }
let(:initial_commit) do
project.commit(create_branch(branch_name, 'master')[:branch].name)
end
describe '#trace' do
describe 'diff scenarios' do
let(:create_file_commit) do
initial_commit
create_file(
branch_name,
file_name,
Base64.encode64('content')
)
end
let(:update_file_commit) do
create_file_commit
update_file(
branch_name,
file_name,
Base64.encode64('updatedcontent')
)
end
let(:update_file_again_commit) do
update_file_commit
update_file(
branch_name,
file_name,
Base64.encode64('updatedcontentagain')
)
end
let(:delete_file_commit) do
create_file_commit
delete_file(branch_name, file_name)
end
let(:rename_file_commit) do
delete_file_commit
create_file(
branch_name,
new_file_name,
Base64.encode64('renamedcontent')
)
end
let(:create_second_file_commit) do
create_file_commit
create_file(
branch_name,
second_file_name,
Base64.encode64('morecontent')
)
end
let(:create_another_file_commit) do
create_file(
branch_name,
second_file_name,
Base64.encode64('morecontent')
)
end
let(:update_another_file_commit) do
update_file(
branch_name,
second_file_name,
Base64.encode64('updatedmorecontent')
)
end
context 'when the file was created in the old diff' do
context 'when the file is unchanged between the old and the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(initial_commit, create_second_file_commit) }
it 'returns the new position' do
expect_new_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was updated between the old and the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(initial_commit, update_file_commit) }
let(:change_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was renamed in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(initial_commit, rename_file_commit) }
let(:change_diff_refs) { diff_refs(create_file_commit, rename_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was removed in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(initial_commit, delete_file_commit) }
let(:change_diff_refs) { diff_refs(create_file_commit, delete_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file is unchanged in the new diff' do
let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
let(:new_diff_refs) { diff_refs(create_another_file_commit, update_another_file_commit) }
let(:change_diff_refs) { diff_refs(initial_commit, create_another_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
end
context 'when the file was changed in the old diff' do
context 'when the file is unchanged in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_file_commit, create_second_file_commit) }
it 'returns the new position' do
expect_new_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was updated in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) }
let(:change_diff_refs) { diff_refs(update_file_commit, update_file_again_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was renamed in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_file_commit, rename_file_commit) }
let(:change_diff_refs) { diff_refs(update_file_commit, rename_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file was removed in between the old and the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_file_commit, delete_file_commit) }
let(:change_diff_refs) { diff_refs(update_file_commit, delete_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
context 'when the file is unchanged in the new diff' do
let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
let(:new_diff_refs) { diff_refs(create_another_file_commit, update_another_file_commit) }
let(:change_diff_refs) { diff_refs(create_file_commit, create_another_file_commit) }
it 'returns the position of the change' do
expect_change_position(
old_path: file_name,
new_path: file_name
)
end
end
end
end
end
end

1805
spec/lib/gitlab/diff/position_tracer/line_strategy_spec.rb Normal file
View File

File diff suppressed because it is too large Load Diff

1930
spec/lib/gitlab/diff/position_tracer_spec.rb
View File

File diff suppressed because it is too large Load Diff

24
spec/services/system_note_service_spec.rb
View File

@ -1175,16 +1175,30 @@ describe SystemNoteService do
end
it 'links to the diff in the system note' do
expect(subject.note).to include('version 1')
diff_id = merge_request.merge_request_diff.id
line_code = change_position.line_code(project.repository)
expect(subject.note).to include(diffs_project_merge_request_path(project, merge_request, diff_id: diff_id, anchor: line_code))
link = diffs_project_merge_request_path(project, merge_request, diff_id: diff_id, anchor: line_code)
expect(subject.note).to eq("changed this line in [version 1 of the diff](#{link})")
end
context 'discussion is on an image' do
let(:discussion) { create(:image_diff_note_on_merge_request, project: project).to_discussion }
it 'links to the diff in the system note' do
diff_id = merge_request.merge_request_diff.id
file_hash = change_position.file_hash
link = diffs_project_merge_request_path(project, merge_request, diff_id: diff_id, anchor: file_hash)
expect(subject.note).to eq("changed this file in [version 1 of the diff](#{link})")
end
end
end
context 'when the change_position is invalid for the discussion' do
let(:change_position) { project.commit(sample_commit.id) }
context 'when the change_position does not point to a valid version' do
before do
allow(merge_request).to receive(:version_params_for).and_return(nil)
end
it 'creates a new note in the discussion' do
# we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded.

93
spec/support/helpers/position_tracer_helpers.rb Normal file
View File

@ -0,0 +1,93 @@
# frozen_string_literal: true
module PositionTracerHelpers
def diff_refs(base_commit, head_commit)
Gitlab::Diff::DiffRefs.new(base_sha: base_commit.id, head_sha: head_commit.id)
end
def position(attrs = {})
attrs.reverse_merge!(
diff_refs: old_diff_refs
)
Gitlab::Diff::Position.new(attrs)
end
def expect_new_position(attrs, result = subject)
aggregate_failures("expect new position #{attrs.inspect}") do
if attrs.nil?
expect(result[:outdated]).to be_truthy
else
new_position = result[:position]
expect(result[:outdated]).to be_falsey
expect(new_position).not_to be_nil
expect(new_position.diff_refs).to eq(new_diff_refs)
attrs.each do |attr, value|
expect(new_position.send(attr)).to eq(value)
end
end
end
end
def expect_change_position(attrs, result = subject)
aggregate_failures("expect change position #{attrs.inspect}") do
change_position = result[:position]
expect(result[:outdated]).to be_truthy
if attrs.nil? || attrs.empty?
expect(change_position).to be_nil
else
expect(change_position).not_to be_nil
expect(change_position.diff_refs).to eq(change_diff_refs)
attrs.each do |attr, value|
expect(change_position.send(attr)).to eq(value)
end
end
end
end
def create_branch(new_name, branch_name)
CreateBranchService.new(project, current_user).execute(new_name, branch_name)
end
def create_file(branch_name, file_name, content)
Files::CreateService.new(
project,
current_user,
start_branch: branch_name,
branch_name: branch_name,
commit_message: "Create file",
file_path: file_name,
file_content: content
).execute
project.commit(branch_name)
end
def update_file(branch_name, file_name, content)
Files::UpdateService.new(
project,
current_user,
start_branch: branch_name,
branch_name: branch_name,
commit_message: "Update file",
file_path: file_name,
file_content: content
).execute
project.commit(branch_name)
end
def delete_file(branch_name, file_name)
Files::DeleteService.new(
project,
current_user,
start_branch: branch_name,
branch_name: branch_name,
commit_message: "Delete file",
file_path: file_name
).execute
project.commit(branch_name)
end
end