570 lines
17 KiB
Ruby
570 lines
17 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
require 'spec_helper'
|
|
|
|
RSpec.describe DiffNote do
|
|
include RepoHelpers
|
|
|
|
let_it_be(:merge_request) { create(:merge_request) }
|
|
let_it_be(:project) { merge_request.project }
|
|
let_it_be(:commit) { project.commit(sample_commit.id) }
|
|
|
|
let_it_be(:path) { "files/ruby/popen.rb" }
|
|
|
|
let(:diff_refs) { merge_request.diff_refs }
|
|
let!(:position) do
|
|
Gitlab::Diff::Position.new(
|
|
old_path: path,
|
|
new_path: path,
|
|
old_line: nil,
|
|
new_line: 14,
|
|
diff_refs: diff_refs
|
|
)
|
|
end
|
|
|
|
let!(:new_position) do
|
|
Gitlab::Diff::Position.new(
|
|
old_path: path,
|
|
new_path: path,
|
|
old_line: 16,
|
|
new_line: 22,
|
|
diff_refs: diff_refs
|
|
)
|
|
end
|
|
|
|
subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }
|
|
|
|
describe 'validations' do
|
|
it_behaves_like 'a valid diff positionable note' do
|
|
subject { build(:diff_note_on_commit, project: project, commit_id: commit_id, position: position) }
|
|
end
|
|
|
|
it "is not valid when noteable is empty" do
|
|
note = build(:diff_note_on_merge_request, project: project, noteable: nil)
|
|
|
|
note.valid?
|
|
|
|
expect(note.errors[:noteable]).to include("doesn't support new-style diff notes")
|
|
end
|
|
|
|
context 'when importing' do
|
|
it "does not check if it's supported" do
|
|
note = build(:diff_note_on_merge_request, project: project, noteable: nil)
|
|
note.importing = true
|
|
note.valid?
|
|
|
|
expect(note.errors.full_messages).not_to include(
|
|
"Noteable doesn't support new-style diff notes"
|
|
)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#position=" do
|
|
context "when provided a string" do
|
|
it "sets the position" do
|
|
subject.position = new_position.to_json
|
|
|
|
expect(subject.position).to eq(new_position)
|
|
end
|
|
end
|
|
|
|
context "when provided a hash" do
|
|
it "sets the position" do
|
|
subject.position = new_position.to_h
|
|
|
|
expect(subject.position).to eq(new_position)
|
|
end
|
|
end
|
|
|
|
context "when provided a position object" do
|
|
it "sets the position" do
|
|
subject.position = new_position
|
|
|
|
expect(subject.position).to eq(new_position)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#original_position=" do
|
|
context "when provided a string" do
|
|
it "sets the original position" do
|
|
subject.original_position = new_position.to_json
|
|
|
|
expect(subject.original_position).to eq(new_position)
|
|
end
|
|
end
|
|
|
|
context "when provided a hash" do
|
|
it "sets the original position" do
|
|
subject.original_position = new_position.to_h
|
|
|
|
expect(subject.original_position).to eq(new_position)
|
|
end
|
|
end
|
|
|
|
context "when provided a position object" do
|
|
it "sets the original position" do
|
|
subject.original_position = new_position
|
|
|
|
expect(subject.original_position).to eq(new_position)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe '#create_diff_file callback' do
|
|
context 'merge request' do
|
|
let(:position) do
|
|
Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb",
|
|
new_path: "files/ruby/popen.rb",
|
|
old_line: nil,
|
|
new_line: 9,
|
|
diff_refs: merge_request.diff_refs)
|
|
end
|
|
|
|
subject { build(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }
|
|
|
|
let(:diff_file_from_repository) do
|
|
position.diff_file(project.repository)
|
|
end
|
|
|
|
let(:diff_file) do
|
|
diffs = merge_request.diffs
|
|
raw_diff = diffs.diffable.raw_diffs(diffs.diff_options.merge(paths: ['files/ruby/popen.rb'])).first
|
|
Gitlab::Diff::File.new(raw_diff,
|
|
repository: diffs.project.repository,
|
|
diff_refs: diffs.diff_refs,
|
|
fallback_diff_refs: diffs.fallback_diff_refs)
|
|
end
|
|
|
|
let(:diff_line) { diff_file.diff_lines.first }
|
|
|
|
let(:line_code) { '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14' }
|
|
|
|
before do
|
|
allow(subject.position).to receive(:line_code).and_return('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_14')
|
|
end
|
|
|
|
context 'when diffs are already created' do
|
|
before do
|
|
allow(subject).to receive(:created_at_diff?).and_return(true)
|
|
end
|
|
|
|
context 'when diff_file is found in persisted diffs' do
|
|
before do
|
|
allow(merge_request).to receive_message_chain(:diffs, :diff_files, :first).and_return(diff_file)
|
|
end
|
|
|
|
context 'when importing' do
|
|
before do
|
|
subject.importing = true
|
|
subject.line_code = line_code
|
|
end
|
|
|
|
context 'when diff_line is found in persisted diff_file' do
|
|
before do
|
|
allow(diff_file).to receive(:line_for_position).with(position).and_return(diff_line)
|
|
end
|
|
|
|
it 'creates a diff note file' do
|
|
subject.save!
|
|
expect(subject.note_diff_file).to be_present
|
|
end
|
|
end
|
|
|
|
context 'when diff_line is not found in persisted diff_file' do
|
|
before do
|
|
allow(diff_file).to receive(:line_for_position).and_return(nil)
|
|
end
|
|
|
|
it_behaves_like 'a valid diff note with after commit callback'
|
|
end
|
|
end
|
|
|
|
context 'when not importing' do
|
|
context 'when diff_line is not found' do
|
|
before do
|
|
allow(diff_file).to receive(:line_for_position).with(position).and_return(nil)
|
|
end
|
|
|
|
it 'raises an error' do
|
|
expect { subject.save! }.to raise_error(::DiffNote::NoteDiffFileCreationError,
|
|
"Failed to find diff line for: #{diff_file.file_path}, "\
|
|
"old_line: #{position.old_line}"\
|
|
", new_line: #{position.new_line}")
|
|
end
|
|
end
|
|
|
|
context 'when diff_line is found' do
|
|
before do
|
|
allow(diff_file).to receive(:line_for_position).with(position).and_return(diff_line)
|
|
end
|
|
|
|
it 'creates a diff note file' do
|
|
subject.save!
|
|
expect(subject.reload.note_diff_file).to be_present
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
context 'when diff file is not found in persisted diffs' do
|
|
before do
|
|
allow_next_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff) do |merge_request_diff|
|
|
allow(merge_request_diff).to receive(:diff_files).and_return([])
|
|
end
|
|
end
|
|
|
|
it_behaves_like 'a valid diff note with after commit callback'
|
|
end
|
|
end
|
|
|
|
context 'when diffs are not already created' do
|
|
before do
|
|
allow(subject).to receive(:created_at_diff?).and_return(false)
|
|
end
|
|
|
|
it_behaves_like 'a valid diff note with after commit callback'
|
|
end
|
|
|
|
it 'does not create diff note file if it is a reply' do
|
|
diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request)
|
|
|
|
expect { create(:diff_note_on_merge_request, noteable: merge_request, in_reply_to: diff_note) }
|
|
.not_to change(NoteDiffFile, :count)
|
|
end
|
|
end
|
|
|
|
context 'commit' do
|
|
let!(:diff_note) { create(:diff_note_on_commit, project: project) }
|
|
|
|
it 'creates a diff note file' do
|
|
expect(diff_note.reload.note_diff_file).to be_present
|
|
end
|
|
|
|
it 'does not create diff note file if it is a reply' do
|
|
expect { create(:diff_note_on_commit, in_reply_to: diff_note) }
|
|
.not_to change(NoteDiffFile, :count)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe '#diff_file', :clean_gitlab_redis_shared_state do
|
|
context 'when note_diff_file association exists' do
|
|
it 'returns persisted diff file data' do
|
|
diff_file = subject.diff_file
|
|
|
|
expect(diff_file.diff.to_hash.with_indifferent_access)
|
|
.to include(subject.note_diff_file.to_hash)
|
|
end
|
|
end
|
|
|
|
context 'when the discussion was created in the diff' do
|
|
it 'returns correct diff file' do
|
|
diff_file = subject.diff_file
|
|
|
|
expect(diff_file.old_path).to eq(position.old_path)
|
|
expect(diff_file.new_path).to eq(position.new_path)
|
|
expect(diff_file.diff_refs).to eq(position.diff_refs)
|
|
end
|
|
end
|
|
|
|
context 'when discussion is outdated or not created in the diff' do
|
|
let(:diff_refs) { project.commit(sample_commit.id).diff_refs }
|
|
let(:position) do
|
|
Gitlab::Diff::Position.new(
|
|
old_path: "files/ruby/popen.rb",
|
|
new_path: "files/ruby/popen.rb",
|
|
old_line: nil,
|
|
new_line: 14,
|
|
diff_refs: diff_refs
|
|
)
|
|
end
|
|
|
|
it 'returns the correct diff file' do
|
|
diff_file = subject.diff_file
|
|
|
|
expect(diff_file.old_path).to eq(position.old_path)
|
|
expect(diff_file.new_path).to eq(position.new_path)
|
|
expect(diff_file.diff_refs).to eq(position.diff_refs)
|
|
end
|
|
end
|
|
|
|
context 'note diff file creation enqueuing' do
|
|
it 'enqueues CreateNoteDiffFileWorker if it is the first note of a discussion' do
|
|
subject.note_diff_file.destroy!
|
|
|
|
expect(CreateNoteDiffFileWorker).to receive(:perform_async).with(subject.id)
|
|
|
|
subject.reload.diff_file
|
|
end
|
|
|
|
it 'does not enqueues CreateNoteDiffFileWorker if not first note of a discussion' do
|
|
mr = create(:merge_request)
|
|
diff_note = create(:diff_note_on_merge_request, project: mr.project, noteable: mr)
|
|
reply_diff_note = create(:diff_note_on_merge_request, in_reply_to: diff_note)
|
|
|
|
expect(CreateNoteDiffFileWorker).not_to receive(:perform_async).with(reply_diff_note.id)
|
|
|
|
reply_diff_note.reload.diff_file
|
|
end
|
|
end
|
|
|
|
context 'when noteable is a Design' do
|
|
it 'does not return a diff file' do
|
|
diff_note = create(:diff_note_on_design)
|
|
|
|
expect(diff_note.diff_file).to be_nil
|
|
end
|
|
end
|
|
end
|
|
|
|
describe '#latest_diff_file' do
|
|
context 'when noteable is a Design' do
|
|
it 'does not return a diff file' do
|
|
diff_note = create(:diff_note_on_design)
|
|
|
|
expect(diff_note.latest_diff_file).to be_nil
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#diff_line" do
|
|
it "returns the correct diff line" do
|
|
diff_line = subject.diff_line
|
|
|
|
expect(diff_line.added?).to be true
|
|
expect(diff_line.new_line).to eq(position.formatter.new_line)
|
|
expect(diff_line.text).to eq("+ vars = {")
|
|
end
|
|
end
|
|
|
|
describe "#line_code" do
|
|
it "returns the correct line code" do
|
|
line_code = Gitlab::Git.diff_line_code(position.file_path, position.formatter.new_line, 15)
|
|
|
|
expect(subject.line_code).to eq(line_code)
|
|
end
|
|
end
|
|
|
|
describe "#active?" do
|
|
context "when noteable is a commit" do
|
|
subject { build(:diff_note_on_commit, project: project, position: position) }
|
|
|
|
it "returns true" do
|
|
expect(subject.active?).to be true
|
|
end
|
|
end
|
|
|
|
context "when noteable is a merge request" do
|
|
context "when the merge request's diff refs match that of the diff note" do
|
|
it "returns true" do
|
|
expect(subject.active?).to be true
|
|
end
|
|
end
|
|
|
|
context "when the merge request's diff refs don't match that of the diff note" do
|
|
before do
|
|
allow(subject.noteable).to receive(:diff_refs).and_return(commit.diff_refs)
|
|
end
|
|
|
|
it "returns false" do
|
|
expect(subject.active?).to be false
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "creation" do
|
|
describe "updating of position" do
|
|
context "when noteable is a commit" do
|
|
let(:diff_refs) { commit.diff_refs }
|
|
|
|
subject { create(:diff_note_on_commit, project: project, position: position, commit_id: commit.id) }
|
|
|
|
it "doesn't update the position" do
|
|
is_expected.to have_attributes(original_position: position,
|
|
position: position)
|
|
end
|
|
end
|
|
|
|
context "when noteable is a merge request" do
|
|
context "when the note is active" do
|
|
it "doesn't update the position" do
|
|
expect(subject.original_position).to eq(position)
|
|
expect(subject.position).to eq(position)
|
|
end
|
|
end
|
|
|
|
context "when the note is outdated" do
|
|
before do
|
|
allow(merge_request).to receive(:diff_refs).and_return(commit.diff_refs)
|
|
end
|
|
|
|
it "updates the position" do
|
|
expect(subject.original_position).to eq(position)
|
|
expect(subject.position).not_to eq(position)
|
|
end
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "#discussion_id" do
|
|
let(:note) { create(:diff_note_on_merge_request) }
|
|
|
|
context "when it is newly created" do
|
|
it "has a discussion id" do
|
|
expect(note.discussion_id).not_to be_nil
|
|
expect(note.discussion_id).to match(/\A\h{40}\z/)
|
|
end
|
|
end
|
|
|
|
context "when it didn't store a discussion id before" do
|
|
before do
|
|
note.update_column(:discussion_id, nil)
|
|
end
|
|
|
|
it "has a discussion id" do
|
|
# The discussion_id is set in `after_initialize`, so `reload` won't work
|
|
reloaded_note = Note.find(note.id)
|
|
|
|
expect(reloaded_note.discussion_id).not_to be_nil
|
|
expect(reloaded_note.discussion_id).to match(/\A\h{40}\z/)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe '#created_at_diff?' do
|
|
let(:diff_refs) { project.commit(sample_commit.id).diff_refs }
|
|
let(:position) do
|
|
Gitlab::Diff::Position.new(
|
|
old_path: "files/ruby/popen.rb",
|
|
new_path: "files/ruby/popen.rb",
|
|
old_line: nil,
|
|
new_line: 14,
|
|
diff_refs: diff_refs
|
|
)
|
|
end
|
|
|
|
context "when noteable is a commit" do
|
|
subject { build(:diff_note_on_commit, project: project, position: position) }
|
|
|
|
it "returns true" do
|
|
expect(subject.created_at_diff?(diff_refs)).to be true
|
|
end
|
|
end
|
|
|
|
context "when noteable is a merge request" do
|
|
context "when the diff refs match the original one of the diff note" do
|
|
it "returns true" do
|
|
expect(subject.created_at_diff?(diff_refs)).to be true
|
|
end
|
|
end
|
|
|
|
context "when the diff refs don't match the original one of the diff note" do
|
|
it "returns false" do
|
|
expect(subject.created_at_diff?(merge_request.diff_refs)).to be false
|
|
end
|
|
end
|
|
end
|
|
end
|
|
|
|
describe '#supports_suggestion?' do
|
|
context 'when noteable does not exist' do
|
|
it 'returns false' do
|
|
allow(subject).to receive(:noteable) { nil }
|
|
|
|
expect(subject.supports_suggestion?).to be(false)
|
|
end
|
|
end
|
|
|
|
context 'when noteable does not support suggestions' do
|
|
it 'returns false' do
|
|
allow(subject.noteable).to receive(:supports_suggestion?) { false }
|
|
|
|
expect(subject.supports_suggestion?).to be(false)
|
|
end
|
|
end
|
|
|
|
context 'when line is not suggestible' do
|
|
it 'returns false' do
|
|
allow_next_instance_of(Gitlab::Diff::Line) do |instance|
|
|
allow(instance).to receive(:suggestible?) { false }
|
|
end
|
|
|
|
expect(subject.supports_suggestion?).to be(false)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe '#banzai_render_context' do
|
|
let(:note) { create(:diff_note_on_merge_request) }
|
|
|
|
it 'includes expected context' do
|
|
context = note.banzai_render_context(:note)
|
|
|
|
expect(context).to include(suggestions_filter_enabled: true, noteable: note.noteable, project: note.project)
|
|
end
|
|
end
|
|
|
|
describe "image diff notes" do
|
|
subject { build(:image_diff_note_on_merge_request, project: project, noteable: merge_request) }
|
|
|
|
describe "validations" do
|
|
it { is_expected.not_to validate_presence_of(:line_code) }
|
|
|
|
it "does not validate diff line" do
|
|
diff_line = subject.diff_line
|
|
|
|
expect(diff_line).to be nil
|
|
expect(subject).to be_valid
|
|
end
|
|
|
|
it "does not update the position" do
|
|
expect(subject).not_to receive(:update_position)
|
|
|
|
subject.save!
|
|
end
|
|
end
|
|
|
|
it "returns true for on_image?" do
|
|
expect(subject.on_image?).to be_truthy
|
|
end
|
|
end
|
|
|
|
describe '#to_ability_name' do
|
|
subject { described_class.new.to_ability_name }
|
|
|
|
it { is_expected.to eq('note') }
|
|
end
|
|
|
|
describe '#shas' do
|
|
it 'returns list of SHAs based on original_position' do
|
|
expect(subject.shas).to match_array(
|
|
[
|
|
position.base_sha,
|
|
position.start_sha,
|
|
position.head_sha
|
|
])
|
|
end
|
|
|
|
context 'when position changes' do
|
|
before do
|
|
subject.position = new_position
|
|
end
|
|
|
|
it 'includes the new position SHAs' do
|
|
expect(subject.shas).to match_array(
|
|
[
|
|
position.base_sha,
|
|
position.start_sha,
|
|
position.head_sha,
|
|
new_position.base_sha,
|
|
new_position.start_sha,
|
|
new_position.head_sha
|
|
])
|
|
end
|
|
end
|
|
end
|
|
end
|