From b202b42cfee6bb8cf0c142c918c545f45464a29c Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Fri, 31 Mar 2017 17:39:14 -0600
Subject: [PATCH 01/11] Link to outdated diff in older MR version from outdated
 diff discussion

---
 .../stylesheets/pages/merge_requests.scss     |  2 ++
 .../projects/merge_requests_controller.rb     | 28 ++++++++-----------
 app/models/concerns/note_on_diff.rb           | 10 +++++++
 app/models/concerns/noteable.rb               |  4 +--
 app/models/diff_discussion.rb                 |  1 +
 app/models/diff_note.rb                       | 14 ++++------
 app/models/legacy_diff_note.rb                |  3 +-
 app/models/merge_request.rb                   |  4 +++
 app/models/merge_request_diff.rb              |  1 +
 app/models/note.rb                            |  8 ++++--
 app/views/discussions/_discussion.html.haml   |  7 ++++-
 .../projects/diffs/_parallel_view.html.haml   |  3 +-
 app/views/projects/diffs/_text_file.html.haml |  3 +-
 .../merge_requests/show/_versions.html.haml   | 10 ++++---
 .../dm-link-discussion-to-outdated-diff.yml   |  4 +++
 15 files changed, 63 insertions(+), 39 deletions(-)
 create mode 100644 changelogs/unreleased/dm-link-discussion-to-outdated-diff.yml

diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 2f946ab2f59..cc43d046b54 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -528,6 +528,8 @@
   }
 
   .comments-disabled-notif {
+    line-height: 28px;
+
     .btn {
       margin-left: 5px;
     }
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 5c1f7e69ee8..bba3e007610 100755
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -16,7 +16,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines]
   before_action :define_widget_vars, only: [:merge, :cancel_merge_when_pipeline_succeeds, :merge_check]
   before_action :define_commit_vars, only: [:diffs]
-  before_action :define_diff_comment_vars, only: [:diffs]
   before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :conflict_for_path, :pipelines]
   before_action :close_merge_request_without_source_project, only: [:show, :diffs, :commits, :builds, :pipelines]
   before_action :apply_diff_view_cookie!, only: [:new_diffs]
@@ -108,6 +107,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController
             @merge_request.merge_request_diff
           end
 
+        define_diff_comment_vars
+
         @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff
         @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
 
@@ -123,11 +124,14 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
         @environment = @merge_request.environments_for(current_user).last
 
-        if @start_sha
-          compared_diff_version
-        else
-          original_diff_version
-        end
+        @diff_notes_disabled = !@merge_request_diff.latest? || @start_sha
+
+        @diffs =
+          if @start_sha
+            @merge_request_diff.compare_with(@start_sha).diffs(diff_options)
+          else
+            @merge_request_diff.diffs(diff_options)
+          end
 
         render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") }
       end
@@ -594,7 +598,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
     @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs?
 
-    @grouped_diff_discussions = @merge_request.grouped_diff_discussions
+    @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@merge_request_diff.diff_refs)
     @notes = prepare_notes_for_rendering(@grouped_diff_discussions.values.flatten.flat_map(&:notes))
   end
 
@@ -678,16 +682,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @merge_request = MergeRequests::BuildService.new(project, current_user, merge_request_params.merge(diff_options: diff_options)).execute
   end
 
-  def compared_diff_version
-    @diff_notes_disabled = true
-    @diffs = @merge_request_diff.compare_with(@start_sha).diffs(diff_options)
-  end
-
-  def original_diff_version
-    @diff_notes_disabled = !@merge_request_diff.latest?
-    @diffs = @merge_request_diff.diffs(diff_options)
-  end
-
   def close_merge_request_without_source_project
     if !@merge_request.source_project && @merge_request.open?
       @merge_request.close
diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb
index 1a5a7007a2b..ac4c3099c00 100644
--- a/app/models/concerns/note_on_diff.rb
+++ b/app/models/concerns/note_on_diff.rb
@@ -25,4 +25,14 @@ module NoteOnDiff
   def diff_attributes
     raise NotImplementedError
   end
+
+  private
+
+  def noteable_diff_refs
+    if noteable.respond_to?(:diff_sha_refs)
+      noteable.diff_sha_refs
+    else
+      noteable.diff_refs
+    end
+  end
 end
diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb
index 772ff6a6d2f..dd1e6630642 100644
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@@ -36,10 +36,10 @@ module Noteable
       .discussions(self)
   end
 
-  def grouped_diff_discussions
+  def grouped_diff_discussions(*args)
     # Doesn't use `discussion_notes`, because this may include commit diff notes
     # besides MR diff notes, that we do no want to display on the MR Changes tab.
-    notes.inc_relations_for_view.grouped_diff_discussions
+    notes.inc_relations_for_view.grouped_diff_discussions(*args)
   end
 
   def resolvable_discussions
diff --git a/app/models/diff_discussion.rb b/app/models/diff_discussion.rb
index d9b7e484e0f..6a6466b493b 100644
--- a/app/models/diff_discussion.rb
+++ b/app/models/diff_discussion.rb
@@ -10,6 +10,7 @@ class DiffDiscussion < Discussion
 
   delegate  :position,
             :original_position,
