From 928907072575a0158e543bdd442755d0b8039b34 Mon Sep 17 00:00:00 2001 From: Jesse Hall Date: Wed, 13 May 2020 19:16:14 -0500 Subject: [PATCH 1/6] Feature for #25486, users can apply multiple suggestions at once. --- app/assets/javascripts/api.js | 7 + .../notes/components/note_body.vue | 32 +- .../javascripts/notes/stores/actions.js | 46 +- .../javascripts/notes/stores/modules/index.js | 1 + .../notes/stores/mutation_types.js | 4 + .../javascripts/notes/stores/mutations.js | 33 ++ .../components/markdown/suggestion_diff.vue | 28 + .../markdown/suggestion_diff_header.vue | 98 +++- .../components/markdown/suggestions.vue | 21 +- app/assets/stylesheets/framework/badges.scss | 8 + app/services/suggestions/apply_service.rb | 110 +--- ...quest_merge_suggestions_settings.html.haml | 4 +- .../unreleased/25486-batch-suggestions.yml | 5 + ...ggestions_custom_commit_messages_v12_7.png | Bin 21720 -> 0 bytes ...ggestions_custom_commit_messages_v13_1.jpg | Bin 0 -> 35055 bytes doc/user/discussions/index.md | 34 +- lib/api/suggestions.rb | 43 +- lib/gitlab/suggestions/commit_message.rb | 54 ++ lib/gitlab/suggestions/file_suggestion.rb | 107 ++++ lib/gitlab/suggestions/suggestion_set.rb | 120 ++++ locale/gitlab.pot | 38 +- .../user_suggests_changes_on_diff_spec.rb | 94 +++ spec/frontend/notes/mock_data.js | 13 + spec/frontend/notes/stores/actions_spec.js | 141 ++++- spec/frontend/notes/stores/mutation_spec.js | 105 ++++ .../suggestion_diff_spec.js.snap | 3 + .../markdown/suggestion_diff_header_spec.js | 122 +++- .../markdown/suggestion_diff_spec.js | 34 +- .../gitlab/suggestions/commit_message_spec.rb | 87 +++ .../suggestions/file_suggestion_spec.rb | 241 ++++++++ .../gitlab/suggestions/suggestion_set_spec.rb | 110 ++++ spec/requests/api/suggestions_spec.rb | 140 ++++- .../suggestions/apply_service_spec.rb | 553 +++++++++++------- spec/views/projects/edit.html.haml_spec.rb | 16 +- 34 files changed, 2059 insertions(+), 393 deletions(-) create mode 100644 changelogs/unreleased/25486-batch-suggestions.yml delete mode 100644 doc/user/discussions/img/suggestions_custom_commit_messages_v12_7.png create mode 100644 doc/user/discussions/img/suggestions_custom_commit_messages_v13_1.jpg create mode 100644 lib/gitlab/suggestions/commit_message.rb create mode 100644 lib/gitlab/suggestions/file_suggestion.rb create mode 100644 lib/gitlab/suggestions/suggestion_set.rb create mode 100644 spec/lib/gitlab/suggestions/commit_message_spec.rb create mode 100644 spec/lib/gitlab/suggestions/file_suggestion_spec.rb create mode 100644 spec/lib/gitlab/suggestions/suggestion_set_spec.rb diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index b75fa65b842e6b..253caedf75ab23 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -38,6 +38,7 @@ const Api = { userPostStatusPath: '/api/:version/user/status', commitPath: '/api/:version/projects/:id/repository/commits', applySuggestionPath: '/api/:version/suggestions/:id/apply', + applySuggestionBatchPath: '/api/:version/suggestions/batch_apply', commitPipelinesPath: '/:project_id/commit/:sha/pipelines', branchSinglePath: '/api/:version/projects/:id/repository/branches/:branch', createBranchPath: '/api/:version/projects/:id/repository/branches', @@ -322,6 +323,12 @@ const Api = { return axios.put(url); }, + applySuggestionBatch(ids) { + const url = Api.buildUrl(Api.applySuggestionBatchPath); + + return axios.put(url, { ids }); + }, + commitPipelines(projectId, sha) { const encodedProjectId = projectId .split('/') diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue index 0ba104d064c8f2..57e9e6a49f7882 100644 --- a/app/assets/javascripts/notes/components/note_body.vue +++ b/app/assets/javascripts/notes/components/note_body.vue @@ -1,5 +1,9 @@ @@ -105,10 +131,14 @@ export default {
{ + const suggestionIds = state.batchSuggestionsInfo.map(({ suggestionId }) => suggestionId); + + const applyAllSuggestions = () => + state.batchSuggestionsInfo.map(suggestionInfo => + commit(types.APPLY_SUGGESTION, suggestionInfo), + ); + + const resolveAllDiscussions = () => + state.batchSuggestionsInfo.map(suggestionInfo => { + const { discussionId } = suggestionInfo; + return dispatch('resolveDiscussion', { discussionId }).catch(() => {}); + }); + + commit(types.SET_APPLYING_BATCH_STATE, true); + + return Api.applySuggestionBatch(suggestionIds) + .then(() => Promise.all(applyAllSuggestions())) + .then(() => Promise.all(resolveAllDiscussions())) + .then(() => commit(types.CLEAR_SUGGESTION_BATCH)) + .catch(err => { + commit(types.SET_APPLYING_BATCH_STATE, false); + + const defaultMessage = __( + 'Something went wrong while applying the batch of suggestions. Please try again.', + ); + + const errorMessage = err.response.data?.message; + + const flashMessage = errorMessage || defaultMessage; + + Flash(__(flashMessage), 'alert', flashContainer); + }); +}; + +export const addSuggestionInfoToBatch = ({ commit }, { suggestionId, noteId, discussionId }) => + commit(types.ADD_SUGGESTION_TO_BATCH, { suggestionId, noteId, discussionId }); + +export const removeSuggestionInfoFromBatch = ({ commit }, suggestionId) => + commit(types.REMOVE_SUGGESTION_FROM_BATCH, suggestionId); + export const convertToDiscussion = ({ commit }, noteId) => commit(types.CONVERT_TO_DISCUSSION, noteId); diff --git a/app/assets/javascripts/notes/stores/modules/index.js b/app/assets/javascripts/notes/stores/modules/index.js index 25f0f546103bec..329bf5e147e2fc 100644 --- a/app/assets/javascripts/notes/stores/modules/index.js +++ b/app/assets/javascripts/notes/stores/modules/index.js @@ -11,6 +11,7 @@ export default () => ({ targetNoteHash: null, lastFetchedAt: null, currentDiscussionId: null, + batchSuggestionsInfo: [], // View layer isToggleStateButtonLoading: false, diff --git a/app/assets/javascripts/notes/stores/mutation_types.js b/app/assets/javascripts/notes/stores/mutation_types.js index 2f7b2788d8a282..5c88e152280ddd 100644 --- a/app/assets/javascripts/notes/stores/mutation_types.js +++ b/app/assets/javascripts/notes/stores/mutation_types.js @@ -17,6 +17,10 @@ export const SET_NOTES_FETCHED_STATE = 'SET_NOTES_FETCHED_STATE'; export const SET_NOTES_LOADING_STATE = 'SET_NOTES_LOADING_STATE'; export const DISABLE_COMMENTS = 'DISABLE_COMMENTS'; export const APPLY_SUGGESTION = 'APPLY_SUGGESTION'; +export const SET_APPLYING_BATCH_STATE = 'SET_APPLYING_BATCH_STATE'; +export const ADD_SUGGESTION_TO_BATCH = 'ADD_SUGGESTION_TO_BATCH'; +export const REMOVE_SUGGESTION_FROM_BATCH = 'REMOVE_SUGGESTION_FROM_BATCH'; +export const CLEAR_SUGGESTION_BATCH = 'CLEAR_SUGGESTION_BATCH'; export const CONVERT_TO_DISCUSSION = 'CONVERT_TO_DISCUSSION'; export const REMOVE_CONVERTED_DISCUSSION = 'REMOVE_CONVERTED_DISCUSSION'; diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index f06874991f018e..38f5551695da42 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -225,6 +225,39 @@ export default { })); }, + [types.SET_APPLYING_BATCH_STATE](state, isApplyingBatch) { + state.batchSuggestionsInfo.forEach(suggestionInfo => { + const { discussionId, noteId, suggestionId } = suggestionInfo; + + const noteObj = utils.findNoteObjectById(state.discussions, discussionId); + const comment = utils.findNoteObjectById(noteObj.notes, noteId); + + comment.suggestions = comment.suggestions.map(suggestion => ({ + ...suggestion, + is_applying_batch: suggestion.id === suggestionId && isApplyingBatch, + })); + }); + }, + + [types.ADD_SUGGESTION_TO_BATCH](state, { noteId, discussionId, suggestionId }) { + state.batchSuggestionsInfo.push({ + suggestionId, + noteId, + discussionId, + }); + }, + + [types.REMOVE_SUGGESTION_FROM_BATCH](state, id) { + const index = state.batchSuggestionsInfo.findIndex(({ suggestionId }) => suggestionId === id); + if (index !== -1) { + state.batchSuggestionsInfo.splice(index, 1); + } + }, + + [types.CLEAR_SUGGESTION_BATCH](state) { + state.batchSuggestionsInfo.splice(0, state.batchSuggestionsInfo.length); + }, + [types.UPDATE_DISCUSSION](state, noteData) { const note = noteData; const selectedDiscussion = state.discussions.find(disc => disc.id === note.id); diff --git a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue index a7cd292e01ddb0..6dac448d5de1a8 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue @@ -13,6 +13,11 @@ export default { type: Object, required: true, }, + batchSuggestionsInfo: { + type: Array, + required: false, + default: () => [], + }, disabled: { type: Boolean, required: false, @@ -24,6 +29,14 @@ export default { }, }, computed: { + batchSuggestionsCount() { + return this.batchSuggestionsInfo.length; + }, + isBatched() { + return Boolean( + this.batchSuggestionsInfo.find(({ suggestionId }) => suggestionId === this.suggestion.id), + ); + }, lines() { return selectDiffLines(this.suggestion.diff_lines); }, @@ -32,6 +45,15 @@ export default { applySuggestion(callback) { this.$emit('apply', { suggestionId: this.suggestion.id, callback }); }, + applySuggestionBatch() { + this.$emit('applyBatch'); + }, + addSuggestionToBatch() { + this.$emit('addToBatch', this.suggestion.id); + }, + removeSuggestionFromBatch() { + this.$emit('removeFromBatch', this.suggestion.id); + }, }, }; @@ -42,8 +64,14 @@ export default { class="qa-suggestion-diff-header js-suggestion-diff-header" :can-apply="suggestion.appliable && suggestion.current_user.can_apply && !disabled" :is-applied="suggestion.applied" + :is-batched="isBatched" + :is-applying-batch="suggestion.is_applying_batch" + :batch-suggestions-count="batchSuggestionsCount" :help-page-path="helpPagePath" @apply="applySuggestion" + @applyBatch="applySuggestionBatch" + @addToBatch="addSuggestionToBatch" + @removeFromBatch="removeSuggestionFromBatch" /> diff --git a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue index af438ce5619add..ee9318f4924abe 100644 --- a/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue +++ b/app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue @@ -1,11 +1,17 @@