Link to outdated diff in older MR version from outdated diff discussion

app/assets/stylesheets/pages/merge_requests.scss

@@ -528,6 +528,8 @@
   }
 
   .comments-disabled-notif {
+    line-height: 28px;
+
     .btn {
       margin-left: 5px;
     }

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,10 +124,13 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
         @environment = @merge_request.environments_for(current_user).last
 
+        @diff_notes_disabled = !@merge_request_diff.latest? || @start_sha
+
+        @diffs =
           if @start_sha
-          compared_diff_version
+            @merge_request_diff.compare_with(@start_sha).diffs(diff_options)
           else
-          original_diff_version
+            @merge_request_diff.diffs(diff_options)
           end
 
         render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") }
@@ -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

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

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

app/models/diff_discussion.rb

@@ -10,6 +10,7 @@ class DiffDiscussion < Discussion
 
   delegate  :position,
             :original_position,
+            :latest_merge_request_diff,
 
             to: :first_note
 

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

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
 

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

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.

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

app/views/discussions/_discussion.html.haml

@@ -33,6 +33,11 @@
               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
                   an outdated diff
 

app/views/projects/diffs/_parallel_view.html.haml

@@ -5,7 +5,6 @@
       - 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)
       %tr.line_holder.parallel
         - if left

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

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.
+          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'

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: