Merge branch 'tor/defect/commit-message-placeholders' into 'master'

Fill default suggestion commit message values in the placeholder instead of showing the variable slugs

See merge request gitlab-org/gitlab!52851

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

@@ -4,6 +4,7 @@ import {
   INLINE_DIFF_VIEW_TYPE,
   INLINE_DIFF_LINES_KEY,
 } from '../constants';
+import { computeSuggestionCommitMessage } from '../utils/suggestions';
 import { parallelizeDiffLines } from './utils';
 
 export * from './getters_versions_dropdowns';
@@ -154,3 +155,18 @@ export const diffLines = (state) => (file, unifiedDiffComponents) => {
     state.diffViewType === INLINE_DIFF_VIEW_TYPE,
   );
 };
+
+export function suggestionCommitMessage(state) {
+  return (values = {}) =>
+    computeSuggestionCommitMessage({
+      message: state.defaultSuggestionCommitMessage,
+      values: {
+        branch_name: state.branchName,
+        project_path: state.projectPath,
+        project_name: state.projectName,
+        username: state.username,
+        user_full_name: state.userFullName,
+        ...values,
+      },
+    });
+}

app/assets/javascripts/diffs/utils/suggestions.js

+function removeEmptyProperties(dict) {
+  const noBlanks = Object.entries(dict).reduce((final, [key, value]) => {
+    const upd = { ...final };
+
+    // The number 0 shouldn't be falsey when we're printing variables
+    if (value || value === 0) {
+      upd[key] = value;
+    }
+
+    return upd;
+  }, {});
+
+  return noBlanks;
+}
+
+export function computeSuggestionCommitMessage({ message, values = {} } = {}) {
+  const noEmpties = removeEmptyProperties(values);
+  const matchPhrases = Object.keys(noEmpties)
+    .map((key) => `%{${key}}`)
+    .join('|');
+  const replacementExpression = new RegExp(`(${matchPhrases})`, 'gm');
+
+  return message.replace(replacementExpression, (match) => {
+    const key = match.replace(/(^%{|}$)/gm, '');
+
+    return noEmpties[key];
+  });
+}

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

@@ -2,6 +2,8 @@
 /* eslint-disable vue/no-v-html */
 import { mapActions, mapGetters, mapState } from 'vuex';
 import $ from 'jquery';
+import { escape } from 'lodash';
+
 import '~/behaviors/markdown/render_gfm';
 import Suggestions from '~/vue_shared/components/markdown/suggestions.vue';
 import autosave from '../mixins/autosave';
@@ -29,6 +31,11 @@ export default {
       required: false,
       default: null,
     },
