summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Eastwood <contact@ericeastwood.com>2017-06-20 00:18:27 -0500
committerEric Eastwood <contact@ericeastwood.com>2017-06-20 00:31:21 -0500
commitf7ad9e5abbfd0916a4f93e22d97cbdac17c46986 (patch)
tree9ab2f5fec18cad9f67b6eacb63a9e9baf566d731
parent1c64fa08aeab7e56a4bef7da24840b12eb03b9ac (diff)
downloadgitlab-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.js2
-rw-r--r--spec/javascripts/fixtures/merge_requests.rb13
-rw-r--r--spec/javascripts/merge_request_notes_spec.js96
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();
+ });
});
});
});