From 6420f5d56bf6c16123e7b0b339468ea597d08a7a Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Mon, 25 Aug 2025 22:06:16 -0600 Subject: [PATCH 1/5] Provide the correct line range when opening the comment editor Changelog: fixed --- .../javascripts/notes/components/discussion_notes.vue | 1 + app/assets/javascripts/notes/components/note_body.vue | 6 ++++++ app/assets/javascripts/notes/components/noteable_note.vue | 5 +++++ 3 files changed, 12 insertions(+) diff --git a/app/assets/javascripts/notes/components/discussion_notes.vue b/app/assets/javascripts/notes/components/discussion_notes.vue index 78953a62733011..f626d689b3d7cb 100644 --- a/app/assets/javascripts/notes/components/discussion_notes.vue +++ b/app/assets/javascripts/notes/components/discussion_notes.vue @@ -137,6 +137,7 @@ export default { :is="componentName(firstNote)" :note="componentData(firstNote)" :line="line || diffLine" + :discussion="discussion" :discussion-file="discussion.diff_file" :commit="commit" :help-page-path="helpPagePath" diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue index 1bf4fec6cebb87..100bd2fcc20685 100644 --- a/app/assets/javascripts/notes/components/note_body.vue +++ b/app/assets/javascripts/notes/components/note_body.vue @@ -36,6 +36,11 @@ export default { required: false, default: null, }, + lines: { + type: Array, + required: false, + default: () => [], + }, file: { type: Object, required: false, @@ -234,6 +239,7 @@ export default { :note-body="noteBody" :note-id="note.id" :line="line" + :lines="lines" :note="note" :diff-file="file" :save-button-title="saveButtonTitle" diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 367e3fbc743b40..da7ff174b67035 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -182,6 +182,9 @@ export default { } return ''; }, + commentLines() { + return this.getLinesForDiscussion({ discussion: this.discussion }); + }, actionText() { if (!this.commit) { return ''; @@ -281,6 +284,7 @@ export default { 'updateAssignees', 'setSelectedCommentPositionHover', ]), + ...mapActions(useLegacyDiffs, ['getLinesForDiscussion']), editHandler() { this.isEditing = true; this.setSelectedCommentPositionHover(); @@ -539,6 +543,7 @@ export default { :note="note" :can-edit="canEdit" :line="line" + :lines="commentLines" :file="diffFile" :is-editing="isEditing" :autosave-key="autosaveKey" -- GitLab From 2f97a34f6fbe1c92107fb52f7d546c34f9228863 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 27 Aug 2025 20:15:58 -0600 Subject: [PATCH 2/5] Move pinia creation to a function so it can be controlled externally --- .../notes/components/noteable_note_spec.js | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/spec/frontend/notes/components/noteable_note_spec.js b/spec/frontend/notes/components/noteable_note_spec.js index 65c234bcddc442..1dd0855f9cf62e 100644 --- a/spec/frontend/notes/components/noteable_note_spec.js +++ b/spec/frontend/notes/components/noteable_note_spec.js @@ -46,8 +46,19 @@ const singleLineNotePosition = { }, }; +function createPinia({ stubActions = true } = {}) { + const pinia = createTestingPinia({ stubActions, plugins: [globalAccessorPlugin] }); + const diffsStore = useLegacyDiffs(); + useNotes().noteableData = noteableDataMock; + useNotes().notesData = notesDataMock; + useNotes().updateNote.mockResolvedValue(); + + return { pinia, diffsStore }; +} + describe('issue_note', () => { let pinia; + let diffsStore; let wrapper; const REPORT_ABUSE_PATH = '/abuse_reports/add_category'; @@ -80,11 +91,7 @@ describe('issue_note', () => { }; beforeEach(() => { - pinia = createTestingPinia({ plugins: [globalAccessorPlugin] }); - useLegacyDiffs(); - useNotes().noteableData = noteableDataMock; - useNotes().notesData = notesDataMock; - useNotes().updateNote.mockResolvedValue(); + ({ pinia, diffsStore } = createPinia()); }); describe('mutiline comments', () => { -- GitLab From bc075ee8113c39db60b22fdd471104b9475d2aad Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 27 Aug 2025 20:31:34 -0600 Subject: [PATCH 3/5] Test that noteable_note is providing the correct lines to note_body --- .../notes/components/noteable_note_spec.js | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/spec/frontend/notes/components/noteable_note_spec.js b/spec/frontend/notes/components/noteable_note_spec.js index 1dd0855f9cf62e..bfffc4a63777b9 100644 --- a/spec/frontend/notes/components/noteable_note_spec.js +++ b/spec/frontend/notes/components/noteable_note_spec.js @@ -5,6 +5,7 @@ import { createTestingPinia } from '@pinia/testing'; import { PiniaVuePlugin } from 'pinia'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; +import { getDiffFileMock } from 'jest/diffs/mock_data/diff_file'; import NoteActions from '~/notes/components/note_actions.vue'; import NoteBody from '~/notes/components/note_body.vue'; import NoteHeader from '~/notes/components/note_header.vue'; @@ -20,7 +21,7 @@ import { useMockInternalEventsTracking } from 'helpers/tracking_internal_events_ import { globalAccessorPlugin } from '~/pinia/plugins'; import { useLegacyDiffs } from '~/diffs/stores/legacy_diffs'; import { useNotes } from '~/notes/store/legacy_notes'; -import { noteableDataMock, notesDataMock, note } from '../mock_data'; +import { noteableDataMock, notesDataMock, discussionMock, note } from '../mock_data'; Vue.use(PiniaVuePlugin); @@ -201,6 +202,48 @@ describe('issue_note', () => { it('should not render if `line_range` is unavailable', () => { expect(findMultilineComment().exists()).toBe(false); }); + + describe('multi-line line range', () => { + let discussion; + let startCode; + let endCode; + + beforeEach(() => { + ({ pinia, diffsStore } = createPinia({ stubActions: false })); + + const file = getDiffFileMock(); + diffsStore.diffFiles = [file]; + + startCode = file.highlighted_diff_lines[6].line_code; + endCode = file.highlighted_diff_lines[7].line_code; + + discussion = { + ...discussionMock, + diff_file: file, + position: { + line_range: { + start: { + line_code: startCode, + }, + end: { + line_code: endCode, + }, + }, + }, + }; + + createWrapper({ discussion }); + }); + + it('passes', () => { + const body = findNoteBody(); + + expect(body.props('lines')).toEqual([ + expect.objectContaining({ line_code: startCode }), + expect.objectContaining({ line_code: endCode }), + ]); + }); + }); }); describe('rendering', () => { -- GitLab From 7d40264b2653fd375f6396347a97eede39679073 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 28 Aug 2025 21:26:11 -0600 Subject: [PATCH 4/5] Test that note_body is providing the correct line range to the form --- .../frontend/notes/components/note_body_spec.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/spec/frontend/notes/components/note_body_spec.js b/spec/frontend/notes/components/note_body_spec.js index 9be25fc4a87e82..b6f90b3a5e0bd4 100644 --- a/spec/frontend/notes/components/note_body_spec.js +++ b/spec/frontend/notes/components/note_body_spec.js @@ -87,6 +87,23 @@ describe('issue_note_body component', () => { expect(wrapper.findComponent(NoteForm).props('saveButtonTitle')).toBe(buttonText); }); + it.each` + type | value + ${'simple'} | ${[]} + ${'complex'} | ${[{ line_code: 'a' }, { line_code: 'b' }]} + `('provides the correct lines to the note form ($type)', ({ value }) => { + createComponent({ + isEditing: true, + autosaveKey, + restoreFromAutosave: true, + lines: value, + }); + + const form = wrapper.findComponent(NoteForm); + + expect(form.props('lines')).toEqual(value); + }); + describe('isInternalNote', () => { beforeEach(() => { wrapper.setProps({ isInternalNote: true }); -- GitLab From f520184145299346f3e6df0b9c417b404658d739 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 2 Sep 2025 12:22:29 -0600 Subject: [PATCH 5/5] Update with review notes --- .../notes/components/noteable_note_spec.js | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/spec/frontend/notes/components/noteable_note_spec.js b/spec/frontend/notes/components/noteable_note_spec.js index bfffc4a63777b9..9ec74adfe37cb6 100644 --- a/spec/frontend/notes/components/noteable_note_spec.js +++ b/spec/frontend/notes/components/noteable_note_spec.js @@ -47,16 +47,6 @@ const singleLineNotePosition = { }, }; -function createPinia({ stubActions = true } = {}) { - const pinia = createTestingPinia({ stubActions, plugins: [globalAccessorPlugin] }); - const diffsStore = useLegacyDiffs(); - useNotes().noteableData = noteableDataMock; - useNotes().notesData = notesDataMock; - useNotes().updateNote.mockResolvedValue(); - - return { pinia, diffsStore }; -} - describe('issue_note', () => { let pinia; let diffsStore; @@ -91,8 +81,16 @@ describe('issue_note', () => { }); }; + const createPinia = ({ stubActions = true } = {}) => { + pinia = createTestingPinia({ stubActions, plugins: [globalAccessorPlugin] }); + diffsStore = useLegacyDiffs(); + useNotes().noteableData = noteableDataMock; + useNotes().notesData = notesDataMock; + useNotes().updateNote.mockResolvedValue(); + }; + beforeEach(() => { - ({ pinia, diffsStore } = createPinia()); + createPinia(); }); describe('mutiline comments', () => { @@ -209,7 +207,7 @@ describe('issue_note', () => { let endCode; beforeEach(() => { - ({ pinia, diffsStore } = createPinia({ stubActions: false })); + createPinia({ stubActions: false }); const file = getDiffFileMock(); diffsStore.diffFiles = [file]; @@ -235,7 +233,7 @@ describe('issue_note', () => { createWrapper({ discussion }); }); - it('passes', () => { + it('passes the correct lines to the note body', () => { const body = findNoteBody(); expect(body.props('lines')).toEqual([ -- GitLab