From feb8d3bc6455e77398321a99d4b5db309d55a43f Mon Sep 17 00:00:00 2001 From: Phil Hughes <me@iamphill.com> Date: Mon, 17 Jul 2023 12:18:11 +0100 Subject: [PATCH] Move summarize review action to submit review dropdown This moves the action to summarize my review from every markdown toolbar to within the submit review dropdown. https://gitlab.com/gitlab-org/gitlab/-/issues/417750 --- .../components/submit_dropdown.vue | 69 +++++++------ .../javascripts/batch_comments/index.js | 5 +- app/assets/javascripts/merge_request_tabs.js | 2 +- app/assets/javascripts/mr_notes/init_notes.js | 3 +- .../projects/merge_requests/show/index.js | 2 +- app/helpers/merge_requests_helper.rb | 4 + .../projects/merge_requests/_page.html.haml | 2 +- .../ai/editor_actions/summarize_my_review.js | 27 ----- .../components/summarize_my_review.vue | 98 +++++++++++++++++++ .../graphql/summarize_review.mutation.graphql | 0 ee/app/assets/javascripts/mr_notes/index.js | 12 +-- .../assets/javascripts/mr_notes/mount_app.js | 14 --- ee/app/helpers/ee/merge_requests_helper.rb | 5 + .../components/summarize_my_review_spec.js | 96 ++++++++++++++++++ locale/gitlab.pot | 8 +- 15 files changed, 256 insertions(+), 91 deletions(-) delete mode 100644 ee/app/assets/javascripts/ai/editor_actions/summarize_my_review.js create mode 100644 ee/app/assets/javascripts/batch_comments/components/summarize_my_review.vue rename ee/app/assets/javascripts/{ai => batch_comments}/graphql/summarize_review.mutation.graphql (100%) delete mode 100644 ee/app/assets/javascripts/mr_notes/mount_app.js create mode 100644 ee/spec/frontend/batch_comments/components/summarize_my_review_spec.js diff --git a/app/assets/javascripts/batch_comments/components/submit_dropdown.vue b/app/assets/javascripts/batch_comments/components/submit_dropdown.vue index 96889f0059c4..baf441f38082 100644 --- a/app/assets/javascripts/batch_comments/components/submit_dropdown.vue +++ b/app/assets/javascripts/batch_comments/components/submit_dropdown.vue @@ -1,5 +1,5 @@ <script> -import { GlDropdown, GlButton, GlIcon, GlForm, GlFormGroup, GlFormCheckbox } from '@gitlab/ui'; +import { GlDropdown, GlButton, GlIcon, GlForm, GlFormCheckbox } from '@gitlab/ui'; import { mapGetters, mapActions, mapState } from 'vuex'; import { __ } from '~/locale'; import { createAlert } from '~/alert'; @@ -16,12 +16,16 @@ export default { GlButton, GlIcon, GlForm, - GlFormGroup, GlFormCheckbox, MarkdownEditor, ApprovalPassword: () => import('ee_component/batch_comments/components/approval_password.vue'), + SummarizeMyReview: () => + import('ee_component/batch_comments/components/summarize_my_review.vue'), }, mixins: [glFeatureFlagsMixin()], + inject: { + canSummarize: { default: false }, + }, data() { return { isSubmitting: false, @@ -107,6 +111,9 @@ export default { this.isSubmitting = false; }, + updateNote(note) { + this.noteData.note = note; + }, }, restrictedToolbarItems: ['full-screen'], }; @@ -128,33 +135,39 @@ export default { <gl-icon class="dropdown-chevron" name="chevron-up" /> </template> <gl-form data-testid="submit-gl-form" @submit.prevent="submitReview"> - <gl-form-group label-for="review-note-body" label-class="gl-mb-2"> - <template #label> + <div class="gl-display-flex gl-mb-4 gl-align-items-center"> + <label for="review-note-body" class="gl-mb-0"> {{ __('Summary comment (optional)') }} - </template> - <div class="common-note-form gfm-form"> - <markdown-editor - ref="markdownEditor" - v-model="noteData.note" - :enable-content-editor="Boolean(glFeatures.contentEditorOnIssues)" - class="js-no-autosize" - :is-submitting="isSubmitting" - :render-markdown-path="getNoteableData.preview_note_path" - :markdown-docs-path="getNotesData.markdownDocsPath" - :form-field-props="formFieldProps" - enable-autocomplete - :autocomplete-data-sources="autocompleteDataSources" - :disabled="isSubmitting" - :restricted-tool-bar-items="$options.restrictedToolbarItems" - :force-autosize="false" - :autosave-key="autosaveKey" - supports-quick-actions - @input="$emit('input', $event)" - @keydown.meta.enter="submitReview" - @keydown.ctrl.enter="submitReview" - /> - </div> - </gl-form-group> + </label> + <summarize-my-review + v-if="canSummarize" + :id="getNoteableData.id" + class="gl-ml-auto" + @input="updateNote" + /> + </div> + <div class="common-note-form gfm-form"> + <markdown-editor + ref="markdownEditor" + v-model="noteData.note" + :enable-content-editor="Boolean(glFeatures.contentEditorOnIssues)" + class="js-no-autosize" + :is-submitting="isSubmitting" + :render-markdown-path="getNoteableData.preview_note_path" + :markdown-docs-path="getNotesData.markdownDocsPath" + :form-field-props="formFieldProps" + enable-autocomplete + :autocomplete-data-sources="autocompleteDataSources" + :disabled="isSubmitting" + :restricted-tool-bar-items="$options.restrictedToolbarItems" + :force-autosize="false" + :autosave-key="autosaveKey" + supports-quick-actions + @input="$emit('input', $event)" + @keydown.meta.enter="submitReview" + @keydown.ctrl.enter="submitReview" + /> + </div> <template v-if="getNoteableData.current_user.can_approve"> <gl-form-checkbox v-model="noteData.approve" data-testid="approve_merge_request"> {{ __('Approve merge request') }} diff --git a/app/assets/javascripts/batch_comments/index.js b/app/assets/javascripts/batch_comments/index.js index bf9769ff3594..c4683e5cd5fe 100644 --- a/app/assets/javascripts/batch_comments/index.js +++ b/app/assets/javascripts/batch_comments/index.js @@ -1,10 +1,11 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import { mapActions, mapGetters } from 'vuex'; +import { parseBoolean } from '~/lib/utils/common_utils'; import { apolloProvider } from '~/graphql_shared/issuable_client'; import store from '~/mr_notes/stores'; -export const initReviewBar = ({ editorAiActions = [] } = {}) => { +export const initReviewBar = () => { const el = document.getElementById('js-review-bar'); if (!el) return; @@ -21,7 +22,7 @@ export const initReviewBar = ({ editorAiActions = [] } = {}) => { }, provide: { newCommentTemplatePath: el.dataset.newCommentTemplatePath, - editorAiActions, + canSummarize: parseBoolean(el.dataset.canSummarize), }, computed: { ...mapGetters('batchComments', ['draftsCount']), diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 883b9e6919be..09fe611262cb 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -183,7 +183,7 @@ function getActionFromHref(href) { } const pageBundles = { - show: () => import(/* webpackPrefetch: true */ 'ee_else_ce/mr_notes/mount_app'), + show: () => import(/* webpackPrefetch: true */ '~/mr_notes/mount_app'), diffs: () => import(/* webpackPrefetch: true */ '~/diffs'), }; diff --git a/app/assets/javascripts/mr_notes/init_notes.js b/app/assets/javascripts/mr_notes/init_notes.js index 1795363f24cc..fc189d058f31 100644 --- a/app/assets/javascripts/mr_notes/init_notes.js +++ b/app/assets/javascripts/mr_notes/init_notes.js @@ -12,7 +12,7 @@ import NotesApp from '../notes/components/notes_app.vue'; import { getNotesFilterData } from '../notes/utils/get_notes_filter_data'; import initWidget from '../vue_merge_request_widget'; -export default ({ editorAiActions = [] } = {}) => { +export default () => { requestIdleCallback( () => { renderGFM(document.getElementById('diff-notes-app')); @@ -42,7 +42,6 @@ export default ({ editorAiActions = [] } = {}) => { provide: { reportAbusePath: notesDataset.reportAbusePath, newCommentTemplatePath: notesDataset.newCommentTemplatePath, - editorAiActions, mrFilter: true, }, data() { diff --git a/app/assets/javascripts/pages/projects/merge_requests/show/index.js b/app/assets/javascripts/pages/projects/merge_requests/show/index.js index 9eaf490abb2c..cacfb00fa2cc 100644 --- a/app/assets/javascripts/pages/projects/merge_requests/show/index.js +++ b/app/assets/javascripts/pages/projects/merge_requests/show/index.js @@ -1,4 +1,4 @@ -import mountNotesApp from 'ee_else_ce/mr_notes/mount_app'; +import mountNotesApp from '~/mr_notes/mount_app'; import { initMrPage } from 'ee_else_ce/pages/projects/merge_requests/page'; initMrPage(); diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index af1c85532c32..6c585a060d1c 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -309,6 +309,10 @@ def hidden_merge_request_icon(merge_request) def tab_count_display(merge_request, count) merge_request.preparing? ? "-" : count end + + def review_bar_data(_merge_request, _user) + { new_comment_template_path: profile_comment_templates_path } + end end MergeRequestsHelper.prepend_mod_with('MergeRequestsHelper') diff --git a/app/views/projects/merge_requests/_page.html.haml b/app/views/projects/merge_requests/_page.html.haml index 5ea67376a866..69e2487152ef 100644 --- a/app/views/projects/merge_requests/_page.html.haml +++ b/app/views/projects/merge_requests/_page.html.haml @@ -106,7 +106,7 @@ - if @merge_request.can_be_cherry_picked? = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit -#js-review-bar{ data: { new_comment_template_path: profile_comment_templates_path } } +#js-review-bar{ data: review_bar_data(@merge_request, current_user) } - if current_user && Feature.enabled?(:mr_experience_survey, current_user) #js-mr-experience-survey{ data: { account_age: current_user.account_age_in_days } } diff --git a/ee/app/assets/javascripts/ai/editor_actions/summarize_my_review.js b/ee/app/assets/javascripts/ai/editor_actions/summarize_my_review.js deleted file mode 100644 index 6e7eeb337d58..000000000000 --- a/ee/app/assets/javascripts/ai/editor_actions/summarize_my_review.js +++ /dev/null @@ -1,27 +0,0 @@ -import { __ } from '~/locale'; -import { convertToGraphQLId } from '~/graphql_shared/utils'; -import { TYPENAME_USER } from '~/graphql_shared/constants'; -import aiActionMutation from '../graphql/summarize_review.mutation.graphql'; - -export const createSummarizeMyReview = (store) => { - return { - title: __('Summarize my code review'), - description: __('Creates a summary of all your draft code comments'), - subscriptionVariables() { - const resourceId = convertToGraphQLId('MergeRequest', store.state.notes.noteableData.id); - return { - userId: convertToGraphQLId(TYPENAME_USER, gon.current_user_id), - resourceId, - }; - }, - apolloMutation() { - const resourceId = convertToGraphQLId('MergeRequest', store.state.notes.noteableData.id); - return { - mutation: aiActionMutation, - variables: { - resourceId, - }, - }; - }, - }; -}; diff --git a/ee/app/assets/javascripts/batch_comments/components/summarize_my_review.vue b/ee/app/assets/javascripts/batch_comments/components/summarize_my_review.vue new file mode 100644 index 000000000000..4ed06d17f43e --- /dev/null +++ b/ee/app/assets/javascripts/batch_comments/components/summarize_my_review.vue @@ -0,0 +1,98 @@ +<script> +import { GlButton } from '@gitlab/ui'; +import * as Sentry from '@sentry/browser'; +import { __ } from '~/locale'; +import { createAlert } from '~/alert'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; +import { TYPENAME_USER, TYPENAME_MERGE_REQUEST } from '~/graphql_shared/constants'; +import aiResponseSubscription from 'ee/graphql_shared/subscriptions/ai_completion_response.subscription.graphql'; +import aiSummarizeReviewMutation from '../graphql/summarize_review.mutation.graphql'; + +export default { + apollo: { + $subscribe: { + testFile: { + query: aiResponseSubscription, + variables() { + return { + resourceId: this.resourceId, + userId: convertToGraphQLId(TYPENAME_USER, window.gon.current_user_id), + }; + }, + skip() { + return !this.loading; + }, + result({ data }) { + const responseBody = data.aiCompletionResponse?.responseBody; + const errors = data.aiCompletionResponse?.errors; + + if (errors?.length) { + createAlert({ message: errors[0] }); + this.loading = false; + } else if (responseBody) { + this.$emit('input', responseBody); + + this.loading = false; + } + }, + }, + }, + }, + components: { + GlButton, + }, + props: { + id: { + required: true, + type: Number, + }, + }, + data() { + return { + loading: false, + }; + }, + computed: { + resourceId() { + return convertToGraphQLId(TYPENAME_MERGE_REQUEST, this.id); + }, + }, + methods: { + triggerAiMutation() { + this.loading = true; + + try { + this.$apollo.mutate({ + mutation: aiSummarizeReviewMutation, + variables: { + resourceId: this.resourceId, + }, + }); + } catch (e) { + Sentry.captureException(e); + + createAlert({ + message: __('There was an summarizing your pending comments.'), + primaryButton: { + text: __('Try again'), + clickHandler: () => this.triggerAiMutation(), + }, + }); + + this.loading = false; + } + }, + }, +}; +</script> + +<template> + <gl-button + icon="tanuki-ai" + :loading="loading" + data-testid="mutation-trigger" + @click="triggerAiMutation" + > + {{ __('Summarize my pending comments') }} + </gl-button> +</template> diff --git a/ee/app/assets/javascripts/ai/graphql/summarize_review.mutation.graphql b/ee/app/assets/javascripts/batch_comments/graphql/summarize_review.mutation.graphql similarity index 100% rename from ee/app/assets/javascripts/ai/graphql/summarize_review.mutation.graphql rename to ee/app/assets/javascripts/batch_comments/graphql/summarize_review.mutation.graphql diff --git a/ee/app/assets/javascripts/mr_notes/index.js b/ee/app/assets/javascripts/mr_notes/index.js index d7940bd62147..ee8f6bef3381 100644 --- a/ee/app/assets/javascripts/mr_notes/index.js +++ b/ee/app/assets/javascripts/mr_notes/index.js @@ -1,15 +1,5 @@ import initMrNotes from '~/mr_notes/init_mr_notes'; -import { createSummarizeMyReview } from 'ee/ai/editor_actions/summarize_my_review'; -import store from '~/mr_notes/stores'; export default () => { - const editorAiActions = []; - if (window.gon?.features?.summarizeMyCodeReview) { - editorAiActions.push(createSummarizeMyReview(store)); - } - initMrNotes({ - reviewBarParams: { - editorAiActions, - }, - }); + initMrNotes(); }; diff --git a/ee/app/assets/javascripts/mr_notes/mount_app.js b/ee/app/assets/javascripts/mr_notes/mount_app.js deleted file mode 100644 index 1280de4e0da1..000000000000 --- a/ee/app/assets/javascripts/mr_notes/mount_app.js +++ /dev/null @@ -1,14 +0,0 @@ -import initNotes from '~/mr_notes/init_notes'; -import { createSummarizeMyReview } from 'ee/ai/editor_actions/summarize_my_review'; -import store from '~/mr_notes/stores'; - -// this module is required for the EE functions to work properly with merge request tabs -export default () => { - const editorAiActions = []; - if (window.gon?.features?.summarizeMyCodeReview) { - editorAiActions.push(createSummarizeMyReview(store)); - } - initNotes({ - editorAiActions, - }); -}; diff --git a/ee/app/helpers/ee/merge_requests_helper.rb b/ee/app/helpers/ee/merge_requests_helper.rb index c65d22aa1757..68a75aace0f4 100644 --- a/ee/app/helpers/ee/merge_requests_helper.rb +++ b/ee/app/helpers/ee/merge_requests_helper.rb @@ -33,5 +33,10 @@ def diffs_tab_pane_data(project, merge_request, params) def summarize_llm_enabled?(project, user) ::Llm::MergeRequests::SummarizeDiffService.enabled?(group: project.root_ancestor, user: user) end + + override :review_bar_data + def review_bar_data(merge_request, user) + super.merge({ can_summarize: Ability.allowed?(user, :summarize_draft_code_review, merge_request).to_s }) + end end end diff --git a/ee/spec/frontend/batch_comments/components/summarize_my_review_spec.js b/ee/spec/frontend/batch_comments/components/summarize_my_review_spec.js new file mode 100644 index 000000000000..430ef342dfb1 --- /dev/null +++ b/ee/spec/frontend/batch_comments/components/summarize_my_review_spec.js @@ -0,0 +1,96 @@ +import Vue, { nextTick } from 'vue'; +import VueApollo from 'vue-apollo'; +import waitForPromises from 'helpers/wait_for_promises'; +import { createAlert } from '~/alert'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import SummarizeMyReview from 'ee/batch_comments/components/summarize_my_review.vue'; +import aiResponseSubscription from 'ee/graphql_shared/subscriptions/ai_completion_response.subscription.graphql'; +import summarizeReviewMutation from 'ee/batch_comments/graphql/summarize_review.mutation.graphql'; + +jest.mock('~/alert'); + +Vue.use(VueApollo); + +let wrapper; +let subscriptionHandlerMock; +let mutationHandlerMock; + +function createComponent() { + const apolloProvider = createMockApollo([ + [aiResponseSubscription, subscriptionHandlerMock], + [summarizeReviewMutation, mutationHandlerMock], + ]); + + wrapper = mountExtended(SummarizeMyReview, { + propsData: { + id: 1, + }, + apolloProvider, + }); +} + +const findButton = () => wrapper.findByTestId('mutation-trigger'); + +describe('Generate test file drawer component', () => { + beforeEach(() => { + window.gon.current_user_id = 1; + mutationHandlerMock = jest + .fn() + .mockResolvedValue({ data: { aiAction: { errors: [], __typename: 'AiActionPayload' } } }); + subscriptionHandlerMock = jest.fn().mockResolvedValue({ + data: { + aiCompletionResponse: { + responseBody: 'This is a summary', + errors: [], + }, + }, + }); + }); + + afterEach(() => { + mutationHandlerMock.mockRestore(); + subscriptionHandlerMock.mockRestore(); + }); + + it('calls mutation button is clicked', async () => { + createComponent(); + + findButton().trigger('click'); + + await nextTick(); + + expect(mutationHandlerMock).toHaveBeenCalledWith({ resourceId: 'gid://gitlab/MergeRequest/1' }); + }); + + it('emits input event when subscription returns value', async () => { + createComponent(); + + findButton().trigger('click'); + + await nextTick(); + await waitForPromises(); + + expect(wrapper.emitted()).toEqual({ input: [['This is a summary']] }); + }); + + it('calls createAlert when subsription returns an error', async () => { + subscriptionHandlerMock = jest.fn().mockResolvedValue({ + data: { + aiCompletionResponse: { + responseBody: null, + errors: ['Error'], + }, + }, + }); + + createComponent(); + + findButton().trigger('click'); + + await nextTick(); + await waitForPromises(); + + expect(createAlert).toHaveBeenCalledWith({ message: 'Error' }); + }); +}); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 858153947bd0..0ef9779fcf66 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13528,9 +13528,6 @@ msgstr "" msgid "Creates a summary of all comments" msgstr "" -msgid "Creates a summary of all your draft code comments" -msgstr "" - msgid "Creates branch '%{branch_name}' and a merge request to resolve this issue." msgstr "" @@ -44932,7 +44929,7 @@ msgstr "" msgid "Summarize comments" msgstr "" -msgid "Summarize my code review" +msgid "Summarize my pending comments" msgstr "" msgid "Summary" @@ -46846,6 +46843,9 @@ msgstr "" msgid "There was an error with the reCAPTCHA. Please solve the reCAPTCHA again." msgstr "" +msgid "There was an summarizing your pending comments." +msgstr "" + msgid "These dates affect how your epics appear in the roadmap. Set a fixed date or one inherited from the milestones assigned to issues in this epic." msgstr "" -- GitLab