From 756900228c7a7a7bf08e203ee6e48ce775e1a30d Mon Sep 17 00:00:00 2001 From: Chaoyue Zhao Date: Fri, 7 Mar 2025 23:15:52 -0500 Subject: [PATCH 1/5] Show a list of open MR details in badge popover This MR adds a popover that is being triggered when user hover/click on the MR badge. It fires a graphql query to fetch MR details and render them in the list. This feature is behind `filter_blob_path` feature flag. --- .../header_area/merge_request_list_item.vue | 61 +++++++ .../components/header_area/open_mr_badge.vue | 61 ++++++- .../repository/queries/open_mr.query.graphql | 36 ++++ .../queries/open_mr_counts.query.graphql | 2 +- .../merge_request_list_item_spec.js | 115 ++++++++++++ .../components/header_area/mock_data.js | 39 +++++ .../header_area/open_mr_badge_spec.js | 163 +++++++++++++++--- 7 files changed, 443 insertions(+), 34 deletions(-) create mode 100644 app/assets/javascripts/repository/components/header_area/merge_request_list_item.vue create mode 100644 app/assets/javascripts/repository/queries/open_mr.query.graphql create mode 100644 spec/frontend/repository/components/header_area/merge_request_list_item_spec.js diff --git a/app/assets/javascripts/repository/components/header_area/merge_request_list_item.vue b/app/assets/javascripts/repository/components/header_area/merge_request_list_item.vue new file mode 100644 index 00000000000000..ed3e9fad808290 --- /dev/null +++ b/app/assets/javascripts/repository/components/header_area/merge_request_list_item.vue @@ -0,0 +1,61 @@ + + + diff --git a/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue b/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue index bfadc6f16dbbd7..644e7c3e3eeb42 100644 --- a/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue +++ b/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue @@ -1,17 +1,22 @@ diff --git a/app/assets/javascripts/repository/queries/open_mr.query.graphql b/app/assets/javascripts/repository/queries/open_mr.query.graphql new file mode 100644 index 00000000000000..8797bc8c959f24 --- /dev/null +++ b/app/assets/javascripts/repository/queries/open_mr.query.graphql @@ -0,0 +1,36 @@ +#import "~/graphql_shared/fragments/user.fragment.graphql" + +query getOpenMrsForBlobPath( + $projectPath: ID! + $targetBranch: [String!] + $blobPath: String! + $createdAfter: Time! +) { + project(fullPath: $projectPath) { + id + mergeRequests( + state: opened + targetBranches: $targetBranch + blobPath: $blobPath + createdAfter: $createdAfter + ) { + nodes { + id + iid + title + createdAt + assignees { + nodes { + ...User + } + } + project { + id + fullPath + } + sourceBranch + } + count + } + } +} diff --git a/app/assets/javascripts/repository/queries/open_mr_counts.query.graphql b/app/assets/javascripts/repository/queries/open_mr_counts.query.graphql index 407d3402f63614..953d07577137e3 100644 --- a/app/assets/javascripts/repository/queries/open_mr_counts.query.graphql +++ b/app/assets/javascripts/repository/queries/open_mr_counts.query.graphql @@ -1,4 +1,4 @@ -query getOpenMrCountsForBlobPath( +query getOpenMrCountForBlobPath( $projectPath: ID! $targetBranch: [String!] $blobPath: String! diff --git a/spec/frontend/repository/components/header_area/merge_request_list_item_spec.js b/spec/frontend/repository/components/header_area/merge_request_list_item_spec.js new file mode 100644 index 00000000000000..eecf4a13762fe0 --- /dev/null +++ b/spec/frontend/repository/components/header_area/merge_request_list_item_spec.js @@ -0,0 +1,115 @@ +import { GlBadge, GlIcon } from '@gitlab/ui'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import MergeRequestListItem from '~/repository/components/header_area/merge_request_list_item.vue'; +import { getTimeago } from '~/lib/utils/datetime/timeago_utility'; + +jest.mock('~/lib/utils/datetime/timeago_utility', () => ({ + getTimeago: jest.fn(), +})); + +describe('MergeRequestListItem', () => { + let wrapper; + const mockTimeago = { + format: jest.fn().mockReturnValue('3 days ago'), + }; + + const mockAssignees = [ + { id: 'gid://gitlab/User/1', name: 'User One' }, + { id: 'gid://gitlab/User/2', name: 'User Two' }, + ]; + + const createMergeRequestMock = (overrides = {}) => ({ + id: 'gid://gitlab/MergeRequest/1', + iid: '123', + title: 'Test MR title', + createdAt: '2023-01-01T00:00:00Z', + sourceBranch: 'feature-branch', + project: { + fullPath: 'group/project', + }, + assignees: { + nodes: mockAssignees, + }, + ...overrides, + }); + + const createComponent = (props) => { + wrapper = shallowMountExtended(MergeRequestListItem, { + propsData: { + mergeRequest: createMergeRequestMock(), + ...props, + }, + }); + }; + + const findProjectInfo = () => wrapper.findByTestId('project-info'); + const findAssigneeInfo = () => wrapper.findAllByTestId('assignee-info'); + const findSourceBranchInfo = () => wrapper.findByTestId('source-branch-info'); + + beforeEach(() => { + getTimeago.mockReturnValue(mockTimeago); + createComponent(); + }); + + describe('rendering', () => { + it('renders the open badge with correct text', () => { + const badge = wrapper.findComponent(GlBadge); + expect(badge.exists()).toBe(true); + expect(badge.text()).toContain('Open'); + expect(badge.findComponent(GlIcon).props('name')).toBe('merge-request'); + }); + + it('renders the formatted creation time', () => { + expect(mockTimeago.format).toHaveBeenCalledWith('2023-01-01T00:00:00Z'); + expect(wrapper.find('time').text()).toBe('3 days ago'); + }); + + it('renders the merge request title', () => { + expect(wrapper.findByText('Test MR title').exists()).toBe(true); + }); + + it('renders the project path and MR ID', () => { + const projectInfo = findProjectInfo(); + expect(projectInfo.findComponent(GlIcon).props('name')).toBe('project'); + expect(projectInfo.text()).toContain('group/project !123'); + }); + + it('renders the source branch', () => { + const branchInfo = findSourceBranchInfo(); + expect(branchInfo.findComponent(GlIcon).props('name')).toBe('branch'); + expect(branchInfo.text()).toContain('feature-branch'); + }); + + it('does not add border class when showBorder is true', () => { + createComponent({ showBorder: true }); + + expect(wrapper.classes('gl-border-b')).toBe(false); + }); + + it('adds border class when showBorder is false (default)', () => { + expect(wrapper.classes('gl-border-b')).toBe(true); + }); + }); + + describe('assignees', () => { + it('renders all assignees', () => { + const assigneeInfos = findAssigneeInfo(); + expect(assigneeInfos.length).toBe(2); + + mockAssignees.forEach((mockUser, index) => { + const assigneeText = assigneeInfos.at(index).text().trim(); + expect(assigneeText).toContain(mockUser.name); + }); + }); + + it('handles merge requests with no assignees', () => { + const mrWithNoAssignees = createMergeRequestMock({ + assignees: { nodes: [] }, + }); + + createComponent({ mergeRequest: mrWithNoAssignees }); + + expect(findAssigneeInfo().length).toBe(0); + }); + }); +}); diff --git a/spec/frontend/repository/components/header_area/mock_data.js b/spec/frontend/repository/components/header_area/mock_data.js index 724e4130396a54..abbe67d2e8de74 100644 --- a/spec/frontend/repository/components/header_area/mock_data.js +++ b/spec/frontend/repository/components/header_area/mock_data.js @@ -21,3 +21,42 @@ export const zeroOpenMRQueryResult = jest.fn().mockResolvedValue({ }, }, }); + +const mockMergeRequests = [ + { + id: '111', + iid: '123', + title: 'MR 1', + createdAt: '2020-07-07T00:00:00Z', + assignees: { nodes: [{ name: 'root' }] }, + project: { + id: '1', + fullPath: 'full/path/to/project', + }, + sourceBranch: 'main', + }, + { + id: '222', + iid: '456', + title: 'MR 2', + createdAt: '2020-07-09T00:00:00Z', + assignees: { nodes: [{ name: 'homer' }] }, + project: { + id: '1', + fullPath: 'full/path/to/project', + }, + sourceBranch: 'main', + }, +]; + +export const openMRsDetailResult = jest.fn().mockResolvedValue({ + data: { + project: { + id: '1', + mergeRequests: { + nodes: mockMergeRequests, + count: 2, + }, + }, + }, +}); diff --git a/spec/frontend/repository/components/header_area/open_mr_badge_spec.js b/spec/frontend/repository/components/header_area/open_mr_badge_spec.js index 88e8b57ceb147f..2958ef63b9f605 100644 --- a/spec/frontend/repository/components/header_area/open_mr_badge_spec.js +++ b/spec/frontend/repository/components/header_area/open_mr_badge_spec.js @@ -1,15 +1,17 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import { shallowMount } from '@vue/test-utils'; -import { GlBadge } from '@gitlab/ui'; +import { GlBadge, GlPopover, GlSkeletonLoader } from '@gitlab/ui'; import OpenMrBadge from '~/repository/components/header_area/open_mr_badge.vue'; import getOpenMrCountsForBlobPath from '~/repository/queries/open_mr_counts.query.graphql'; +import getOpenMrsForBlobPath from '~/repository/queries/open_mr.query.graphql'; +import MergeRequestListItem from '~/repository/components/header_area/merge_request_list_item.vue'; import { logError } from '~/lib/logger'; import waitForPromises from 'helpers/wait_for_promises'; import createMockApollo from 'helpers/mock_apollo_helper'; import { useFakeDate } from 'helpers/fake_date'; import * as Sentry from '~/sentry/sentry_browser_wrapper'; -import { openMRQueryResult, zeroOpenMRQueryResult } from './mock_data'; +import { openMRQueryResult, zeroOpenMRQueryResult, openMRsDetailResult } from './mock_data'; Vue.use(VueApollo); jest.mock('~/lib/logger'); @@ -17,16 +19,26 @@ jest.mock('~/sentry/sentry_browser_wrapper'); describe('OpenMrBadge', () => { let wrapper; - let requestHandler; + let openMrsCountQueryHandler; + let openMrsQueryHandler; const defaultProps = { projectPath: 'group/project', blobPath: 'path/to/file.js', }; - function createComponent(props = {}, mockResolver = openMRQueryResult) { - requestHandler = mockResolver; - const mockApollo = createMockApollo([[getOpenMrCountsForBlobPath, mockResolver]]); + function createComponent( + props = {}, + mockResolver = openMRQueryResult, + mrDetailResolver = openMRsDetailResult, + ) { + openMrsCountQueryHandler = mockResolver; + openMrsQueryHandler = mrDetailResolver; + + const mockApollo = createMockApollo([ + [getOpenMrCountsForBlobPath, mockResolver], + [getOpenMrsForBlobPath, mrDetailResolver], + ]); wrapper = shallowMount(OpenMrBadge, { propsData: { @@ -40,6 +52,10 @@ describe('OpenMrBadge', () => { }); } + const findPopover = () => wrapper.findComponent(GlPopover); + const findAllMergeRequestItems = () => wrapper.findAllComponents(MergeRequestListItem); + const findLoader = () => wrapper.findComponent(GlSkeletonLoader); + describe('rendering', () => { it('does not render badge when query is loading', () => { createComponent(); @@ -52,17 +68,6 @@ describe('OpenMrBadge', () => { expect(wrapper.findComponent(GlBadge).exists()).toBe(false); }); - - it('renders badge when when there are open MRs', async () => { - createComponent(); - await waitForPromises(); - - const badge = wrapper.findComponent(GlBadge); - expect(badge.exists()).toBe(true); - expect(badge.props('variant')).toBe('success'); - expect(badge.props('icon')).toBe('merge-request'); - expect(wrapper.text()).toBe('3 open'); - }); }); describe('computed properties', () => { @@ -73,7 +78,7 @@ describe('OpenMrBadge', () => { }); it('computes queryVariables correctly', () => { - expect(requestHandler).toHaveBeenCalledWith({ + expect(openMrsCountQueryHandler).toHaveBeenCalledWith({ blobPath: 'path/to/file.js', createdAfter: '2020-06-07', projectPath: 'group/project', @@ -83,17 +88,121 @@ describe('OpenMrBadge', () => { }); describe('apollo query', () => { - it('handles apollo error correctly', async () => { - const mockError = new Error(); - createComponent({}, jest.fn().mockRejectedValueOnce(mockError)); + describe('fetchOpenMrCount', () => { + it('fetch mr count and render badge correctly', async () => { + createComponent(); + await waitForPromises(); + + const badge = wrapper.findComponent(GlBadge); + expect(badge.exists()).toBe(true); + expect(badge.props('variant')).toBe('success'); + expect(badge.props('icon')).toBe('merge-request'); + expect(wrapper.text()).toBe('3 open'); + }); + + it('handles errors when fetching MR count', async () => { + const mockError = new Error(); + createComponent({}, jest.fn().mockRejectedValueOnce(mockError)); + await waitForPromises(); + + expect(wrapper.findComponent(GlBadge).exists()).toBe(false); + expect(logError).toHaveBeenCalledWith( + 'Failed to fetch merge request count. See exception details for more information.', + mockError, + ); + expect(Sentry.captureException).toHaveBeenCalledWith(mockError); + }); + }); + + describe('fetchOpenMrs', () => { + it('fetches MRs and updates data', async () => { + createComponent(); + findPopover().vm.$emit('show'); + await waitForPromises(); + + expect(openMrsQueryHandler).toHaveBeenCalledWith({ + blobPath: 'path/to/file.js', + createdAfter: '2020-06-07', + projectPath: 'group/project', + targetBranch: ['main'], + }); + + expect(findAllMergeRequestItems().length).toEqual(2); + expect(findLoader().exists()).toBe(false); + }); + + it('handles errors when fetching MRs', async () => { + const mockError = new Error('Failed to fetch MRs'); + const errorResolver = jest.fn().mockRejectedValue(mockError); + + createComponent({}, openMRQueryResult, errorResolver); + await waitForPromises(); + + findPopover().vm.$emit('show'); + await waitForPromises(); + + expect(logError).toHaveBeenCalledWith( + 'Failed to fetch merge requests. See exception details for more information.', + mockError, + ); + expect(Sentry.captureException).toHaveBeenCalledWith(mockError); + }); + }); + }); + + describe('popover functionality', () => { + beforeEach(() => { + createComponent(); + }); + + it('sets correct props on the popover', () => { + expect(findPopover().props()).toMatchObject({ + target: 'open-mr-badge', + boundary: 'viewport', + placement: 'bottomleft', + }); + }); + + it('shows skeleton loader when loading MRs', () => { + expect(findLoader().exists()).toBe(true); + }); + + it('calls fetchOpenMrs when popover is shown', async () => { await waitForPromises(); - expect(wrapper.findComponent(GlBadge).exists()).toBe(false); - expect(logError).toHaveBeenCalledWith( - 'Failed to fetch merge request count. See exception details for more information.', - mockError, - ); - expect(Sentry.captureException).toHaveBeenCalledWith(mockError); + findPopover().vm.$emit('show'); + + expect(openMrsQueryHandler).toHaveBeenCalled(); + }); + + it('passes correct showBorder prop based on item position', async () => { + await waitForPromises(); + findPopover().vm.$emit('show'); + await waitForPromises(); + + const items = findAllMergeRequestItems(); + expect(items.length).toBe(2); + + expect(items.at(0).props('showBorder')).toBe(false); + + expect(items.at(1).props('showBorder')).toBe(true); + }); + }); + + describe('isLastItem method', () => { + beforeEach(async () => { + createComponent(); + await waitForPromises(); + findPopover().vm.$emit('show'); + await waitForPromises(); + }); + + it('returns false for non-last items', () => { + expect(wrapper.vm.isLastItem(0)).toBe(false); + }); + + it('returns true for the last item', () => { + expect(wrapper.vm.isLastItem(1)).toBe(true); }); }); }); -- GitLab From 259b3d0185485c8da5416eb375ea800720ce1ba6 Mon Sep 17 00:00:00 2001 From: Chaoyue Zhao Date: Mon, 10 Mar 2025 15:15:26 -0400 Subject: [PATCH 2/5] Add namespace for translation strings --- .../header_area/merge_request_list_item.vue | 6 ++++-- .../components/header_area/open_mr_badge.vue | 4 ++-- locale/gitlab.pot | 12 +++++++++--- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/repository/components/header_area/merge_request_list_item.vue b/app/assets/javascripts/repository/components/header_area/merge_request_list_item.vue index ed3e9fad808290..1380cee3cb3a9b 100644 --- a/app/assets/javascripts/repository/components/header_area/merge_request_list_item.vue +++ b/app/assets/javascripts/repository/components/header_area/merge_request_list_item.vue @@ -33,9 +33,11 @@ export default {
- {{ __('Open') }} + {{ s__('OpenMrBadge|Open') }} - {{ __('Opened') }} + + {{ s__('OpenMrBadge|Opened') }}
{{ mergeRequest.title }}
diff --git a/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue b/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue index 644e7c3e3eeb42..2efcf4f1a2883f 100644 --- a/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue +++ b/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue @@ -1,6 +1,6 @@ diff --git a/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue b/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue index b14752500641ab..06a9d4549c82be 100644 --- a/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue +++ b/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue @@ -33,7 +33,7 @@ export default { return { openMrsCount: null, openMrs: [], - isLoadingMrs: false, + isPopoverOpen: false, }; }, computed: { @@ -76,32 +76,22 @@ export default { Sentry.captureException(error); }, }, - }, - methods: { - async fetchOpenMrs() { - this.isLoadingMrs = true; - try { - const { data } = await this.$apollo.query({ - query: getOpenMrsForBlobPath, - variables: this.queryVariables, - }); - - this.openMrs = data?.project?.mergeRequests?.nodes || []; - } catch (error) { + openMrs: { + query: getOpenMrsForBlobPath, + variables() { + return this.queryVariables; + }, + skip() { + return !this.isPopoverOpen; + }, + update: (data) => data?.project?.mergeRequests?.nodes || [], + error(error) { logError( `Failed to fetch merge requests. See exception details for more information.`, error, ); Sentry.captureException(error); - } finally { - this.isLoadingMrs = false; - } - }, - onShowPopover() { - this.fetchOpenMrs(); - }, - isLastItem(index) { - return index === this.openMrs.length - 1; + }, }, }, }; @@ -116,14 +106,22 @@ export default { target="open-mr-badge" boundary="viewport" placement="bottomleft" - @show="onShowPopover" + @show="isPopoverOpen = true" + @hide="isPopoverOpen = false" > - + -
- -
+
    +
  • + +
    • +
diff --git a/spec/frontend/repository/components/header_area/merge_request_list_item_spec.js b/spec/frontend/repository/components/header_area/merge_request_list_item_spec.js index eecf4a13762fe0..78e0926dc6b598 100644 --- a/spec/frontend/repository/components/header_area/merge_request_list_item_spec.js +++ b/spec/frontend/repository/components/header_area/merge_request_list_item_spec.js @@ -79,16 +79,6 @@ describe('MergeRequestListItem', () => { expect(branchInfo.findComponent(GlIcon).props('name')).toBe('branch'); expect(branchInfo.text()).toContain('feature-branch'); }); - - it('does not add border class when showBorder is true', () => { - createComponent({ showBorder: true }); - - expect(wrapper.classes('gl-border-b')).toBe(false); - }); - - it('adds border class when showBorder is false (default)', () => { - expect(wrapper.classes('gl-border-b')).toBe(true); - }); }); describe('assignees', () => { diff --git a/spec/frontend/repository/components/header_area/open_mr_badge_spec.js b/spec/frontend/repository/components/header_area/open_mr_badge_spec.js index 7a2771c79b1f28..f2eb713546c153 100644 --- a/spec/frontend/repository/components/header_area/open_mr_badge_spec.js +++ b/spec/frontend/repository/components/header_area/open_mr_badge_spec.js @@ -1,4 +1,4 @@ -import Vue from 'vue'; +import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import { shallowMount } from '@vue/test-utils'; import { GlBadge, GlPopover, GlSkeletonLoader } from '@gitlab/ui'; @@ -169,40 +169,10 @@ describe('OpenMrBadge', () => { it('calls fetchOpenMrs when popover is shown', async () => { await waitForPromises(); - findPopover().vm.$emit('show'); + await nextTick(); expect(openMrsQueryHandler).toHaveBeenCalled(); }); - - it('passes correct showBorder prop based on item position', async () => { - await waitForPromises(); - findPopover().vm.$emit('show'); - await waitForPromises(); - - const items = findAllMergeRequestItems(); - expect(items.length).toBe(2); - - expect(items.at(0).props('showBorder')).toBe(false); - - expect(items.at(1).props('showBorder')).toBe(true); - }); - }); - - describe('isLastItem method', () => { - beforeEach(async () => { - createComponent(); - await waitForPromises(); - findPopover().vm.$emit('show'); - await waitForPromises(); - }); - - it('returns false for non-last items', () => { - expect(wrapper.vm.isLastItem(0)).toBe(false); - }); - - it('returns true for the last item', () => { - expect(wrapper.vm.isLastItem(1)).toBe(true); - }); }); }); -- GitLab From 77de88390e90ebd83060b440e9c86c27d30a3cd4 Mon Sep 17 00:00:00 2001 From: Chaoyue Zhao Date: Fri, 14 Mar 2025 10:34:45 -0400 Subject: [PATCH 5/5] Update to fetch MR badge only the first time --- .../repository/components/header_area/open_mr_badge.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue b/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue index 06a9d4549c82be..0929377e7e985a 100644 --- a/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue +++ b/app/assets/javascripts/repository/components/header_area/open_mr_badge.vue @@ -106,8 +106,8 @@ export default { target="open-mr-badge" boundary="viewport" placement="bottomleft" - @show="isPopoverOpen = true" - @hide="isPopoverOpen = false" + @show.once="isPopoverOpen = true" + @hide.once="isPopoverOpen = false" > -- GitLab