From 43d50117187db1d8e034dbfc01e894a108f55369 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Thu, 25 Aug 2016 16:42:20 +0100
Subject: [PATCH] Fix diff comments on legacy MRs

---
 CHANGELOG                                     |  3 ++
 app/models/legacy_diff_note.rb                |  4 +++
 .../merge_requests/diff_notes_spec.rb         | 31 +++++++++++++++++++
 3 files changed, 38 insertions(+)

diff --git a/CHANGELOG b/CHANGELOG
index df8dec7bdde..f8391e996fb 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -32,6 +32,9 @@ v 8.12.0 (unreleased)
   - Use the default branch for displaying the project icon instead of master !5792 (Hannes Rosenögger)
   - Adds response mime type to transaction metric action when it's not HTML
 
+v 8.11.4 (unreleased)
+  - Fix diff commenting on merge requests created prior to 8.10
+
 v 8.11.3 (unreleased)
   - Allow system info page to handle case where info is unavailable
   - Label list shows all issues (opened or closed) with that label
diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb
index 40277a9b139..0e1649aafe5 100644
--- a/app/models/legacy_diff_note.rb
+++ b/app/models/legacy_diff_note.rb
@@ -53,6 +53,10 @@ class LegacyDiffNote < Note
     self.line_code
   end
 
+  def to_discussion
+    Discussion.new([self])
+  end
+
   # Check if this note is part of an "active" discussion
   #
   # This will always return true for anything except MergeRequest noteables,
diff --git a/spec/features/merge_requests/diff_notes_spec.rb b/spec/features/merge_requests/diff_notes_spec.rb
index a818679a874..06fad1007e8 100644
--- a/spec/features/merge_requests/diff_notes_spec.rb
+++ b/spec/features/merge_requests/diff_notes_spec.rb
@@ -147,6 +147,37 @@ feature 'Diff notes', js: true, feature: true do
       end
     end
 
+    context 'when the MR only supports legacy diff notes' do
+      before do
+        @merge_request.merge_request_diff.update_attributes(start_commit_sha: nil)
+        visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request, view: 'inline')
+      end
+
+      context 'with a new line' do
+        it 'should allow commenting' do
+          should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
+        end
+      end
+
+      context 'with an old line' do
+        it 'should allow commenting' do
+          should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'))
+        end
+      end
+
+      context 'with an unchanged line' do
+        it 'should allow commenting' do
+          should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]'))
+        end
+      end
+
+      context 'with a match line' do
+        it 'should not allow commenting' do
+          should_not_allow_commenting(find('.match', match: :first))
+        end
+      end
+    end
+
     def should_allow_commenting(line_holder, diff_side = nil)
       line = get_line_components(line_holder, diff_side)
       line[:content].hover