From 5469d92ee913391156f87e36c20ff3568513943a Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Tue, 13 Aug 2024 00:02:02 +1000 Subject: [PATCH 1/3] Replace work_item_iid query param with show Changelog: changed --- .../components/related_issuable_item.vue | 9 +++++---- .../components/work_item_detail.vue | 11 ++++++---- .../work_item_links/work_item_links.vue | 20 +++++++++++++------ .../javascripts/work_items/constants.js | 2 ++ .../components/related_issuable_item_spec.js | 6 +++--- .../work_item_links/work_item_links_spec.js | 9 +++++++++ spec/frontend/work_items/mock_data.js | 2 +- 7 files changed, 41 insertions(+), 18 deletions(-) diff --git a/app/assets/javascripts/issuable/components/related_issuable_item.vue b/app/assets/javascripts/issuable/components/related_issuable_item.vue index 4eacad6c435124..027c31081207bb 100644 --- a/app/assets/javascripts/issuable/components/related_issuable_item.vue +++ b/app/assets/javascripts/issuable/components/related_issuable_item.vue @@ -10,6 +10,7 @@ import { setUrlParams, updateHistory } from '~/lib/utils/url_utility'; import { sprintf } from '~/locale'; import CiIcon from '~/vue_shared/components/ci_icon/ci_icon.vue'; import WorkItemDetailModal from '~/work_items/components/work_item_detail_modal.vue'; +import { DETAIL_VIEW_QUERY_PARAM_NAME } from '~/work_items/constants'; import AbuseCategorySelector from '~/abuse_reports/components/abuse_category_selector.vue'; import relatedIssuableMixin from '../mixins/related_issuable_mixin'; import IssueAssignees from './issue_assignees.vue'; @@ -97,15 +98,15 @@ export default { } event.preventDefault(); this.$refs.modal.show(); - this.updateWorkItemIidUrlQuery(this.iid); + this.updateQueryParam(this.idKey); } }, handleWorkItemDeleted(workItemId) { this.$emit('relatedIssueRemoveRequest', workItemId); }, - updateWorkItemIidUrlQuery(iid) { + updateQueryParam(id) { updateHistory({ - url: setUrlParams({ work_item_iid: iid }), + url: setUrlParams({ [DETAIL_VIEW_QUERY_PARAM_NAME]: id }), replace: true, }); }, @@ -251,7 +252,7 @@ export default { ref="modal" :work-item-id="workItemId" :work-item-iid="workItemIid" - @close="updateWorkItemIidUrlQuery" + @close="updateQueryParam" @workItemDeleted="handleWorkItemDeleted" @openReportAbuse="openReportAbuseDrawer" /> diff --git a/app/assets/javascripts/work_items/components/work_item_detail.vue b/app/assets/javascripts/work_items/components/work_item_detail.vue index b6618265ee8020..ecb4e3af404e08 100644 --- a/app/assets/javascripts/work_items/components/work_item_detail.vue +++ b/app/assets/javascripts/work_items/components/work_item_detail.vue @@ -11,6 +11,7 @@ import { isLoggedIn } from '~/lib/utils/common_utils'; import { WORKSPACE_PROJECT } from '~/issues/constants'; import { i18n, + DETAIL_VIEW_QUERY_PARAM_NAME, WIDGET_TYPE_ASSIGNEES, WIDGET_TYPE_NOTIFICATIONS, WIDGET_TYPE_CURRENT_USER_TODOS, @@ -111,7 +112,7 @@ export default { updateError: undefined, workItem: {}, updateInProgress: false, - modalWorkItemId: undefined, + modalWorkItemId: getParameterByName(DETAIL_VIEW_QUERY_PARAM_NAME), modalWorkItemIid: getParameterByName('work_item_iid'), modalWorkItemNamespaceFullPath: '', isReportModalOpen: false, @@ -339,10 +340,10 @@ export default { }, }, mounted() { - if (this.modalWorkItemIid) { + if (this.modalWorkItemId) { this.openInModal({ event: undefined, - modalWorkItem: { iid: this.modalWorkItemIid }, + modalWorkItem: { id: this.modalWorkItemId }, }); } }, @@ -397,7 +398,9 @@ export default { }, updateUrl(modalWorkItem) { updateHistory({ - url: setUrlParams({ work_item_iid: modalWorkItem?.iid }), + url: setUrlParams({ + [DETAIL_VIEW_QUERY_PARAM_NAME]: modalWorkItem?.id, + }), replace: true, }); }, diff --git a/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue index 5f27743c27c607..604cd7a3e11ea6 100644 --- a/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue +++ b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue @@ -22,6 +22,7 @@ import { I18N_WORK_ITEM_SHOW_LABELS, TASKS_ANCHOR, DEFAULT_PAGE_SIZE_CHILD_ITEMS, + DETAIL_VIEW_QUERY_PARAM_NAME, } from '../../constants'; import { findHierarchyWidgets } from '../../utils'; import { removeHierarchyChild } from '../../graphql/cache_utils'; @@ -82,13 +83,17 @@ export default { }, async result() { const iid = getParameterByName('work_item_iid'); - this.activeChild = this.children.find((child) => child.iid === iid) ?? {}; + const id = getParameterByName(DETAIL_VIEW_QUERY_PARAM_NAME); + this.activeChild = + this.children.find( + (child) => getIdFromGraphQLId(child.id) === getIdFromGraphQLId(id) || child.iid === iid, + ) ?? {}; await this.$nextTick(); if (!isEmpty(this.activeChild)) { this.$refs.modal.show(); return; } - this.updateWorkItemIdUrlQuery(); + this.updateQueryParam(); }, }, parentIssue: { @@ -187,10 +192,10 @@ export default { event.preventDefault(); this.activeChild = child; this.$refs.modal.show(); - this.updateWorkItemIdUrlQuery(child); + this.updateQueryParam(child.id); }, async closeModal() { - this.updateWorkItemIdUrlQuery(); + this.updateQueryParam(); }, handleWorkItemDeleted(child) { const { defaultClient: cache } = this.$apollo.provider.clients; @@ -203,8 +208,11 @@ export default { }); this.$toast.show(s__('WorkItem|Task deleted')); }, - updateWorkItemIdUrlQuery({ iid } = {}) { - updateHistory({ url: setUrlParams({ work_item_iid: iid }), replace: true }); + updateQueryParam(id) { + updateHistory({ + url: setUrlParams({ [DETAIL_VIEW_QUERY_PARAM_NAME]: getIdFromGraphQLId(id) }), + replace: true, + }); }, toggleReportAbuseModal(isOpen, reply = {}) { this.isReportModalOpen = isOpen; diff --git a/app/assets/javascripts/work_items/constants.js b/app/assets/javascripts/work_items/constants.js index 8be385f3636ac4..931db3d192a85e 100644 --- a/app/assets/javascripts/work_items/constants.js +++ b/app/assets/javascripts/work_items/constants.js @@ -363,3 +363,5 @@ export const NEW_EPIC_FEEDBACK_PROMPT_EXPIRY = '2024-11-01'; export const FEATURE_NAME = 'work_item_epic_feedback'; export const CLEAR_VALUE = 'CLEAR_VALUE'; + +export const DETAIL_VIEW_QUERY_PARAM_NAME = 'show'; diff --git a/spec/frontend/issuable/components/related_issuable_item_spec.js b/spec/frontend/issuable/components/related_issuable_item_spec.js index bffa708436f67f..3055e20f0d3e57 100644 --- a/spec/frontend/issuable/components/related_issuable_item_spec.js +++ b/spec/frontend/issuable/components/related_issuable_item_spec.js @@ -25,7 +25,7 @@ describe('RelatedIssuableItem', () => { let showModalSpy; const defaultProps = { - idKey: 1, + idKey: 10, iid: 1, displayReference: 'gitlab-org/gitlab-test#1', pathIdSeparator: '#', @@ -243,7 +243,7 @@ describe('RelatedIssuableItem', () => { }); describe('work item modal', () => { - const workItemId = 'gid://gitlab/WorkItem/1'; + const workItemId = 'gid://gitlab/WorkItem/10'; it('renders', () => { mountComponent(); @@ -276,7 +276,7 @@ describe('RelatedIssuableItem', () => { it('updates the url params with the work item id', () => { expect(updateHistory).toHaveBeenCalledWith({ - url: `${TEST_HOST}/?work_item_iid=1`, + url: `${TEST_HOST}/?show=10`, replace: true, }); }); diff --git a/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js index 128ba9218bc86c..8516a7417fdd66 100644 --- a/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js +++ b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js @@ -230,6 +230,15 @@ describe('WorkItemLinks', () => { expect(findWorkItemDetailModal().props('workItemIid')).toBe('37'); }); + it('opens the modal if work item id URL parameter is found in child items', async () => { + setWindowLocation('?show=31'); + await createComponent(); + + expect(showModal).toHaveBeenCalled(); + expect(findWorkItemDetailModal().props('workItemId')).toBe('gid://gitlab/WorkItem/31'); + expect(findWorkItemDetailModal().props('workItemIid')).toBe('37'); + }); + describe('abuse category selector', () => { beforeEach(async () => { setWindowLocation('?work_item_id=2'); diff --git a/spec/frontend/work_items/mock_data.js b/spec/frontend/work_items/mock_data.js index e6f2b1178b1683..4d7e06629271aa 100644 --- a/spec/frontend/work_items/mock_data.js +++ b/spec/frontend/work_items/mock_data.js @@ -1943,7 +1943,7 @@ export const workItemHierarchyTreeEmptyResponse = { export const mockHierarchyChildren = [ { - id: 'gid://gitlab/WorkItem/37', + id: 'gid://gitlab/WorkItem/31', iid: '37', workItemType: { id: 'gid://gitlab/WorkItems::Type/2411', -- GitLab From 58798f2673b4bded0a897b3b624b1853373a1fe9 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Thu, 15 Aug 2024 18:55:46 +1000 Subject: [PATCH 2/3] Add query by id and tests for detail component --- .../components/work_item_detail.vue | 36 +++++++++++++--- .../components/work_item_detail_modal.vue | 1 + .../graphql/work_item_by_id.query.graphql | 7 ++++ .../components/work_item_detail_modal_spec.js | 1 + .../components/work_item_detail_spec.js | 42 ++++++++++++++++--- .../work_items/pages/work_item_root_spec.js | 1 + 6 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 app/assets/javascripts/work_items/graphql/work_item_by_id.query.graphql diff --git a/app/assets/javascripts/work_items/components/work_item_detail.vue b/app/assets/javascripts/work_items/components/work_item_detail.vue index ecb4e3af404e08..9fceead6d5e649 100644 --- a/app/assets/javascripts/work_items/components/work_item_detail.vue +++ b/app/assets/javascripts/work_items/components/work_item_detail.vue @@ -6,7 +6,8 @@ import * as Sentry from '~/sentry/sentry_browser_wrapper'; import { s__ } from '~/locale'; import { getParameterByName, updateHistory, setUrlParams } from '~/lib/utils/url_utility'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; -import { getIdFromGraphQLId } from '~/graphql_shared/utils'; +import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; +import { TYPENAME_WORK_ITEM } from '~/graphql_shared/constants'; import { isLoggedIn } from '~/lib/utils/common_utils'; import { WORKSPACE_PROJECT } from '~/issues/constants'; import { @@ -31,6 +32,7 @@ import { import workItemUpdatedSubscription from '../graphql/work_item_updated.subscription.graphql'; import updateWorkItemMutation from '../graphql/update_work_item.mutation.graphql'; +import workItemByIdQuery from '../graphql/work_item_by_id.query.graphql'; import workItemByIidQuery from '../graphql/work_item_by_iid.query.graphql'; import getAllowedWorkItemChildTypes from '../graphql/work_item_allowed_children.query.graphql'; import { findHierarchyWidgetDefinition } from '../utils'; @@ -90,6 +92,11 @@ export default { required: false, default: false, }, + workItemId: { + type: String, + required: false, + default: null, + }, workItemIid: { type: String, required: false, @@ -107,12 +114,18 @@ export default { }, }, data() { + let modalWorkItemId = getParameterByName(DETAIL_VIEW_QUERY_PARAM_NAME); + + if (modalWorkItemId) { + modalWorkItemId = convertToGraphQLId(TYPENAME_WORK_ITEM, modalWorkItemId); + } + return { error: undefined, updateError: undefined, workItem: {}, updateInProgress: false, - modalWorkItemId: getParameterByName(DETAIL_VIEW_QUERY_PARAM_NAME), + modalWorkItemId, modalWorkItemIid: getParameterByName('work_item_iid'), modalWorkItemNamespaceFullPath: '', isReportModalOpen: false, @@ -126,17 +139,30 @@ export default { }, apollo: { workItem: { - query: workItemByIidQuery, + query() { + if (this.workItemId) { + return workItemByIdQuery; + } + return workItemByIidQuery; + }, variables() { + if (this.workItemId) { + return { + id: this.workItemId, + }; + } return { fullPath: this.workItemFullPath, iid: this.workItemIid, }; }, skip() { - return !this.workItemIid; + return !this.workItemIid && !this.workItemId; }, update(data) { + if (this.workItemId) { + return data.workItem ?? {}; + } return data.workspace.workItem ?? {}; }, error() { @@ -399,7 +425,7 @@ export default { updateUrl(modalWorkItem) { updateHistory({ url: setUrlParams({ - [DETAIL_VIEW_QUERY_PARAM_NAME]: modalWorkItem?.id, + [DETAIL_VIEW_QUERY_PARAM_NAME]: getIdFromGraphQLId(modalWorkItem?.id), }), replace: true, }); diff --git a/app/assets/javascripts/work_items/components/work_item_detail_modal.vue b/app/assets/javascripts/work_items/components/work_item_detail_modal.vue index 7230e96a2c424e..afabf4691b1cfe 100644 --- a/app/assets/javascripts/work_items/components/work_item_detail_modal.vue +++ b/app/assets/javascripts/work_items/components/work_item_detail_modal.vue @@ -124,6 +124,7 @@ export default { { expect(findWorkItemDetail().props()).toEqual({ isModal: true, + workItemId, workItemIid: '1', modalWorkItemFullPath: '', isDrawer: false, diff --git a/spec/frontend/work_items/components/work_item_detail_spec.js b/spec/frontend/work_items/components/work_item_detail_spec.js index 285c03899b02ce..91d8d7392eb7d4 100644 --- a/spec/frontend/work_items/components/work_item_detail_spec.js +++ b/spec/frontend/work_items/components/work_item_detail_spec.js @@ -24,6 +24,7 @@ import WorkItemAbuseModal from '~/work_items/components/work_item_abuse_modal.vu import WorkItemTodos from '~/work_items/components/work_item_todos.vue'; import DesignWidget from '~/work_items/components/design_management/design_management_widget.vue'; import { i18n } from '~/work_items/constants'; +import workItemByIdQuery from '~/work_items/graphql/work_item_by_id.query.graphql'; import workItemByIidQuery from '~/work_items/graphql/work_item_by_iid.query.graphql'; import updateWorkItemMutation from '~/work_items/graphql/update_work_item.mutation.graphql'; import workItemUpdatedSubscription from '~/work_items/graphql/work_item_updated.subscription.graphql'; @@ -32,6 +33,7 @@ import getAllowedWorkItemChildTypes from '~/work_items/graphql/work_item_allowed import { mockParent, workItemByIidResponseFactory, + workItemQueryResponse, objectiveType, epicType, mockWorkItemCommentNote, @@ -46,7 +48,10 @@ describe('WorkItemDetail component', () => { Vue.use(VueApollo); - const workItemQueryResponse = workItemByIidResponseFactory({ canUpdate: true, canDelete: true }); + const workItemByIidQueryResponse = workItemByIidResponseFactory({ + canUpdate: true, + canDelete: true, + }); const workItemQueryResponseWithNoPermissions = workItemByIidResponseFactory({ canUpdate: false, canDelete: false, @@ -56,12 +61,13 @@ describe('WorkItemDetail component', () => { canUpdate: true, canDelete: true, }); - const successHandler = jest.fn().mockResolvedValue(workItemQueryResponse); + const workItemByIdQueryHandler = jest.fn().mockResolvedValue(workItemQueryResponse); + const successHandler = jest.fn().mockResolvedValue(workItemByIidQueryResponse); const successHandlerWithNoPermissions = jest .fn() .mockResolvedValue(workItemQueryResponseWithNoPermissions); const showModalHandler = jest.fn(); - const { id } = workItemQueryResponse.data.workspace.workItem; + const { id } = workItemByIidQueryResponse.data.workspace.workItem; const workItemUpdatedSubscriptionHandler = jest .fn() .mockResolvedValue({ data: { workItemUpdated: null } }); @@ -97,8 +103,10 @@ describe('WorkItemDetail component', () => { isModal = false, isDrawer = false, updateInProgress = false, + workItemId = '', workItemIid = '1', handler = successHandler, + workItemByIdHandler = workItemByIdQueryHandler, mutationHandler, error = undefined, workItemsAlphaEnabled = false, @@ -110,12 +118,14 @@ describe('WorkItemDetail component', () => { wrapper = shallowMountExtended(WorkItemDetail, { apolloProvider: createMockApollo([ [workItemByIidQuery, handler], + [workItemByIdQuery, workItemByIdHandler], [updateWorkItemMutation, mutationHandler], [workItemUpdatedSubscription, workItemUpdatedSubscriptionHandler], [getAllowedWorkItemChildTypes, allowedChildrenTypesHandler], ]), isLoggedIn: isLoggedIn(), propsData: { + workItemId, isModal, workItemIid, isDrawer, @@ -469,8 +479,28 @@ describe('WorkItemDetail component', () => { expect(successHandler).toHaveBeenCalledWith({ fullPath: 'group/project', iid: '1' }); }); - it('skips calling the work item query when there is no workItemIid', async () => { - createComponent({ workItemIid: null }); + it('calls the work item query by workItemId', async () => { + const workItemId = workItemQueryResponse.data.workItem.id; + createComponent({ workItemId }); + await waitForPromises(); + + expect(workItemByIdQueryHandler).toHaveBeenCalledWith({ id: workItemId }); + expect(successHandler).not.toHaveBeenCalled(); + }); + + it('shows work item modal if "show" query param set', async () => { + const workItemId = workItemQueryResponse.data.workItem.id; + setWindowLocation(`?show=${workItemId}`); + + createComponent(); + await waitForPromises(); + + expect(findModal().exists()).toBe(true); + expect(findModal().props('workItemId')).toBe(workItemId); + }); + + it('skips calling the work item query when there is no workItemIid and no workItemId', async () => { + createComponent({ workItemIid: null, workItemId: null }); await waitForPromises(); expect(successHandler).not.toHaveBeenCalled(); @@ -674,7 +704,7 @@ describe('WorkItemDetail component', () => { createComponent(); await waitForPromises(); - const { confidential } = workItemQueryResponse.data.workspace.workItem; + const { confidential } = workItemByIidQueryResponse.data.workspace.workItem; expect(findNotesWidget().exists()).toBe(true); expect(findNotesWidget().props('isWorkItemConfidential')).toBe(confidential); diff --git a/spec/frontend/work_items/pages/work_item_root_spec.js b/spec/frontend/work_items/pages/work_item_root_spec.js index 0848aea4eb1c6d..d5869f75063052 100644 --- a/spec/frontend/work_items/pages/work_item_root_spec.js +++ b/spec/frontend/work_items/pages/work_item_root_spec.js @@ -49,6 +49,7 @@ describe('Work items root component', () => { expect(findWorkItemDetail().props()).toEqual({ isModal: false, + workItemId: null, workItemIid: '1', modalWorkItemFullPath: '', isDrawer: false, -- GitLab From ddeeb6585b4136373a50c83861006288b1133786 Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Thu, 15 Aug 2024 20:03:29 +1000 Subject: [PATCH 3/3] Fix conflicts --- app/assets/javascripts/work_items/constants.js | 4 ---- .../work_items/graphql/work_item_by_id.query.graphql | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/app/assets/javascripts/work_items/constants.js b/app/assets/javascripts/work_items/constants.js index b238e0c8240038..e78b06abe1a87b 100644 --- a/app/assets/javascripts/work_items/constants.js +++ b/app/assets/javascripts/work_items/constants.js @@ -354,14 +354,10 @@ export const FEATURE_NAME = 'work_item_epic_feedback'; export const CLEAR_VALUE = 'CLEAR_VALUE'; -<<<<<<< HEAD export const DETAIL_VIEW_QUERY_PARAM_NAME = 'show'; -||||||| b166e1ab5ae4 -======= export const ROUTES = { index: 'workItemList', workItem: 'workItem', new: 'new', design: 'design', }; ->>>>>>> 48a9f58ee0d1bd3b8b903f5419f423a94cf282e9 diff --git a/app/assets/javascripts/work_items/graphql/work_item_by_id.query.graphql b/app/assets/javascripts/work_items/graphql/work_item_by_id.query.graphql index 07d4e794e6e5a2..500a04e91239fa 100644 --- a/app/assets/javascripts/work_items/graphql/work_item_by_id.query.graphql +++ b/app/assets/javascripts/work_items/graphql/work_item_by_id.query.graphql @@ -1,6 +1,6 @@ #import "./work_item.fragment.graphql" -query namespaceWorkItem($id: WorkItemID!) { +query workItemById($id: WorkItemID!) { workItem(id: $id) { ...WorkItem } -- GitLab