+            :latest_merge_request_diff,
 
             to: :first_note
 
diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb
index 1523244f8a8..abe4518d62a 100644
--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@@ -65,20 +65,18 @@ class DiffNote < Note
     self.position.diff_refs == diff_refs
   end
 
+  def latest_merge_request_diff
+    return unless for_merge_request?
+
+    self.noteable.merge_request_diff_for(self.position.diff_refs)
+  end
+
   private
 
   def supported?
     for_commit? || self.noteable.has_complete_diff_refs?
   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 unless self.original_position&.complete?
   end
diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb
index 9a77557ebcd..d7c627432d2 100644
--- a/app/models/legacy_diff_note.rb
+++ b/app/models/legacy_diff_note.rb
@@ -56,11 +56,12 @@ class LegacyDiffNote < Note
   #
   # If the note's current diff cannot be matched in the MergeRequest's current
   # diff, it's considered inactive.
-  def active?
+  def active?(diff_refs = nil)
     return @active if defined?(@active)
     return true if for_commit?
     return true unless diff_line
     return false unless noteable
+    return false if diff_refs && diff_refs != noteable_diff_refs
 
     noteable_diff = find_noteable_diff
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index b71a9e17a93..1b6e898a7fd 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -367,6 +367,10 @@ class MergeRequest < ActiveRecord::Base
     merge_request_diff(true)
   end
 
+  def merge_request_diff_for(diff_refs)
+    merge_request_diffs.viewable.select_without_diff.with_diff_refs(diff_refs).take
+  end
+
   def reload_diff_if_branch_changed
     if source_branch_changed? || target_branch_changed?
       reload_diff
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 6ad56b842b2..bf9289086f0 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -26,6 +26,7 @@ class MergeRequestDiff < ActiveRecord::Base
   end
 
   scope :viewable, -> { without_state(:empty) }
+  scope :with_diff_refs, ->(diff_refs) { where(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha) }
 
   # All diff information is collected from repository after object is created.
   # It allows you to override variables like head_commit_sha before getting diff.
diff --git a/app/models/note.rb b/app/models/note.rb
index 1ea7b946061..c85692c5aec 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -113,11 +113,11 @@ class Note < ActiveRecord::Base
       Discussion.build(notes)
     end
 
-    def grouped_diff_discussions
+    def grouped_diff_discussions(diff_refs = nil)
       diff_notes.
         fresh.
         discussions.
-        select(&:active?).
+        select { |n| n.active?(diff_refs) }.
         group_by(&:line_code)
     end
 
@@ -140,6 +140,10 @@ class Note < ActiveRecord::Base
     true
   end
 
+  def latest_merge_request_diff
+    nil
+  end
+
   def max_attachment_size
     current_application_settings.max_attachment_size.megabytes.to_i
   end
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index e04958817e4..f12778be305 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -34,7 +34,12 @@
               - if discussion.active?
                 = link_to 'the diff', discussion_diff_path(discussion)
               - else
-                an outdated diff
+                - merge_request_diff = discussion.latest_merge_request_diff
+                - if merge_request_diff
+                  = link_to diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, diff_id: merge_request_diff, anchor: discussion.line_code) do
+                    an outdated diff
+                - else
+                  an outdated diff
 
             = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago")
           = render "discussions/headline", discussion: discussion
diff --git a/app/views/projects/diffs/_parallel_view.html.haml b/app/views/projects/diffs/_parallel_view.html.haml
index f920f359de2..45c95f7ab6a 100644
--- a/app/views/projects/diffs/_parallel_view.html.haml
+++ b/app/views/projects/diffs/_parallel_view.html.haml
@@ -5,8 +5,7 @@
       - left = line[:left]
       - right = line[:right]
       - last_line = right.new_pos if right
-      - unless @diff_notes_disabled
-        - discussions_left, discussions_right = parallel_diff_discussions(left, right, diff_file)
+      - discussions_left, discussions_right = parallel_diff_discussions(left, right, diff_file)
       %tr.line_holder.parallel
         - if left
           - case left.type
diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml
index ebd1a914ee7..5f3968b6709 100644
--- a/app/views/projects/diffs/_text_file.html.haml
+++ b/app/views/projects/diffs/_text_file.html.haml
@@ -4,11 +4,10 @@
     %a.show-suppressed-diff.js-show-suppressed-diff Changes suppressed. Click to show.
 
 %table.text-file.code.js-syntax-highlight{ data: diff_view_data, class: too_big ? 'hide' : '' }
-  - discussions = @grouped_diff_discussions unless @diff_notes_disabled
   = render partial: "projects/diffs/line",
     collection: diff_file.highlighted_diff_lines,
     as: :line,
-    locals: { diff_file: diff_file, discussions: discussions }
+    locals: { diff_file: diff_file, discussions: @grouped_diff_discussions }
 
   - if !diff_file.new_file && !diff_file.deleted_file && diff_file.highlighted_diff_lines.any?
     - last_line = diff_file.highlighted_diff_lines.last
diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml
index 74a7b1dc498..47f45e8e061 100644
--- a/app/views/projects/merge_requests/show/_versions.html.haml
+++ b/app/views/projects/merge_requests/show/_versions.html.haml
@@ -74,11 +74,13 @@
         from
         %code= @merge_request.target_branch
 
-    - unless @merge_request_diff.latest? && !@start_sha
+    - if @diff_notes_disabled
       .comments-disabled-notif.content-block
         = icon('info-circle')
         - if @start_sha
-          Comments are disabled because you're comparing two versions of this merge request.
+          Comment creation is disabled because you're comparing two versions of this merge request.
         - else
-          Comments are disabled because you're viewing an old version of this merge request.
-        = link_to 'Show latest version', diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm'
+          Discussions on this old version of the merge request are displayed but comment creation has been disabled.
+
+        .pull-right
+          = link_to 'Show latest version', diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm'
diff --git a/changelogs/unreleased/dm-link-discussion-to-outdated-diff.yml b/changelogs/unreleased/dm-link-discussion-to-outdated-diff.yml
new file mode 100644
index 00000000000..d489bada7ea
--- /dev/null
+++ b/changelogs/unreleased/dm-link-discussion-to-outdated-diff.yml
@@ -0,0 +1,4 @@
+---
+title: Link to outdated diff in older MR version from outdated diff discussion
+merge_request:
+author:

From 2c0de7aaafd5fb842618bb7fa218e11255363bc8 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Fri, 31 Mar 2017 17:49:43 -0600
Subject: [PATCH 02/11] Cache MR diffs by diff refs

---
 app/models/merge_request.rb  | 6 +++++-
 lib/gitlab/diff/diff_refs.rb | 6 ++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 1b6e898a7fd..95e41106b49 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -368,7 +368,11 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def merge_request_diff_for(diff_refs)
-    merge_request_diffs.viewable.select_without_diff.with_diff_refs(diff_refs).take
+    @merge_request_diffs_by_diff_refs ||= Hash.new do |h, diff_refs|
+      h[diff_refs] = merge_request_diffs.viewable.select_without_diff.with_diff_refs(diff_refs).take
+    end
+
+    @merge_request_diffs_by_diff_refs[diff_refs]
   end
 
   def reload_diff_if_branch_changed
diff --git a/lib/gitlab/diff/diff_refs.rb b/lib/gitlab/diff/diff_refs.rb
index 8406ca4269c..7948782aecc 100644
--- a/lib/gitlab/diff/diff_refs.rb
+++ b/lib/gitlab/diff/diff_refs.rb
@@ -18,6 +18,12 @@ module Gitlab
           head_sha == other.head_sha
       end
 
+      alias_method :eql?, :==
+
+      def hash
+        [base_sha, start_sha, head_sha].hash
+      end
+
       # There is only one case in which we will have `start_sha` and `head_sha`,
       # but not `base_sha`, which is when a diff is generated between an
       # orphaned branch and another branch, which means there _is_ no base, but

From d65d245e06fbe45455e130e2d4ca0ca1d066a8c6 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Fri, 31 Mar 2017 18:10:53 -0600
Subject: [PATCH 03/11] Add link to diff header too

---
 app/helpers/notes_helper.rb                 | 13 ++++++++++---
 app/views/discussions/_discussion.html.haml | 17 ++++++-----------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 5f3d89cf6cb..b244ae4fcbf 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -61,10 +61,17 @@ module NotesHelper
   end
 
   def discussion_diff_path(discussion)
-    return unless discussion.diff_discussion?
+    if discussion.for_merge_request?
+      if discussion.active?
+        # Without a diff ID, the link always points to the latest diff version
+        diff_id = nil
+      elsif merge_request_diff = discussion.latest_merge_request_diff
+        diff_id = merge_request_diff.id
+      else
+        return
+      end
 
-    if discussion.for_merge_request? && discussion.active?
-      diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code)
+      diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, diff_id: diff_id, anchor: discussion.line_code)
     elsif discussion.for_commit?
       namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code)
     end
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index f12778be305..47739809108 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -20,24 +20,19 @@
             = discussion.author.to_reference
             started a discussion
 
+            - url = discussion_diff_path(discussion)
             - if discussion.for_commit? && @noteable != discussion.noteable
               on
               - commit = discussion.noteable
               - if commit
                 commit
-                - anchor = discussion.line_code if discussion.diff_discussion?
-                = link_to commit.short_id, namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: anchor), class: 'monospace'
+                = link_to commit.short_id, discussion_diff_path(discussion), class: 'monospace'
               - else
                 a deleted commit
-            - elsif discussion.diff_discussion?
-              on
-              - if discussion.active?
-                = link_to 'the diff', discussion_diff_path(discussion)
-              - else
-                - merge_request_diff = discussion.latest_merge_request_diff
-                - if merge_request_diff
-                  = link_to diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, diff_id: merge_request_diff, anchor: discussion.line_code) do
-                    an outdated diff
+            - else
+              = conditional_link_to url.present?, url do
+                - if discussion.active?
+                  the diff
                 - else
                   an outdated diff
 

