Refactor straight compare diff code

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

4 files changed, 22 insertions, 12 deletions

diff --git a/app/models/compare.rb b/app/models/compare.rb
index 4b568a1d11c..3a8bbcb1acd 100644
--- a/app/models/compare.rb
+++ b/app/models/compare.rb
@@ -11,7 +11,7 @@ class Compare
     end
   end
 
-  def initialize(compare, project, straight = false)
+  def initialize(compare, project, straight: false)
     @compare = compare
     @project = project
     @straight = straight
@@ -37,8 +37,6 @@ class Compare
   alias_method :commit, :head_commit
 
   def base_commit
-    return start_commit if @straight
-
     return @base_commit if defined?(@base_commit)
 
     @base_commit = if start_commit && head_commit
@@ -48,6 +46,18 @@ class Compare
                    end
   end
 
+  def start_commit_sha
+    start_commit.try(:sha)
+  end
+
+  def base_commit_sha
+    base_commit.try(:sha)
+  end
+
+  def head_commit_sha
+    commit.try(:sha)
+  end
+
   def raw_diffs(*args)
     @compare.diffs(*args)
   end
@@ -61,9 +71,9 @@ class Compare
 
   def diff_refs
     Gitlab::Diff::DiffRefs.new(
-      base_sha:  base_commit.try(:sha),
-      start_sha: start_commit.try(:sha),
-      head_sha: commit.try(:sha)
+      base_sha:  @straight ? start_commit_sha : base_commit_sha,
+      start_sha: start_commit_sha,
+      head_sha: head_commit_sha
     )
   end
 end
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 6ad2201573d..b8a10b7968e 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -167,11 +167,11 @@ class MergeRequestDiff < ActiveRecord::Base
     self == merge_request.merge_request_diff
   end
 
-  def compare_with(sha, straight = true)
+  def compare_with(sha, straight: true)
     # When compare merge request versions we want diff A..B instead of A...B
     # so we handle cases when user does squash and rebase of the commits between versions.
     # For this reason we set straight to true by default.
-    CompareService.new.execute(project, head_commit_sha, project, sha, straight)
+    CompareService.new.execute(project, head_commit_sha, project, sha, straight: straight)
   end
 
   private
diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb
index 6df3b958b8a..5e8fafca98c 100644
--- a/app/services/compare_service.rb
+++ b/app/services/compare_service.rb
@@ -3,7 +3,7 @@ require 'securerandom'
 # Compare 2 branches for one repo or between repositories
 # and return Gitlab::Git::Compare object that responds to commits and diffs
 class CompareService
-  def execute(source_project, source_branch, target_project, target_branch, straight = false)
+  def execute(source_project, source_branch, target_project, target_branch, straight: false)
     source_commit = source_project.commit(source_branch)
     return unless source_commit
 
@@ -27,6 +27,6 @@ class CompareService
       straight
     )
 
-    Compare.new(raw_compare, target_project, straight)
+    Compare.new(raw_compare, target_project, straight: straight)
   end
 end
diff --git a/spec/services/compare_service_spec.rb b/spec/services/compare_service_spec.rb
index d809f291b44..3760f19aaa2 100644
--- a/spec/services/compare_service_spec.rb
+++ b/spec/services/compare_service_spec.rb
@@ -7,13 +7,13 @@ describe CompareService, services: true do
 
   describe '#execute' do
     context 'compare with base, like feature...fix' do
-      subject { service.execute(project, 'feature', project, 'fix', false) }
+      subject { service.execute(project, 'feature', project, 'fix', straight: false) }
 
       it { expect(subject.diffs.size).to eq(1) }
     end
 
     context 'straight compare, like feature..fix' do
-      subject { service.execute(project, 'feature', project, 'fix', true) }
+      subject { service.execute(project, 'feature', project, 'fix', straight: true) }
 
       it { expect(subject.diffs.size).to eq(3) }
     end