2016-08-24 12:49:21 +05:30
|
|
|
require 'spec_helper'
|
|
|
|
|
2017-09-10 17:25:29 +05:30
|
|
|
describe DiffNote do
|
2016-08-24 12:49:21 +05:30
|
|
|
include RepoHelpers
|
|
|
|
|
2018-03-17 18:26:18 +05:30
|
|
|
let!(:merge_request) { create(:merge_request) }
|
2017-08-17 22:00:37 +05:30
|
|
|
let(:project) { merge_request.project }
|
2016-08-24 12:49:21 +05:30
|
|
|
let(:commit) { project.commit(sample_commit.id) }
|
|
|
|
|
|
|
|
let(:path) { "files/ruby/popen.rb" }
|
|
|
|
|
2018-03-17 18:26:18 +05:30
|
|
|
let(:diff_refs) { merge_request.diff_refs }
|
2016-08-24 12:49:21 +05:30
|
|
|
let!(:position) do
|
|
|
|
Gitlab::Diff::Position.new(
|
|
|
|
old_path: path,
|
|
|
|
new_path: path,
|
|
|
|
old_line: nil,
|
|
|
|
new_line: 14,
|
2018-03-17 18:26:18 +05:30
|
|
|
diff_refs: diff_refs
|
2016-08-24 12:49:21 +05:30
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
let!(:new_position) do
|
|
|
|
Gitlab::Diff::Position.new(
|
|
|
|
old_path: path,
|
|
|
|
new_path: path,
|
|
|
|
old_line: 16,
|
|
|
|
new_line: 22,
|
2018-03-17 18:26:18 +05:30
|
|
|
diff_refs: diff_refs
|
2016-08-24 12:49:21 +05:30
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
subject { create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }
|
|
|
|
|
|
|
|
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
|
|
|
|
|
2017-08-17 22:00:37 +05:30
|
|
|
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
|
|
|
|
|
2018-11-08 19:23:39 +05:30
|
|
|
describe '#create_diff_file callback' do
|
|
|
|
let(:noteable) { create(:merge_request) }
|
|
|
|
let(:project) { noteable.project }
|
|
|
|
|
|
|
|
context 'merge request' do
|
|
|
|
let!(:diff_note) { create(:diff_note_on_merge_request, project: project, noteable: noteable) }
|
|
|
|
|
|
|
|
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_merge_request, noteable: noteable, 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
|
|
|
|
|
2018-10-15 14:42:47 +05:30
|
|
|
context 'when the discussion was created in the diff' do
|
|
|
|
it 'returns correct diff file' do
|
|
|
|
diff_file = subject.diff_file
|
2016-08-24 12:49:21 +05:30
|
|
|
|
2018-10-15 14:42:47 +05:30
|
|
|
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
|
2016-08-24 12:49:21 +05:30
|
|
|
end
|
2018-11-08 19:23:39 +05:30
|
|
|
|
|
|
|
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
|
2016-08-24 12:49:21 +05:30
|
|
|
end
|
|
|
|
|
|
|
|
describe "#diff_line" do
|
|
|
|
it "returns the correct diff line" do
|
|
|
|
diff_line = subject.diff_line
|
|
|
|
|
|
|
|
expect(diff_line.added?).to be true
|
2018-03-17 18:26:18 +05:30
|
|
|
expect(diff_line.new_line).to eq(position.formatter.new_line)
|
2016-08-24 12:49:21 +05:30
|
|
|
expect(diff_line.text).to eq("+ vars = {")
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
describe "#line_code" do
|
|
|
|
it "returns the correct line code" do
|
2018-03-17 18:26:18 +05:30
|
|
|
line_code = Gitlab::Git.diff_line_code(position.file_path, position.formatter.new_line, 15)
|
2016-08-24 12:49:21 +05:30
|
|
|
|
|
|
|
expect(subject.line_code).to eq(line_code)
|
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
describe "#active?" do
|
|
|
|
context "when noteable is a commit" do
|
2016-09-13 17:45:13 +05:30
|
|
|
subject { build(:diff_note_on_commit, project: project, position: position) }
|
2016-08-24 12:49:21 +05:30
|
|
|
|
|
|
|
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
|
2017-09-10 17:25:29 +05:30
|
|
|
allow(subject.noteable).to receive(:diff_refs).and_return(commit.diff_refs)
|
2016-08-24 12:49:21 +05:30
|
|
|
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
|
2018-03-17 18:26:18 +05:30
|
|
|
let(:diff_refs) { commit.diff_refs }
|
2016-08-24 12:49:21 +05:30
|
|
|
|
2018-03-17 18:26:18 +05:30
|
|
|
subject { create(:diff_note_on_commit, project: project, position: position, commit_id: commit.id) }
|
2016-08-24 12:49:21 +05:30
|
|
|
|
2018-03-17 18:26:18 +05:30
|
|
|
it "doesn't update the position" do
|
|
|
|
is_expected.to have_attributes(original_position: position,
|
|
|
|
position: position)
|
2016-08-24 12:49:21 +05:30
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context "when noteable is a merge request" do
|
|
|
|
context "when the note is active" do
|
|
|
|
it "doesn't update the position" do
|
2018-03-17 18:26:18 +05:30
|
|
|
expect(subject.original_position).to eq(position)
|
|
|
|
expect(subject.position).to eq(position)
|
2016-08-24 12:49:21 +05:30
|
|
|
end
|
|
|
|
end
|
|
|
|
|
|
|
|
context "when the note is outdated" do
|
|
|
|
before do
|
2017-09-10 17:25:29 +05:30
|
|
|
allow(merge_request).to receive(:diff_refs).and_return(commit.diff_refs)
|
2016-08-24 12:49:21 +05:30
|
|
|
end
|
|
|
|
|
2017-09-10 17:25:29 +05:30
|
|
|
it "updates the position" do
|
2018-03-17 18:26:18 +05:30
|
|
|
expect(subject.original_position).to eq(position)
|
|
|
|
expect(subject.position).not_to eq(position)
|
2016-08-24 12:49:21 +05:30
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2016-09-13 17:45:13 +05:30
|
|
|
|
|
|
|
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
|
|
|
|
|
2017-08-17 22:00:37 +05:30
|
|
|
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
|
2016-09-13 17:45:13 +05:30
|
|
|
|
2017-08-17 22:00:37 +05:30
|
|
|
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
|
2016-09-13 17:45:13 +05:30
|
|
|
end
|
|
|
|
end
|
|
|
|
|
2017-08-17 22:00:37 +05:30
|
|
|
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
|
2016-09-13 17:45:13 +05:30
|
|
|
end
|
|
|
|
|
2017-08-17 22:00:37 +05:30
|
|
|
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
|
2016-09-13 17:45:13 +05:30
|
|
|
end
|
|
|
|
end
|
|
|
|
end
|
2018-03-17 18:26:18 +05:30
|
|
|
|
|
|
|
describe "image diff notes" do
|
|
|
|
let(:path) { "files/images/any_image.png" }
|
|
|
|
|
|
|
|
let!(:position) do
|
|
|
|
Gitlab::Diff::Position.new(
|
|
|
|
old_path: path,
|
|
|
|
new_path: path,
|
|
|
|
width: 10,
|
|
|
|
height: 10,
|
|
|
|
x: 1,
|
|
|
|
y: 1,
|
|
|
|
diff_refs: merge_request.diff_refs,
|
|
|
|
position_type: "image"
|
|
|
|
)
|
|
|
|
end
|
|
|
|
|
|
|
|
describe "validations" do
|
|
|
|
subject { build(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) }
|
|
|
|
|
|
|
|
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
|
2016-08-24 12:49:21 +05:30
|
|
|
end
|