From 0d554df6eb9e1c14060cf6be51d5527fe3f95baa Mon Sep 17 00:00:00 2001 From: Clement Ho Date: Fri, 26 May 2017 14:26:24 +0000 Subject: 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 --- app/assets/javascripts/merge_request_tabs.js | 2 +- 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: `
foo
` }); + }); + + 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); -- cgit v1.2.1 From 0ef4d3cc569f9a04c6872146a2c293e6d457bdde Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Thu, 8 Jun 2017 10:07:51 -0500 Subject: Fix up merge_reqest_tabs import/require conflicts See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12011 --- spec/javascripts/fixtures/merge_requests.rb | 16 ++++++++++++++++ spec/javascripts/merge_request_tabs_spec.js | 13 +------------ 2 files changed, 17 insertions(+), 12 deletions(-) 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 36bc5cf02c2..d119fc0c11e 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -1,7 +1,6 @@ /* eslint-disable no-var, comma-dangle, object-shorthand */ /* global Notes */ -<<<<<<< HEAD require('~/merge_request_tabs'); require('~/commit/pipelines/pipelines_bundle.js'); require('~/breakpoints'); @@ -9,18 +8,8 @@ require('~/lib/utils/common_utils'); require('~/diff'); require('~/single_file_diff'); require('~/files_comment_button'); +require('~/notes'); 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! -- cgit v1.2.1