From 47195539026fdd70208cedb1e68c2a7f048ea9ac Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 5 Apr 2021 12:07:50 -0600 Subject: [PATCH 1/2] Pass pseudo-diff file from the app to the discussion note component Change prop file name --- .../notes/components/discussion_notes.vue | 2 + .../notes/components/noteable_note.vue | 15 +- .../notes/components/noteable_note_spec.js | 133 ++++++++++++++++-- 3 files changed, 133 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue index 0f74d78c8e04e7..dfe2763d8bd39c 100644 --- a/app/assets/javascripts/notes/components/discussion_notes.vue +++ b/app/assets/javascripts/notes/components/discussion_notes.vue @@ -121,6 +121,7 @@ export default { :is="componentName(firstNote)" :note="componentData(firstNote)" :line="line || diffLine" + :discussion-file="discussion.diff_file" :commit="commit" :help-page-path="helpPagePath" :show-reply-button="userCanReply" @@ -167,6 +168,7 @@ export default { v-for="(note, index) in discussion.notes" :key="note.id" :note="componentData(note)" + :discussion-file="discussion.diff_file" :help-page-path="helpPagePath" :line="diffLine" :discussion-root="index === 0" diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 0b5abacc96340a..0feb77be653e19 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -48,6 +48,11 @@ export default { required: false, default: null, }, + discussionFile: { + type: Object, + required: false, + default: null, + }, helpPagePath: { type: String, required: false, @@ -167,12 +172,18 @@ export default { return commentLineOptions(lines, this.commentLineStart, this.line.line_code); }, diffFile() { + let fileResolvedFromAvailableSource; + if (this.commentLineStart.line_code) { const lineCode = this.commentLineStart.line_code.split('_')[0]; - return this.getDiffFileByHash(lineCode); + fileResolvedFromAvailableSource = this.getDiffFileByHash(lineCode); + } + + if (!fileResolvedFromAvailableSource && this.discussionFile) { + fileResolvedFromAvailableSource = this.discussionFile; } - return null; + return fileResolvedFromAvailableSource || null; }, }, created() { diff --git a/spec/frontend/notes/components/noteable_note_spec.js b/spec/frontend/notes/components/noteable_note_spec.js index 112983f3ac2008..7444c441e06785 100644 --- a/spec/frontend/notes/components/noteable_note_spec.js +++ b/spec/frontend/notes/components/noteable_note_spec.js @@ -1,32 +1,65 @@ -import { mount, createLocalVue } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; import { escape } from 'lodash'; +import Vue from 'vue'; +import Vuex from 'vuex'; + import waitForPromises from 'helpers/wait_for_promises'; + +import DiffsModule from '~/diffs/store/modules'; + import NoteActions from '~/notes/components/note_actions.vue'; import NoteBody from '~/notes/components/note_body.vue'; import NoteHeader from '~/notes/components/note_header.vue'; import issueNote from '~/notes/components/noteable_note.vue'; -import createStore from '~/notes/stores'; +import NotesModule from '~/notes/stores/modules'; + import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; + import { noteableDataMock, notesDataMock, note } from '../mock_data'; +Vue.use(Vuex); + +const singleLineNotePosition = { + line_range: { + start: { + line_code: 'abc_1_1', + type: null, + old_line: '1', + new_line: '1', + }, + end: { + line_code: 'abc_1_1', + type: null, + old_line: '1', + new_line: '1', + }, + }, +}; + describe('issue_note', () => { let store; let wrapper; const findMultilineComment = () => wrapper.find('[data-testid="multiline-comment"]'); - const createWrapper = (props = {}) => { - store = createStore(); + const createWrapper = (props = {}, storeUpdater = (s) => s) => { + store = new Vuex.Store( + storeUpdater({ + modules: { + notes: NotesModule(), + diffs: DiffsModule(), + }, + }), + ); + store.dispatch('setNoteableData', noteableDataMock); store.dispatch('setNotesData', notesDataMock); - const localVue = createLocalVue(); - wrapper = mount(localVue.extend(issueNote), { + wrapper = mount(issueNote, { store, propsData: { note, ...props, }, - localVue, stubs: [ 'note-header', 'user-avatar-link', @@ -216,9 +249,13 @@ describe('issue_note', () => { const noteBodyComponent = wrapper.findComponent(NoteBody); store.hotUpdate({ - actions: { - updateNote() {}, - setSelectedCommentPositionHover() {}, + modules: { + notes: { + actions: { + updateNote() {}, + setSelectedCommentPositionHover() {}, + }, + }, }, }); @@ -238,8 +275,12 @@ describe('issue_note', () => { it('restores content of updated note', async () => { const updatedText = 'updated note text'; store.hotUpdate({ - actions: { - updateNote() {}, + modules: { + notes: { + actions: { + updateNote() {}, + }, + }, }, }); const noteBody = wrapper.findComponent(NoteBody); @@ -267,9 +308,13 @@ describe('issue_note', () => { const updateActions = () => { store.hotUpdate({ - actions: { - updateNote, - setSelectedCommentPositionHover() {}, + modules: { + notes: { + actions: { + updateNote, + setSelectedCommentPositionHover() {}, + }, + }, }, }); }; @@ -299,4 +344,62 @@ describe('issue_note', () => { expect(updateNote.mock.calls[0][1].note.note.position).toBe(expectation); }); }); + + describe('diffFile', () => { + it.each` + scenario | files | noteDef + ${'the note has no position'} | ${undefined} | ${note} + ${'the Diffs store has no data'} | ${[]} | ${{ ...note, position: singleLineNotePosition }} + `( + 'returns `null` when $scenario and no diff file is provided as a prop', + ({ noteDef, diffs }) => { + const storeUpdater = (rawStore) => { + const updatedStore = { ...rawStore }; + + if (diffs) { + updatedStore.modules.diffs.state.diffFiles = diffs; + } + + return updatedStore; + }; + + createWrapper({ note: noteDef, discussionFile: null }, storeUpdater); + + expect(wrapper.vm.diffFile).toBe(null); + }, + ); + + it("returns the correct diff file from the Diffs store if it's available", () => { + createWrapper( + { + note: { ...note, position: singleLineNotePosition }, + }, + (rawStore) => { + const updatedStore = { ...rawStore }; + updatedStore.modules.diffs.state.diffFiles = [ + { file_hash: 'abc', testId: 'diffFileTest' }, + ]; + return updatedStore; + }, + ); + + expect(wrapper.vm.diffFile.testId).toBe('diffFileTest'); + }); + + it('returns the provided diff file if the more robust getters fail', () => { + createWrapper( + { + note: { ...note, position: singleLineNotePosition }, + discussionFile: { testId: 'diffFileTest' }, + }, + (rawStore) => { + const updatedStore = { ...rawStore }; + updatedStore.modules.diffs.state.diffFiles = []; + return updatedStore; + }, + ); + + expect(wrapper.vm.diffFile.testId).toBe('diffFileTest'); + }); + }); }); -- GitLab From 3982a8cd353c8631cf3986de3431a7a273f06e89 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 1 Apr 2021 17:58:43 -0600 Subject: [PATCH 2/2] Add changelog for filling the last placeholder variable --- ...ct-overview-placeholder-message-overview-file-source.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelogs/unreleased/tor-defect-overview-placeholder-message-overview-file-source.yml diff --git a/changelogs/unreleased/tor-defect-overview-placeholder-message-overview-file-source.yml b/changelogs/unreleased/tor-defect-overview-placeholder-message-overview-file-source.yml new file mode 100644 index 00000000000000..8ceb2067a4ba0f --- /dev/null +++ b/changelogs/unreleased/tor-defect-overview-placeholder-message-overview-file-source.yml @@ -0,0 +1,6 @@ +--- +title: Fill in all placeholder values in the apply suggestion commit message placeholder + text +merge_request: 58136 +author: +type: other -- GitLab