From f47b737456f848784fddb5958c3fa781d2ede2f1 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Fri, 31 Mar 2017 18:20:44 -0600
Subject: [PATCH 04/11] Change discussion headline to 'a now outdated portion
 of the diff'

---
 app/views/discussions/_discussion.html.haml | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index 47739809108..6f74948f498 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -30,11 +30,10 @@
               - else
                 a deleted commit
             - else
+              - unless discussion.active?
+                a now outdated portion of
               = conditional_link_to url.present?, url do
-                - if discussion.active?
-                  the diff
-                - else
-                  an outdated diff
+                the diff
 
             = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago")
           = render "discussions/headline", discussion: discussion

From 50eae640dbbfa42a42e8b6966bd739cfda3adabc Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Thu, 6 Apr 2017 17:13:28 -0500
Subject: [PATCH 05/11] Fix specs and make tweaks

---
 .../projects/merge_requests_controller.rb     | 65 ++++++++++---------
 app/helpers/notes_helper.rb                   |  2 +
 app/models/merge_request.rb                   |  2 +-
 app/models/merge_request_diff.rb              |  5 +-
 app/views/discussions/_discussion.html.haml   |  9 +--
 .../merge_requests/show/_versions.html.haml   |  2 +-
 .../merge_request_versions_spec.rb            |  4 +-
 7 files changed, 50 insertions(+), 39 deletions(-)

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index bba3e007610..c25d33e12cf 100755
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -100,39 +100,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     respond_to do |format|
       format.html { define_discussion_vars }
       format.json do
-        @merge_request_diff =
-          if params[:diff_id]
-            @merge_request.merge_request_diffs.viewable.find(params[:diff_id])
-          else
-            @merge_request.merge_request_diff
-          end
-
+        define_diff_vars
         define_diff_comment_vars
 
-        @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff
-        @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
-
-        if params[:start_sha].present?
-          @start_sha = params[:start_sha]
-          @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == @start_sha }
-
-          unless @start_version
-            @start_sha = @merge_request_diff.head_commit_sha
-            @start_version = @merge_request_diff
-          end
-        end
-
         @environment = @merge_request.environments_for(current_user).last
 
-        @diff_notes_disabled = !@merge_request_diff.latest? || @start_sha
-
-        @diffs =
-          if @start_sha
-            @merge_request_diff.compare_with(@start_sha).diffs(diff_options)
-          else
-            @merge_request_diff.diffs(diff_options)
-          end
-
         render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") }
       end
     end
@@ -144,16 +116,18 @@ class Projects::MergeRequestsController < Projects::ApplicationController
   def diff_for_path
     if params[:id]
       merge_request
+      define_diff_vars
       define_diff_comment_vars
     else
       build_merge_request
+      @diffs = @merge_request.diffs(diff_options)
       @diff_notes_disabled = true
       @grouped_diff_discussions = {}
     end
 
     define_commit_vars
 
-    render_diff_for_path(@merge_request.diffs(diff_options))
+    render_diff_for_path(@diffs)
   end
 
   def commits
@@ -590,12 +564,43 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @base_commit = @merge_request.diff_base_commit || @merge_request.likely_diff_base_commit
   end
 
+  def define_diff_vars
+    @merge_request_diff =
+      if params[:diff_id]
+        @merge_request.merge_request_diffs.viewable.find(params[:diff_id])
+      else
+        @merge_request.merge_request_diff
+      end
+
+    @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff
+    @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
+
+    if params[:start_sha].present?
+      @start_sha = params[:start_sha]
+      @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == @start_sha }
+
+      unless @start_version
+        @start_sha = @merge_request_diff.head_commit_sha
+        @start_version = @merge_request_diff
+      end
+    end
+
+    @diffs =
+      if @start_sha
+        @merge_request_diff.compare_with(@start_sha).diffs(diff_options)
+      else
+        @merge_request_diff.diffs(diff_options)
+      end
+  end
+
   def define_diff_comment_vars
     @new_diff_note_attrs = {
       noteable_type: 'MergeRequest',
       noteable_id: @merge_request.id
     }
 
+    @diff_notes_disabled = !@merge_request_diff.latest? || @start_sha
+
     @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs?
 
     @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@merge_request_diff.diff_refs)
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index b244ae4fcbf..6f4ba79b80b 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -68,6 +68,8 @@ module NotesHelper
       elsif merge_request_diff = discussion.latest_merge_request_diff
         diff_id = merge_request_diff.id
       else
+        # If the discussion is not active, and we cannot find the latest
+        # merge request diff for this discussion, we return no path at all.
         return
       end
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 95e41106b49..1e24c03e5b6 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -369,7 +369,7 @@ class MergeRequest < ActiveRecord::Base
 
   def merge_request_diff_for(diff_refs)
     @merge_request_diffs_by_diff_refs ||= Hash.new do |h, diff_refs|
