Merge remote-tracking branch 'origin/fix-9-2-stable-conflicts-for-mr-11700' into 9-2-stable

3 files changed, 61 insertions, 2 deletions

diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
index c709730f78f..e40d6572b18 100644
--- a/app/assets/javascripts/merge_request_tabs.js
+++ b/app/assets/javascripts/merge_request_tabs.js
@@ -285,7 +285,7 @@ import BlobForkSuggestion from './blob/blob_fork_suggestion';
           // Similar to `toggler_behavior` in the discussion tab
           const hash = window.gl.utils.getLocationHash();
           const anchor = hash && $container.find(`[id="${hash}"]`);
-          if (anchor) {
+          if (anchor && anchor.length > 0) {
             const notesContent = anchor.closest('.notes_content');
             const lineType = notesContent.hasClass('new') ? 'new' : 'old';
             notes.toggleDiffNote({
diff --git a/spec/javascripts/fixtures/merge_requests.rb b/spec/javascripts/fixtures/merge_requests.rb
index 47d904b865b..a746a776548 100644
--- a/spec/javascripts/fixtures/merge_requests.rb
+++ b/spec/javascripts/fixtures/merge_requests.rb
@@ -16,6 +16,16 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont
       sha: merge_request.diff_head_sha
     )
   end
+  let(:path) { "files/ruby/popen.rb" }
+  let(:position) do
+    Gitlab::Diff::Position.new(
+      old_path: path,
+      new_path: path,
+      old_line: nil,
+      new_line: 14,
+      diff_refs: merge_request.diff_refs
+    )
+  end
 
   render_views
 
@@ -39,6 +49,12 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont
     render_merge_request(example.description, merged_merge_request)
   end
 
+  it 'merge_requests/diff_comment.html.raw' do |example|
+    create(:diff_note_on_merge_request, project: project, author: admin, position: position, noteable: merge_request)
+    create(:note_on_merge_request, author: admin, project: project, noteable: merge_request)
+    render_merge_request(example.description, merge_request)
+  end
+
   private
 
   def render_merge_request(fixture_file_name, merge_request)
diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js
index e437333d522..d119fc0c11e 100644
--- a/spec/javascripts/merge_request_tabs_spec.js
+++ b/spec/javascripts/merge_request_tabs_spec.js
@@ -1,4 +1,5 @@
 /* eslint-disable no-var, comma-dangle, object-shorthand */
+/* global Notes */
 
 require('~/merge_request_tabs');
 require('~/commit/pipelines/pipelines_bundle.js');
@@ -7,6 +8,7 @@ require('~/lib/utils/common_utils');
 require('~/diff');
 require('~/single_file_diff');
 require('~/files_comment_button');
+require('~/notes');
 require('vendor/jquery.scrollTo');
 
 (function () {
@@ -29,7 +31,7 @@ require('vendor/jquery.scrollTo');
       };
       $.extend(stubLocation, defaults, stubs || {});
     };
-    preloadFixtures('merge_requests/merge_request_with_task_list.html.raw');
+    preloadFixtures('merge_requests/merge_request_with_task_list.html.raw', 'merge_requests/diff_comment.html.raw');
 
     beforeEach(function () {
       this.class = new gl.MergeRequestTabs({ stubLocation: stubLocation });
@@ -286,8 +288,49 @@ require('vendor/jquery.scrollTo');
         spyOn($, 'ajax').and.callFake(function (options) {
           expect(options.url).toEqual('/foo/bar/merge_requests/1/diffs.json');
         });
+
         this.class.loadDiff('/foo/bar/merge_requests/1/diffs');
       });
+
+      describe('with note fragment hash', () => {
+        beforeEach(() => {
+          loadFixtures('merge_requests/diff_comment.html.raw');
+          spyOn(window.gl.utils, 'getPagePath').and.returnValue('merge_requests');
+          window.notes = new Notes('', []);
+          spyOn(window.notes, 'toggleDiffNote').and.callThrough();
+        });
+
+        afterEach(() => {
+          delete window.notes;
+        });
+
+        it('should expand and scroll to linked fragment hash #note_xxx', function () {
+          const noteId = 'note_1';
+          spyOn(window.gl.utils, 'getLocationHash').and.returnValue(noteId);
+          spyOn($, 'ajax').and.callFake(function (options) {
+            options.success({ html: `<div id="${noteId}">foo</div>` });
+          });
+
+          this.class.loadDiff('/foo/bar/merge_requests/1/diffs');
+
+          expect(window.notes.toggleDiffNote).toHaveBeenCalledWith({
+            target: jasmine.any(Object),
+            lineType: 'old',
+            forceShow: true,
+          });
+        });
+
+        it('should gracefully ignore non-existant fragment hash', function () {
+          spyOn(window.gl.utils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist');
+          spyOn($, 'ajax').and.callFake(function (options) {
+            options.success({ html: '' });
+          });
+
+          this.class.loadDiff('/foo/bar/merge_requests/1/diffs');
+
+          expect(window.notes.toggleDiffNote).not.toHaveBeenCalled();
+        });
+      });
     });
   });
 }).call(window);