Merge branch '25381-apply-suggestion-with-custom-message' into 'master'

Add a custom commit message when applying a suggested change (FE)

See merge request gitlab-org/gitlab!48597

app/assets/javascripts/api.js

@@ -442,10 +442,10 @@ const Api = {
     });
   },
 
-  applySuggestion(id) {
+  applySuggestion(id, message) {
     const url = Api.buildUrl(Api.applySuggestionPath).replace(':id', encodeURIComponent(id));
 
-    return axios.put(url);
+    return axios.put(url, { commit_message: message });
   },
 
   applySuggestionBatch(ids) {

app/assets/javascripts/diffs/components/app.vue

@@ -124,6 +124,11 @@ export default {
       required: false,
       default: false,
     },
+    defaultSuggestionCommitMessage: {
+      type: String,
+      required: false,
+      default: '',
+    },
     mrReviews: {
       type: Object,
       required: false,
@@ -268,6 +273,7 @@ export default {
       dismissEndpoint: this.dismissEndpoint,
       showSuggestPopover: this.showSuggestPopover,
       viewDiffsFileByFile: fileByFile(this.fileByFileUserPreference),
+      defaultSuggestionCommitMessage: this.defaultSuggestionCommitMessage,
       mrReviews: this.mrReviews || {},
     });
 

app/assets/javascripts/diffs/index.js

@@ -83,6 +83,7 @@ export default function initDiffsApp(store) {
         showSuggestPopover: parseBoolean(dataset.showSuggestPopover),
         showWhitespaceDefault: parseBoolean(dataset.showWhitespaceDefault),
         viewDiffsFileByFile: parseBoolean(dataset.fileByFileDefault),
+        defaultSuggestionCommitMessage: dataset.defaultSuggestionCommitMessage,
       };
     },
     computed: {
@@ -123,6 +124,7 @@ export default function initDiffsApp(store) {
           dismissEndpoint: this.dismissEndpoint,
           showSuggestPopover: this.showSuggestPopover,
           fileByFileUserPreference: this.viewDiffsFileByFile,
+          defaultSuggestionCommitMessage: this.defaultSuggestionCommitMessage,
           mrReviews: getReviewsForMergeRequest(mrPath),
         },
       });

app/assets/javascripts/diffs/store/actions.js

@@ -62,6 +62,7 @@ export const setBaseConfig = ({ commit }, options) => {
     projectPath,
     dismissEndpoint,
     showSuggestPopover,
+    defaultSuggestionCommitMessage,
     viewDiffsFileByFile,
     mrReviews,
   } = options;
@@ -73,6 +74,7 @@ export const setBaseConfig = ({ commit }, options) => {
     projectPath,
     dismissEndpoint,
     showSuggestPopover,
+    defaultSuggestionCommitMessage,
     viewDiffsFileByFile,
     mrReviews,
   });

app/assets/javascripts/diffs/store/modules/diff_state.js

@@ -45,5 +45,6 @@ export default () => ({
   fileFinderVisible: false,
   dismissEndpoint: '',
   showSuggestPopover: true,
+  defaultSuggestionCommitMessage: '',
   mrReviews: {},
 });

app/assets/javascripts/diffs/store/mutations.js

@@ -36,6 +36,7 @@ export default {
       projectPath,
       dismissEndpoint,
       showSuggestPopover,
+      defaultSuggestionCommitMessage,
       viewDiffsFileByFile,
       mrReviews,
     } = options;
@@ -47,6 +48,7 @@ export default {
       projectPath,
       dismissEndpoint,
       showSuggestPopover,
+      defaultSuggestionCommitMessage,
       viewDiffsFileByFile,
       mrReviews,
     });

app/assets/javascripts/notes/components/note_body.vue

