diff options
author | Eric Eastwood <contact@ericeastwood.com> | 2017-06-20 00:18:27 -0500 |
---|---|---|
committer | Eric Eastwood <contact@ericeastwood.com> | 2017-06-20 00:31:21 -0500 |
commit | f7ad9e5abbfd0916a4f93e22d97cbdac17c46986 (patch) | |
tree | 9ab2f5fec18cad9f67b6eacb63a9e9baf566d731 | |
parent | 1c64fa08aeab7e56a4bef7da24840b12eb03b9ac (diff) | |
download | gitlab-ce-f7ad9e5abbfd0916a4f93e22d97cbdac17c46986.tar.gz |
Fix MR "Changes" diff note up arrow to edit last note33868-fix-mr-changes-diff-note-up-arrow-editing
Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/33868
While in a MR "Changes" tab looking at some diff note discussions.
Clicking reply and pressing the up-arrow should edit your last comment in
that discussion.
I suspect this regressed in
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11173/diffs#f4bb984ec495c5cb58516785c62978f5209c39b4_179_178
where we changed the selector
-rw-r--r-- | app/assets/javascripts/notes.js | 2 | ||||
-rw-r--r-- | spec/javascripts/fixtures/merge_requests.rb | 13 | ||||
-rw-r--r-- | spec/javascripts/merge_request_notes_spec.js | 96 |
3 files changed, 78 insertions, 33 deletions
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index f12a35f0485..ab6322a613e 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -187,7 +187,7 @@ const normalizeNewlines = function(str) { if ($textarea.val() !== '') { return; } - myLastNote = $(`li.note[data-author-id='${gon.current_user_id}'][data-editable]:last`, $textarea.closest('.note, #notes')); + myLastNote = $(`li.note[data-author-id='${gon.current_user_id}'][data-editable]:last`, $textarea.closest('.note, .notes_holder, #notes')); if (myLastNote.length) { myLastNoteEditBtn = myLastNote.find('.js-note-edit'); return myLastNoteEditBtn.trigger('click', [true, myLastNote]); diff --git a/spec/javascripts/fixtures/merge_requests.rb b/spec/javascripts/fixtures/merge_requests.rb index a746a776548..0715f4d5f6b 100644 --- a/spec/javascripts/fixtures/merge_requests.rb +++ b/spec/javascripts/fixtures/merge_requests.rb @@ -55,13 +55,20 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont render_merge_request(example.description, merge_request) end + it 'merge_requests/changes_tab_with_comments.json' 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, action: :diffs, format: :json) + end + private - def render_merge_request(fixture_file_name, merge_request) - get :show, + def render_merge_request(fixture_file_name, merge_request, action: :show, format: :html) + get action, namespace_id: project.namespace.to_param, project_id: project, - id: merge_request.to_param + id: merge_request.to_param, + format: format expect(response).to be_success store_frontend_fixture(response, fixture_file_name) diff --git a/spec/javascripts/merge_request_notes_spec.js b/spec/javascripts/merge_request_notes_spec.js index e54acfa8e44..b6d0ce02c4f 100644 --- a/spec/javascripts/merge_request_notes_spec.js +++ b/spec/javascripts/merge_request_notes_spec.js @@ -7,54 +7,92 @@ import '~/render_gfm'; import '~/render_math'; import '~/notes'; +const upArrowKeyCode = 38; + describe('Merge request notes', () => { window.gon = window.gon || {}; window.gl = window.gl || {}; gl.utils = gl.utils || {}; - const fixture = 'merge_requests/diff_comment.html.raw'; - preloadFixtures(fixture); + const discussionTabFixture = 'merge_requests/diff_comment.html.raw'; + const changesTabJsonFixture = 'merge_requests/changes_tab_with_comments.json'; + preloadFixtures(discussionTabFixture, changesTabJsonFixture); - beforeEach(() => { - loadFixtures(fixture); - gl.utils.disableButtonIfEmptyField = _.noop; - window.project_uploads_path = 'http://test.host/uploads'; - $('body').data('page', 'projects:merge_requests:show'); - window.gon.current_user_id = $('.note:last').data('author-id'); + describe('Discussion tab with diff comments', () => { + beforeEach(() => { + loadFixtures(discussionTabFixture); + gl.utils.disableButtonIfEmptyField = _.noop; + window.project_uploads_path = 'http://test.host/uploads'; + $('body').data('page', 'projects:merge_requests:show'); + window.gon.current_user_id = $('.note:last').data('author-id'); - return new Notes('', []); - }); + return new Notes('', []); + }); + + describe('up arrow', () => { + it('edits last comment when triggered in main form', () => { + const upArrowEvent = $.Event('keydown'); + upArrowEvent.which = upArrowKeyCode; + + spyOnEvent('.note:last .js-note-edit', 'click'); + + $('.js-note-text').trigger(upArrowEvent); + + expect('click').toHaveBeenTriggeredOn('.note:last .js-note-edit'); + }); + + it('edits last comment in discussion when triggered in discussion form', (done) => { + const upArrowEvent = $.Event('keydown'); + upArrowEvent.which = upArrowKeyCode; + + spyOnEvent('.note-discussion .js-note-edit', 'click'); + + $('.js-discussion-reply-button').click(); - describe('up arrow', () => { - it('edits last comment when triggered in main form', () => { - const upArrowEvent = $.Event('keydown'); - upArrowEvent.which = 38; + setTimeout(() => { + expect( + $('.note-discussion .js-note-text'), + ).toExist(); - spyOnEvent('.note:last .js-note-edit', 'click'); + $('.note-discussion .js-note-text').trigger(upArrowEvent); - $('.js-note-text').trigger(upArrowEvent); + expect('click').toHaveBeenTriggeredOn('.note-discussion .js-note-edit'); - expect('click').toHaveBeenTriggeredOn('.note:last .js-note-edit'); + done(); + }); + }); }); + }); - it('edits last comment in discussion when triggered in discussion form', (done) => { - const upArrowEvent = $.Event('keydown'); - upArrowEvent.which = 38; + describe('Changes tab with diff comments', () => { + beforeEach(() => { + const diffsResponse = getJSONFixture(changesTabJsonFixture); + const noteFormHtml = `<form class="js-new-note-form"> + <textarea class="js-note-text"></textarea> + </form>`; + setFixtures(diffsResponse.html + noteFormHtml); + $('body').data('page', 'projects:merge_requests:show'); + window.gon.current_user_id = $('.note:last').data('author-id'); + + return new Notes('', []); + }); - spyOnEvent('.note-discussion .js-note-edit', 'click'); + describe('up arrow', () => { + it('edits last comment in discussion when triggered in discussion form', (done) => { + const upArrowEvent = $.Event('keydown'); + upArrowEvent.which = upArrowKeyCode; - $('.js-discussion-reply-button').click(); + spyOnEvent('.note:last .js-note-edit', 'click'); - setTimeout(() => { - expect( - $('.note-discussion .js-note-text'), - ).toExist(); + $('.js-discussion-reply-button').trigger('click'); - $('.note-discussion .js-note-text').trigger(upArrowEvent); + setTimeout(() => { + $('.js-note-text').trigger(upArrowEvent); - expect('click').toHaveBeenTriggeredOn('.note-discussion .js-note-edit'); + expect('click').toHaveBeenTriggeredOn('.note:last .js-note-edit'); - done(); + done(); + }); }); }); }); |