fix: eliminate workflow editing flicker on active-to-draft transitions (#21176)
## 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
This commit is contained in:
@@ -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 };
|
||||
};
|
||||
@@ -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<Workflow, 'versions'> & {
|
||||
export const useWorkflowWithCurrentVersion = (
|
||||
workflowId: string | undefined,
|
||||
): WorkflowWithCurrentVersion | undefined => {
|
||||
const shouldWorkflowRefetchRequest = useAtomFamilyStateValue(
|
||||
shouldWorkflowRefetchRequestFamilyState,
|
||||
workflowId ?? '',
|
||||
);
|
||||
const setShouldWorkflowRefetchRequest = useSetAtomFamilyState(
|
||||
shouldWorkflowRefetchRequestFamilyState,
|
||||
workflowId ?? '',
|
||||
);
|
||||
|
||||
const { record: workflow, refetch: refetchWorkflow } =
|
||||
useFindOneRecord<WorkflowWithAllVersions>({
|
||||
objectNameSingular: CoreObjectNameSingular.Workflow,
|
||||
objectRecordId: workflowId,
|
||||
recordGqlFields: {
|
||||
const { record: workflow } = useFindOneRecord<WorkflowWithAllVersions>({
|
||||
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<WorkflowVersion>(
|
||||
{
|
||||
objectNameSingular: CoreObjectNameSingular.WorkflowVersion,
|
||||
objectRecordId: currentVersionWithoutSteps?.id,
|
||||
skip: !isDefined(currentVersionWithoutSteps?.id),
|
||||
objectRecordId: currentVersionId,
|
||||
skip: !isDefined(currentVersionId),
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
import { atom } from 'jotai';
|
||||
|
||||
export const lastDiscardedDraftIdState = atom<string | undefined>(undefined);
|
||||
lastDiscardedDraftIdState.debugLabel = 'lastDiscardedDraftIdState';
|
||||
+13
-3
@@ -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<string>();
|
||||
|
||||
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(() => {
|
||||
|
||||
-3
@@ -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,
|
||||
},
|
||||
],
|
||||
}
|
||||
|
||||
+21
-10
@@ -2,8 +2,9 @@ import {
|
||||
type WorkflowDiagram,
|
||||
type WorkflowDiagramNode,
|
||||
} from '@/workflow/workflow-diagram/types/WorkflowDiagram';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
const nodePropertiesToPreserve: Array<keyof WorkflowDiagramNode> = [
|
||||
const BASE_PROPERTIES_TO_PRESERVE: Array<keyof WorkflowDiagramNode> = [
|
||||
'selected',
|
||||
'measured',
|
||||
];
|
||||
@@ -11,22 +12,32 @@ const nodePropertiesToPreserve: Array<keyof WorkflowDiagramNode> = [
|
||||
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<WorkflowDiagramNode>,
|
||||
);
|
||||
if (!isDefined(previousNode)) {
|
||||
return nextNode;
|
||||
}
|
||||
|
||||
return Object.assign(nodeWithPreservedProperties, nextNode);
|
||||
const preservedProperties: Partial<WorkflowDiagramNode> = {};
|
||||
|
||||
for (const property of propertiesToPreserve) {
|
||||
if (isDefined(previousNode[property])) {
|
||||
Object.assign(preservedProperties, {
|
||||
[property]: previousNode[property],
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return { ...nextNode, ...preservedProperties };
|
||||
});
|
||||
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user