From a747e62970d0016fa7f52aa063b8da9745134217 Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Thu, 30 Jul 2026 18:45:25 +0200 Subject: [PATCH] fix(workflow): use as draft with an existing draft and cross-object record page filters (#23524) Fixes two workflow issues. ### "Use as draft" fails when a draft already exists `UseAsDraftWorkflowVersionSingleRecordCommand` rendered its own `OverrideWorkflowDraftConfirmationModal`. Headless commands are unmounted by `HeadlessEngineCommandWrapperEffect` as soon as `execute` resolves, so the modal was removed from the tree right after `openModal` was called and the user saw nothing happen. This regressed when the command was converted from a rendered `` to a self-unmounting effect. The command now uses `HeadlessConfirmationModalEngineCommandEffect`, the existing mechanism for headless commands that need a confirmation: it opens the app-wide `CommandMenuConfirmationModalManager` and keeps the command mounted until the modal emits its result. `OverrideWorkflowDraftConfirmationModal`, its modal id and its config state are deleted. To keep the "Go to Draft" shortcut, the shared confirmation modal config gains an optional `linkButton` rendered as a secondary link button that emits a `cancel` result on click. The command also resolved the workflow id through `useWorkflowVersion`, which is `undefined` while the query is in flight, so it threw and surfaced an error snackbar through the command error boundary. It now reads the workflow id off the selected record like the sibling workflow version commands, and waits for the workflow to load before deciding whether a confirmation is needed. ### `workflow object doesn't have any "workflowId" field` when opening a workflow from a version `useRecordShowPagePagination` builds prev/next queries from the parent view stored in `contextStoreRecordShowParentViewComponentState`. Navigating from a workflow version record page to its workflow through the relation chip keeps the workflow version parent view, whose relation filter compiles to `workflowId: { in: [...] }` and is then sent against the `workflow` object, which the API rejects. `useQueryVariablesFromParentView` now ignores the parent view when `parentViewObjectNameSingular` does not match the current object, which covers every navigation path between record pages of different objects. ### Test Added `useQueryVariablesFromParentView.test.tsx` covering both the matching and mismatching parent view object. Manually checked on a local instance: override with an existing draft, "Go to Draft", cancel then re-trigger, and the no-existing-draft path, plus navigating from a filtered workflow versions view to a workflow. --------- Co-authored-by: Tom --- .../CommandMenuConfirmationModalManager.tsx | 19 ++++- .../commandMenuItemConfirmationModalState.ts | 6 ++ ...ssConfirmationModalEngineCommandEffect.tsx | 5 ++ ...raftWorkflowVersionSingleRecordCommand.tsx | 83 +++++++++--------- .../useQueryVariablesFromParentView.test.tsx | 84 +++++++++++++++++++ .../hooks/useQueryVariablesFromParentView.ts | 15 ++-- ...OverrideWorkflowDraftConfirmationModal.tsx | 67 --------------- ...verrideWorkflowDraftConfirmationModalId.ts | 2 - 8 files changed, 162 insertions(+), 119 deletions(-) create mode 100644 packages/twenty-front/src/modules/views/hooks/__tests__/useQueryVariablesFromParentView.test.tsx delete mode 100644 packages/twenty-front/src/modules/workflow/components/OverrideWorkflowDraftConfirmationModal.tsx delete mode 100644 packages/twenty-front/src/modules/workflow/constants/OverrideWorkflowDraftConfirmationModalId.ts diff --git a/packages/twenty-front/src/modules/command-menu-item/confirmation-modal/components/CommandMenuConfirmationModalManager.tsx b/packages/twenty-front/src/modules/command-menu-item/confirmation-modal/components/CommandMenuConfirmationModalManager.tsx index 921783f59a..4e4b8904ac 100644 --- a/packages/twenty-front/src/modules/command-menu-item/confirmation-modal/components/CommandMenuConfirmationModalManager.tsx +++ b/packages/twenty-front/src/modules/command-menu-item/confirmation-modal/components/CommandMenuConfirmationModalManager.tsx @@ -5,7 +5,10 @@ import { type CommandMenuConfirmationModalResult, type CommandMenuConfirmationModalResultBrowserEventDetail, } from 'twenty-shared/types'; -import { ConfirmationModal } from '@/ui/layout/modal/components/ConfirmationModal'; +import { + ConfirmationModal, + StyledCenteredButton, +} from '@/ui/layout/modal/components/ConfirmationModal'; import { isModalOpenedComponentState } from '@/ui/layout/modal/states/isModalOpenedComponentState'; import { useAtomComponentStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateValue'; import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue'; @@ -52,6 +55,8 @@ export const CommandMenuConfirmationModalManager = () => { return null; } + const linkButton = commandMenuItemConfirmationModalConfig.linkButton; + return ( { confirmButtonAccent={ commandMenuItemConfirmationModalConfig.confirmButtonAccent } + AdditionalButtons={ + isDefined(linkButton) ? ( + emitConfirmationResult('cancel')} + variant="secondary" + title={linkButton.title} + fullWidth + justify="center" + /> + ) : undefined + } /> ); }; diff --git a/packages/twenty-front/src/modules/command-menu-item/confirmation-modal/states/commandMenuItemConfirmationModalState.ts b/packages/twenty-front/src/modules/command-menu-item/confirmation-modal/states/commandMenuItemConfirmationModalState.ts index 3a77b03023..2cfc96f334 100644 --- a/packages/twenty-front/src/modules/command-menu-item/confirmation-modal/states/commandMenuItemConfirmationModalState.ts +++ b/packages/twenty-front/src/modules/command-menu-item/confirmation-modal/states/commandMenuItemConfirmationModalState.ts @@ -4,12 +4,18 @@ import { type ConfirmationModalCaller } from 'twenty-shared/types'; import { createAtomState } from '@/ui/utilities/state/jotai/utils/createAtomState'; import { type ButtonAccent } from 'twenty-ui/input'; +export type CommandMenuItemConfirmationModalLinkButton = { + title: string; + to: string; +}; + export type CommandMenuItemConfirmationModalConfig = { caller: ConfirmationModalCaller; title: string; subtitle: ReactNode; confirmButtonText?: string; confirmButtonAccent?: ButtonAccent; + linkButton?: CommandMenuItemConfirmationModalLinkButton; }; export const commandMenuItemConfirmationModalConfigState = diff --git a/packages/twenty-front/src/modules/command-menu-item/engine-command/components/HeadlessConfirmationModalEngineCommandEffect.tsx b/packages/twenty-front/src/modules/command-menu-item/engine-command/components/HeadlessConfirmationModalEngineCommandEffect.tsx index 12894f3b35..2593bed917 100644 --- a/packages/twenty-front/src/modules/command-menu-item/engine-command/components/HeadlessConfirmationModalEngineCommandEffect.tsx +++ b/packages/twenty-front/src/modules/command-menu-item/engine-command/components/HeadlessConfirmationModalEngineCommandEffect.tsx @@ -3,6 +3,7 @@ import { type ReactNode, useEffect } from 'react'; import { COMMAND_MENU_CONFIRMATION_MODAL_RESULT_BROWSER_EVENT_NAME } from 'twenty-shared/constants'; import { useCommandMenuConfirmationModal } from '@/command-menu-item/confirmation-modal/hooks/useCommandMenuConfirmationModal'; +import { type CommandMenuItemConfirmationModalLinkButton } from '@/command-menu-item/confirmation-modal/states/commandMenuItemConfirmationModalState'; import { type CommandMenuConfirmationModalResultBrowserEventDetail } from 'twenty-shared/types'; import { useUnmountCommand } from '@/command-menu-item/engine-command/hooks/useUnmountEngineCommand'; import { CommandComponentInstanceContext } from '@/command-menu-item/engine-command/states/contexts/CommandComponentInstanceContext'; @@ -14,6 +15,7 @@ export type HeadlessConfirmationModalEngineCommandEffectProps = { subtitle: ReactNode; confirmButtonText: string; confirmButtonAccent?: ButtonAccent; + linkButton?: CommandMenuItemConfirmationModalLinkButton; execute: () => void | Promise; }; @@ -22,6 +24,7 @@ export const HeadlessConfirmationModalEngineCommandEffect = ({ subtitle, confirmButtonText, confirmButtonAccent = 'danger', + linkButton, execute, }: HeadlessConfirmationModalEngineCommandEffectProps) => { const { isInitializedRef, setIsInitialized } = @@ -46,6 +49,7 @@ export const HeadlessConfirmationModalEngineCommandEffect = ({ subtitle, confirmButtonText, confirmButtonAccent, + linkButton, }); }, [ isInitializedRef, @@ -56,6 +60,7 @@ export const HeadlessConfirmationModalEngineCommandEffect = ({ subtitle, confirmButtonText, confirmButtonAccent, + linkButton, ]); useEffect(() => { diff --git a/packages/twenty-front/src/modules/command-menu-item/engine-command/record/single-record/workflow-versions/components/UseAsDraftWorkflowVersionSingleRecordCommand.tsx b/packages/twenty-front/src/modules/command-menu-item/engine-command/record/single-record/workflow-versions/components/UseAsDraftWorkflowVersionSingleRecordCommand.tsx index d7cfda85ca..919c63157b 100644 --- a/packages/twenty-front/src/modules/command-menu-item/engine-command/record/single-record/workflow-versions/components/UseAsDraftWorkflowVersionSingleRecordCommand.tsx +++ b/packages/twenty-front/src/modules/command-menu-item/engine-command/record/single-record/workflow-versions/components/UseAsDraftWorkflowVersionSingleRecordCommand.tsx @@ -1,14 +1,11 @@ +import { HeadlessConfirmationModalEngineCommandEffect } from '@/command-menu-item/engine-command/components/HeadlessConfirmationModalEngineCommandEffect'; import { HeadlessEngineCommandWrapperEffect } from '@/command-menu-item/engine-command/components/HeadlessEngineCommandWrapperEffect'; import { useHeadlessCommandContextApi } from '@/command-menu-item/engine-command/hooks/useHeadlessCommandContextApi'; -import { useModal } from '@/ui/layout/modal/hooks/useModal'; -import { OverrideWorkflowDraftConfirmationModal } from '@/workflow/components/OverrideWorkflowDraftConfirmationModal'; -import { OVERRIDE_WORKFLOW_DRAFT_CONFIRMATION_MODAL_ID } from '@/workflow/constants/OverrideWorkflowDraftConfirmationModalId'; import { useCreateDraftFromWorkflowVersion } from '@/workflow/hooks/useCreateDraftFromWorkflowVersion'; -import { useWorkflowVersion } from '@/workflow/hooks/useWorkflowVersion'; import { useWorkflowWithCurrentVersion } from '@/workflow/hooks/useWorkflowWithCurrentVersion'; -import { useState } from 'react'; +import { useLingui } from '@lingui/react/macro'; import { AppPath, CoreObjectNameSingular } from 'twenty-shared/types'; -import { isDefined } from 'twenty-shared/utils'; +import { getAppPath, isDefined } from 'twenty-shared/utils'; import { useNavigateApp } from '~/hooks/useNavigateApp'; const UseAsDraftWorkflowVersionSingleRecordCommandContent = ({ @@ -18,60 +15,58 @@ const UseAsDraftWorkflowVersionSingleRecordCommandContent = ({ workflowId: string; workflowVersionId: string; }) => { - const { openModal } = useModal(); + const { t } = useLingui(); const workflow = useWorkflowWithCurrentVersion(workflowId); const { createDraftFromWorkflowVersion } = useCreateDraftFromWorkflowVersion(); const navigate = useNavigateApp(); - const [hasNavigated, setHasNavigated] = useState(false); const hasAlreadyDraftVersion = - workflow?.versions.some((version) => version.status === 'DRAFT') || false; + workflow?.versions.some((version) => version.status === 'DRAFT') ?? false; - const handleExecute = () => { - if (!isDefined(workflow) || hasNavigated) { - return; - } + const handleExecute = async () => { + await createDraftFromWorkflowVersion({ + workflowId, + workflowVersionIdToCopy: workflowVersionId, + }); - if (hasAlreadyDraftVersion) { - openModal(OVERRIDE_WORKFLOW_DRAFT_CONFIRMATION_MODAL_ID); - } else { - const executeCommandWithoutWaiting = async () => { - await createDraftFromWorkflowVersion({ - workflowId, - workflowVersionIdToCopy: workflowVersionId, - }); - - navigate(AppPath.RecordShowPage, { - objectNameSingular: CoreObjectNameSingular.Workflow, - objectRecordId: workflowId, - }); - - setHasNavigated(true); - }; - - executeCommandWithoutWaiting(); - } + navigate(AppPath.RecordShowPage, { + objectNameSingular: CoreObjectNameSingular.Workflow, + objectRecordId: workflowId, + }); }; + if (!isDefined(workflow)) { + return null; + } + + if (!hasAlreadyDraftVersion) { + return ; + } + return ( - <> - - - + ); }; export const UseAsDraftWorkflowVersionSingleRecordCommand = () => { const { selectedRecords } = useHeadlessCommandContextApi(); - const recordId = selectedRecords[0]?.id; - const workflowVersion = useWorkflowVersion(recordId ?? ''); + const selectedRecord = selectedRecords[0]; - if (!recordId || !isDefined(workflowVersion?.workflow?.id)) { + if (!isDefined(selectedRecord) || !isDefined(selectedRecord.workflowId)) { throw new Error( 'Record ID and workflow ID are required to use as draft workflow version', ); @@ -79,8 +74,8 @@ export const UseAsDraftWorkflowVersionSingleRecordCommand = () => { return ( ); }; diff --git a/packages/twenty-front/src/modules/views/hooks/__tests__/useQueryVariablesFromParentView.test.tsx b/packages/twenty-front/src/modules/views/hooks/__tests__/useQueryVariablesFromParentView.test.tsx new file mode 100644 index 0000000000..f0639beade --- /dev/null +++ b/packages/twenty-front/src/modules/views/hooks/__tests__/useQueryVariablesFromParentView.test.tsx @@ -0,0 +1,84 @@ +import { renderHook } from '@testing-library/react'; + +import { MAIN_CONTEXT_STORE_INSTANCE_ID } from '@/context-store/constants/MainContextStoreInstanceId'; +import { contextStoreRecordShowParentViewComponentState } from '@/context-store/states/contextStoreRecordShowParentViewComponentState'; +import { type RecordFilter } from '@/object-record/record-filter/types/RecordFilter'; +import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem'; +import { useQueryVariablesFromParentView } from '@/views/hooks/useQueryVariablesFromParentView'; +import { ViewFilterOperand } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; +import { getJestMetadataAndApolloMocksWrapper } from '~/testing/jest/getJestMetadataAndApolloMocksWrapper'; +import { getMockObjectMetadataItemOrThrow } from '~/testing/utils/getMockObjectMetadataItemOrThrow'; + +const WORKFLOW_RECORD_ID = '20202020-1c25-4d02-bf25-6aeccf7ea419'; + +const workflowVersionObjectMetadataItem = + getMockObjectMetadataItemOrThrow('workflowVersion'); +const workflowObjectMetadataItem = getMockObjectMetadataItemOrThrow('workflow'); + +const workflowRelationFieldMetadataItem = + workflowVersionObjectMetadataItem.fields.find( + (field) => field.name === 'workflow', + ); + +if (!isDefined(workflowRelationFieldMetadataItem)) { + throw new Error('Missing "workflow" field on the workflowVersion object'); +} + +const workflowRecordFilter: RecordFilter = { + id: 'record-filter-1', + fieldMetadataId: workflowRelationFieldMetadataItem.id, + operand: ViewFilterOperand.IS, + value: JSON.stringify({ + isCurrentWorkspaceMemberSelected: false, + isCurrentRecordSelected: false, + selectedRecordIds: [WORKFLOW_RECORD_ID], + }), + displayValue: 'My workflow', + type: 'RELATION', + label: 'Workflow', +}; + +const renderUseQueryVariablesFromParentView = ( + objectMetadataItem: EnrichedObjectMetadataItem, +) => + renderHook(() => useQueryVariablesFromParentView({ objectMetadataItem }), { + wrapper: getJestMetadataAndApolloMocksWrapper({ + apolloMocks: [], + onInitializeJotaiStore: (store) => { + store.set( + contextStoreRecordShowParentViewComponentState.atomFamily({ + instanceId: MAIN_CONTEXT_STORE_INSTANCE_ID, + }), + { + parentViewComponentId: 'record-index-workflow-versions', + parentViewObjectNameSingular: + workflowVersionObjectMetadataItem.nameSingular, + parentViewFilterGroups: [], + parentViewFilters: [workflowRecordFilter], + parentViewSorts: [], + }, + ); + }, + }), + }); + +describe('useQueryVariablesFromParentView', () => { + it('should compute the filter when the parent view targets the same object', () => { + const { result } = renderUseQueryVariablesFromParentView( + workflowVersionObjectMetadataItem, + ); + + expect(result.current.filter).toEqual({ + workflowId: { in: [WORKFLOW_RECORD_ID] }, + }); + }); + + it('should ignore the parent view when it targets another object', () => { + const { result } = renderUseQueryVariablesFromParentView( + workflowObjectMetadataItem, + ); + + expect(result.current.filter).toEqual({}); + }); +}); diff --git a/packages/twenty-front/src/modules/views/hooks/useQueryVariablesFromParentView.ts b/packages/twenty-front/src/modules/views/hooks/useQueryVariablesFromParentView.ts index 3da1969a00..296fff060a 100644 --- a/packages/twenty-front/src/modules/views/hooks/useQueryVariablesFromParentView.ts +++ b/packages/twenty-front/src/modules/views/hooks/useQueryVariablesFromParentView.ts @@ -27,11 +27,16 @@ export const useQueryVariablesFromParentView = ({ const { filterValueDependencies } = useFilterValueDependencies(); + const parentView = + contextStoreRecordShowParentView?.parentViewObjectNameSingular === + objectMetadataItem.nameSingular + ? contextStoreRecordShowParentView + : undefined; + const { filter, orderBy } = getQueryVariablesFromFiltersAndSorts({ - recordFilterGroups: - contextStoreRecordShowParentView?.parentViewFilterGroups ?? [], - recordFilters: contextStoreRecordShowParentView?.parentViewFilters ?? [], - recordSorts: contextStoreRecordShowParentView?.parentViewSorts ?? [], + recordFilterGroups: parentView?.parentViewFilterGroups ?? [], + recordFilters: parentView?.parentViewFilters ?? [], + recordSorts: parentView?.parentViewSorts ?? [], objectMetadataItem, objectMetadataItems, fieldMetadataItems: flattenedFieldMetadataItems, @@ -39,7 +44,7 @@ export const useQueryVariablesFromParentView = ({ }); const isSoftDeleteFilterActive = - contextStoreRecordShowParentView?.parentViewFilters.some((recordFilter) => + parentView?.parentViewFilters.some((recordFilter) => isRecordFilterAboutSoftDelete({ recordFilter, objectMetadataItems }), ) ?? false; diff --git a/packages/twenty-front/src/modules/workflow/components/OverrideWorkflowDraftConfirmationModal.tsx b/packages/twenty-front/src/modules/workflow/components/OverrideWorkflowDraftConfirmationModal.tsx deleted file mode 100644 index 8e87ef00de..0000000000 --- a/packages/twenty-front/src/modules/workflow/components/OverrideWorkflowDraftConfirmationModal.tsx +++ /dev/null @@ -1,67 +0,0 @@ -import { - ConfirmationModal, - StyledCenteredButton, -} from '@/ui/layout/modal/components/ConfirmationModal'; -import { useModal } from '@/ui/layout/modal/hooks/useModal'; -import { OVERRIDE_WORKFLOW_DRAFT_CONFIRMATION_MODAL_ID } from '@/workflow/constants/OverrideWorkflowDraftConfirmationModalId'; -import { useCreateDraftFromWorkflowVersion } from '@/workflow/hooks/useCreateDraftFromWorkflowVersion'; -import { useLingui } from '@lingui/react/macro'; -import { AppPath, CoreObjectNameSingular } from 'twenty-shared/types'; -import { getAppPath } from 'twenty-shared/utils'; -import { useNavigateApp } from '~/hooks/useNavigateApp'; - -export const OverrideWorkflowDraftConfirmationModal = ({ - workflowId, - workflowVersionIdToCopy, -}: { - workflowId: string; - workflowVersionIdToCopy: string; -}) => { - const { closeModal } = useModal(); - - const { createDraftFromWorkflowVersion } = - useCreateDraftFromWorkflowVersion(); - - const navigate = useNavigateApp(); - - const handleOverrideDraft = async () => { - await createDraftFromWorkflowVersion({ - workflowId, - workflowVersionIdToCopy, - }); - - navigate(AppPath.RecordShowPage, { - objectNameSingular: CoreObjectNameSingular.Workflow, - objectRecordId: workflowId, - }); - }; - - const { t } = useLingui(); - - return ( - <> - { - closeModal(OVERRIDE_WORKFLOW_DRAFT_CONFIRMATION_MODAL_ID); - }} - variant="secondary" - title={t`Go to Draft`} - fullWidth - justify="center" - /> - } - /> - - ); -}; diff --git a/packages/twenty-front/src/modules/workflow/constants/OverrideWorkflowDraftConfirmationModalId.ts b/packages/twenty-front/src/modules/workflow/constants/OverrideWorkflowDraftConfirmationModalId.ts deleted file mode 100644 index 4e87a40db5..0000000000 --- a/packages/twenty-front/src/modules/workflow/constants/OverrideWorkflowDraftConfirmationModalId.ts +++ /dev/null @@ -1,2 +0,0 @@ -export const OVERRIDE_WORKFLOW_DRAFT_CONFIRMATION_MODAL_ID = - 'override-workflow-draft-confirmation-modal';