diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 149afc01056a83651253a57a5bf6944894ede609..1fc2a684e951b1f119bc0ec32ac4ac1591cbaee6 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/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, + }, + }); +} diff --git a/app/assets/javascripts/diffs/utils/suggestions.js b/app/assets/javascripts/diffs/utils/suggestions.js new file mode 100644 index 0000000000000000000000000000000000000000..a272f7f32571f7849055cf6d7c57f967b86f7ec1 --- /dev/null +++ b/app/assets/javascripts/diffs/utils/suggestions.js @@ -0,0 +1,28 @@ +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]; + }); +} diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue index 03a8a8f93766a73cd39dd20829e78a3124f0a6b4..df56e6fd8b01c82dc89c8c7c78d3489ff6df2845 100644 --- a/app/assets/javascripts/notes/components/note_body.vue +++ b/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" diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index a1738b993d7476a69e771b6bc10dc5e690d28ecc..22941857f939e7a87d6ac8af606d0f9f71683731 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/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" diff --git a/changelogs/unreleased/tor-defect-commit-message-placeholders.yml b/changelogs/unreleased/tor-defect-commit-message-placeholders.yml new file mode 100644 index 0000000000000000000000000000000000000000..093dfdda81a0293c3cbf550bccdd51a07e6a3b20 --- /dev/null +++ b/changelogs/unreleased/tor-defect-commit-message-placeholders.yml @@ -0,0 +1,6 @@ +--- +title: Fill default commit message values in the placeholder instead of showing the + variable slugs +merge_request: 52851 +author: +type: fixed diff --git a/spec/frontend/diffs/store/getters_spec.js b/spec/frontend/diffs/store/getters_spec.js index 85dc52f8bf70f20c807a28cd0bf92d36526afa5d..56d6bc844c926035c25e12965820a862504ec1aa 100644 --- a/spec/frontend/diffs/store/getters_spec.js +++ b/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); + }, + ); + }); }); diff --git a/spec/frontend/diffs/utils/suggestions_spec.js b/spec/frontend/diffs/utils/suggestions_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..fbfe9cef85775dc4bb14e871a62f133bf36e91e8 --- /dev/null +++ b/spec/frontend/diffs/utils/suggestions_spec.js @@ -0,0 +1,15 @@ +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); + }); + }); +}); diff --git a/spec/frontend/notes/components/note_body_spec.js b/spec/frontend/notes/components/note_body_spec.js index 3c11c266f90068212d15d84217bf8029e06db23f..07b8d99a79a12840006bee2b6c5894541028ae8a 100644 --- a/spec/frontend/notes/components/note_body_spec.js +++ b/spec/frontend/notes/components/note_body_spec.js @@ -1,6 +1,14 @@ 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'); + }); + }); });