From 645593e5af1fe9e7fa345788aeaa90d2313d6486 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Fri, 5 May 2017 10:57:29 +0000 Subject: [PATCH] Add instant comments support --- .../javascripts/behaviors/quick_submit.js | 2 +- .../javascripts/lib/utils/common_utils.js | 8 + app/assets/javascripts/notes.js | 345 +++++++++++++++--- .../stylesheets/framework/animations.scss | 28 ++ app/assets/stylesheets/pages/notes.scss | 23 ++ app/views/discussions/_notes.html.haml | 1 + app/views/projects/notes/_edit_form.html.haml | 2 +- .../unreleased/27614-instant-comments.yml | 4 + features/steps/project/merge_requests.rb | 7 + features/steps/shared/note.rb | 4 + .../merge_requests/user_posts_notes_spec.rb | 1 + .../user_uses_slash_commands_spec.rb | 1 + .../lib/utils/common_utils_spec.js | 11 + spec/javascripts/notes_spec.js | 229 +++++++++++- ...issuable_slash_commands_shared_examples.rb | 1 + spec/support/time_tracking_shared_examples.rb | 5 + 16 files changed, 607 insertions(+), 65 deletions(-) create mode 100644 changelogs/unreleased/27614-instant-comments.yml diff --git a/app/assets/javascripts/behaviors/quick_submit.js b/app/assets/javascripts/behaviors/quick_submit.js index 3d162b24413..1f9e0448084 100644 --- a/app/assets/javascripts/behaviors/quick_submit.js +++ b/app/assets/javascripts/behaviors/quick_submit.js @@ -43,8 +43,8 @@ $(document).on('keydown.quick_submit', '.js-quick-submit', (e) => { const $submitButton = $form.find('input[type=submit], button[type=submit]'); if (!$submitButton.attr('disabled')) { + $submitButton.trigger('click', [e]); $submitButton.disable(); - $form.submit(); } }); diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index 8058672eaa9..2f682fbd2fb 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -35,6 +35,14 @@ }); }; + w.gl.utils.ajaxPost = function(url, data) { + return $.ajax({ + type: 'POST', + url: url, + data: data, + }); + }; + w.gl.utils.extractLast = function(term) { return this.split(term).pop(); }; diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 87f03a40eba..72709f68070 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -26,12 +26,13 @@ const normalizeNewlines = function(str) { this.Notes = (function() { const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; + const REGEX_SLASH_COMMANDS = /\/\w+/g; Notes.interval = null; function Notes(notes_url, note_ids, last_fetched_at, view) { this.updateTargetButtons = bind(this.updateTargetButtons, this); - this.updateCloseButton = bind(this.updateCloseButton, this); + this.updateComment = bind(this.updateComment, this); this.visibilityChange = bind(this.visibilityChange, this); this.cancelDiscussionForm = bind(this.cancelDiscussionForm, this); this.addDiffNote = bind(this.addDiffNote, this); @@ -47,6 +48,7 @@ const normalizeNewlines = function(str) { this.refresh = bind(this.refresh, this); this.keydownNoteText = bind(this.keydownNoteText, this); this.toggleCommitList = bind(this.toggleCommitList, this); + this.postComment = bind(this.postComment, this); this.notes_url = notes_url; this.note_ids = note_ids; @@ -82,28 +84,19 @@ const normalizeNewlines = function(str) { }; Notes.prototype.addBinding = function() { - // add note to UI after creation - $(document).on("ajax:success", ".js-main-target-form", this.addNote); - $(document).on("ajax:success", ".js-discussion-note-form", this.addDiscussionNote); - // catch note ajax errors - $(document).on("ajax:error", ".js-main-target-form", this.addNoteError); - // change note in UI after update - $(document).on("ajax:success", "form.edit-note", this.updateNote); // Edit note link $(document).on("click", ".js-note-edit", this.showEditForm.bind(this)); $(document).on("click", ".note-edit-cancel", this.cancelEdit); // Reopen and close actions for Issue/MR combined with note form submit - $(document).on("click", ".js-comment-button", this.updateCloseButton); + $(document).on("click", ".js-comment-submit-button", this.postComment); + $(document).on("click", ".js-comment-save-button", this.updateComment); $(document).on("keyup input", ".js-note-text", this.updateTargetButtons); // resolve a discussion - $(document).on('click', '.js-comment-resolve-button', this.resolveDiscussion); + $(document).on('click', '.js-comment-resolve-button', this.postComment); // remove a note (in general) $(document).on("click", ".js-note-delete", this.removeNote); // delete note attachment $(document).on("click", ".js-note-attachment-delete", this.removeAttachment); - // reset main target form after submit - $(document).on("ajax:complete", ".js-main-target-form", this.reenableTargetFormSubmitButton); - $(document).on("ajax:success", ".js-main-target-form", this.resetMainTargetForm); // reset main target form when clicking discard $(document).on("click", ".js-note-discard", this.resetMainTargetForm); // update the file name when an attachment is selected @@ -120,20 +113,20 @@ const normalizeNewlines = function(str) { $(document).on("visibilitychange", this.visibilityChange); // when issue status changes, we need to refresh data $(document).on("issuable:change", this.refresh); + // ajax:events that happen on Form when actions like Reopen, Close are performed on Issues and MRs. + $(document).on("ajax:success", ".js-main-target-form", this.addNote); + $(document).on("ajax:success", ".js-discussion-note-form", this.addDiscussionNote); + $(document).on("ajax:success", ".js-main-target-form", this.resetMainTargetForm); + $(document).on("ajax:complete", ".js-main-target-form", this.reenableTargetFormSubmitButton); // when a key is clicked on the notes return $(document).on("keydown", ".js-note-text", this.keydownNoteText); }; Notes.prototype.cleanBinding = function() { - $(document).off("ajax:success", ".js-main-target-form"); - $(document).off("ajax:success", ".js-discussion-note-form"); - $(document).off("ajax:success", "form.edit-note"); $(document).off("click", ".js-note-edit"); $(document).off("click", ".note-edit-cancel"); $(document).off("click", ".js-note-delete"); $(document).off("click", ".js-note-attachment-delete"); - $(document).off("ajax:complete", ".js-main-target-form"); - $(document).off("ajax:success", ".js-main-target-form"); $(document).off("click", ".js-discussion-reply-button"); $(document).off("click", ".js-add-diff-note-button"); $(document).off("visibilitychange"); @@ -144,6 +137,9 @@ const normalizeNewlines = function(str) { $(document).off("keydown", ".js-note-text"); $(document).off('click', '.js-comment-resolve-button'); $(document).off("click", '.system-note-commit-list-toggler'); + $(document).off("ajax:success", ".js-main-target-form"); + $(document).off("ajax:success", ".js-discussion-note-form"); + $(document).off("ajax:complete", ".js-main-target-form"); }; Notes.initCommentTypeToggle = function (form) { @@ -276,12 +272,8 @@ const normalizeNewlines = function(str) { return this.initRefresh(); }; - Notes.prototype.handleCreateChanges = function(noteEntity) { + Notes.prototype.handleSlashCommands = function(noteEntity) { var votesBlock; - if (typeof noteEntity === 'undefined') { - return; - } - if (noteEntity.commands_changes) { if ('merge' in noteEntity.commands_changes) { $.get(mrRefreshWidgetUrl); @@ -556,24 +548,29 @@ const normalizeNewlines = function(str) { Adds new note to list. */ - Notes.prototype.addNote = function(xhr, note, status) { - this.handleCreateChanges(note); + Notes.prototype.addNote = function($form, note) { return this.renderNote(note); }; - Notes.prototype.addNoteError = function(xhr, note, status) { - return new Flash('Your comment could not be submitted! Please check your network connection and try again.', 'alert', this.parentTimeline); + Notes.prototype.addNoteError = ($form) => { + let formParentTimeline; + if ($form.hasClass('js-main-target-form')) { + formParentTimeline = $form.parents('.timeline'); + } else if ($form.hasClass('js-discussion-note-form')) { + formParentTimeline = $form.closest('.discussion-notes').find('.notes'); + } + return new Flash('Your comment could not be submitted! Please check your network connection and try again.', 'alert', formParentTimeline); }; + Notes.prototype.updateNoteError = $parentTimeline => new Flash('Your comment could not be updated! Please check your network connection and try again.'); + /* Called in response to the new note form being submitted Adds new note to list. */ - Notes.prototype.addDiscussionNote = function(xhr, note, status) { - var $form = $(xhr.target); - + Notes.prototype.addDiscussionNote = function($form, note, isNewDiffComment) { if ($form.attr('data-resolve-all') != null) { var projectPath = $form.data('project-path'); var discussionId = $form.data('discussion-id'); @@ -586,7 +583,9 @@ const normalizeNewlines = function(str) { this.renderNote(note, $form); // cleanup after successfully creating a diff/discussion note - this.removeDiscussionNoteForm($form); + if (isNewDiffComment) { + this.removeDiscussionNoteForm($form); + } }; /* @@ -596,17 +595,18 @@ const normalizeNewlines = function(str) { */ Notes.prototype.updateNote = function(_xhr, noteEntity, _status) { - var $html, $note_li; + var $noteEntityEl, $note_li; // Convert returned HTML to a jQuery object so we can modify it further - $html = $(noteEntity.html); + $noteEntityEl = $(noteEntity.html); + $noteEntityEl.addClass('fade-in-full'); this.revertNoteEditForm(); - gl.utils.localTimeAgo($('.js-timeago', $html)); - $html.renderGFM(); - $html.find('.js-task-list-container').taskList('enable'); + gl.utils.localTimeAgo($('.js-timeago', $noteEntityEl)); + $noteEntityEl.renderGFM(); + $noteEntityEl.find('.js-task-list-container').taskList('enable'); // Find the note's `li` element by ID and replace it with the updated HTML $note_li = $('.note-row-' + noteEntity.id); - $note_li.replaceWith($html); + $note_li.replaceWith($noteEntityEl); if (typeof gl.diffNotesCompileComponents !== 'undefined') { gl.diffNotesCompileComponents(); @@ -698,7 +698,7 @@ const normalizeNewlines = function(str) { var $editForm = $(selector); $editForm.insertBefore('.notes-form'); - $editForm.find('.js-comment-button').enable(); + $editForm.find('.js-comment-save-button').enable(); $editForm.find('.js-finish-edit-warning').hide(); }; @@ -982,14 +982,6 @@ const normalizeNewlines = function(str) { return this.refresh(); }; - Notes.prototype.updateCloseButton = function(e) { - var closebtn, form, textarea; - textarea = $(e.target); - form = textarea.parents('form'); - closebtn = form.find('.js-note-target-close'); - return closebtn.text(closebtn.data('original-text')); - }; - Notes.prototype.updateTargetButtons = function(e) { var closebtn, closetext, discardbtn, form, reopenbtn, reopentext, textarea; textarea = $(e.target); @@ -1078,17 +1070,6 @@ const normalizeNewlines = function(str) { return this.notesCountBadge.text(parseInt(this.notesCountBadge.text(), 10) + updateCount); }; - Notes.prototype.resolveDiscussion = function() { - var $this = $(this); - var discussionId = $this.attr('data-discussion-id'); - - $this - .closest('form') - .attr('data-discussion-id', discussionId) - .attr('data-resolve-all', 'true') - .attr('data-project-path', $this.attr('data-project-path')); - }; - Notes.prototype.toggleCommitList = function(e) { const $element = $(e.currentTarget); const $closestSystemCommitList = $element.siblings('.system-note-commit-list'); @@ -1137,7 +1118,7 @@ const normalizeNewlines = function(str) { Notes.animateAppendNote = function(noteHtml, $notesList) { const $note = $(noteHtml); - $note.addClass('fade-in').renderGFM(); + $note.addClass('fade-in-full').renderGFM(); $notesList.append($note); return $note; }; @@ -1150,6 +1131,254 @@ const normalizeNewlines = function(str) { return $updatedNote; }; + /** + * Get data from Form attributes to use for saving/submitting comment. + */ + Notes.prototype.getFormData = function($form) { + return { + formData: $form.serialize(), + formContent: $form.find('.js-note-text').val(), + formAction: $form.attr('action'), + }; + }; + + /** + * Identify if comment has any slash commands + */ + Notes.prototype.hasSlashCommands = function(formContent) { + return REGEX_SLASH_COMMANDS.test(formContent); + }; + + /** + * Remove slash commands and leave comment with pure message + */ + Notes.prototype.stripSlashCommands = function(formContent) { + return formContent.replace(REGEX_SLASH_COMMANDS, '').trim(); + }; + + /** + * Create placeholder note DOM element populated with comment body + * that we will show while comment is being posted. + * Once comment is _actually_ posted on server, we will have final element + * in response that we will show in place of this temporary element. + */ + Notes.prototype.createPlaceholderNote = function({ formContent, uniqueId, isDiscussionNote, currentUsername, currentUserFullname }) { + const discussionClass = isDiscussionNote ? 'discussion' : ''; + const $tempNote = $( + `
  • +
    +
    + +
    +
    +
    +
    + + + @${currentUsername} + + + + +
    +
    +
    +
    +

    ${formContent}

    +
    +
    +
    +
    +
    • ` + ); + + return $tempNote; + }; + + /** + * This method does following tasks step-by-step whenever a new comment + * is submitted by user (both main thread comments as well as discussion comments). + * + * 1) Get Form metadata + * 2) Identify comment type; a) Main thread b) Discussion thread c) Discussion resolve + * 3) Build temporary placeholder element (using `createPlaceholderNote`) + * 4) Show placeholder note on UI + * 5) Perform network request to submit the note using `gl.utils.ajaxPost` + * a) If request is successfully completed + * 1. Remove placeholder element + * 2. Show submitted Note element + * 3. Perform post-submit errands + * a. Mark discussion as resolved if comment submission was for resolve. + * b. Reset comment form to original state. + * b) If request failed + * 1. Remove placeholder element + * 2. Show error Flash message about failure + */ + Notes.prototype.postComment = function(e) { + e.preventDefault(); + + // Get Form metadata + const $submitBtn = $(e.target); + let $form = $submitBtn.parents('form'); + const $closeBtn = $form.find('.js-note-target-close'); + const isDiscussionNote = $submitBtn.parent().find('li.droplab-item-selected').attr('id') === 'discussion'; + const isMainForm = $form.hasClass('js-main-target-form'); + const isDiscussionForm = $form.hasClass('js-discussion-note-form'); + const isDiscussionResolve = $submitBtn.hasClass('js-comment-resolve-button'); + const { formData, formContent, formAction } = this.getFormData($form); + const uniqueId = _.uniqueId('tempNote_'); + let $notesContainer; + let tempFormContent; + + // Get reference to notes container based on type of comment + if (isDiscussionForm) { + $notesContainer = $form.parent('.discussion-notes').find('.notes'); + } else if (isMainForm) { + $notesContainer = $('ul.main-notes-list'); + } + + // If comment is to resolve discussion, disable submit buttons while + // comment posting is finished. + if (isDiscussionResolve) { + $submitBtn.disable(); + $form.find('.js-comment-submit-button').disable(); + } + + tempFormContent = formContent; + if (this.hasSlashCommands(formContent)) { + tempFormContent = this.stripSlashCommands(formContent); + } + + if (tempFormContent) { + // Show placeholder note + $notesContainer.append(this.createPlaceholderNote({ + formContent: tempFormContent, + uniqueId, + isDiscussionNote, + currentUsername: gon.current_username, + currentUserFullname: gon.current_user_fullname, + })); + } + + // Clear the form textarea + if ($notesContainer.length) { + if (isMainForm) { + this.resetMainTargetForm(e); + } else if (isDiscussionForm) { + this.removeDiscussionNoteForm($form); + } + } + + /* eslint-disable promise/catch-or-return */ + // Make request to submit comment on server + gl.utils.ajaxPost(formAction, formData) + .then((note) => { + // Submission successful! remove placeholder + $notesContainer.find(`#${uniqueId}`).remove(); + + // Check if this was discussion comment + if (isDiscussionForm) { + // Remove flash-container + $notesContainer.find('.flash-container').remove(); + + // If comment intends to resolve discussion, do the same. + if (isDiscussionResolve) { + $form + .attr('data-discussion-id', $submitBtn.data('discussion-id')) + .attr('data-resolve-all', 'true') + .attr('data-project-path', $submitBtn.data('project-path')); + } + + // Show final note element on UI + this.addDiscussionNote($form, note, $notesContainer.length === 0); + + // append flash-container to the Notes list + if ($notesContainer.length) { + $notesContainer.append(''); + } + } else if (isMainForm) { // Check if this was main thread comment + // Show final note element on UI and perform form and action buttons cleanup + this.addNote($form, note); + this.reenableTargetFormSubmitButton(e); + } + + if (note.commands_changes) { + this.handleSlashCommands(note); + } + + $form.trigger('ajax:success', [note]); + }).fail(() => { + // Submission failed, remove placeholder note and show Flash error message + $notesContainer.find(`#${uniqueId}`).remove(); + + // Show form again on UI on failure + if (isDiscussionForm && $notesContainer.length) { + const replyButton = $notesContainer.parent().find('.js-discussion-reply-button'); + $.proxy(this.replyToDiscussionNote, replyButton[0], { target: replyButton[0] }).call(); + $form = $notesContainer.parent().find('form'); + } + + $form.find('.js-note-text').val(formContent); + this.reenableTargetFormSubmitButton(e); + this.addNoteError($form); + }); + + return $closeBtn.text($closeBtn.data('original-text')); + }; + + /** + * This method does following tasks step-by-step whenever an existing comment + * is updated by user (both main thread comments as well as discussion comments). + * + * 1) Get Form metadata + * 2) Update note element with new content + * 3) Perform network request to submit the updated note using `gl.utils.ajaxPost` + * a) If request is successfully completed + * 1. Show submitted Note element + * b) If request failed + * 1. Revert Note element to original content + * 2. Show error Flash message about failure + */ + Notes.prototype.updateComment = function(e) { + e.preventDefault(); + + // Get Form metadata + const $submitBtn = $(e.target); + const $form = $submitBtn.parents('form'); + const $closeBtn = $form.find('.js-note-target-close'); + const $editingNote = $form.parents('.note.is-editing'); + const $noteBody = $editingNote.find('.js-task-list-container'); + const $noteBodyText = $noteBody.find('.note-text'); + const { formData, formContent, formAction } = this.getFormData($form); + + // Cache original comment content + const cachedNoteBodyText = $noteBodyText.html(); + + // Show updated comment content temporarily + $noteBodyText.html(formContent); + $editingNote.removeClass('is-editing').addClass('being-posted fade-in-half'); + $editingNote.find('.note-headline-meta a').html(''); + + /* eslint-disable promise/catch-or-return */ + // Make request to update comment on server + gl.utils.ajaxPost(formAction, formData) + .then((note) => { + // Submission successful! render final note element + this.updateNote(null, note, null); + }) + .fail(() => { + // Submission failed, revert back to original note + $noteBodyText.html(cachedNoteBodyText); + $editingNote.removeClass('being-posted fade-in'); + $editingNote.find('.fa.fa-spinner').remove(); + + // Show Flash message about failure + this.updateNoteError(); + }); + + return $closeBtn.text($closeBtn.data('original-text')); + }; + return Notes; })(); }).call(window); diff --git a/app/assets/stylesheets/framework/animations.scss b/app/assets/stylesheets/framework/animations.scss index 7c50b80fd2b..3cd7f81da47 100644 --- a/app/assets/stylesheets/framework/animations.scss +++ b/app/assets/stylesheets/framework/animations.scss @@ -159,3 +159,31 @@ a { .fade-in { animation: fadeIn $fade-in-duration 1; } + +@keyframes fadeInHalf { + 0% { + opacity: 0; + } + + 100% { + opacity: 0.5; + } +} + +.fade-in-half { + animation: fadeInHalf $fade-in-duration 1; +} + +@keyframes fadeInFull { + 0% { + opacity: 0.5; + } + + 100% { + opacity: 1; + } +} + +.fade-in-full { + animation: fadeInFull $fade-in-duration 1; +} diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index f89150ebead..cfea52c6e57 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -57,6 +57,25 @@ ul.notes { position: relative; border-bottom: 1px solid $white-normal; + &.being-posted { + pointer-events: none; + opacity: 0.5; + + .dummy-avatar { + display: inline-block; + height: 40px; + width: 40px; + border-radius: 50%; + background-color: $kdb-border; + border: 1px solid darken($kdb-border, 25%); + } + + .note-headline-light, + .fa-spinner { + margin-left: 3px; + } + } + &.note-discussion { &.timeline-entry { padding: 14px 10px; @@ -687,6 +706,10 @@ ul.notes { } } +.discussion-notes .flash-container { + margin-bottom: 0; +} + // Merge request notes in diffs .diff-file { // Diff is side by side diff --git a/app/views/discussions/_notes.html.haml b/app/views/discussions/_notes.html.haml index 964473ee3e0..7ba3f3f6c42 100644 --- a/app/views/discussions/_notes.html.haml +++ b/app/views/discussions/_notes.html.haml @@ -1,6 +1,7 @@ .discussion-notes %ul.notes{ data: { discussion_id: discussion.id } } = render partial: "shared/notes/note", collection: discussion.notes, as: :note + .flash-container - if current_user .discussion-reply-holder diff --git a/app/views/projects/notes/_edit_form.html.haml b/app/views/projects/notes/_edit_form.html.haml index 00230b0bdf8..3867072225f 100644 --- a/app/views/projects/notes/_edit_form.html.haml +++ b/app/views/projects/notes/_edit_form.html.haml @@ -9,6 +9,6 @@ .note-form-actions.clearfix .settings-message.note-edit-warning.js-finish-edit-warning Finish editing this message first! - = submit_tag 'Save comment', class: 'btn btn-nr btn-save js-comment-button' + = submit_tag 'Save comment', class: 'btn btn-nr btn-save js-comment-save-button' %button.btn.btn-nr.btn-cancel.note-edit-cancel{ type: 'button' } Cancel diff --git a/changelogs/unreleased/27614-instant-comments.yml b/changelogs/unreleased/27614-instant-comments.yml new file mode 100644 index 00000000000..7b2592f46ed --- /dev/null +++ b/changelogs/unreleased/27614-instant-comments.yml @@ -0,0 +1,4 @@ +--- +title: Add support for instantly updating comments +merge_request: 10760 +author: diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index a06b2f2911f..4b7d6cd840b 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -458,6 +458,8 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps click_button "Comment" end + wait_for_ajax + page.within ".files>div:nth-child(2) .note-body > .note-text" do expect(page).to have_content "Line is correct" end @@ -470,6 +472,8 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps fill_in "note_note", with: "Line is wrong on here" click_button "Comment" end + + wait_for_ajax end step 'I should still see a comment like "Line is correct" in the second file' do @@ -574,6 +578,9 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps fill_in "note_note", with: message click_button "Comment" end + + wait_for_ajax + page.within(".notes_holder", visible: true) do expect(page).to have_content message end diff --git a/features/steps/shared/note.rb b/features/steps/shared/note.rb index 7885cc7ab77..7d260025052 100644 --- a/features/steps/shared/note.rb +++ b/features/steps/shared/note.rb @@ -24,6 +24,8 @@ module SharedNote fill_in "note[note]", with: "XML attached" click_button "Comment" end + + wait_for_ajax end step 'I preview a comment text like "Bug fixed :smile:"' do @@ -37,6 +39,8 @@ module SharedNote page.within(".js-main-target-form") do click_button "Comment" end + + wait_for_ajax end step 'I write a comment like ":+1: Nice"' do diff --git a/spec/features/merge_requests/user_posts_notes_spec.rb b/spec/features/merge_requests/user_posts_notes_spec.rb index c7cc4d6bc72..7fc0e2ce6ec 100644 --- a/spec/features/merge_requests/user_posts_notes_spec.rb +++ b/spec/features/merge_requests/user_posts_notes_spec.rb @@ -98,6 +98,7 @@ describe 'Merge requests > User posts notes', :js do find('.btn-save').click end + wait_for_ajax find('.note').hover find('.js-note-edit').click diff --git a/spec/features/merge_requests/user_uses_slash_commands_spec.rb b/spec/features/merge_requests/user_uses_slash_commands_spec.rb index 1c0f21e5616..f0ad57eb92f 100644 --- a/spec/features/merge_requests/user_uses_slash_commands_spec.rb +++ b/spec/features/merge_requests/user_uses_slash_commands_spec.rb @@ -160,6 +160,7 @@ feature 'Merge Requests > User uses slash commands', feature: true, js: true do it 'changes target branch from a note' do write_note("message start \n/target_branch merge-test\n message end.") + wait_for_ajax expect(page).not_to have_content('/target_branch') expect(page).to have_content('message start') expect(page).to have_content('message end.') diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index a00efa10119..5eb147ed888 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -362,5 +362,16 @@ require('~/lib/utils/common_utils'); gl.utils.setCiStatusFavicon(BUILD_URL); }); }); + + describe('gl.utils.ajaxPost', () => { + it('should perform `$.ajax` call and do `POST` request', () => { + const requestURL = '/some/random/api'; + const data = { keyname: 'value' }; + const ajaxSpy = spyOn($, 'ajax').and.callFake(() => {}); + + gl.utils.ajaxPost(requestURL, data); + expect(ajaxSpy.calls.allArgs()[0][0].type).toEqual('POST'); + }); + }); }); })(); diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index cdc5c4510ff..7bffa90ab14 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -26,7 +26,7 @@ import '~/notes'; describe('task lists', function() { beforeEach(function() { - $('form').on('submit', function(e) { + $('.js-comment-button').on('click', function(e) { e.preventDefault(); }); this.notes = new Notes(); @@ -60,9 +60,12 @@ import '~/notes'; reset: function() {} }); - $('form').on('submit', function(e) { + $('.js-comment-button').on('click', (e) => { + const $form = $(this); e.preventDefault(); - $('.js-main-target-form').trigger('ajax:success'); + this.notes.addNote($form); + this.notes.reenableTargetFormSubmitButton(e); + this.notes.resetMainTargetForm(e); }); }); @@ -238,8 +241,8 @@ import '~/notes'; $resultantNote = Notes.animateAppendNote(noteHTML, $notesList); }); - it('should have `fade-in` class', () => { - expect($resultantNote.hasClass('fade-in')).toEqual(true); + it('should have `fade-in-full` class', () => { + expect($resultantNote.hasClass('fade-in-full')).toEqual(true); }); it('should append note to the notes list', () => { @@ -269,5 +272,221 @@ import '~/notes'; expect($note.replaceWith).toHaveBeenCalledWith($updatedNote); }); }); + + describe('getFormData', () => { + it('should return form metadata object from form reference', () => { + this.notes = new Notes(); + + const $form = $('form'); + const sampleComment = 'foobar'; + $form.find('textarea.js-note-text').val(sampleComment); + const { formData, formContent, formAction } = this.notes.getFormData($form); + + expect(formData.indexOf(sampleComment) > -1).toBe(true); + expect(formContent).toEqual(sampleComment); + expect(formAction).toEqual($form.attr('action')); + }); + }); + + describe('hasSlashCommands', () => { + beforeEach(() => { + this.notes = new Notes(); + }); + + it('should return true when comment has slash commands', () => { + const sampleComment = '/wip /milestone %1.0 /merge /unassign Merging this'; + const hasSlashCommands = this.notes.hasSlashCommands(sampleComment); + + expect(hasSlashCommands).toBeTruthy(); + }); + + it('should return false when comment does NOT have any slash commands', () => { + const sampleComment = 'Looking good, Awesome!'; + const hasSlashCommands = this.notes.hasSlashCommands(sampleComment); + + expect(hasSlashCommands).toBeFalsy(); + }); + }); + + describe('stripSlashCommands', () => { + const REGEX_SLASH_COMMANDS = /\/\w+/g; + + it('should strip slash commands from the comment', () => { + this.notes = new Notes(); + const sampleComment = '/wip /milestone %1.0 /merge /unassign Merging this'; + const stripedComment = this.notes.stripSlashCommands(sampleComment); + + expect(REGEX_SLASH_COMMANDS.test(stripedComment)).toBeFalsy(); + }); + }); + + describe('createPlaceholderNote', () => { + const sampleComment = 'foobar'; + const uniqueId = 'b1234-a4567'; + const currentUsername = 'root'; + const currentUserFullname = 'Administrator'; + + beforeEach(() => { + this.notes = new Notes(); + }); + + it('should return constructed placeholder element for regular note based on form contents', () => { + const $tempNote = this.notes.createPlaceholderNote({ + formContent: sampleComment, + uniqueId, + isDiscussionNote: false, + currentUsername, + currentUserFullname + }); + const $tempNoteHeader = $tempNote.find('.note-header'); + + expect($tempNote.prop('nodeName')).toEqual('LI'); + expect($tempNote.attr('id')).toEqual(uniqueId); + $tempNote.find('.timeline-icon > a, .note-header-info > a').each(function() { + expect($(this).attr('href')).toEqual(`/${currentUsername}`); + }); + expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeFalsy(); + expect($tempNoteHeader.find('.hidden-xs').text().trim()).toEqual(currentUserFullname); + expect($tempNoteHeader.find('.note-headline-light').text().trim()).toEqual(`@${currentUsername}`); + expect($tempNote.find('.note-body .note-text').text().trim()).toEqual(sampleComment); + }); + + it('should return constructed placeholder element for discussion note based on form contents', () => { + const $tempNote = this.notes.createPlaceholderNote({ + formContent: sampleComment, + uniqueId, + isDiscussionNote: true, + currentUsername, + currentUserFullname + }); + + expect($tempNote.prop('nodeName')).toEqual('LI'); + expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeTruthy(); + }); + }); + + describe('postComment & updateComment', () => { + const sampleComment = 'foo'; + const updatedComment = 'bar'; + const note = { + id: 1234, + html: `
  • +
    ${sampleComment}
    +
    • `, + note: sampleComment, + valid: true + }; + let $form; + let $notesContainer; + + beforeEach(() => { + this.notes = new Notes(); + window.gon.current_username = 'root'; + window.gon.current_user_fullname = 'Administrator'; + $form = $('form'); + $notesContainer = $('ul.main-notes-list'); + $form.find('textarea.js-note-text').val(sampleComment); + $('.js-comment-button').click(); + }); + + it('should show placeholder note while new comment is being posted', () => { + expect($notesContainer.find('.note.being-posted').length > 0).toEqual(true); + }); + + it('should remove placeholder note when new comment is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + expect($notesContainer.find('.note.being-posted').length).toEqual(0); + }); + }); + + it('should show actual note element when new comment is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + expect($notesContainer.find(`#${note.id}`).length > 0).toEqual(true); + }); + }); + + it('should reset Form when new comment is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + expect($form.find('textarea.js-note-text')).toEqual(''); + }); + }); + + it('should trigger ajax:success event on Form when new comment is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + spyOn($form, 'trigger'); + expect($form.trigger).toHaveBeenCalledWith('ajax:success', [note]); + }); + }); + + it('should show flash error message when new comment failed to be posted', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.error(); + expect($notesContainer.parent().find('.flash-container .flash-text').is(':visible')).toEqual(true); + }); + }); + + it('should refill form textarea with original comment content when new comment failed to be posted', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.error(); + expect($form.find('textarea.js-note-text')).toEqual(sampleComment); + }); + }); + + it('should show updated comment as _actively being posted_ while comment being updated', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').val(updatedComment); + $noteEl.find('.js-comment-save-button').click(); + expect($noteEl.hasClass('.being-posted')).toEqual(true); + expect($noteEl.find('.note-text').text()).toEqual(updatedComment); + }); + }); + + it('should show updated comment when comment update is done posting', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').val(updatedComment); + $noteEl.find('.js-comment-save-button').click(); + + spyOn($, 'ajax').and.callFake((updateOptions) => { + const updatedNote = Object.assign({}, note); + updatedNote.note = updatedComment; + updatedNote.html = `
  • +
    ${updatedComment}
    +
    • `; + updateOptions.success(updatedNote); + const $updatedNoteEl = $notesContainer.find(`#note_${updatedNote.id}`); + expect($updatedNoteEl.hasClass('.being-posted')).toEqual(false); // Remove being-posted visuals + expect($updatedNoteEl.find('note-text').text().trim()).toEqual(updatedComment); // Verify if comment text updated + }); + }); + }); + + it('should show flash error message when comment failed to be updated', () => { + spyOn($, 'ajax').and.callFake((options) => { + options.success(note); + const $noteEl = $notesContainer.find(`#note_${note.id}`); + $noteEl.find('.js-note-edit').click(); + $noteEl.find('textarea.js-note-text').val(updatedComment); + $noteEl.find('.js-comment-save-button').click(); + + spyOn($, 'ajax').and.callFake((updateOptions) => { + updateOptions.error(); + const $updatedNoteEl = $notesContainer.find(`#note_${note.id}`); + expect($updatedNoteEl.hasClass('.being-posted')).toEqual(false); // Remove being-posted visuals + expect($updatedNoteEl.find('note-text').text().trim()).toEqual(sampleComment); // See if comment reverted back to original + expect($notesContainer.parent().find('.flash-container .flash-text').is(':visible')).toEqual(true); // Flash error message shown + }); + }); + }); + }); }); }).call(window); diff --git a/spec/support/features/issuable_slash_commands_shared_examples.rb b/spec/support/features/issuable_slash_commands_shared_examples.rb index 6efcdd7a1de..610decdcddb 100644 --- a/spec/support/features/issuable_slash_commands_shared_examples.rb +++ b/spec/support/features/issuable_slash_commands_shared_examples.rb @@ -58,6 +58,7 @@ shared_examples 'issuable record that supports slash commands in its description expect(page).not_to have_content '/label ~bug' expect(page).not_to have_content '/milestone %"ASAP"' + wait_for_ajax issuable.reload note = issuable.notes.user.first diff --git a/spec/support/time_tracking_shared_examples.rb b/spec/support/time_tracking_shared_examples.rb index 01bc80f957e..e94e17da7e5 100644 --- a/spec/support/time_tracking_shared_examples.rb +++ b/spec/support/time_tracking_shared_examples.rb @@ -8,6 +8,7 @@ shared_examples 'issuable time tracker' do it 'updates the sidebar component when estimate is added' do submit_time('/estimate 3w 1d 1h') + wait_for_ajax page.within '.time-tracking-estimate-only-pane' do expect(page).to have_content '3w 1d 1h' end @@ -16,6 +17,7 @@ shared_examples 'issuable time tracker' do it 'updates the sidebar component when spent is added' do submit_time('/spend 3w 1d 1h') + wait_for_ajax page.within '.time-tracking-spend-only-pane' do expect(page).to have_content '3w 1d 1h' end @@ -25,6 +27,7 @@ shared_examples 'issuable time tracker' do submit_time('/estimate 3w 1d 1h') submit_time('/spend 3w 1d 1h') + wait_for_ajax page.within '.time-tracking-comparison-pane' do expect(page).to have_content '3w 1d 1h' end @@ -34,6 +37,7 @@ shared_examples 'issuable time tracker' do submit_time('/estimate 3w 1d 1h') submit_time('/remove_estimate') + wait_for_ajax page.within '#issuable-time-tracker' do expect(page).to have_content 'No estimate or time spent' end @@ -43,6 +47,7 @@ shared_examples 'issuable time tracker' do submit_time('/spend 3w 1d 1h') submit_time('/remove_time_spent') + wait_for_ajax page.within '#issuable-time-tracker' do expect(page).to have_content 'No estimate or time spent' end