From 05d00e6604ddc2e096fc37b1edd7d766766ba91b Mon Sep 17 00:00:00 2001 From: Baptiste Devessier Date: Thu, 20 Feb 2025 17:12:03 +0100 Subject: [PATCH] Store the current flow definition in a state to not depend on a specific workflow version (#10352) This PR introduces a new Recoil state to store the flow. A few parts of the application need to know the definition of the current flow. Previously, we stored the workflow version's ID and fetched its definition with the `useWorkflowVersion` hook. However, we must use another strategy to visualize workflow runs. Indeed, we now store the definition of the workflow in the workflow run output when it is executed. This is useful for draft versions, which can change between the moment they were executed and the moment they are visualized. --- .../tests/workflow-run.spec.ts | 60 +++++++++++++++++++ .../WorkflowRunVisualizerContent.tsx | 5 +- .../modules/workflow/hooks/useFlowOrThrow.ts | 12 ++++ .../src/modules/workflow/states/flowState.ts | 13 ++++ .../workflow/states/workflowVersionIdState.ts | 6 -- .../utils/getStepDefinitionOrThrow.ts | 16 ++--- .../components/WorkflowDiagramEffect.tsx | 9 +++ .../WorkflowRunVisualizerEffect.tsx | 19 ++++-- .../WorkflowVersionVisualizerEffect.tsx | 17 ++++-- .../RightDrawerWorkflowEditStepContent.tsx | 6 +- .../RightDrawerWorkflowViewStep.tsx | 27 +++++---- .../RightDrawerWorkflowViewStepContent.tsx | 26 -------- .../components/WorkflowStepDetail.tsx | 24 ++++---- .../useAvailableVariablesInWorkflowStep.ts | 32 +++++----- 14 files changed, 180 insertions(+), 92 deletions(-) create mode 100644 packages/twenty-e2e-testing/tests/workflow-run.spec.ts create mode 100644 packages/twenty-front/src/modules/workflow/hooks/useFlowOrThrow.ts create mode 100644 packages/twenty-front/src/modules/workflow/states/flowState.ts delete mode 100644 packages/twenty-front/src/modules/workflow/states/workflowVersionIdState.ts delete mode 100644 packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowViewStepContent.tsx diff --git a/packages/twenty-e2e-testing/tests/workflow-run.spec.ts b/packages/twenty-e2e-testing/tests/workflow-run.spec.ts new file mode 100644 index 000000000..2ba3341d5 --- /dev/null +++ b/packages/twenty-e2e-testing/tests/workflow-run.spec.ts @@ -0,0 +1,60 @@ +import { expect } from '@playwright/test'; +import { test } from '../lib/fixtures/blank-workflow'; + +test('The workflow run visualizer shows the executed draft version without the last draft changes', async ({ + workflowVisualizer, + page, +}) => { + await workflowVisualizer.createInitialTrigger('manual'); + + const manualTriggerAvailabilitySelect = page.getByRole('button', { + name: 'When record(s) are selected', + }); + + await manualTriggerAvailabilitySelect.click(); + + const alwaysAvailableOption = page.getByText( + 'When no record(s) are selected', + ); + + await alwaysAvailableOption.click(); + + await workflowVisualizer.closeSidePanel(); + + const { createdStepId: firstStepId } = + await workflowVisualizer.createStep('create-record'); + + await workflowVisualizer.closeSidePanel(); + + const launchTestButton = page.getByRole('button', { name: 'Test' }); + + await launchTestButton.click(); + + const goToExecutionPageLink = page.getByRole('link', { + name: 'View execution details', + }); + const executionPageUrl = await goToExecutionPageLink.getAttribute('href'); + expect(executionPageUrl).not.toBeNull(); + + await workflowVisualizer.deleteStep(firstStepId); + + await page.goto(executionPageUrl!); + + const workflowRunName = page.getByText('Execution of v1'); + + await expect(workflowRunName).toBeVisible(); + + const flowTab = page.getByText('Flow', { exact: true }); + + await flowTab.click(); + + const executedFirstStepNode = workflowVisualizer.getStepNode(firstStepId); + + await expect(executedFirstStepNode).toBeVisible(); + + await executedFirstStepNode.click(); + + await expect( + workflowVisualizer.commandMenu.getByRole('textbox').first(), + ).toHaveValue('Create Record'); +}); diff --git a/packages/twenty-front/src/modules/workflow/components/WorkflowRunVisualizerContent.tsx b/packages/twenty-front/src/modules/workflow/components/WorkflowRunVisualizerContent.tsx index 903234264..49d4e58bf 100644 --- a/packages/twenty-front/src/modules/workflow/components/WorkflowRunVisualizerContent.tsx +++ b/packages/twenty-front/src/modules/workflow/components/WorkflowRunVisualizerContent.tsx @@ -16,10 +16,7 @@ export const WorkflowRunVisualizerContent = ({ return ( <> - + diff --git a/packages/twenty-front/src/modules/workflow/hooks/useFlowOrThrow.ts b/packages/twenty-front/src/modules/workflow/hooks/useFlowOrThrow.ts new file mode 100644 index 000000000..99fd038a8 --- /dev/null +++ b/packages/twenty-front/src/modules/workflow/hooks/useFlowOrThrow.ts @@ -0,0 +1,12 @@ +import { flowState } from '@/workflow/states/flowState'; +import { useRecoilValue } from 'recoil'; +import { isDefined } from 'twenty-shared'; + +export const useFlowOrThrow = () => { + const flow = useRecoilValue(flowState); + if (!isDefined(flow)) { + throw new Error('Expected the flow to be defined'); + } + + return flow; +}; diff --git a/packages/twenty-front/src/modules/workflow/states/flowState.ts b/packages/twenty-front/src/modules/workflow/states/flowState.ts new file mode 100644 index 000000000..21216f77a --- /dev/null +++ b/packages/twenty-front/src/modules/workflow/states/flowState.ts @@ -0,0 +1,13 @@ +import { WorkflowAction, WorkflowTrigger } from '@/workflow/types/Workflow'; +import { createState } from '@ui/utilities/state/utils/createState'; + +export const flowState = createState< + | { + trigger: WorkflowTrigger | null; + steps: WorkflowAction[] | null; + } + | undefined +>({ + key: 'flowState', + defaultValue: undefined, +}); diff --git a/packages/twenty-front/src/modules/workflow/states/workflowVersionIdState.ts b/packages/twenty-front/src/modules/workflow/states/workflowVersionIdState.ts deleted file mode 100644 index f72cb3933..000000000 --- a/packages/twenty-front/src/modules/workflow/states/workflowVersionIdState.ts +++ /dev/null @@ -1,6 +0,0 @@ -import { createState } from '@ui/utilities/state/utils/createState'; - -export const workflowVersionIdState = createState({ - key: 'workflowVersionIdState', - defaultValue: undefined, -}); diff --git a/packages/twenty-front/src/modules/workflow/utils/getStepDefinitionOrThrow.ts b/packages/twenty-front/src/modules/workflow/utils/getStepDefinitionOrThrow.ts index 74fe0bbea..6aa897936 100644 --- a/packages/twenty-front/src/modules/workflow/utils/getStepDefinitionOrThrow.ts +++ b/packages/twenty-front/src/modules/workflow/utils/getStepDefinitionOrThrow.ts @@ -1,17 +1,19 @@ -import { WorkflowVersion } from '@/workflow/types/Workflow'; +import { WorkflowAction, WorkflowTrigger } from '@/workflow/types/Workflow'; import { findStepPosition } from '@/workflow/utils/findStepPosition'; import { TRIGGER_STEP_ID } from '@/workflow/workflow-trigger/constants/TriggerStepId'; import { isDefined } from 'twenty-shared'; export const getStepDefinitionOrThrow = ({ stepId, - workflowVersion, + trigger, + steps, }: { stepId: string; - workflowVersion: WorkflowVersion; + trigger: WorkflowTrigger | null; + steps: Array | null; }) => { if (stepId === TRIGGER_STEP_ID) { - if (!isDefined(workflowVersion.trigger)) { + if (!isDefined(trigger)) { return { type: 'trigger', definition: undefined, @@ -20,18 +22,18 @@ export const getStepDefinitionOrThrow = ({ return { type: 'trigger', - definition: workflowVersion.trigger, + definition: trigger, } as const; } - if (!isDefined(workflowVersion.steps)) { + if (!isDefined(steps)) { throw new Error( 'Malformed workflow version: missing steps information; be sure to create at least one step before trying to edit one', ); } const selectedNodePosition = findStepPosition({ - steps: workflowVersion.steps, + steps, stepId: stepId, }); if (!isDefined(selectedNodePosition)) { 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 a85807c22..f781581fe 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 @@ -1,4 +1,5 @@ import { getSnapshotValue } from '@/ui/utilities/state/utils/getSnapshotValue'; +import { flowState } from '@/workflow/states/flowState'; import { workflowLastCreatedStepIdState } from '@/workflow/states/workflowLastCreatedStepIdState'; import { WorkflowVersion, @@ -20,6 +21,7 @@ export const WorkflowDiagramEffect = ({ workflowWithCurrentVersion: WorkflowWithCurrentVersion | undefined; }) => { const setWorkflowDiagram = useSetRecoilState(workflowDiagramState); + const setFlow = useSetRecoilState(flowState); const computeAndMergeNewWorkflowDiagram = useRecoilCallback( ({ snapshot, set }) => { @@ -67,14 +69,21 @@ export const WorkflowDiagramEffect = ({ useEffect(() => { const currentVersion = workflowWithCurrentVersion?.currentVersion; if (!isDefined(currentVersion)) { + setFlow(undefined); setWorkflowDiagram(undefined); return; } + setFlow({ + trigger: currentVersion.trigger, + steps: currentVersion.steps, + }); + computeAndMergeNewWorkflowDiagram(currentVersion); }, [ computeAndMergeNewWorkflowDiagram, + setFlow, setWorkflowDiagram, workflowWithCurrentVersion?.currentVersion, ]); diff --git a/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowRunVisualizerEffect.tsx b/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowRunVisualizerEffect.tsx index 23f3be1e9..f105b0493 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowRunVisualizerEffect.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowRunVisualizerEffect.tsx @@ -1,4 +1,4 @@ -import { workflowVersionIdState } from '@/workflow/states/workflowVersionIdState'; +import { flowState } from '@/workflow/states/flowState'; import { WorkflowRun } from '@/workflow/types/Workflow'; import { workflowDiagramState } from '@/workflow/workflow-diagram/states/workflowDiagramState'; import { generateWorkflowRunDiagram } from '@/workflow/workflow-diagram/utils/generateWorkflowRunDiagram'; @@ -7,18 +7,25 @@ import { useSetRecoilState } from 'recoil'; import { isDefined } from 'twenty-shared'; export const WorkflowRunVisualizerEffect = ({ - workflowVersionId, workflowRun, }: { - workflowVersionId: string; workflowRun: WorkflowRun; }) => { - const setWorkflowVersionId = useSetRecoilState(workflowVersionIdState); + const setFlow = useSetRecoilState(flowState); const setWorkflowDiagram = useSetRecoilState(workflowDiagramState); useEffect(() => { - setWorkflowVersionId(workflowVersionId); - }, [setWorkflowVersionId, workflowVersionId]); + if (!isDefined(workflowRun.output)) { + setFlow(undefined); + + return; + } + + setFlow({ + trigger: workflowRun.output.flow.trigger, + steps: workflowRun.output.flow.steps, + }); + }, [setFlow, workflowRun.output]); useEffect(() => { if (!isDefined(workflowRun.output)) { diff --git a/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowVersionVisualizerEffect.tsx b/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowVersionVisualizerEffect.tsx index 3d5a3e4c1..47c0024fe 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowVersionVisualizerEffect.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowVersionVisualizerEffect.tsx @@ -1,5 +1,5 @@ import { useWorkflowVersion } from '@/workflow/hooks/useWorkflowVersion'; -import { workflowVersionIdState } from '@/workflow/states/workflowVersionIdState'; +import { flowState } from '@/workflow/states/flowState'; import { workflowDiagramState } from '@/workflow/workflow-diagram/states/workflowDiagramState'; import { getWorkflowVersionDiagram } from '@/workflow/workflow-diagram/utils/getWorkflowVersionDiagram'; import { markLeafNodes } from '@/workflow/workflow-diagram/utils/markLeafNodes'; @@ -14,12 +14,21 @@ export const WorkflowVersionVisualizerEffect = ({ }) => { const workflowVersion = useWorkflowVersion(workflowVersionId); - const setWorkflowVersionId = useSetRecoilState(workflowVersionIdState); + const setFlow = useSetRecoilState(flowState); const setWorkflowDiagram = useSetRecoilState(workflowDiagramState); useEffect(() => { - setWorkflowVersionId(workflowVersionId); - }, [setWorkflowVersionId, workflowVersionId]); + if (!isDefined(workflowVersion)) { + setFlow(undefined); + + return; + } + + setFlow({ + trigger: workflowVersion.trigger, + steps: workflowVersion.steps, + }); + }, [setFlow, workflowVersion]); useEffect(() => { if (!isDefined(workflowVersion)) { diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowEditStepContent.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowEditStepContent.tsx index 37aadc98d..4dfd8183b 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowEditStepContent.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowEditStepContent.tsx @@ -1,3 +1,4 @@ +import { useFlowOrThrow } from '@/workflow/hooks/useFlowOrThrow'; import { WorkflowWithCurrentVersion } from '@/workflow/types/Workflow'; import { workflowSelectedNodeState } from '@/workflow/workflow-diagram/states/workflowSelectedNodeState'; import { WorkflowStepDetail } from '@/workflow/workflow-steps/components/WorkflowStepDetail'; @@ -11,6 +12,8 @@ export const RightDrawerWorkflowEditStepContent = ({ }: { workflow: WorkflowWithCurrentVersion; }) => { + const flow = useFlowOrThrow(); + const workflowSelectedNode = useRecoilValue(workflowSelectedNodeState); if (!isDefined(workflowSelectedNode)) { throw new Error( @@ -26,7 +29,8 @@ export const RightDrawerWorkflowEditStepContent = ({ return ( diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowViewStep.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowViewStep.tsx index c1f6540cb..a029adf75 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowViewStep.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowViewStep.tsx @@ -1,22 +1,25 @@ -import { useWorkflowVersion } from '@/workflow/hooks/useWorkflowVersion'; -import { workflowVersionIdState } from '@/workflow/states/workflowVersionIdState'; -import { RightDrawerWorkflowViewStepContent } from '@/workflow/workflow-steps/components/RightDrawerWorkflowViewStepContent'; +import { useFlowOrThrow } from '@/workflow/hooks/useFlowOrThrow'; +import { workflowSelectedNodeState } from '@/workflow/workflow-diagram/states/workflowSelectedNodeState'; +import { WorkflowStepDetail } from '@/workflow/workflow-steps/components/WorkflowStepDetail'; import { useRecoilValue } from 'recoil'; import { isDefined } from 'twenty-shared'; export const RightDrawerWorkflowViewStep = () => { - const workflowVersionId = useRecoilValue(workflowVersionIdState); - if (!isDefined(workflowVersionId)) { - throw new Error('Expected a workflow version id'); - } + const flow = useFlowOrThrow(); - const workflowVersion = useWorkflowVersion(workflowVersionId); - - if (!isDefined(workflowVersion)) { - return null; + const workflowSelectedNode = useRecoilValue(workflowSelectedNodeState); + if (!isDefined(workflowSelectedNode)) { + throw new Error( + 'Expected a node to be selected. Selecting a node is mandatory to view its details.', + ); } return ( - + ); }; diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowViewStepContent.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowViewStepContent.tsx deleted file mode 100644 index c7a635664..000000000 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/components/RightDrawerWorkflowViewStepContent.tsx +++ /dev/null @@ -1,26 +0,0 @@ -import { WorkflowVersion } from '@/workflow/types/Workflow'; -import { workflowSelectedNodeState } from '@/workflow/workflow-diagram/states/workflowSelectedNodeState'; -import { WorkflowStepDetail } from '@/workflow/workflow-steps/components/WorkflowStepDetail'; -import { useRecoilValue } from 'recoil'; -import { isDefined } from 'twenty-shared'; - -export const RightDrawerWorkflowViewStepContent = ({ - workflowVersion, -}: { - workflowVersion: WorkflowVersion; -}) => { - const workflowSelectedNode = useRecoilValue(workflowSelectedNodeState); - if (!isDefined(workflowSelectedNode)) { - throw new Error( - 'Expected a node to be selected. Selecting a node is mandatory to edit it.', - ); - } - - return ( - - ); -}; diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/components/WorkflowStepDetail.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/components/WorkflowStepDetail.tsx index 373570359..ed9b8af02 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/components/WorkflowStepDetail.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/components/WorkflowStepDetail.tsx @@ -1,8 +1,4 @@ -import { - WorkflowAction, - WorkflowTrigger, - WorkflowVersion, -} from '@/workflow/types/Workflow'; +import { WorkflowAction, WorkflowTrigger } from '@/workflow/types/Workflow'; import { assertUnreachable } from '@/workflow/utils/assertUnreachable'; import { getStepDefinitionOrThrow } from '@/workflow/utils/getStepDefinitionOrThrow'; import { WorkflowEditActionFormCreateRecord } from '@/workflow/workflow-steps/workflow-actions/components/WorkflowEditActionFormCreateRecord'; @@ -25,30 +21,34 @@ const WorkflowEditActionFormServerlessFunction = lazy(() => })), ); -type WorkflowStepDetailProps = +type WorkflowStepDetailProps = { + stepId: string; + trigger: WorkflowTrigger | null; + steps: Array | null; +} & ( | { - stepId: string; - workflowVersion: WorkflowVersion; readonly: true; onTriggerUpdate?: undefined; onActionUpdate?: undefined; } | { stepId: string; - workflowVersion: WorkflowVersion; readonly?: false; onTriggerUpdate: (trigger: WorkflowTrigger) => void; onActionUpdate: (action: WorkflowAction) => void; - }; + } +); export const WorkflowStepDetail = ({ stepId, - workflowVersion, + trigger, + steps, ...props }: WorkflowStepDetailProps) => { const stepDefinition = getStepDefinitionOrThrow({ stepId, - workflowVersion, + trigger, + steps, }); if (!isDefined(stepDefinition) || !isDefined(stepDefinition.definition)) { return null; diff --git a/packages/twenty-front/src/modules/workflow/workflow-variables/hooks/useAvailableVariablesInWorkflowStep.ts b/packages/twenty-front/src/modules/workflow/workflow-variables/hooks/useAvailableVariablesInWorkflowStep.ts index 233cfa25c..9a1de459e 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-variables/hooks/useAvailableVariablesInWorkflowStep.ts +++ b/packages/twenty-front/src/modules/workflow/workflow-variables/hooks/useAvailableVariablesInWorkflowStep.ts @@ -1,3 +1,4 @@ +import { useFlowOrThrow } from '@/workflow/hooks/useFlowOrThrow'; import { useWorkflowWithCurrentVersion } from '@/workflow/hooks/useWorkflowWithCurrentVersion'; import { workflowIdState } from '@/workflow/states/workflowIdState'; import { getStepDefinitionOrThrow } from '@/workflow/utils/getStepDefinitionOrThrow'; @@ -24,27 +25,32 @@ export const useAvailableVariablesInWorkflowStep = ({ const workflowId = useRecoilValue(workflowIdState); const workflow = useWorkflowWithCurrentVersion(workflowId); const workflowSelectedNode = useRecoilValue(workflowSelectedNodeState); + const flow = useFlowOrThrow(); if (!isDefined(workflowSelectedNode) || !isDefined(workflow)) { return []; } + const trigger = flow.trigger; + const steps = flow.steps; + const stepDefinition = getStepDefinitionOrThrow({ stepId: workflowSelectedNode, - workflowVersion: workflow.currentVersion, + trigger, + steps, }); if ( !isDefined(stepDefinition) || stepDefinition.type === 'trigger' || - !isDefined(workflow.currentVersion.steps) + !isDefined(steps) ) { return []; } const previousSteps = []; - for (const step of workflow.currentVersion.steps) { + for (const step of steps) { if (step.id === workflowSelectedNode) { break; } @@ -54,34 +60,32 @@ export const useAvailableVariablesInWorkflowStep = ({ const result = []; const filteredTriggerOutputSchema = filterOutputSchema( - workflow.currentVersion.trigger?.settings?.outputSchema as - | OutputSchema - | undefined, + trigger?.settings?.outputSchema as OutputSchema | undefined, objectNameSingularToSelect, ); if ( - isDefined(workflow.currentVersion.trigger) && + isDefined(trigger) && isDefined(filteredTriggerOutputSchema) && !isEmptyObject(filteredTriggerOutputSchema) ) { const triggerIconKey = - workflow.currentVersion.trigger.type === 'DATABASE_EVENT' + trigger.type === 'DATABASE_EVENT' ? getTriggerIcon({ - type: workflow.currentVersion.trigger.type, + type: trigger.type, eventName: splitWorkflowTriggerEventName( - workflow.currentVersion.trigger.settings?.eventName, + trigger.settings?.eventName, ).event, }) : getTriggerIcon({ - type: workflow.currentVersion.trigger.type, + type: trigger.type, }); result.push({ id: 'trigger', - name: isDefined(workflow.currentVersion.trigger.name) - ? workflow.currentVersion.trigger.name - : getTriggerStepName(workflow.currentVersion.trigger), + name: isDefined(trigger.name) + ? trigger.name + : getTriggerStepName(trigger), icon: triggerIconKey, outputSchema: filteredTriggerOutputSchema, });