diff options
author | Clement Ho <clemmakesapps@gmail.com> | 2017-05-26 14:26:24 +0000 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2017-06-12 08:03:06 +0000 |
commit | 0d554df6eb9e1c14060cf6be51d5527fe3f95baa (patch) | |
tree | febbf7abeb260457c0a27614f6c68098b8a47bec | |
parent | 04ba6ae564e406b3bfa244f5bdb4ab80151fbfd3 (diff) | |
download | gitlab-ce-0d554df6eb9e1c14060cf6be51d5527fe3f95baa.tar.gz |
Merge branch '32888-fix-error-after-missing-note-hash-fragment-in-dom' into 'master'
Fix error thrown with missing note fragment in DOM
Closes #32888
See merge request !11700
Conflicts:
spec/javascripts/merge_request_tabs_spec.js
-rw-r--r-- | app/assets/javascripts/merge_request_tabs.js | 2 | ||||
-rw-r--r-- | spec/javascripts/merge_request_tabs_spec.js | 56 |
2 files changed, 56 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/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index e437333d522..36bc5cf02c2 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -1,5 +1,7 @@ /* eslint-disable no-var, comma-dangle, object-shorthand */ +/* global Notes */ +<<<<<<< HEAD require('~/merge_request_tabs'); require('~/commit/pipelines/pipelines_bundle.js'); require('~/breakpoints'); @@ -8,6 +10,17 @@ require('~/diff'); require('~/single_file_diff'); require('~/files_comment_button'); require('vendor/jquery.scrollTo'); +======= +import '~/merge_request_tabs'; +import '~/commit/pipelines/pipelines_bundle'; +import '~/breakpoints'; +import '~/lib/utils/common_utils'; +import '~/diff'; +import '~/single_file_diff'; +import '~/files_comment_button'; +import '~/notes'; +import 'vendor/jquery.scrollTo'; +>>>>>>> a59165e... Merge branch '32888-fix-error-after-missing-note-hash-fragment-in-dom' into 'master' (function () { // TODO: remove this hack! @@ -29,7 +42,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 +299,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); |