diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/WorkflowEditActionFilterBodyEffect.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/WorkflowEditActionFilterBodyEffect.tsx index 80c6657808..861be00f60 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/WorkflowEditActionFilterBodyEffect.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/WorkflowEditActionFilterBodyEffect.tsx @@ -1,12 +1,8 @@ -import { useAtomComponentFamilyState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentFamilyState'; -import { useAtomComponentStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateValue'; import { useSetAtomComponentState } from '@/ui/utilities/state/jotai/hooks/useSetAtomComponentState'; import { currentStepFilterGroupsComponentState } from '@/workflow/workflow-steps/filters/states/currentStepFilterGroupsComponentState'; import { currentStepFiltersComponentState } from '@/workflow/workflow-steps/filters/states/currentStepFiltersComponentState'; -import { hasInitializedCurrentStepFilterGroupsComponentFamilyState } from '@/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFilterGroupsComponentFamilyState'; -import { hasInitializedCurrentStepFiltersComponentFamilyState } from '@/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFiltersComponentFamilyState'; import { type FilterSettingsWithPotentiallyDeprecatedOperand } from '@/workflow/workflow-steps/filters/types/FilterSettings'; -import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { convertViewFilterOperandToCoreOperand, isDefined, @@ -14,35 +10,10 @@ import { import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; export const WorkflowEditActionFilterBodyEffect = ({ - stepId, defaultValue, }: { - stepId: string; defaultValue?: FilterSettingsWithPotentiallyDeprecatedOperand; }) => { - const [ - hasInitializedCurrentStepFilters, - setHasInitializedCurrentStepFilters, - ] = useAtomComponentFamilyState( - hasInitializedCurrentStepFiltersComponentFamilyState, - { stepId }, - ); - - const [ - hasInitializedCurrentStepFilterGroups, - setHasInitializedCurrentStepFilterGroups, - ] = useAtomComponentFamilyState( - hasInitializedCurrentStepFilterGroupsComponentFamilyState, - { stepId }, - ); - - const currentStepFilters = useAtomComponentStateValue( - currentStepFiltersComponentState, - ); - const currentStepFilterGroups = useAtomComponentStateValue( - currentStepFilterGroupsComponentState, - ); - const setCurrentStepFilters = useSetAtomComponentState( currentStepFiltersComponentState, ); @@ -58,64 +29,48 @@ export const WorkflowEditActionFilterBodyEffect = ({ })); }, [defaultValue?.stepFilters]); + const stepFilterGroups = defaultValue?.stepFilterGroups; + + const [lastSyncedStepFilters, setLastSyncedStepFilters] = + useState(undefined); + + const [lastSyncedStepFilterGroups, setLastSyncedStepFilterGroups] = + useState(undefined); + useEffect(() => { if (!isDefined(stepFiltersConverted)) { return; } - if ( - hasInitializedCurrentStepFilters && - isDeeplyEqual(currentStepFilters, stepFiltersConverted) - ) { + if (isDeeplyEqual(lastSyncedStepFilters, stepFiltersConverted)) { return; } - setCurrentStepFilters(stepFiltersConverted ?? []); - - if (!hasInitializedCurrentStepFilters) { - setHasInitializedCurrentStepFilters(true); - } + setLastSyncedStepFilters(stepFiltersConverted); + setCurrentStepFilters(stepFiltersConverted); }, [ - setCurrentStepFilters, - hasInitializedCurrentStepFilters, - setHasInitializedCurrentStepFilters, stepFiltersConverted, - currentStepFilters, + lastSyncedStepFilters, + setCurrentStepFilters, + setLastSyncedStepFilters, ]); useEffect(() => { - if (!isDefined(defaultValue?.stepFilterGroups)) { + if (!isDefined(stepFilterGroups)) { return; } - if ( - !hasInitializedCurrentStepFilterGroups && - defaultValue.stepFilterGroups.length === 0 - ) { + if (isDeeplyEqual(lastSyncedStepFilterGroups, stepFilterGroups)) { return; } - if ( - hasInitializedCurrentStepFilterGroups && - isDeeplyEqual( - currentStepFilterGroups, - defaultValue.stepFilterGroups ?? [], - ) - ) { - return; - } - - setCurrentStepFilterGroups(defaultValue.stepFilterGroups ?? []); - - if (!hasInitializedCurrentStepFilterGroups) { - setHasInitializedCurrentStepFilterGroups(true); - } + setLastSyncedStepFilterGroups(stepFilterGroups); + setCurrentStepFilterGroups(stepFilterGroups); }, [ + stepFilterGroups, + lastSyncedStepFilterGroups, setCurrentStepFilterGroups, - hasInitializedCurrentStepFilterGroups, - setHasInitializedCurrentStepFilterGroups, - defaultValue?.stepFilterGroups, - currentStepFilterGroups, + setLastSyncedStepFilterGroups, ]); return null; diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/WorkflowStepFilterBuilder.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/WorkflowStepFilterBuilder.tsx index d9f925d66e..4f46a39f3e 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/WorkflowStepFilterBuilder.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/WorkflowStepFilterBuilder.tsx @@ -117,10 +117,7 @@ export const WorkflowStepFilterBuilder = ({ > - + ); diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/__tests__/WorkflowEditActionFilterBodyEffect.test.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/__tests__/WorkflowEditActionFilterBodyEffect.test.tsx new file mode 100644 index 0000000000..2c69b147c7 --- /dev/null +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/components/__tests__/WorkflowEditActionFilterBodyEffect.test.tsx @@ -0,0 +1,160 @@ +import { render } from '@testing-library/react'; +import { act } from 'react'; +import { + type StepFilter, + type StepFilterGroup, + StepLogicalOperator, + ViewFilterOperand, +} from 'twenty-shared/types'; +import { convertViewFilterOperandToCoreOperand } from 'twenty-shared/utils'; + +import { useSetAtomComponentState } from '@/ui/utilities/state/jotai/hooks/useSetAtomComponentState'; +import { useAtomComponentStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateValue'; +import { WorkflowEditActionFilterBodyEffect } from '@/workflow/workflow-steps/filters/components/WorkflowEditActionFilterBodyEffect'; +import { StepFilterGroupsComponentInstanceContext } from '@/workflow/workflow-steps/filters/states/context/StepFilterGroupsComponentInstanceContext'; +import { StepFiltersComponentInstanceContext } from '@/workflow/workflow-steps/filters/states/context/StepFiltersComponentInstanceContext'; +import { currentStepFilterGroupsComponentState } from '@/workflow/workflow-steps/filters/states/currentStepFilterGroupsComponentState'; +import { currentStepFiltersComponentState } from '@/workflow/workflow-steps/filters/states/currentStepFiltersComponentState'; +import { type FilterSettingsWithPotentiallyDeprecatedOperand } from '@/workflow/workflow-steps/filters/types/FilterSettings'; + +const makeStepFilterGroup = (id: string): StepFilterGroup => ({ + id, + logicalOperator: StepLogicalOperator.AND, +}); + +const makeStepFilter = (id: string, stepFilterGroupId: string): StepFilter => ({ + id, + type: 'unknown', + stepOutputKey: '', + operand: ViewFilterOperand.IS, + value: '', + stepFilterGroupId, + positionInStepFilterGroup: 0, +}); + +// `defaultValue` filters are stored with potentially deprecated operands and are +// normalized to core operands when synced into the atoms. +const toExpectedSyncedFilter = (filter: StepFilter) => ({ + ...filter, + operand: convertViewFilterOperandToCoreOperand(filter.operand), +}); + +type Captured = { + filters: StepFilter[]; + filterGroups: StepFilterGroup[]; + setFilters: (value: StepFilter[]) => void; + setFilterGroups: (value: StepFilterGroup[]) => void; +}; + +const renderBodyEffect = ( + instanceId: string, + defaultValue?: FilterSettingsWithPotentiallyDeprecatedOperand, +) => { + const captured: Captured = {} as Captured; + + const ProbeEffect = () => { + captured.filters = useAtomComponentStateValue( + currentStepFiltersComponentState, + ); + captured.filterGroups = useAtomComponentStateValue( + currentStepFilterGroupsComponentState, + ); + captured.setFilters = useSetAtomComponentState( + currentStepFiltersComponentState, + ); + captured.setFilterGroups = useSetAtomComponentState( + currentStepFilterGroupsComponentState, + ); + + return null; + }; + + const Harness = ({ + value, + }: { + value?: FilterSettingsWithPotentiallyDeprecatedOperand; + }) => ( + + + + + + + ); + + const utils = render(); + + return { + captured, + rerender: (value?: FilterSettingsWithPotentiallyDeprecatedOperand) => + utils.rerender(), + }; +}; + +describe('WorkflowEditActionFilterBodyEffect', () => { + it('seeds the local atoms from defaultValue on mount', () => { + const stepFilterGroup = makeStepFilterGroup('group-1'); + const stepFilter = makeStepFilter('filter-1', 'group-1'); + + const { captured } = renderBodyEffect('seed', { + stepFilterGroups: [stepFilterGroup], + stepFilters: [stepFilter], + }); + + expect(captured.filterGroups).toEqual([stepFilterGroup]); + expect(captured.filters).toEqual([toExpectedSyncedFilter(stepFilter)]); + }); + + // Regression test for the "Conditions" flash: when the user adds a condition, + // the local atoms are updated synchronously while the persisted value (passed + // as defaultValue) only catches up after an async save. The effect must not + // overwrite the local edit during that window. + it('does not overwrite a local edit while defaultValue is still stale', () => { + const { captured, rerender } = renderBodyEffect('no-clobber', { + stepFilterGroups: [], + stepFilters: [], + }); + + expect(captured.filters).toEqual([]); + expect(captured.filterGroups).toEqual([]); + + const editedGroup = makeStepFilterGroup('edited-group'); + const editedFilter = makeStepFilter('edited-filter', 'edited-group'); + + act(() => { + captured.setFilterGroups([editedGroup]); + captured.setFilters([editedFilter]); + }); + + expect(captured.filters).toEqual([editedFilter]); + expect(captured.filterGroups).toEqual([editedGroup]); + + // The save has not landed yet, so defaultValue is still the stale empty value. + act(() => { + rerender({ stepFilterGroups: [], stepFilters: [] }); + }); + + expect(captured.filters).toEqual([editedFilter]); + expect(captured.filterGroups).toEqual([editedGroup]); + }); + + it('resyncs the atoms when defaultValue actually changes', () => { + const { captured, rerender } = renderBodyEffect('resync', { + stepFilterGroups: [], + stepFilters: [], + }); + + const nextGroup = makeStepFilterGroup('group-2'); + const nextFilter = makeStepFilter('filter-2', 'group-2'); + + act(() => { + rerender({ + stepFilterGroups: [nextGroup], + stepFilters: [nextFilter], + }); + }); + + expect(captured.filterGroups).toEqual([nextGroup]); + expect(captured.filters).toEqual([toExpectedSyncedFilter(nextFilter)]); + }); +}); diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/hooks/useAddRootStepFilter.ts b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/hooks/useAddRootStepFilter.ts index ce49b056c4..ef02fe5c3c 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/hooks/useAddRootStepFilter.ts +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/hooks/useAddRootStepFilter.ts @@ -1,10 +1,7 @@ import { useAtomComponentStateCallbackState } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateCallbackState'; -import { useSetAtomComponentFamilyState } from '@/ui/utilities/state/jotai/hooks/useSetAtomComponentFamilyState'; import { WorkflowStepFilterContext } from '@/workflow/workflow-steps/filters/states/context/WorkflowStepFilterContext'; import { currentStepFilterGroupsComponentState } from '@/workflow/workflow-steps/filters/states/currentStepFilterGroupsComponentState'; import { currentStepFiltersComponentState } from '@/workflow/workflow-steps/filters/states/currentStepFiltersComponentState'; -import { hasInitializedCurrentStepFilterGroupsComponentFamilyState } from '@/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFilterGroupsComponentFamilyState'; -import { hasInitializedCurrentStepFiltersComponentFamilyState } from '@/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFiltersComponentFamilyState'; import { useStore } from 'jotai'; import { useCallback, useContext } from 'react'; import { @@ -16,9 +13,7 @@ import { import { v4 } from 'uuid'; export const useAddRootStepFilter = () => { - const { stepId, onFilterSettingsUpdate } = useContext( - WorkflowStepFilterContext, - ); + const { onFilterSettingsUpdate } = useContext(WorkflowStepFilterContext); const currentStepFilterGroups = useAtomComponentStateCallbackState( currentStepFilterGroupsComponentState, ); @@ -27,17 +22,6 @@ export const useAddRootStepFilter = () => { currentStepFiltersComponentState, ); - const setHasInitializedCurrentStepFilters = useSetAtomComponentFamilyState( - hasInitializedCurrentStepFiltersComponentFamilyState, - { stepId }, - ); - - const setHasInitializedCurrentStepFilterGroups = - useSetAtomComponentFamilyState( - hasInitializedCurrentStepFilterGroupsComponentFamilyState, - { stepId }, - ); - const store = useStore(); const addRootStepFilter = useCallback(() => { @@ -59,9 +43,6 @@ export const useAddRootStepFilter = () => { store.set(currentStepFilterGroups, [newStepFilterGroup]); store.set(currentStepFilters, [newStepFilter]); - setHasInitializedCurrentStepFilters(true); - setHasInitializedCurrentStepFilterGroups(true); - onFilterSettingsUpdate({ stepFilterGroups: [newStepFilterGroup], stepFilters: [newStepFilter], @@ -70,8 +51,6 @@ export const useAddRootStepFilter = () => { onFilterSettingsUpdate, currentStepFilterGroups, currentStepFilters, - setHasInitializedCurrentStepFilters, - setHasInitializedCurrentStepFilterGroups, store, ]); diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFilterGroupsComponentFamilyState.ts b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFilterGroupsComponentFamilyState.ts deleted file mode 100644 index 5db9dd821e..0000000000 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFilterGroupsComponentFamilyState.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { createAtomComponentFamilyState } from '@/ui/utilities/state/jotai/utils/createAtomComponentFamilyState'; -import { StepFilterGroupsComponentInstanceContext } from '@/workflow/workflow-steps/filters/states/context/StepFilterGroupsComponentInstanceContext'; - -export const hasInitializedCurrentStepFilterGroupsComponentFamilyState = - createAtomComponentFamilyState({ - key: 'hasInitializedCurrentStepFilterGroupsComponentFamilyState', - defaultValue: false, - componentInstanceContext: StepFilterGroupsComponentInstanceContext, - }); diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFiltersComponentFamilyState.ts b/packages/twenty-front/src/modules/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFiltersComponentFamilyState.ts deleted file mode 100644 index dd1147b1cf..0000000000 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/filters/states/hasInitializedCurrentStepFiltersComponentFamilyState.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { createAtomComponentFamilyState } from '@/ui/utilities/state/jotai/utils/createAtomComponentFamilyState'; -import { StepFiltersComponentInstanceContext } from '@/workflow/workflow-steps/filters/states/context/StepFiltersComponentInstanceContext'; - -export const hasInitializedCurrentStepFiltersComponentFamilyState = - createAtomComponentFamilyState({ - key: 'hasInitializedCurrentStepFiltersComponentFamilyState', - defaultValue: false, - componentInstanceContext: StepFiltersComponentInstanceContext, - }); diff --git a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/if-else-action/components/WorkflowEditActionIfElse.tsx b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/if-else-action/components/WorkflowEditActionIfElse.tsx index 8deefe0108..69a8459bf1 100644 --- a/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/if-else-action/components/WorkflowEditActionIfElse.tsx +++ b/packages/twenty-front/src/modules/workflow/workflow-steps/workflow-actions/if-else-action/components/WorkflowEditActionIfElse.tsx @@ -38,7 +38,6 @@ export const WorkflowEditActionIfElse = ({ actionOptions={actionOptions} />