Fix and refactor merge request diff_refs method

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

4 files changed, 11 insertions, 27 deletions

diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index c816deb4e0c..364e1a8341b 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -79,11 +79,7 @@ class DiffNote < Note
   end
 
   def noteable_diff_refs
-    if noteable.respond_to?(:diff_sha_refs)
-      noteable.diff_sha_refs
-    else
-      noteable.diff_refs
-    end
+    noteable.diff_refs
   end
 
   def set_original_position
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 104f9c7223d..007c2aa74e1 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -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:)
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 9a34d099acd..a769d31ac93 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -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,
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 9de9f19a27c..7242c4fc8d3 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -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