+    file: {
+      type: Object,
+      required: false,
+      default: null,
+    },
     canEdit: {
       type: Boolean,
       required: true,
@@ -46,6 +53,7 @@ export default {
   },
   computed: {
     ...mapGetters(['getDiscussion', 'suggestionsCount']),
+    ...mapGetters('diffs', ['suggestionCommitMessage']),
     discussion() {
       if (!this.note.isDraft) return {};
 
@@ -54,7 +62,6 @@ export default {
     ...mapState({
       batchSuggestionsInfo: (state) => state.notes.batchSuggestionsInfo,
     }),
-    ...mapState('diffs', ['defaultSuggestionCommitMessage']),
     noteBody() {
       return this.note.note;
     },
@@ -64,6 +71,21 @@ export default {
     lineType() {
       return this.line ? this.line.type : null;
     },
+    commitMessage() {
+      // Please see this issue comment for why these
+      //  are hard-coded to 1:
+      //  https://gitlab.com/gitlab-org/gitlab/-/issues/291027#note_468308022
+      const suggestionsCount = 1;
+      const filesCount = 1;
+      const filePaths = this.file ? [this.file.file_path] : [];
+      const suggestion = this.suggestionCommitMessage({
+        file_paths: filePaths.join(', '),
+        suggestions_count: suggestionsCount,
+        files_count: filesCount,
+      });
+
+      return escape(suggestion);
+    },
   },
   mounted() {
     this.renderGFM();
@@ -135,7 +157,7 @@ export default {
       :note-html="note.note_html"
       :line-type="lineType"
       :help-page-path="helpPagePath"
-      :default-commit-message="defaultSuggestionCommitMessage"
+      :default-commit-message="commitMessage"
       @apply="applySuggestion"
       @applyBatch="applySuggestionBatch"
       @addToBatch="addSuggestionToBatch"

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

@@ -431,6 +431,7 @@ export default {
           ref="noteBody"
           :note="note"
           :line="line"
+          :file="diffFile"
           :can-edit="note.current_user.can_edit"
           :is-editing="isEditing"
           :help-page-path="helpPagePath"

changelogs/unreleased/tor-defect-commit-message-placeholders.yml

+---
+title: Fill default commit message values in the placeholder instead of showing the
+  variable slugs
+merge_request: 52851
+author:
+type: fixed

spec/frontend/diffs/store/getters_spec.js

@@ -375,4 +375,64 @@ describe('Diffs Module Getters', () => {
       });
     });
   });
+
+  describe('suggestionCommitMessage', () => {
+    beforeEach(() => {
+      Object.assign(localState, {
+        defaultSuggestionCommitMessage:
+          '%{branch_name}%{project_path}%{project_name}%{username}%{user_full_name}%{file_paths}%{suggestions_count}%{files_count}',
+        branchName: 'branch',
+        projectPath: '/path',
+        projectName: 'name',
+        username: 'user',
+        userFullName: 'user userton',
+      });
+    });
+
+    it.each`
+      specialState              | output
+      ${{}}                     | ${'branch/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ userFullName: null }} | ${'branch/pathnameuser%{user_full_name}%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ username: null }}     | ${'branch/pathname%{username}user userton%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ projectName: null }}  | ${'branch/path%{project_name}useruser userton%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ projectPath: null }}  | ${'branch%{project_path}nameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ branchName: null }}   | ${'%{branch_name}/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
+    `(
+      'provides the correct "base" default commit message based on state ($specialState)',
+      ({ specialState, output }) => {
+        Object.assign(localState, specialState);
+
+        expect(getters.suggestionCommitMessage(localState)()).toBe(output);
+      },
+    );
+
+    it.each`
+      stateOverrides              | output
+      ${{}}                       | ${'branch/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ user_full_name: null }} | ${'branch/pathnameuser%{user_full_name}%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ username: null }}       | ${'branch/pathname%{username}user userton%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ project_name: null }}   | ${'branch/path%{project_name}useruser userton%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ project_path: null }}   | ${'branch%{project_path}nameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
+      ${{ branch_name: null }}    | ${'%{branch_name}/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
+    `(
+      "properly overrides state values ($stateOverrides) if they're provided",
+      ({ stateOverrides, output }) => {
+        expect(getters.suggestionCommitMessage(localState)(stateOverrides)).toBe(output);
+      },
+    );
+
+    it.each`
+      providedValues                                                          | output
+      ${{ file_paths: 'path1, path2', suggestions_count: 1, files_count: 1 }} | ${'branch/pathnameuseruser usertonpath1, path211'}
+      ${{ suggestions_count: 1, files_count: 1 }}                             | ${'branch/pathnameuseruser userton%{file_paths}11'}
+      ${{ file_paths: 'path1, path2', files_count: 1 }}                       | ${'branch/pathnameuseruser usertonpath1, path2%{suggestions_count}1'}
+      ${{ file_paths: 'path1, path2', suggestions_count: 1 }}                 | ${'branch/pathnameuseruser usertonpath1, path21%{files_count}'}
+      ${{ something_unused: 'CrAzY TeXt' }}                                   | ${'branch/pathnameuseruser userton%{file_paths}%{suggestions_count}%{files_count}'}
+    `(
+      "fills in any missing interpolations ($providedValues) when they're provided at the getter callsite",
+      ({ providedValues, output }) => {
+        expect(getters.suggestionCommitMessage(localState)(providedValues)).toBe(output);
+      },
+    );
+  });
 });

spec/frontend/diffs/utils/suggestions_spec.js

+import { computeSuggestionCommitMessage } from '~/diffs/utils/suggestions';
+
+describe('Diff Suggestions utilities', () => {
+  describe('computeSuggestionCommitMessage', () => {
+    it.each`
+      description                                                     | input              | values                        | output
+      ${'makes the appropriate replacements'}                         | ${'%{foo} %{bar}'} | ${{ foo: 'foo', bar: 'bar' }} | ${'foo bar'}
+      ${"skips replacing values that aren't passed"}                  | ${'%{foo} %{bar}'} | ${{ foo: 'foo' }}             | ${'foo %{bar}'}
+      ${'treats the number 0 as a valid value (not falsey)'}          | ${'%{foo} %{bar}'} | ${{ foo: 'foo', bar: 0 }}     | ${'foo 0'}
+      ${"works when the variables don't have any space between them"} | ${'%{foo}%{bar}'}  | ${{ foo: 'foo', bar: 'bar' }} | ${'foobar'}
+    `('$description', ({ input, output, values }) => {
+      expect(computeSuggestionCommitMessage({ message: input, values })).toBe(output);
+    });
+  });
+});

spec/frontend/notes/components/note_body_spec.js

 import Vue from 'vue';
+import { shallowMount } from '@vue/test-utils';
+import Vuex from 'vuex';
+
+import notes from '~/notes/stores/modules/index';
 import createStore from '~/notes/stores';
+import { suggestionCommitMessage } from '~/diffs/store/getters';
+
+import Suggestions from '~/vue_shared/components/markdown/suggestions.vue';
 import noteBody from '~/notes/components/note_body.vue';
+
 import { noteableDataMock, notesDataMock, note } from '../mock_data';
 
 describe('issue_note_body component', () => {
@@ -54,4 +62,50 @@ describe('issue_note_body component', () => {
       expect(vm.autosave.key).toEqual(autosaveKey);
     });
   });
+
+  describe('commitMessage', () => {
+    let wrapper;
+
+    Vue.use(Vuex);
+
+    beforeEach(() => {
+      const notesStore = notes();
+
+      notesStore.state.notes = {};
+
+      store = new Vuex.Store({
+        modules: {
+          notes: notesStore,
+          diffs: {
+            namespaced: true,
+            state: {
+              defaultSuggestionCommitMessage:
+                '%{branch_name}%{project_path}%{project_name}%{username}%{user_full_name}%{file_paths}%{suggestions_count}%{files_count}',
+              branchName: 'branch',
+              projectPath: '/path',
+              projectName: 'name',
+              username: 'user',
+              userFullName: 'user userton',
+            },
+            getters: { suggestionCommitMessage },
+          },
+        },
+      });
+
+      wrapper = shallowMount(noteBody, {
+        store,
+        propsData: {
+          note: { ...note, suggestions: [12345] },
+          canEdit: true,
+          file: { file_path: 'abc' },
+        },
+      });
+    });
+
+    it('passes the correct default placeholder commit message for a suggestion to the suggestions component', () => {
+      const commitMessage = wrapper.find(Suggestions).attributes('defaultcommitmessage');
+
+      expect(commitMessage).toBe('branch/pathnameuseruser usertonabc11');
+    });
+  });
 });