@@ -54,6 +54,7 @@ export default {
     ...mapState({
       batchSuggestionsInfo: (state) => state.notes.batchSuggestionsInfo,
     }),
+    ...mapState('diffs', ['defaultSuggestionCommitMessage']),
     noteBody() {
       return this.note.note;
     },
@@ -98,12 +99,16 @@ export default {
     formCancelHandler(shouldConfirm, isDirty) {
       this.$emit('cancelForm', shouldConfirm, isDirty);
     },
-    applySuggestion({ suggestionId, flashContainer, callback = () => {} }) {
+    applySuggestion({ suggestionId, flashContainer, callback = () => {}, message }) {
       const { discussion_id: discussionId, id: noteId } = this.note;
 
-      return this.submitSuggestion({ discussionId, noteId, suggestionId, flashContainer }).then(
-        callback,
-      );
+      return this.submitSuggestion({
+        discussionId,
+        noteId,
+        suggestionId,
+        flashContainer,
+        message,
+      }).then(callback);
     },
     applySuggestionBatch({ flashContainer }) {
       return this.submitSuggestionBatch({ flashContainer });
@@ -130,6 +135,7 @@ export default {
       :note-html="note.note_html"
       :line-type="lineType"
       :help-page-path="helpPagePath"
+      :default-commit-message="defaultSuggestionCommitMessage"
       @apply="applySuggestion"
       @applyBatch="applySuggestionBatch"
       @addToBatch="addSuggestionToBatch"

app/assets/javascripts/notes/stores/actions.js

@@ -559,7 +559,7 @@ export const updateResolvableDiscussionsCounts = ({ commit }) =>
 
 export const submitSuggestion = (
   { commit, dispatch },
-  { discussionId, suggestionId, flashContainer },
+  { discussionId, suggestionId, flashContainer, message },
 ) => {
   const dispatchResolveDiscussion = () =>
     dispatch('resolveDiscussion', { discussionId }).catch(() => {});
@@ -567,7 +567,7 @@ export const submitSuggestion = (
   commit(types.SET_RESOLVING_DISCUSSION, true);
   dispatch('stopPolling');
 
-  return Api.applySuggestion(suggestionId)
+  return Api.applySuggestion(suggestionId, message)
     .then(dispatchResolveDiscussion)
     .catch((err) => {
       const defaultMessage = __(

app/assets/javascripts/vue_shared/components/markdown/apply_suggestion.vue

 <script>
 import { GlDropdown, GlDropdownForm, GlFormTextarea, GlButton } from '@gitlab/ui';
-import { __, sprintf } from '~/locale';
 
 export default {
   components: { GlDropdown, GlDropdownForm, GlFormTextarea, GlButton },
@@ -10,7 +9,7 @@ export default {
       required: false,
       default: false,
     },
-    fileName: {
+    defaultCommitMessage: {
       type: String,
       required: true,
     },
@@ -18,18 +17,11 @@ export default {
   data() {
     return {
       message: null,
-      buttonText: __('Apply suggestion'),
-      headerText: __('Apply suggestion commit message'),
     };
   },
-  computed: {
-    placeholderText() {
-      return sprintf(__('Apply suggestion on %{fileName}'), { fileName: this.fileName });
-    },
-  },
   methods: {
     onApply() {
-      this.$emit('apply', this.message || this.placeholderText);
+      this.$emit('apply', this.message);
     },
   },
 };
@@ -37,18 +29,26 @@ export default {
 
 <template>
   <gl-dropdown
-    :text="buttonText"
-    :header-text="headerText"
+    :text="__('Apply suggestion')"
     :disabled="disabled"
     boundary="window"
     right
-    menu-class="gl-w-full! gl-pb-0!"
+    menu-class="gl-w-full!"
+    @shown="$refs.commitMessage.$el.focus()"
   >
-    <gl-dropdown-form class="gl-m-3!">
-      <gl-form-textarea v-model="message" :placeholder="placeholderText" />
+    <gl-dropdown-form class="gl-px-4! gl-m-0!">
+      <label for="commit-message">{{ __('Commit message') }}</label>
+      <gl-form-textarea
+        id="commit-message"
+        ref="commitMessage"
+        v-model="message"
+        :placeholder="defaultCommitMessage"
+        submit-on-enter
+        @submit="onApply"
+      />
       <gl-button
-        class="gl-w-quarter! gl-mt-3 gl-text-center! float-right"
-        category="secondary"
+        class="gl-w-auto! gl-mt-3 gl-text-center! gl-hover-text-white! gl-transition-medium! float-right"
+        category="primary"
         variant="success"
         @click="onApply"
       >

app/assets/javascripts/vue_shared/components/markdown/suggestion_diff.vue

@@ -27,6 +27,10 @@ export default {
       type: String,
       required: true,
     },
+    defaultCommitMessage: {
+      type: String,
+      required: true,
+    },
     suggestionsCount: {
       type: Number,
       required: false,
@@ -47,8 +51,8 @@ export default {
     },
   },
   methods: {
-    applySuggestion(callback) {
-      this.$emit('apply', { suggestionId: this.suggestion.id, callback });
+    applySuggestion(callback, message) {
+      this.$emit('apply', { suggestionId: this.suggestion.id, callback, message });
     },
     applySuggestionBatch() {
       this.$emit('applyBatch');
@@ -74,6 +78,7 @@ export default {
       :is-applying-batch="suggestion.is_applying_batch"
       :batch-suggestions-count="batchSuggestionsCount"
       :help-page-path="helpPagePath"
+      :default-commit-message="defaultCommitMessage"
       :inapplicable-reason="suggestion.inapplicable_reason"
       @apply="applySuggestion"
       @applyBatch="applySuggestionBatch"

app/assets/javascripts/vue_shared/components/markdown/suggestion_diff_header.vue

@@ -2,9 +2,10 @@
 import { GlButton, GlLoadingIcon, GlTooltipDirective, GlIcon } from '@gitlab/ui';
 import { __ } from '~/locale';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import ApplySuggestion from './apply_suggestion.vue';
 
 export default {
-  components: { GlIcon, GlButton, GlLoadingIcon },
+  components: { GlIcon, GlButton, GlLoadingIcon, ApplySuggestion },
   directives: { 'gl-tooltip': GlTooltipDirective },
   mixins: [glFeatureFlagsMixin()],
   props: {
@@ -37,6 +38,10 @@ export default {
       type: String,
       required: true,
     },
+    defaultCommitMessage: {
+      type: String,
+      required: true,
+    },
     inapplicableReason: {
       type: String,
       required: false,
@@ -57,6 +62,9 @@ export default {
     canBeBatched() {
       return Boolean(this.glFeatures.batchSuggestions);
     },
+    canAddCustomCommitMessage() {
+      return this.glFeatures.suggestionsCustomCommit;
+    },
     isApplying() {
       return this.isApplyingSingle || this.isApplyingBatch;
     },
@@ -77,10 +85,10 @@ export default {
     },
   },
   methods: {
-    applySuggestion() {
+    applySuggestion(message) {
       if (!this.canApply) return;
       this.isApplyingSingle = true;
-      this.$emit('apply', this.applySuggestionCallback);
+      this.$emit('apply', this.applySuggestionCallback, message);
     },
     applySuggestionCallback() {
       this.isApplyingSingle = false;
@@ -142,7 +150,14 @@ export default {
       >
         {{ __('Add suggestion to batch') }}
       </gl-button>
-      <span v-gl-tooltip.viewport="tooltipMessage" tabindex="0">
+      <apply-suggestion
+        v-if="canAddCustomCommitMessage"
+        :disabled="isDisableButton"
+        :default-commit-message="defaultCommitMessage"
+        class="gl-ml-3"
+        @apply="applySuggestion"
+      />
+      <span v-else v-gl-tooltip.viewport="tooltipMessage" tabindex="0">
         <gl-button
           v-if="isLoggedIn"
           class="btn-inverted js-apply-btn btn-grouped"

app/assets/javascripts/vue_shared/components/markdown/suggestions.vue

@@ -38,6 +38,10 @@ export default {
       type: String,
       required: true,
     },
+    defaultCommitMessage: {
+      type: String,
+      required: true,
+    },
     suggestionsCount: {
       type: Number,
       required: false,
@@ -82,16 +86,30 @@ export default {
       this.isRendered = true;
     },
     generateDiff(suggestionIndex) {
-      const { suggestions, disabled, batchSuggestionsInfo, helpPagePath, suggestionsCount } = this;
+      const {
+        suggestions,
+        disabled,
+        batchSuggestionsInfo,
+        helpPagePath,
+        defaultCommitMessage,
+        suggestionsCount,
+      } = this;
       const suggestion =
         suggestions && suggestions[suggestionIndex] ? suggestions[suggestionIndex] : {};
       const SuggestionDiffComponent = Vue.extend(SuggestionDiff);
       const suggestionDiff = new SuggestionDiffComponent({
-        propsData: { disabled, suggestion, batchSuggestionsInfo, helpPagePath, suggestionsCount },
+        propsData: {
+          disabled,
+          suggestion,
+          batchSuggestionsInfo,
+          helpPagePath,
+          defaultCommitMessage,
+          suggestionsCount,
+        },
       });
 
-      suggestionDiff.$on('apply', ({ suggestionId, callback }) => {
-        this.$emit('apply', { suggestionId, callback, flashContainer: this.$el });
+      suggestionDiff.$on('apply', ({ suggestionId, callback, message }) => {
+        this.$emit('apply', { suggestionId, callback, flashContainer: this.$el, message });
       });
 
       suggestionDiff.$on('applyBatch', () => {

app/controllers/projects/merge_requests_controller.rb

@@ -42,6 +42,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     push_frontend_feature_flag(:remove_resolve_note, @project, default_enabled: true)
     push_frontend_feature_flag(:diffs_gradual_load, @project, default_enabled: true)
     push_frontend_feature_flag(:codequality_mr_diff, @project)
+    push_frontend_feature_flag(:suggestions_custom_commit, @project)
 
     record_experiment_user(:invite_members_version_a)
     record_experiment_user(:invite_members_version_b)

app/helpers/merge_requests_helper.rb

@@ -185,6 +185,10 @@ module MergeRequestsHelper
 
     current_user.review_requested_open_merge_requests_count
   end
+
+  def default_suggestion_commit_message
+    @project.suggestion_commit_message.presence || Gitlab::Suggestions::CommitMessage::DEFAULT_SUGGESTION_COMMIT_MESSAGE
+  end
 end
 
 MergeRequestsHelper.prepend_if_ee('EE::MergeRequestsHelper')

app/views/projects/merge_requests/show.html.haml

@@ -90,7 +90,8 @@
         dismiss_endpoint: user_callouts_path,
         show_suggest_popover: show_suggest_popover?.to_s,
         show_whitespace_default: @show_whitespace_default.to_s,
-        file_by_file_default: @file_by_file_default.to_s }
+        file_by_file_default: @file_by_file_default.to_s,
+        default_suggestion_commit_message: default_suggestion_commit_message }
 
     .mr-loading-status
       .loading.hide

config/feature_flags/development/suggestions_custom_commit.yml

+---
+name: suggestions_custom_commit
+introduced_by_url:
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/297404
+milestone: '13.9'
+type: development
+group: group::code review
+default_enabled: false

locale/gitlab.pot

@@ -3543,12 +3543,6 @@ msgstr ""
 msgid "Apply suggestion"
 msgstr ""
 
-msgid "Apply suggestion commit message"
-msgstr ""
-
-msgid "Apply suggestion on %{fileName}"
-msgstr ""
-
 msgid "Apply suggestions"
 msgstr ""
 

spec/features/merge_request/user_suggests_changes_on_diff_spec.rb

@@ -87,6 +87,7 @@ RSpec.describe 'User comments on a diff', :js do
         expect(page).not_to have_content('Applied')
 
         click_button('Apply suggestion')
+        click_button('Apply')
         wait_for_requests
 
         expect(page).to have_content('Applied')
@@ -338,6 +339,7 @@ RSpec.describe 'User comments on a diff', :js do
         expect(page).not_to have_content('Applied')
 
         click_button('Apply suggestion')
+        click_button('Apply')
         wait_for_requests
 
         expect(page).to have_content('Applied')
@@ -349,6 +351,7 @@ RSpec.describe 'User comments on a diff', :js do
         expect(page).not_to have_content('Unresolve thread')
 
         click_button('Apply suggestion')
+        click_button('Apply')
         wait_for_requests
 
         expect(page).to have_content('Unresolve thread')

spec/frontend/vue_shared/components/markdown/__snapshots__/suggestion_diff_spec.js.snap

@@ -7,6 +7,7 @@ exports[`Suggestion Diff component matches snapshot 1`] = `
   <suggestion-diff-header-stub
     batchsuggestionscount="1"
     class="qa-suggestion-diff-header js-suggestion-diff-header"
+    defaultcommitmessage="Apply suggestion"
     helppagepath="path_to_docs"
     isapplyingbatch="true"
     isbatched="true"

spec/frontend/vue_shared/components/markdown/apply_suggestion_spec.js

@@ -3,7 +3,7 @@ import { GlDropdown, GlFormTextarea, GlButton } from '@gitlab/ui';
 import ApplySuggestionComponent from '~/vue_shared/components/markdown/apply_suggestion.vue';
 
 describe('Apply Suggestion component', () => {
-  const propsData = { fileName: 'test.js', disabled: false };
+  const propsData = { defaultCommitMessage: 'Apply suggestion', disabled: false };
   let wrapper;
 
   const createWrapper = (props) => {
@@ -27,7 +27,6 @@ describe('Apply Suggestion component', () => {
 
       expect(dropdown.exists()).toBe(true);
       expect(dropdown.props('text')).toBe('Apply suggestion');
-      expect(dropdown.props('headerText')).toBe('Apply suggestion commit message');
       expect(dropdown.props('disabled')).toBe(false);
     });
 
@@ -35,7 +34,7 @@ describe('Apply Suggestion component', () => {
       const textArea = findTextArea();
 
       expect(textArea.exists()).toBe(true);
-      expect(textArea.attributes('placeholder')).toBe('Apply suggestion on test.js');
+      expect(textArea.attributes('placeholder')).toBe('Apply suggestion');
     });
 
     it('renders an apply button', () => {
@@ -55,11 +54,11 @@ describe('Apply Suggestion component', () => {
   });
 
   describe('apply suggestion', () => {
-    it('emits an apply event with a default message if no message was added', () => {
+    it('emits an apply event with no message if no message was added', () => {
       findTextArea().vm.$emit('input', null);
       findApplyButton().vm.$emit('click');
 
-      expect(wrapper.emitted('apply')).toEqual([['Apply suggestion on test.js']]);
+      expect(wrapper.emitted('apply')).toEqual([[null]]);
     });
 
     it('emits an apply event with a user-defined message', () => {

spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js

@@ -9,6 +9,7 @@ const DEFAULT_PROPS = {
   isBatched: false,
   isApplyingBatch: false,
   helpPagePath: 'path_to_docs',
+  defaultCommitMessage: 'Apply suggestion',
 };
 
 describe('Suggestion Diff component', () => {
@@ -91,7 +92,7 @@ describe('Suggestion Diff component', () => {
     });
 
     it('emits apply', () => {
-      expect(wrapper.emitted().apply).toEqual([[expect.any(Function)]]);
+      expect(wrapper.emitted().apply).toEqual([[expect.any(Function), undefined]]);
     });
 
     it('does not render apply suggestion and add to batch buttons', () => {

spec/frontend/vue_shared/components/markdown/suggestion_diff_spec.js

@@ -42,6 +42,7 @@ const MOCK_DATA = {
     is_applying_batch: true,
   },
   helpPagePath: 'path_to_docs',
+  defaultCommitMessage: 'Apply suggestion',
   batchSuggestionsInfo: [{ suggestionId }],
 };
 

spec/frontend/vue_shared/components/markdown/suggestions_spec.js

@@ -44,6 +44,7 @@ const MOCK_DATA = {
   `,
   isApplied: false,
   helpPagePath: 'path_to_docs',
+  defaultCommitMessage: 'Apply suggestion',
 };
 
 describe('Suggestion component', () => {