-      h[diff_refs] = merge_request_diffs.viewable.select_without_diff.with_diff_refs(diff_refs).take
+      h[diff_refs] = merge_request_diffs.viewable.select_without_diff.find_by_diff_refs(diff_refs)
     end
 
     @merge_request_diffs_by_diff_refs[diff_refs]
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index bf9289086f0..3a26b4ee401 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -26,12 +26,15 @@ class MergeRequestDiff < ActiveRecord::Base
   end
 
   scope :viewable, -> { without_state(:empty) }
-  scope :with_diff_refs, ->(diff_refs) { where(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha) }
 
   # All diff information is collected from repository after object is created.
   # It allows you to override variables like head_commit_sha before getting diff.
   after_create :save_git_content, unless: :importing?
 
+  def self.find_by_diff_refs(diff_refs)
+    where(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha)
+  end
+
   def self.select_without_diff
     select(column_names - ['st_diffs'])
   end
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index 6f74948f498..0ee27b6ff20 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -26,14 +26,15 @@
               - commit = discussion.noteable
               - if commit
                 commit
-                = link_to commit.short_id, discussion_diff_path(discussion), class: 'monospace'
+                = link_to commit.short_id, url, class: 'monospace'
               - else
                 a deleted commit
             - else
-              - unless discussion.active?
-                a now outdated portion of
               = conditional_link_to url.present?, url do
-                the diff
+                - if discussion.active?
+                  the diff
+                - else
+                  an outdated diff
 
             = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago")
           = render "discussions/headline", discussion: discussion
diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml
index 47f45e8e061..9842f737ff0 100644
--- a/app/views/projects/merge_requests/show/_versions.html.haml
+++ b/app/views/projects/merge_requests/show/_versions.html.haml
@@ -80,7 +80,7 @@
         - if @start_sha
           Comment creation is disabled because you're comparing two versions of this merge request.
         - else
-          Discussions on this old version of the merge request are displayed but comment creation has been disabled.
+          Discussions on this old version of the merge request are displayed but comment creation is disabled.
 
         .pull-right
           = link_to 'Show latest version', diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm'
diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb
index 04e85ed3f73..2577336bf0f 100644
--- a/spec/features/merge_requests/merge_request_versions_spec.rb
+++ b/spec/features/merge_requests/merge_request_versions_spec.rb
@@ -37,7 +37,7 @@ feature 'Merge Request versions', js: true, feature: true do
     end
 
     it 'show the message about disabled comments' do
-      expect(page).to have_content 'Comments are disabled'
+      expect(page).to have_content 'comment creation is disabled'
     end
   end
 
@@ -66,7 +66,7 @@ feature 'Merge Request versions', js: true, feature: true do
     end
 
     it 'show the message about disabled comments' do
-      expect(page).to have_content 'Comments are disabled'
+      expect(page).to have_content 'Comment creation is disabled'
     end
 
     it 'show diff between new and old version' do

From f112f81d3d463a81c8b5e9225d5e9bb40e82abe8 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Thu, 6 Apr 2017 17:24:51 -0500
Subject: [PATCH 06/11] Fix find_by_diff_refs

---
 app/models/merge_request_diff.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 3a26b4ee401..0143dd83501 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -32,7 +32,7 @@ class MergeRequestDiff < ActiveRecord::Base
   after_create :save_git_content, unless: :importing?
 
   def self.find_by_diff_refs(diff_refs)
-    where(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha)
+    find_by(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha)
   end
 
   def self.select_without_diff

From a8339fe1aa95d489e00cb0b79a20557d711639ec Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Thu, 6 Apr 2017 18:18:53 -0500
Subject: [PATCH 07/11] Fix views after rebase

---
 app/helpers/notes_helper.rb                 | 6 ++++--
 app/views/discussions/_discussion.html.haml | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 6f4ba79b80b..c831f89dc19 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -61,7 +61,7 @@ module NotesHelper
   end
 
   def discussion_diff_path(discussion)
-    if discussion.for_merge_request?
+    if discussion.for_merge_request? && discussion.diff_discussion?
       if discussion.active?
         # Without a diff ID, the link always points to the latest diff version
         diff_id = nil
@@ -75,7 +75,9 @@ module NotesHelper
 
       diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, diff_id: diff_id, anchor: discussion.line_code)
     elsif discussion.for_commit?
-      namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code)
+      anchor = discussion.line_code if discussion.diff_discussion?
+      
+      namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: anchor)
     end
   end
 end
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index 0ee27b6ff20..f48dc575661 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -29,7 +29,7 @@
                 = link_to commit.short_id, url, class: 'monospace'
               - else
                 a deleted commit
-            - else
+            - elsif discussion.diff_discussion?
               = conditional_link_to url.present?, url do
                 - if discussion.active?
                   the diff

From 1817f877e19628417dd8209f070386b111de59c4 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Sat, 8 Apr 2017 14:58:08 -0500
Subject: [PATCH 08/11] Some code tweaks

---
 app/controllers/projects/merge_requests_controller.rb    | 9 +++------
 app/helpers/notes_helper.rb                              | 2 +-
 app/models/concerns/discussion_on_diff.rb                | 8 --------
 app/models/concerns/note_on_diff.rb                      | 4 ++++
 app/models/legacy_diff_discussion.rb                     | 8 ++++++++
 app/views/discussions/_discussion.html.haml              | 1 +
 .../projects/merge_requests/show/_versions.html.haml     | 5 +++--
 .../merge_requests/merge_request_versions_spec.rb        | 2 +-
 8 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index c25d33e12cf..87d684e5c7a 100755
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -576,13 +576,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
 
     if params[:start_sha].present?
-      @start_sha = params[:start_sha]
-      @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == @start_sha }
+      start_sha = params[:start_sha]
+      @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == start_sha }
 
-      unless @start_version
-        @start_sha = @merge_request_diff.head_commit_sha
-        @start_version = @merge_request_diff
-      end
+      @start_sha = start_sha if @start_version
     end
 
     @diffs =
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index c831f89dc19..eab0738a368 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -76,7 +76,7 @@ module NotesHelper
       diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, diff_id: diff_id, anchor: discussion.line_code)
     elsif discussion.for_commit?
       anchor = discussion.line_code if discussion.diff_discussion?
-      
+
       namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: anchor)
     end
   end
diff --git a/app/models/concerns/discussion_on_diff.rb b/app/models/concerns/discussion_on_diff.rb
index 87db0c810c3..67b1cace3eb 100644
--- a/app/models/concerns/discussion_on_diff.rb
+++ b/app/models/concerns/discussion_on_diff.rb
@@ -5,8 +5,6 @@ module DiscussionOnDiff
   included do
     NUMBER_OF_TRUNCATED_DIFF_LINES = 16
 
-    memoized_values << :active
-
     delegate  :line_code,
               :original_line_code,
               :diff_file,
@@ -29,12 +27,6 @@ module DiscussionOnDiff
     true
   end
 
-  def active?
-    return @active if @active.present?
-
-    @active = first_note.active?
-  end
-
   # Returns an array of at most 16 highlighted lines above a diff note
   def truncated_diff_lines(highlight: true)
     lines = highlight ? highlighted_diff_lines : diff_lines
diff --git a/app/models/concerns/note_on_diff.rb b/app/models/concerns/note_on_diff.rb
index ac4c3099c00..6c27dd5aa5c 100644
--- a/app/models/concerns/note_on_diff.rb
+++ b/app/models/concerns/note_on_diff.rb
@@ -26,6 +26,10 @@ module NoteOnDiff
     raise NotImplementedError
   end
 
+  def active?(diff_refs = nil)
+    raise NotImplementedError
+  end
+
   private
 
   def noteable_diff_refs
diff --git a/app/models/legacy_diff_discussion.rb b/app/models/legacy_diff_discussion.rb
index cb2651a03f8..e617ce36f56 100644
--- a/app/models/legacy_diff_discussion.rb
+++ b/app/models/legacy_diff_discussion.rb
@@ -7,6 +7,8 @@
 class LegacyDiffDiscussion < Discussion
   include DiscussionOnDiff
 
+  memoized_values << :active
+
   def legacy_diff_discussion?
     true
   end
@@ -15,6 +17,12 @@ class LegacyDiffDiscussion < Discussion
     LegacyDiffNote
   end
 
+  def active?(*args)
+    return @active if @active.present?
+
+    @active = first_note.active?(*args)
+  end
+
   def collapsed?
     !active?
   end
diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml
index f48dc575661..8440fb3d785 100644
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@@ -30,6 +30,7 @@
               - else
                 a deleted commit
             - elsif discussion.diff_discussion?
+              on
               = conditional_link_to url.present?, url do
                 - if discussion.active?
                   the diff
diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml
index 9842f737ff0..434d9b5837f 100644
--- a/app/views/projects/merge_requests/show/_versions.html.haml
+++ b/app/views/projects/merge_requests/show/_versions.html.haml
@@ -72,13 +72,14 @@
         = link_to namespace_project_compare_path(@project.namespace, @project, from: @start_version.base_commit_sha, to: @merge_request_diff.base_commit_sha) do
           new commits
         from
-        %code= @merge_request.target_branch
+        = succeed '.' do
+          %code= @merge_request.target_branch
 
     - if @diff_notes_disabled
       .comments-disabled-notif.content-block
         = icon('info-circle')
         - if @start_sha
-          Comment creation is disabled because you're comparing two versions of this merge request.
+          Comments are disabled because you're comparing two versions of this merge request.
         - else
           Discussions on this old version of the merge request are displayed but comment creation is disabled.
 
diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/merge_request_versions_spec.rb
index 2577336bf0f..2f627004578 100644
--- a/spec/features/merge_requests/merge_request_versions_spec.rb
+++ b/spec/features/merge_requests/merge_request_versions_spec.rb
@@ -66,7 +66,7 @@ feature 'Merge Request versions', js: true, feature: true do
     end
 
     it 'show the message about disabled comments' do
