Date: Tue, 11 Feb 2025 16:05:35 -0700 Subject: [PATCH 2/6] Test that overview discussions link to the file directly --- .../notes/components/diff_with_note_spec.js | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/spec/frontend/notes/components/diff_with_note_spec.js b/spec/frontend/notes/components/diff_with_note_spec.js index 8a4708ad9b0ff6..2ee169d982aaf3 100644 --- a/spec/frontend/notes/components/diff_with_note_spec.js +++ b/spec/frontend/notes/components/diff_with_note_spec.js @@ -184,4 +184,48 @@ describe('diff_with_note', () => { expect(findDiffViewer().props('newSha')).toBe(mockCommitId); }); }); + + describe('diff header', () => { + let fileDiscussion; + + beforeEach(() => { + fileDiscussion = JSON.parse(JSON.stringify(discussionFixture[0])); + fileDiscussion.position.position_type = 'file'; + fileDiscussion.original_position.position_type = 'file'; + }); + + describe('when the discussion has a diff_file', () => { + beforeEach(() => { + wrapper = shallowMount(DiffWithNote, { + propsData: { discussion: fileDiscussion, diffFile: {} }, + store, + }); + }); + + it('links directly to the file to take advantage of the prioritized Linked File feature', () => { + const header = findDiffFileHeader(); + + expect(header.attributes('discussionpath')).toContain( + `file=${fileDiscussion.diff_file.file_hash}`, + ); + }); + }); + + describe('when the discussion does not have a diff_file', () => { + beforeEach(() => { + delete fileDiscussion.diff_file; + + wrapper = shallowMount(DiffWithNote, { + propsData: { discussion: fileDiscussion, diffFile: {} }, + store, + }); + }); + + it('does not include the `file` search parameter in the file link', () => { + const header = findDiffFileHeader(); + + expect(header.attributes('discussionpath')).not.toContain('file='); + }); + }); + }); }); -- GitLab From bc0eb0676b1cc57eee5c308ef4f273aa5068f305 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 13 Feb 2025 13:29:23 -0700 Subject: [PATCH 3/6] Use `~` instead of `../..` From review --- app/assets/javascripts/notes/components/diff_with_note.vue | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue index 9ed2d5da60cfef..72babe9486c9b1 100644 --- a/app/assets/javascripts/notes/components/diff_with_note.vue +++ b/app/assets/javascripts/notes/components/diff_with_note.vue @@ -3,6 +3,7 @@ import { GlButton, GlSkeletonLoader } from '@gitlab/ui'; // eslint-disable-next-line no-restricted-imports import { mapActions } from 'vuex'; import { mapState } from 'pinia'; +import { mergeUrlParams } from '~/lib/utils/url_utility'; import SafeHtml from '~/vue_shared/directives/safe_html'; import DiffFileHeader from '~/diffs/components/diff_file_header.vue'; import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue'; @@ -13,8 +14,6 @@ import { isCollapsed } from '~/diffs/utils/diff_file'; import { FILE_DIFF_POSITION_TYPE, IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants'; import { useLegacyDiffs } from '~/diffs/stores/legacy_diffs'; -import { mergeUrlParams } from '../../lib/utils/url_utility'; - const FIRST_CHAR_REGEX = /^(\+|-| )/; export default { -- GitLab From 854d64a931b390459ef43657b751b8ebbc616e70 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 12 Jun 2025 12:05:02 -0600 Subject: [PATCH 4/6] Switch stringify/parse to _.cloneDeep --- spec/frontend/notes/components/diff_with_note_spec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/frontend/notes/components/diff_with_note_spec.js b/spec/frontend/notes/components/diff_with_note_spec.js index 2ee169d982aaf3..2b0e7f1b3140a5 100644 --- a/spec/frontend/notes/components/diff_with_note_spec.js +++ b/spec/frontend/notes/components/diff_with_note_spec.js @@ -1,3 +1,4 @@ +import { cloneDeep } from "lodash"; import Vue from 'vue'; import { shallowMount } from '@vue/test-utils'; import { PiniaVuePlugin } from 'pinia'; @@ -105,7 +106,7 @@ describe('diff_with_note', () => { describe('when discussion does not have a diff_file', () => { beforeEach(() => { - const imageDiscussion = JSON.parse(JSON.stringify(imageDiscussionFixture[0])); + const imageDiscussion = cloneDeep(imageDiscussionFixture[0]); delete imageDiscussion.diff_file; createComponent({ discussion: imageDiscussion, diffFile: {} }); -- GitLab From f1e6b2fb91953ac7735ac828fdca4b6d1ab96d2f Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 12 Jun 2025 12:07:59 -0600 Subject: [PATCH 5/6] Use ternary return for conciseness --- .../notes/components/diff_with_note.vue | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue index 72babe9486c9b1..3e4b11d810efe5 100644 --- a/app/assets/javascripts/notes/components/diff_with_note.vue +++ b/app/assets/javascripts/notes/components/diff_with_note.vue @@ -80,18 +80,12 @@ export default { return this.positionType === FILE_DIFF_POSITION_TYPE; }, linkedFileDiscussionPath() { - let path = this.discussion.discussion_path; + const { discussion_path: discussionPath } = this.discussion; + const file = this.discussion.diff_file?.file_hash; - if (path && this.discussion.diff_file?.file_hash) { - path = mergeUrlParams( - { - file: this.discussion.diff_file.file_hash, - }, - path, - ); - } - - return path; + return discussionPath && file + ? mergeUrlParams({ file }, discussionPath) + : discussionPath; }, showHeader() { if (this.discussion.diff_file) return true; -- GitLab From 9136affa330bb51d7ca4b135d103d97cb01ca81d Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Thu, 12 Jun 2025 12:14:03 -0600 Subject: [PATCH 6/6] "Prettify" the latest updates --- app/assets/javascripts/notes/components/diff_with_note.vue | 4 +--- spec/frontend/notes/components/diff_with_note_spec.js | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue index 3e4b11d810efe5..1df5ee04b37ab9 100644 --- a/app/assets/javascripts/notes/components/diff_with_note.vue +++ b/app/assets/javascripts/notes/components/diff_with_note.vue @@ -83,9 +83,7 @@ export default { const { discussion_path: discussionPath } = this.discussion; const file = this.discussion.diff_file?.file_hash; - return discussionPath && file - ? mergeUrlParams({ file }, discussionPath) - : discussionPath; + return discussionPath && file ? mergeUrlParams({ file }, discussionPath) : discussionPath; }, showHeader() { if (this.discussion.diff_file) return true; diff --git a/spec/frontend/notes/components/diff_with_note_spec.js b/spec/frontend/notes/components/diff_with_note_spec.js index 2b0e7f1b3140a5..03bb35bd5548ed 100644 --- a/spec/frontend/notes/components/diff_with_note_spec.js +++ b/spec/frontend/notes/components/diff_with_note_spec.js @@ -1,4 +1,4 @@ -import { cloneDeep } from "lodash"; +import { cloneDeep } from 'lodash'; import Vue from 'vue'; import { shallowMount } from '@vue/test-utils'; import { PiniaVuePlugin } from 'pinia'; -- GitLab