Add code improvements to merge request version feature

* Add MergeRequestDiff#latest?
* Remove unnecessary variable assignment

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

4 files changed, 19 insertions, 11 deletions

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index eff959c781c..3be9bdb076d 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -93,7 +93,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     respond_to do |format|
       format.html { define_discussion_vars }
       format.json do
-        if @merge_request_diff != @merge_request.merge_request_diff
+        unless @merge_request_diff.latest?
           # Disable comments if browsing older version of the diff
           @diff_notes_disabled = true
         end
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 42ab6b620bd..e353bdb24b8 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -79,14 +79,10 @@ class MergeRequestDiff < ActiveRecord::Base
   def raw_diffs(options = {})
     if options[:ignore_whitespace_change]
       @diffs_no_whitespace ||=
-        begin
-          compare = Gitlab::Git::Compare.new(
-            repository.raw_repository,
-            safe_start_commit_sha,
-            safe_head_commit_sha
-          )
-          compare.diffs(options)
-        end
+        Gitlab::Git::Compare.new(
+          repository.raw_repository,
+          safe_start_commit_sha,
+          safe_head_commit_sha).diffs(options)
     else
       @raw_diffs ||= {}
       @raw_diffs[options] ||= load_diffs(st_diffs, options)
@@ -164,6 +160,10 @@ class MergeRequestDiff < ActiveRecord::Base
       end
   end
 
+  def latest?
+    self == merge_request.merge_request_diff
+  end
+
   private
 
   def dump_commits(commits)
diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml
index 15d94b16ced..08c54f0aeff 100644
--- a/app/views/projects/merge_requests/show/_versions.html.haml
+++ b/app/views/projects/merge_requests/show/_versions.html.haml
@@ -1,5 +1,4 @@
 - merge_request_diffs = @merge_request.merge_request_diffs.select_without_diff
-- latest_diff = merge_request_diffs.first
 
 - if merge_request_diffs.size > 1
   .mr-version-switch
@@ -7,7 +6,7 @@
     %span.dropdown.inline
       %a.btn-link.dropdown-toggle{ data: {toggle: :dropdown} }
         %strong.monospace<
-          - if latest_diff == @merge_request_diff
+          - if @merge_request_diff.latest?
             #{"latest"}
           - else
             #{@merge_request_diff.head_commit.short_id}
diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb
index 2f3bb3ed2c4..e5b185dc3f6 100644
--- a/spec/models/merge_request_diff_spec.rb
+++ b/spec/models/merge_request_diff_spec.rb
@@ -13,6 +13,15 @@ describe MergeRequestDiff, models: true do
     it { expect(subject.start_commit_sha).to eq('0b4bc9a49b562e85de7cc9e834518ea6828729b9') }
   end
 
+  describe '#latest' do
+    let!(:mr) { create(:merge_request, :with_diffs) }
+    let!(:first_diff) { mr.merge_request_diff }
+    let!(:last_diff) { mr.create_merge_request_diff }
+
+    it { expect(last_diff.latest?).to be_truthy }
+    it { expect(first_diff.latest?).to be_falsey }
+  end
+
   describe '#diffs' do
     let(:mr) { create(:merge_request, :with_diffs) }
     let(:mr_diff) { mr.merge_request_diff }