-      expect(page).to have_content 'Comment creation is disabled'
+      expect(page).to have_content 'Comments are disabled'
     end
 
     it 'show diff between new and old version' do

From 48a0e9f6aece36e83e3ffde5be42f964b5362221 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Sun, 9 Apr 2017 22:27:45 -0500
Subject: [PATCH 09/11] Fix specs

---
 app/controllers/projects/compare_controller.rb        |  1 -
 app/controllers/projects/merge_requests_controller.rb | 10 ++++++----
 app/helpers/diff_helper.rb                            |  2 ++
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb
index c6651254d70..008d2f5815f 100644
--- a/app/controllers/projects/compare_controller.rb
+++ b/app/controllers/projects/compare_controller.rb
@@ -61,7 +61,6 @@ class Projects::CompareController < Projects::ApplicationController
       @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last
 
       @diff_notes_disabled = true
-      @grouped_diff_discussions = {}
     end
   end
 
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 87d684e5c7a..224b44db397 100755
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -122,7 +122,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       build_merge_request
       @diffs = @merge_request.diffs(diff_options)
       @diff_notes_disabled = true
-      @grouped_diff_discussions = {}
     end
 
     define_commit_vars
@@ -576,10 +575,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
 
     if params[:start_sha].present?
-      start_sha = params[:start_sha]
-      @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == start_sha }
+      @start_sha = params[:start_sha]
+      @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == @start_sha }
 
-      @start_sha = start_sha if @start_version
+      unless @start_version
+        @start_sha = @merge_request_diff.head_commit_sha
+        @start_version = @merge_request_diff
+      end
     end
 
     @diffs =
diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb
index 5e0886cc599..dc144906548 100644
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@@ -62,6 +62,8 @@ module DiffHelper
   end
 
   def parallel_diff_discussions(left, right, diff_file)
+    return unless @grouped_diff_discussions
+    
     discussions_left = discussions_right = nil
 
     if left && (left.unchanged? || left.removed?)

From 4f8c36c03c0f0aebc3956fa530f1b78ae3c76fc0 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Mon, 10 Apr 2017 15:17:47 -0500
Subject: [PATCH 10/11] Add specs

---
 app/models/merge_request_diff.rb              | 12 +++++
 spec/factories/notes.rb                       |  3 +-
 .../merge_requests/discussion_spec.rb         | 51 +++++++++++++++++++
 ...uest_versions_spec.rb => versions_spec.rb} | 17 ++++++-
 spec/models/diff_note_spec.rb                 | 17 +++++++
 5 files changed, 98 insertions(+), 2 deletions(-)
 create mode 100644 spec/features/merge_requests/discussion_spec.rb
 rename spec/features/merge_requests/{merge_request_versions_spec.rb => versions_spec.rb} (85%)

diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 0143dd83501..08066db4767 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -134,6 +134,18 @@ class MergeRequestDiff < ActiveRecord::Base
     st_commits.map { |commit| commit[:id] }
   end
 
+  def diff_refs=(new_diff_refs)
+    if new_diff_refs
+      self.base_commit_sha = new_diff_refs.base_sha
+      self.start_commit_sha = new_diff_refs.start_sha
+      self.head_commit_sha = new_diff_refs.head_sha
+    else
+      self.base_commit_sha = nil
+      self.start_commit_sha = nil
+      self.head_commit_sha = nil
+    end
+  end
+
   def diff_refs
     return unless start_commit_sha || base_commit_sha
 
diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index 90c35e2c7f8..93f4903119c 100644
--- a/spec/factories/notes.rb
+++ b/spec/factories/notes.rb
@@ -40,6 +40,7 @@ FactoryGirl.define do
 
       transient do
         line_number 14
+        diff_refs { noteable.try(:diff_refs) }
       end
 
       position do
@@ -48,7 +49,7 @@ FactoryGirl.define do
           new_path: "files/ruby/popen.rb",
           old_line: nil,
           new_line: line_number,
-          diff_refs: noteable.try(:diff_refs)
+          diff_refs: diff_refs
         )
       end
 
