From ac0368e876131a6aa14014d3f3b062d220fc76ca Mon Sep 17 00:00:00 2001 From: Thomas Trompette Date: Wed, 3 Jun 2026 17:38:38 +0200 Subject: [PATCH] fix: eliminate workflow editing flicker on active-to-draft transitions (#21176) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Before https://github.com/user-attachments/assets/5108a9d8-2017-41d5-855c-98714cbd4237 ## After https://github.com/user-attachments/assets/0a78d1e1-354f-4f3f-8ec4-6f46517619e4 ## Summary - Fixes visual flickering/glitching in the workflow show page header and canvas when editing an active workflow or discarding a draft - Root cause: SSE events re-added discarded drafts to Apollo cache, and multiple hook instances had independent state causing version oscillation between DRAFT and ACTIVE - Rewrites `useWorkflowWithCurrentVersion` with a module-level `discardedDraftId` variable shared across all instances, Apollo cache seeding in mutation callbacks, and `lastValidResult` caching to prevent null renders ## Test plan - [x] Open a workflow show page with an ACTIVE workflow - [x] Drag a node to change position → verify no flicker, status shows DRAFT smoothly - [x] Discard the draft → verify header does NOT flicker between DRAFT/ACTIVE, position resets cleanly - [x] Click on manual trigger and edit settings → verify the edit works (draft created, settings saved) - [x] Repeat discard + edit cycle multiple times to confirm stability --- .../hooks/useEffectiveDraftVersionId.ts | 70 ++++++++++++++++++ .../hooks/useWorkflowWithCurrentVersion.ts | 71 +++++++------------ .../states/lastDiscardedDraftIdState.ts | 4 ++ .../components/WorkflowDiagramEffect.tsx | 16 ++++- .../__tests__/mergeWorkflowDiagrams.test.ts | 3 - .../utils/mergeWorkflowDiagrams.ts | 31 +++++--- 6 files changed, 133 insertions(+), 62 deletions(-) create mode 100644 packages/twenty-front/src/modules/workflow/hooks/useEffectiveDraftVersionId.ts create mode 100644 packages/twenty-front/src/modules/workflow/states/lastDiscardedDraftIdState.ts diff --git a/packages/twenty-front/src/modules/workflow/hooks/useEffectiveDraftVersionId.ts b/packages/twenty-front/src/modules/workflow/hooks/useEffectiveDraftVersionId.ts new file mode 100644 index 0000000000..689ea2aeed --- /dev/null +++ b/packages/twenty-front/src/modules/workflow/hooks/useEffectiveDraftVersionId.ts @@ -0,0 +1,70 @@ +import { useGetRecordFromCache } from '@/object-record/cache/hooks/useGetRecordFromCache'; +import { lastDiscardedDraftIdState } from '@/workflow/states/lastDiscardedDraftIdState'; +import { useAtom } from 'jotai'; +import { useState } from 'react'; +import { CoreObjectNameSingular } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; + +export const useEffectiveDraftVersionId = ( + draftVersionFromServer: { id: string } | undefined, +): { + effectiveDraftId: string | undefined; + lastDiscardedDraftId: string | undefined; +} => { + const getVersionFromCache = useGetRecordFromCache({ + objectNameSingular: CoreObjectNameSingular.WorkflowVersion, + recordGqlFields: { id: true, status: true, deletedAt: true }, + }); + + const [lastDiscardedDraftId, setLastDiscardedDraftId] = useAtom( + lastDiscardedDraftIdState, + ); + + const [previouslyKnownDraftId, setPreviouslyKnownDraftId] = useState< + string | undefined + >(); + + if (isDefined(draftVersionFromServer)) { + const isReaddedBySSEAfterDiscard = + draftVersionFromServer.id === lastDiscardedDraftId; + + if (isReaddedBySSEAfterDiscard) { + return { effectiveDraftId: undefined, lastDiscardedDraftId }; + } + + const isNewDraft = draftVersionFromServer.id !== previouslyKnownDraftId; + + if (isNewDraft) { + setPreviouslyKnownDraftId(draftVersionFromServer.id); + } + + if (isDefined(lastDiscardedDraftId)) { + setLastDiscardedDraftId(undefined); + } + + return { + effectiveDraftId: draftVersionFromServer.id, + lastDiscardedDraftId, + }; + } + + if (!isDefined(previouslyKnownDraftId)) { + return { effectiveDraftId: undefined, lastDiscardedDraftId }; + } + + const cachedDraft = getVersionFromCache(previouslyKnownDraftId); + + const isDraftStillAlive = + isDefined(cachedDraft) && + cachedDraft.status === 'DRAFT' && + !isDefined(cachedDraft.deletedAt); + + if (isDraftStillAlive) { + return { effectiveDraftId: previouslyKnownDraftId, lastDiscardedDraftId }; + } + + setLastDiscardedDraftId(previouslyKnownDraftId); + setPreviouslyKnownDraftId(undefined); + + return { effectiveDraftId: undefined, lastDiscardedDraftId }; +}; diff --git a/packages/twenty-front/src/modules/workflow/hooks/useWorkflowWithCurrentVersion.ts b/packages/twenty-front/src/modules/workflow/hooks/useWorkflowWithCurrentVersion.ts index 203d8ba032..7f62e1d714 100644 --- a/packages/twenty-front/src/modules/workflow/hooks/useWorkflowWithCurrentVersion.ts +++ b/packages/twenty-front/src/modules/workflow/hooks/useWorkflowWithCurrentVersion.ts @@ -1,9 +1,5 @@ -import { useEffect } from 'react'; - import { useFindOneRecord } from '@/object-record/hooks/useFindOneRecord'; -import { useAtomFamilyStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomFamilyStateValue'; -import { useSetAtomFamilyState } from '@/ui/utilities/state/jotai/hooks/useSetAtomFamilyState'; -import { shouldWorkflowRefetchRequestFamilyState } from '@/workflow/states/shouldWorkflowRefetchRequestFamilyState'; +import { useEffectiveDraftVersionId } from '@/workflow/hooks/useEffectiveDraftVersionId'; import { type Workflow, type WorkflowVersion, @@ -21,62 +17,45 @@ type WorkflowWithAllVersions = Omit & { export const useWorkflowWithCurrentVersion = ( workflowId: string | undefined, ): WorkflowWithCurrentVersion | undefined => { - const shouldWorkflowRefetchRequest = useAtomFamilyStateValue( - shouldWorkflowRefetchRequestFamilyState, - workflowId ?? '', - ); - const setShouldWorkflowRefetchRequest = useSetAtomFamilyState( - shouldWorkflowRefetchRequestFamilyState, - workflowId ?? '', - ); - - const { record: workflow, refetch: refetchWorkflow } = - useFindOneRecord({ - objectNameSingular: CoreObjectNameSingular.Workflow, - objectRecordId: workflowId, - recordGqlFields: { + const { record: workflow } = useFindOneRecord({ + objectNameSingular: CoreObjectNameSingular.Workflow, + objectRecordId: workflowId, + recordGqlFields: { + id: true, + name: true, + statuses: true, + lastPublishedVersionId: true, + versions: { id: true, + status: true, name: true, - statuses: true, - lastPublishedVersionId: true, - versions: { - id: true, - status: true, - name: true, - createdAt: true, - }, + createdAt: true, }, - skip: !isDefined(workflowId), - }); + }, + skip: !isDefined(workflowId), + }); - useEffect(() => { - if (shouldWorkflowRefetchRequest) { - setShouldWorkflowRefetchRequest(false); - refetchWorkflow(); - } - }, [ - shouldWorkflowRefetchRequest, - setShouldWorkflowRefetchRequest, - refetchWorkflow, - ]); - - const draftVersion = workflow?.versions.find( + const draftVersionFromServer = workflow?.versions.find( (workflowVersion) => workflowVersion.status === 'DRAFT', ); - const workflowVersions = [...(workflow?.versions ?? [])]; + const { effectiveDraftId, lastDiscardedDraftId } = useEffectiveDraftVersionId( + draftVersionFromServer, + ); - workflowVersions.sort((a, b) => (a.createdAt > b.createdAt ? -1 : 1)); + const workflowVersions = [...(workflow?.versions ?? [])] + .filter((version) => version.id !== lastDiscardedDraftId) + .sort((a, b) => (a.createdAt > b.createdAt ? -1 : 1)); const latestVersion = workflowVersions[0]; - const currentVersionWithoutSteps = draftVersion ?? latestVersion; + const currentVersionId = effectiveDraftId ?? latestVersion?.id; const { record: currentVersionWithSteps } = useFindOneRecord( { objectNameSingular: CoreObjectNameSingular.WorkflowVersion, - objectRecordId: currentVersionWithoutSteps?.id, - skip: !isDefined(currentVersionWithoutSteps?.id), + objectRecordId: currentVersionId, + skip: !isDefined(currentVersionId), }, ); diff --git a/packages/twenty-front/src/modules/workflow/states/lastDiscardedDraftIdState.ts b/packages/twenty-front/src/modules/workflow/states/lastDiscardedDraftIdState.ts new file mode 100644 index 0000000000..d8ee605df8 --- /dev/null +++ b/packages/twenty-front/src/modules/workflow/states/lastDiscardedDraftIdState.ts @@ -0,0 +1,4 @@ +import { atom } from 'jotai'; + +export const lastDiscardedDraftIdState = atom(undefined); +lastDiscardedDraftIdState.debugLabel = 'lastDiscardedDraftIdState'; diff --git a/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowDiagramEffect.tsx b/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowDiagramEffect.tsx index ab96e5c77c..90454cee61 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowDiagramEffect.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowDiagramEffect.tsx @@ -12,7 +12,7 @@ import { useStepsOutputSchema } from '@/workflow/workflow-variables/hooks/useSte import { getWorkflowVersionDiagram } from '@/workflow/workflow-diagram/utils/getWorkflowVersionDiagram'; import { mergeWorkflowDiagrams } from '@/workflow/workflow-diagram/utils/mergeWorkflowDiagrams'; import { useStore } from 'jotai'; -import { useCallback, useEffect } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { isDefined } from 'twenty-shared/utils'; export const WorkflowDiagramEffect = () => { @@ -37,8 +37,10 @@ export const WorkflowDiagramEffect = () => { const store = useStore(); const currentVersion = workflowWithCurrentVersion?.currentVersion; + const [previousVersionId, setPreviousVersionId] = useState(); + const computeAndMergeNewWorkflowDiagram = useCallback( - (version: WorkflowVersion) => { + (version: WorkflowVersion, preservePositions: boolean) => { const previousWorkflowDiagram = store.get(workflowDiagram); const nextWorkflowDiagram = getWorkflowVersionDiagram({ @@ -52,6 +54,7 @@ export const WorkflowDiagramEffect = () => { mergedWorkflowDiagram = mergeWorkflowDiagrams( previousWorkflowDiagram, nextWorkflowDiagram, + { preservePositions }, ); } @@ -80,13 +83,20 @@ export const WorkflowDiagramEffect = () => { return; } + const isSameVersion = previousVersionId === currentVersion.id; + const isTransitionToDraft = currentVersion.status === 'DRAFT'; + const shouldPreservePositions = isSameVersion || isTransitionToDraft; + + setPreviousVersionId(currentVersion.id); + setFlow({ workflowVersionId: currentVersion.id, trigger: currentVersion.trigger, steps: currentVersion.steps, }); - computeAndMergeNewWorkflowDiagram(currentVersion); + computeAndMergeNewWorkflowDiagram(currentVersion, shouldPreservePositions); + // eslint-disable-next-line react-hooks/exhaustive-deps }, [computeAndMergeNewWorkflowDiagram, setFlow, currentVersion]); useEffect(() => { diff --git a/packages/twenty-front/src/modules/workflow/workflow-diagram/utils/__tests__/mergeWorkflowDiagrams.test.ts b/packages/twenty-front/src/modules/workflow/workflow-diagram/utils/__tests__/mergeWorkflowDiagrams.test.ts index 6ecd4c1812..9c689421e3 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-diagram/utils/__tests__/mergeWorkflowDiagrams.test.ts +++ b/packages/twenty-front/src/modules/workflow/workflow-diagram/utils/__tests__/mergeWorkflowDiagrams.test.ts @@ -56,7 +56,6 @@ it('Preserves the properties defined in the previous version but not in the next "stepId": "", }, "id": "1", - "measured": undefined, "position": { "x": 0, "y": 0, @@ -122,12 +121,10 @@ it('Replaces duplicated properties with the next value', () => { "stepId": "", }, "id": "1", - "measured": undefined, "position": { "x": 0, "y": 0, }, - "selected": undefined, }, ], } diff --git a/packages/twenty-front/src/modules/workflow/workflow-diagram/utils/mergeWorkflowDiagrams.ts b/packages/twenty-front/src/modules/workflow/workflow-diagram/utils/mergeWorkflowDiagrams.ts index 196a990d78..8a57303f30 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-diagram/utils/mergeWorkflowDiagrams.ts +++ b/packages/twenty-front/src/modules/workflow/workflow-diagram/utils/mergeWorkflowDiagrams.ts @@ -2,8 +2,9 @@ import { type WorkflowDiagram, type WorkflowDiagramNode, } from '@/workflow/workflow-diagram/types/WorkflowDiagram'; +import { isDefined } from 'twenty-shared/utils'; -const nodePropertiesToPreserve: Array = [ +const BASE_PROPERTIES_TO_PRESERVE: Array = [ 'selected', 'measured', ]; @@ -11,22 +12,32 @@ const nodePropertiesToPreserve: Array = [ export const mergeWorkflowDiagrams = ( previousDiagram: WorkflowDiagram, nextDiagram: WorkflowDiagram, + options?: { preservePositions?: boolean }, ): WorkflowDiagram => { + const propertiesToPreserve = options?.preservePositions + ? [...BASE_PROPERTIES_TO_PRESERVE, 'position' as const] + : BASE_PROPERTIES_TO_PRESERVE; + const lastNodes = nextDiagram.nodes.map((nextNode) => { const previousNode = previousDiagram.nodes.find( (previousNode) => previousNode.id === nextNode.id, ); - const nodeWithPreservedProperties = nodePropertiesToPreserve.reduce( - (nodeToSet, propertyToPreserve) => { - return Object.assign(nodeToSet, { - [propertyToPreserve]: previousNode?.[propertyToPreserve], - }); - }, - {} as Partial, - ); + if (!isDefined(previousNode)) { + return nextNode; + } - return Object.assign(nodeWithPreservedProperties, nextNode); + const preservedProperties: Partial = {}; + + for (const property of propertiesToPreserve) { + if (isDefined(previousNode[property])) { + Object.assign(preservedProperties, { + [property]: previousNode[property], + }); + } + } + + return { ...nextNode, ...preservedProperties }; }); return {