From 8e5c0e68ec837fe247df271a58a819238b8c20b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Lu=C3=ADs?= <me@andr3.net> Date: Sat, 6 Oct 2018 17:16:40 +0000 Subject: [PATCH] Backport CE changes for: [Frontend only] Batch comments on merge requests --- .../javascripts/diffs/components/app.vue | 1 - app/assets/javascripts/diffs/store/utils.js | 12 +++++-- .../javascripts/mr_notes/stores/index.js | 17 +++++---- .../notes/components/note_actions.vue | 33 +++++++++++++++-- .../notes/components/note_body.vue | 2 +- .../notes/components/note_form.vue | 2 +- .../notes/components/note_header.vue | 36 +++++++++++-------- .../notes/components/noteable_note.vue | 28 +++++++++++---- .../javascripts/notes/stores/actions.js | 19 ++++++++-- .../javascripts/notes/stores/getters.js | 3 ++ .../framework/contextual_sidebar.scss | 8 ++--- app/assets/stylesheets/pages/notes.scss | 6 +++- app/helpers/notes_helper.rb | 2 +- .../projects/issues/_discussion.html.haml | 2 +- .../projects/merge_requests/show.html.haml | 2 +- 15 files changed, 123 insertions(+), 50 deletions(-) diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index e60c53338fe..edca45f22f9 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -127,7 +127,6 @@ export default { 'startRenderDiffsQueue', 'assignDiscussionsToDiff', ]), - fetchData() { this.fetchDiffFiles() .then(() => { diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index c39403f1021..a482a2b82c0 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -25,7 +25,7 @@ export const getReversePosition = linePosition => { return LINE_POSITION_RIGHT; }; -export function getNoteFormData(params) { +export function getFormData(params) { const { note, noteableType, @@ -70,9 +70,15 @@ export function getNoteFormData(params) { }, }; + return postData; +} + +export function getNoteFormData(params) { + const data = getFormData(params); + return { - endpoint: noteableData.create_note_path, - data: postData, + endpoint: params.noteableData.create_note_path, + data, }; } diff --git a/app/assets/javascripts/mr_notes/stores/index.js b/app/assets/javascripts/mr_notes/stores/index.js index 446eb477efc..c4225c8ec08 100644 --- a/app/assets/javascripts/mr_notes/stores/index.js +++ b/app/assets/javascripts/mr_notes/stores/index.js @@ -6,10 +6,13 @@ import mrPageModule from './modules'; Vue.use(Vuex); -export default new Vuex.Store({ - modules: { - page: mrPageModule, - notes: notesModule(), - diffs: diffsModule(), - }, -}); +export const createStore = () => + new Vuex.Store({ + modules: { + page: mrPageModule, + notes: notesModule(), + diffs: diffsModule(), + }, + }); + +export default createStore(); diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue index beb53da0e6d..e075f94b82b 100644 --- a/app/assets/javascripts/notes/components/note_actions.vue +++ b/app/assets/javascripts/notes/components/note_actions.vue @@ -7,10 +7,14 @@ import editSvg from 'icons/_icon_pencil.svg'; import resolveDiscussionSvg from 'icons/_icon_resolve_discussion.svg'; import resolvedDiscussionSvg from 'icons/_icon_status_success_solid.svg'; import ellipsisSvg from 'icons/_ellipsis_v.svg'; +import Icon from '~/vue_shared/components/icon.vue'; import tooltip from '~/vue_shared/directives/tooltip'; export default { name: 'NoteActions', + components: { + Icon, + }, directives: { tooltip, }, @@ -20,7 +24,7 @@ export default { required: true, }, noteId: { - type: String, + type: [String, Number], required: true, }, noteUrl: { @@ -35,7 +39,8 @@ export default { }, reportAbusePath: { type: String, - required: true, + required: false, + default: null, }, canEdit: { type: Boolean, @@ -84,6 +89,9 @@ export default { shouldShowActionsDropdown() { return this.currentUserId && (this.canEdit || this.canReportAsAbuse); }, + showDeleteAction() { + return this.canDelete && !this.canReportAsAbuse && !this.noteUrl; + }, isAuthoredByCurrentUser() { return this.authorId === this.currentUserId; }, @@ -201,7 +209,26 @@ export default { </button> </div> <div - v-if="shouldShowActionsDropdown" + v-if="showDeleteAction" + class="note-actions-item" + > + <button + v-tooltip + type="button" + title="Delete comment" + class="note-action-button js-note-delete btn btn-transparent" + data-container="body" + data-placement="bottom" + @click="onDelete" + > + <icon + name="remove" + class="link-highlight" + /> + </button> + </div> + <div + v-else-if="shouldShowActionsDropdown" class="dropdown more-actions note-actions-item"> <button v-tooltip diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue index 6f4a0709825..cf4c35de42c 100644 --- a/app/assets/javascripts/notes/components/note_body.vue +++ b/app/assets/javascripts/notes/components/note_body.vue @@ -109,7 +109,7 @@ export default { class="note_edited_ago" /> <note-awards-list - v-if="note.award_emoji.length" + v-if="note.award_emoji && note.award_emoji.length" :note-id="note.id" :note-author-id="note.author.id" :awards="note.award_emoji" diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue index 2d47d55f33c..33998394a69 100644 --- a/app/assets/javascripts/notes/components/note_form.vue +++ b/app/assets/javascripts/notes/components/note_form.vue @@ -20,7 +20,7 @@ export default { default: '', }, noteId: { - type: String, + type: [String, Number], required: false, default: '', }, diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue index d669d12a39b..7b6e7b72caf 100644 --- a/app/assets/javascripts/notes/components/note_header.vue +++ b/app/assets/javascripts/notes/components/note_header.vue @@ -14,7 +14,8 @@ export default { }, createdAt: { type: String, - required: true, + required: false, + default: null, }, actionText: { type: String, @@ -22,8 +23,9 @@ export default { default: '', }, noteId: { - type: String, - required: true, + type: [String, Number], + required: false, + default: null, }, includeToggle: { type: Boolean, @@ -96,18 +98,22 @@ export default { <span class="system-note-message"> <slot></slot> </span> - <span class="system-note-separator"> - · - </span> - <a - :href="noteTimestampLink" - class="note-timestamp system-note-separator" - @click="updateTargetNoteHash"> - <time-ago-tooltip - :time="createdAt" - tooltip-placement="bottom" - /> - </a> + <template + v-if="createdAt" + > + <span class="system-note-separator"> + · + </span> + <a + :href="noteTimestampLink" + class="note-timestamp system-note-separator" + @click="updateTargetNoteHash"> + <time-ago-tooltip + :time="createdAt" + tooltip-placement="bottom" + /> + </a> + </template> <i class="fa fa-spinner fa-spin editing-spinner" aria-label="Comment is being updated" diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 7579fc852c6..f391ed848a4 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -52,7 +52,7 @@ export default { return this.note.resolvable && !!this.getUserData.id; }, canReportAsAbuse() { - return this.note.report_abuse_path && this.author.id !== this.getUserData.id; + return !!this.note.report_abuse_path && this.author.id !== this.getUserData.id; }, noteAnchorId() { return `note_${this.note.id}`; @@ -81,13 +81,17 @@ export default { ...mapActions(['deleteNote', 'updateNote', 'toggleResolveNote', 'scrollToNoteIfNeeded']), editHandler() { this.isEditing = true; + this.$emit('handleEdit'); }, deleteHandler() { + const typeOfComment = this.note.isDraft ? 'pending comment' : 'comment'; // eslint-disable-next-line no-alert - if (window.confirm('Are you sure you want to delete this comment?')) { + if (window.confirm(`Are you sure you want to delete this ${typeOfComment}?`)) { this.isDeleting = true; this.$emit('handleDeleteNote', this.note); + if (this.note.isDraft) return; + this.deleteNote(this.note) .then(() => { this.isDeleting = false; @@ -98,7 +102,20 @@ export default { }); } }, + updateSuccess() { + this.isEditing = false; + this.isRequesting = false; + this.oldContent = null; + $(this.$refs.noteBody.$el).renderGFM(); + this.$refs.noteBody.resetAutoSave(); + this.$emit('updateSuccess'); + }, formUpdateHandler(noteText, parentElement, callback) { + this.$emit('handleUpdateNote', { + note: this.note, + noteText, + callback: () => this.updateSuccess(), + }); const data = { endpoint: this.note.path, note: { @@ -113,11 +130,7 @@ export default { this.updateNote(data) .then(() => { - this.isEditing = false; - this.isRequesting = false; - this.oldContent = null; - $(this.$refs.noteBody.$el).renderGFM(); - this.$refs.noteBody.resetAutoSave(); + this.updateSuccess(); callback(); }) .catch(() => { @@ -142,6 +155,7 @@ export default { this.oldContent = null; } this.isEditing = false; + this.$emit('cancelForm'); }, recoverNoteContent(noteText) { // we need to do this to prevent noteForm inconsistent content warning diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index 320dfa47d5a..7ab7e5a9abb 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -150,11 +150,24 @@ export const toggleIssueLocalState = ({ commit }, newState) => { export const saveNote = ({ commit, dispatch }, noteData) => { // For MR discussuions we need to post as `note[note]` and issue we use `note.note`. - const note = noteData.data['note[note]'] || noteData.data.note.note; + // For batch comments, we use draft_note + const note = noteData.data.draft_note || noteData.data['note[note]'] || noteData.data.note.note; let placeholderText = note; const hasQuickActions = utils.hasQuickActions(placeholderText); const replyId = noteData.data.in_reply_to_discussion_id; - const methodToDispatch = replyId ? 'replyToDiscussion' : 'createNewNote'; + let methodToDispatch; + const postData = Object.assign({}, noteData); + if (postData.isDraft === true) { + methodToDispatch = replyId + ? 'batchComments/addDraftToDiscussion' + : 'batchComments/createNewDraft'; + if (!postData.draft_note && noteData.note) { + postData.draft_note = postData.note; + delete postData.note; + } + } else { + methodToDispatch = replyId ? 'replyToDiscussion' : 'createNewNote'; + } $('.notes-form .flash-container').hide(); // hide previous flash notification commit(types.REMOVE_PLACEHOLDER_NOTES); // remove previous placeholders @@ -180,7 +193,7 @@ export const saveNote = ({ commit, dispatch }, noteData) => { } } - return dispatch(methodToDispatch, noteData).then(res => { + return dispatch(methodToDispatch, postData, { root: true }).then(res => { const { errors } = res; const commandsChanges = res.commands_changes; diff --git a/app/assets/javascripts/notes/stores/getters.js b/app/assets/javascripts/notes/stores/getters.js index 75832884711..a829149a17e 100644 --- a/app/assets/javascripts/notes/stores/getters.js +++ b/app/assets/javascripts/notes/stores/getters.js @@ -74,6 +74,9 @@ export const allDiscussions = (state, getters) => { return Object.values(resolved).concat(unresolved); }; +export const isDiscussionResolved = (state, getters) => discussionId => + getters.resolvedDiscussionsById[discussionId] !== undefined; + export const allResolvableDiscussions = (state, getters) => getters.allDiscussions.filter(d => !d.individual_note && d.resolvable); diff --git a/app/assets/stylesheets/framework/contextual_sidebar.scss b/app/assets/stylesheets/framework/contextual_sidebar.scss index 2193e8e8de3..2e7f25d975e 100644 --- a/app/assets/stylesheets/framework/contextual_sidebar.scss +++ b/app/assets/stylesheets/framework/contextual_sidebar.scss @@ -9,8 +9,7 @@ padding-left: $contextual-sidebar-width; } - .issues-bulk-update.right-sidebar.right-sidebar-expanded - .issuable-sidebar-header { + .issues-bulk-update.right-sidebar.right-sidebar-expanded .issuable-sidebar-header { padding: 10px 0 15px; } } @@ -75,7 +74,7 @@ .nav-sidebar { transition: width $sidebar-transition-duration, left $sidebar-transition-duration; position: fixed; - z-index: 400; + z-index: 600; width: $contextual-sidebar-width; top: $header-height; bottom: 0; @@ -86,8 +85,7 @@ &:not(.sidebar-collapsed-desktop) { @media (min-width: map-get($grid-breakpoints, sm)) and (max-width: map-get($grid-breakpoints, sm)) { - box-shadow: inset -1px 0 0 $border-color, - 2px 1px 3px $dropdown-shadow-color; + box-shadow: inset -1px 0 0 $border-color, 2px 1px 3px $dropdown-shadow-color; } } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index a2070087963..bfba1bf1b2b 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -343,6 +343,10 @@ ul.notes { &.parallel { border-width: 1px; + + &.new { + border-right-width: 0; + } } .discussion-notes { @@ -738,7 +742,7 @@ ul.notes { padding-top: 0; .discussion-wrapper { - border-color: transparent; + border: 0; } } } diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index a80c8f273a8..e0905584803 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -178,7 +178,7 @@ module NotesHelper notesPath: notes_url, totalNotes: issuable.discussions.length, lastFetchedAt: Time.now.to_i - }.to_json + } end def discussion_resolved_intro(discussion) diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml index 665968a64e1..28998acdc13 100644 --- a/app/views/projects/issues/_discussion.html.haml +++ b/app/views/projects/issues/_discussion.html.haml @@ -6,7 +6,7 @@ = link_to 'Close issue', issue_path(@issue, issue: {state_event: :close}, format: 'json'), data: {original_text: "Close issue", alternative_text: "Comment & close issue"}, class: "btn btn-nr btn-close btn-comment js-note-target-close #{issue_button_visibility(@issue, true)}", title: 'Close issue' %section.js-vue-notes-event - #js-vue-notes{ data: { notes_data: notes_data(@issue), + #js-vue-notes{ data: { notes_data: notes_data(@issue).to_json, noteable_data: serialize_issuable(@issue), noteable_type: 'Issue', target_type: 'issue', diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index b23baa22d8b..ef2fa8668c0 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -60,7 +60,7 @@ %section.col-md-12 %script.js-notes-data{ type: "application/json" }= initial_notes_data(true).to_json.html_safe .issuable-discussion.js-vue-notes-event - #js-vue-mr-discussions{ data: { notes_data: notes_data(@merge_request), + #js-vue-mr-discussions{ data: { notes_data: notes_data(@merge_request).to_json, noteable_data: serialize_issuable(@merge_request), noteable_type: 'MergeRequest', target_type: 'merge_request', -- 2.30.9