diff --git a/spec/features/merge_requests/discussion_spec.rb b/spec/features/merge_requests/discussion_spec.rb
new file mode 100644
index 00000000000..f59d0faa274
--- /dev/null
+++ b/spec/features/merge_requests/discussion_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+feature 'Merge Request Discussions', feature: true do
+  before do
+    login_as :admin
+  end
+
+  context "Diff discussions" do
+    let(:merge_request) { create(:merge_request, importing: true) }
+    let(:project) { merge_request.source_project }
+    let!(:old_merge_request_diff) { merge_request.merge_request_diffs.create(diff_refs: outdated_diff_refs) }
+    let!(:new_merge_request_diff) { merge_request.merge_request_diffs.create }
+
+    let!(:outdated_discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position).to_discussion }
+    let!(:active_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
+
+    let(:outdated_position) do
+      Gitlab::Diff::Position.new(
+        old_path: "files/ruby/popen.rb",
+        new_path: "files/ruby/popen.rb",
+        old_line: nil,
+        new_line: 9,
+        diff_refs: outdated_diff_refs
+      )
+    end
+
+    let(:outdated_diff_refs) { project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs }
+
+    before(:each) do
+      visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+    end
+
+    context 'active discussions' do
+      it 'shows a link to the diff' do
+        within(".discussion[data-discussion-id='#{active_discussion.id}']") do
+          path = diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, anchor: active_discussion.line_code)
+          expect(page).to have_link('the diff', href: path)
+        end
+      end
+    end
+
+    context 'outdated discussions' do
+      it 'shows a link to the outdated diff' do
+        within(".discussion[data-discussion-id='#{outdated_discussion.id}']") do
+          path = diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, diff_id: old_merge_request_diff.id, anchor: outdated_discussion.line_code)
+          expect(page).to have_link('an outdated diff', href: path)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/features/merge_requests/merge_request_versions_spec.rb b/spec/features/merge_requests/versions_spec.rb
similarity index 85%
rename from spec/features/merge_requests/merge_request_versions_spec.rb
rename to spec/features/merge_requests/versions_spec.rb
index 2f627004578..68a68f5d3f3 100644
--- a/spec/features/merge_requests/merge_request_versions_spec.rb
+++ b/spec/features/merge_requests/versions_spec.rb
@@ -36,9 +36,24 @@ feature 'Merge Request versions', js: true, feature: true do
       expect(page).to have_content '5 changed files'
     end
 
-    it 'show the message about disabled comments' do
+    it 'show the message about disabled comment creation' do
       expect(page).to have_content 'comment creation is disabled'
     end
+
+    it 'shows comments that were last relevant at that version' do
+      position = Gitlab::Diff::Position.new(
+        old_path: ".gitmodules",
+        new_path: ".gitmodules",
+        old_line: nil,
+        new_line: 4,
+        diff_refs: merge_request_diff1.diff_refs
+      )
+      outdated_diff_note = create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position)
+      outdated_diff_note.position = outdated_diff_note.original_position
+      outdated_diff_note.save!
+
+      expect(page).to have_css(".diffs .notes[data-discussion-id='#{outdated_diff_note.discussion_id}']")
+    end
   end
 
   describe 'compare with older version' do
diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb
index fb80b74b226..f32b6b99b3d 100644
--- a/spec/models/diff_note_spec.rb
+++ b/spec/models/diff_note_spec.rb
@@ -155,6 +155,23 @@ describe DiffNote, models: true do
     end
   end
 
+  describe '#latest_merge_request_diff' do
+    context 'when active' do
+      it 'returns the current merge request diff' do
+        expect(subject.latest_merge_request_diff).to eq(merge_request.merge_request_diff)
+      end
+    end
+
+    context 'when outdated' do
+      let!(:old_merge_request_diff) { merge_request.merge_request_diff }
+      let!(:new_merge_request_diff) { merge_request.merge_request_diffs.create(diff_refs: commit.diff_refs) }
+
+      it 'returns the latest merge request diff that this diff note applied to' do
+        expect(subject.latest_merge_request_diff).to eq(old_merge_request_diff)
+      end
+    end
+  end
+
   describe "creation" do
     describe "updating of position" do
       context "when noteable is a commit" do

From 543dcdacc2d4536ddeb1d88a7fa85e5fefed3f80 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Tue, 11 Apr 2017 10:51:48 -0500
Subject: [PATCH 11/11] Statisfy Robertcop and Seancop

---
 app/models/merge_request_diff.rb                     | 12 +++---------
 .../projects/merge_requests/show/_versions.html.haml |  2 +-
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 08066db4767..6604af2b47e 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -135,15 +135,9 @@ class MergeRequestDiff < ActiveRecord::Base
   end
 
   def diff_refs=(new_diff_refs)
-    if new_diff_refs
-      self.base_commit_sha = new_diff_refs.base_sha
-      self.start_commit_sha = new_diff_refs.start_sha
-      self.head_commit_sha = new_diff_refs.head_sha
-    else
-      self.base_commit_sha = nil
-      self.start_commit_sha = nil
-      self.head_commit_sha = nil
-    end
+    self.base_commit_sha = new_diff_refs&.base_sha
+    self.start_commit_sha = new_diff_refs&.start_sha
+    self.head_commit_sha = new_diff_refs&.head_sha
   end
 
   def diff_refs
diff --git a/app/views/projects/merge_requests/show/_versions.html.haml b/app/views/projects/merge_requests/show/_versions.html.haml
index 434d9b5837f..547be78992e 100644
--- a/app/views/projects/merge_requests/show/_versions.html.haml
+++ b/app/views/projects/merge_requests/show/_versions.html.haml
@@ -81,7 +81,7 @@
         - if @start_sha
           Comments are disabled because you're comparing two versions of this merge request.
         - else
-          Discussions on this old version of the merge request are displayed but comment creation is disabled.
+          Discussions on this version of the merge request are displayed but comment creation is disabled.
 
         .pull-right
           = link_to 'Show latest version', diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm'