Commit 49d63dc1 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Fix and refactor merge request diff_refs method

Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 643a368f
......@@ -79,12 +79,8 @@ class DiffNote < Note
end
def noteable_diff_refs
if noteable.respond_to?(:diff_sha_refs)
noteable.diff_sha_refs
else
noteable.diff_refs
end
end
def set_original_position
self.original_position = self.position.dup
......
......@@ -270,7 +270,7 @@ class MergeRequest < ActiveRecord::Base
end
def diff_refs
if persisted?
if merge_request_diff && merge_request_diff.diff_refs_by_sha?
merge_request_diff.diff_refs
else
start_sha = target_branch_sha
......@@ -287,19 +287,6 @@ class MergeRequest < ActiveRecord::Base
end
end
# Return diff_refs instance trying to not touch the git repository
def diff_sha_refs
if merge_request_diff && merge_request_diff.diff_refs_by_sha?
return Gitlab::Diff::DiffRefs.new(
base_sha: merge_request_diff.base_commit_sha,
start_sha: merge_request_diff.start_commit_sha,
head_sha: merge_request_diff.head_commit_sha
)
else
diff_refs
end
end
def validate_branches
if target_project == source_project && target_branch == source_branch
errors.add :branch_conflict, "You can not use same project/branch for source and target"
......@@ -716,7 +703,7 @@ class MergeRequest < ActiveRecord::Base
end
def support_new_diff_notes?
diff_sha_refs && diff_sha_refs.complete?
diff_refs && diff_refs.complete?
end
def update_diff_notes_positions(old_diff_refs:, new_diff_refs:)
......
......@@ -128,7 +128,7 @@ class MergeRequestDiff < ActiveRecord::Base
end
def diff_refs
return unless start_commit || base_commit
return unless start_commit_sha || base_commit_sha
Gitlab::Diff::DiffRefs.new(
base_sha: base_commit_sha,
......
......@@ -723,7 +723,6 @@ describe MergeRequest, models: true do
end
end
<<<<<<< HEAD
describe '#branch_merge_base_commit' do
context 'source and target branch exist' do
it { expect(subject.branch_merge_base_commit.sha).to eq('ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }
......@@ -737,8 +736,11 @@ describe MergeRequest, models: true do
it 'returns nil' do
expect(subject.branch_merge_base_commit).to be_nil
=======
describe "#diff_sha_refs" do
end
end
end
describe "#diff_refs" do
context "with diffs" do
subject { create(:merge_request, :with_diffs) }
......@@ -747,7 +749,7 @@ describe MergeRequest, models: true do
expect_any_instance_of(Repository).not_to receive(:commit)
subject.diff_sha_refs
subject.diff_refs
end
it "returns expected diff_refs" do
......@@ -757,8 +759,7 @@ describe MergeRequest, models: true do
head_sha: subject.merge_request_diff.head_commit_sha
)
expect(subject.diff_sha_refs).to eq(expected_diff_refs)
>>>>>>> master
expect(subject.diff_refs).to eq(expected_diff_refs)
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