fix(workflow): stop trigger/action filter Conditions from flashing on edit (#21952)
## Problem
Adding a condition to a database-event **trigger** (the new Conditions
section), the **Filter** action, or the **If/Else** action causes the
just-added condition to flash out and back in.
## Root cause
`WorkflowEditActionFilterBodyEffect` seeds the builder's local jotai
atoms from the persisted `defaultValue` through an effect that
**resynced whenever the live atoms differed from `defaultValue`** (the
atoms were in the effect deps and the equality check compared atoms vs
`defaultValue`).
A local edit writes the atoms **synchronously**, then persists through
an **async** mutation — and for an *active* workflow that mutation first
creates a draft version over the network. During that window the atoms
are ahead of the still-stale `defaultValue`, so the effect treated it as
"out of sync" and overwrote the edit back to the stale value, then wrote
it again once the save landed. That round-trip is the flash.
The resync existed for a real reason: the atoms are module-cached per
`instanceId` and persist across mounts, and the trigger shares a single
**constant** `instanceId` (`'trigger'`), so a previous trigger's filters
must be overwritten when a different one is opened. (This is also why
the `?? { stepFilterGroups: [], stepFilters: [] }` fallback was added in
#21868 — to reset builder state deterministically between trigger
edits.) So a naive "init-once" fix would reintroduce that stale-state
leak.
## Fix
Resync from `defaultValue` **only when `defaultValue` itself changes**,
tracked via the last-synced value in `useState` (not the live atoms).
This:
- never clobbers an in-flight local edit → no flash;
- still re-seeds when switching the trigger/action being edited → no
stale-state leak;
- preserves reflecting genuine external `defaultValue` changes.
The `hasInitialized*` flags are no longer needed and are removed (along
with the now-unused `stepId` prop on the effect).
## Tests
Adds a regression test covering: seeding from `defaultValue` on mount,
the **no-clobber-while-stale** invariant (the flash), and resync on a
genuine `defaultValue` change. Verified the no-clobber test **fails**
against the old "resync against live atoms" behavior and passes with the
fix.
## Verification
- `nx typecheck twenty-front` ✅
- `nx lint:diff-with-main twenty-front` ✅ (0 warnings / 0 errors)
- New unit test: 3 passing ✅
## Known residual / follow-up
On an *active* workflow, the first edit creates a draft version over the
network; making a second edit before that round-trip completes leaves a
narrow window where the optimistic echo of the first value could
momentarily win. Far narrower than the current flash-on-every-edit.
Eliminating it entirely (and resolving the still-open HIGH-severity
"constant `instanceId`" review flag from #21868) would mean giving the
trigger a unique `instanceId` per workflow version + a React `key` to
reset on remount — proposed as a separate, scoped follow-up.
https://claude.ai/code/session_01XnjtzFepMJX2VFwnQVcQbV
---
_Generated by [Claude
Code](https://claude.ai/code/session_01XnjtzFepMJX2VFwnQVcQbV)_
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/21952?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
This commit is contained in:
+22
-67
@@ -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<typeof stepFiltersConverted>(undefined);
|
||||
|
||||
const [lastSyncedStepFilterGroups, setLastSyncedStepFilterGroups] =
|
||||
useState<typeof stepFilterGroups>(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;
|
||||
|
||||
+1
-4
@@ -117,10 +117,7 @@ export const WorkflowStepFilterBuilder = ({
|
||||
>
|
||||
<WorkflowStepFilterBuilderConditions readonly={readonly} />
|
||||
</WorkflowStepFilterContext.Provider>
|
||||
<WorkflowEditActionFilterBodyEffect
|
||||
stepId={instanceId}
|
||||
defaultValue={defaultValue}
|
||||
/>
|
||||
<WorkflowEditActionFilterBodyEffect defaultValue={defaultValue} />
|
||||
</StepFilterGroupsComponentInstanceContext.Provider>
|
||||
</StepFiltersComponentInstanceContext.Provider>
|
||||
);
|
||||
|
||||
+160
@@ -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;
|
||||
}) => (
|
||||
<StepFiltersComponentInstanceContext.Provider value={{ instanceId }}>
|
||||
<StepFilterGroupsComponentInstanceContext.Provider value={{ instanceId }}>
|
||||
<WorkflowEditActionFilterBodyEffect defaultValue={value} />
|
||||
<ProbeEffect />
|
||||
</StepFilterGroupsComponentInstanceContext.Provider>
|
||||
</StepFiltersComponentInstanceContext.Provider>
|
||||
);
|
||||
|
||||
const utils = render(<Harness value={defaultValue} />);
|
||||
|
||||
return {
|
||||
captured,
|
||||
rerender: (value?: FilterSettingsWithPotentiallyDeprecatedOperand) =>
|
||||
utils.rerender(<Harness value={value} />),
|
||||
};
|
||||
};
|
||||
|
||||
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)]);
|
||||
});
|
||||
});
|
||||
+1
-22
@@ -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,
|
||||
]);
|
||||
|
||||
|
||||
-9
@@ -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<boolean, { stepId: string }>({
|
||||
key: 'hasInitializedCurrentStepFilterGroupsComponentFamilyState',
|
||||
defaultValue: false,
|
||||
componentInstanceContext: StepFilterGroupsComponentInstanceContext,
|
||||
});
|
||||
-9
@@ -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<boolean, { stepId: string }>({
|
||||
key: 'hasInitializedCurrentStepFiltersComponentFamilyState',
|
||||
defaultValue: false,
|
||||
componentInstanceContext: StepFiltersComponentInstanceContext,
|
||||
});
|
||||
-1
@@ -38,7 +38,6 @@ export const WorkflowEditActionIfElse = ({
|
||||
actionOptions={actionOptions}
|
||||
/>
|
||||
<WorkflowEditActionFilterBodyEffect
|
||||
stepId={action.id}
|
||||
defaultValue={{
|
||||
stepFilterGroups: action.settings.input.stepFilterGroups,
|
||||
stepFilters: action.settings.input.stepFilters,
|
||||
|
||||
Reference in New Issue
Block a user