Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,23 +1,18 @@
import { AdvancedFilterCommandMenuColumn } from '@/object-record/advanced-filter/command-menu/components/AdvancedFilterCommandMenuColumn';
import { useRecoilComponentValue } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValue';
import { WorkflowStepFilterFieldSelect } from '@/workflow/workflow-steps/filters/components/WorkflowStepFilterFieldSelect';
import { WorkflowStepFilterLogicalOperatorCell } from '@/workflow/workflow-steps/filters/components/WorkflowStepFilterLogicalOperatorCell';
import { WorkflowStepFilterOperandSelect } from '@/workflow/workflow-steps/filters/components/WorkflowStepFilterOperandSelect';
import { WorkflowStepFilterOptionsDropdown } from '@/workflow/workflow-steps/filters/components/WorkflowStepFilterOptionsDropdown';
import { WorkflowStepFilterValueInput } from '@/workflow/workflow-steps/filters/components/WorkflowStepFilterValueInput';
import { useChildStepFiltersAndChildStepFilterGroups } from '@/workflow/workflow-steps/filters/hooks/useChildStepFiltersAndChildStepFilterGroups';
import { WorkflowStepFilterContext } from '@/workflow/workflow-steps/filters/states/context/WorkflowStepFilterContext';
import { currentStepFilterGroupsComponentState } from '@/workflow/workflow-steps/filters/states/currentStepFilterGroupsComponentState';
import styled from '@emotion/styled';
import { useContext } from 'react';
import { type StepFilter, type StepFilterGroup } from 'twenty-shared/types';
import { isDefined } from 'twenty-shared/utils';

type WorkflowStepFilterColumnProps = {
stepFilterGroup: StepFilterGroup;
stepFilter: StepFilter;
stepFilterIndex: number;
isIfBranch?: boolean;
firstFilterLabel?: string;
};

Expand All @@ -32,32 +27,11 @@ export const WorkflowStepFilterColumn = ({
stepFilterGroup,
stepFilter,
stepFilterIndex,
isIfBranch,
firstFilterLabel,
}: WorkflowStepFilterColumnProps) => {
const { readonly } = useContext(WorkflowStepFilterContext);

const stepFilterGroups = useRecoilComponentValue(
currentStepFilterGroupsComponentState,
);

const rootStepFilterGroup = stepFilterGroups?.find(
(filterGroup) => !isDefined(filterGroup.parentStepFilterGroupId),
);

const { childStepFilters, childStepFilterGroups } =
useChildStepFiltersAndChildStepFilterGroups({
stepFilterGroupId: rootStepFilterGroup?.id ?? '',
});

const isLastFilterInIfBranch =
isIfBranch &&
isDefined(rootStepFilterGroup) &&
stepFilter.stepFilterGroupId === rootStepFilterGroup.id &&
childStepFilters.length === 1 &&
childStepFilterGroups.length === 0;

const shouldShowDropdown = !readonly && !isLastFilterInIfBranch;
const shouldShowDropdown = !readonly;

return (
<AdvancedFilterCommandMenuColumn>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,8 @@ import { useRecoilComponentValue } from '@/ui/utilities/state/component-state/ho
import { useGetUpdatableWorkflowVersionOrThrow } from '@/workflow/hooks/useGetUpdatableWorkflowVersionOrThrow';
import { useWorkflowWithCurrentVersion } from '@/workflow/hooks/useWorkflowWithCurrentVersion';
import { workflowVisualizerWorkflowIdComponentState } from '@/workflow/states/workflowVisualizerWorkflowIdComponentState';
import { type WorkflowIfElseAction } from '@/workflow/types/Workflow';
import { useDeleteWorkflowVersionStep } from '@/workflow/workflow-steps/hooks/useDeleteWorkflowVersionStep';
import { useResetWorkflowAiAgentPermissionsStateOnCommandMenuClose } from '@/workflow/workflow-steps/workflow-actions/ai-agent-action/hooks/useResetWorkflowAiAgentPermissionsStateOnCommandMenuClose';
import { getEmptyChildStepIds } from '@/workflow/workflow-steps/workflow-actions/if-else-action/utils/getEmptyChildStepIds';
import { useStepsOutputSchema } from '@/workflow/workflow-variables/hooks/useStepsOutputSchema';
import { isDefined } from 'twenty-shared/utils';

Expand All @@ -32,31 +30,6 @@ export const useDeleteStep = () => {
? steps.find((step) => step.id === stepId)
: undefined;

if (
isDefined(stepToDelete) &&
isDefined(steps) &&
stepToDelete.type === 'IF_ELSE'
) {
const emptyChildStepIds = getEmptyChildStepIds({
ifElseAction: stepToDelete as WorkflowIfElseAction,
allSteps: steps,
});

for (const emptyChildStepId of emptyChildStepIds) {
await deleteWorkflowVersionStep({
workflowVersionId,
stepId: emptyChildStepId,
});
}

if (emptyChildStepIds.length > 0) {
deleteStepsOutputSchema({
stepIds: emptyChildStepIds,
workflowVersionId,
});
}
}

await deleteWorkflowVersionStep({
workflowVersionId,
stepId,
Comment on lines 30 to 35

This comment was marked as outdated.

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { useApolloCoreClient } from '@/object-metadata/hooks/useApolloCoreClient';
import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
import { useFindOneRecordQuery } from '@/object-record/hooks/useFindOneRecordQuery';
import { DELETE_WORKFLOW_VERSION_STEP } from '@/workflow/graphql/mutations/deleteWorkflowVersionStep';
import { useUpdateWorkflowVersionCache } from '@/workflow/workflow-steps/hooks/useUpdateWorkflowVersionCache';
import { useMutation } from '@apollo/client';
Expand All @@ -13,6 +15,11 @@ export const useDeleteWorkflowVersionStep = () => {

const { updateWorkflowVersionCache } = useUpdateWorkflowVersionCache();

const { findOneRecordQuery: findOneWorkflowVersionQuery } =
useFindOneRecordQuery({
objectNameSingular: CoreObjectNameSingular.WorkflowVersion,
});

const [mutate] = useMutation<
DeleteWorkflowVersionStepMutation,
DeleteWorkflowVersionStepMutationVariables
Expand All @@ -23,7 +30,16 @@ export const useDeleteWorkflowVersionStep = () => {
const deleteWorkflowVersionStep = async (
input: DeleteWorkflowVersionStepInput,
) => {
const result = await mutate({ variables: { input } });
const result = await mutate({
variables: { input },
awaitRefetchQueries: true,
refetchQueries: [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we stop removing the empty node, we can remove that refetch logic. Updating cache will be enough

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixMalfait suggested that we should remove the empty nodes. LMK what should we do @thomtrp ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's keep it for now

{
query: findOneWorkflowVersionQuery,
variables: { objectRecordId: input.workflowVersionId },
},
],
});

const workflowVersionStepChanges = result?.data?.deleteWorkflowVersionStep;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,45 +58,70 @@ export const WorkflowIfElseBranchEditor = ({

const isIfBranch = branchIndex === 0;

const isLastFilterInIfBranch = (stepFilter: {
stepFilterGroupId: string;
}): boolean => {
const childStepFilters = childStepFiltersAndChildStepFilterGroups.filter(
(child) => !isStepFilterGroupChildAStepFilterGroup(child),
);
const childStepFilterGroups =
childStepFiltersAndChildStepFilterGroups.filter(
isStepFilterGroupChildAStepFilterGroup,
);

return (
isIfBranch &&
isDefined(branchFilterGroup) &&
stepFilter.stepFilterGroupId === branchFilterGroup.id &&
childStepFilters.length === 1 &&
childStepFilterGroups.length === 0
);
};

return (
<WorkflowStepFilterContext.Provider
value={{
stepId: action.id,
readonly,
onFilterSettingsUpdate,
}}
>
<StyledBranchContainer>
<StyledFiltersContainer>
{isDefined(branchFilterGroup) &&
childStepFiltersAndChildStepFilterGroups.map(
(stepFilterGroupChild, stepFilterGroupChildIndex) =>
isStepFilterGroupChildAStepFilterGroup(stepFilterGroupChild) ? (
<StyledBranchContainer>
<StyledFiltersContainer>
{isDefined(branchFilterGroup) &&
childStepFiltersAndChildStepFilterGroups.map(
(stepFilterGroupChild, stepFilterGroupChildIndex) => (
<WorkflowStepFilterContext.Provider
key={stepFilterGroupChild.id}
value={{
stepId: action.id,
readonly: isStepFilterGroupChildAStepFilterGroup(
stepFilterGroupChild,
)
? readonly
: readonly || isLastFilterInIfBranch(stepFilterGroupChild),
onFilterSettingsUpdate,
}}
>
{isStepFilterGroupChildAStepFilterGroup(
stepFilterGroupChild,
) ? (
<WorkflowStepFilterGroupColumn
key={stepFilterGroupChild.id}
parentStepFilterGroup={branchFilterGroup}
stepFilterGroup={stepFilterGroupChild}
stepFilterGroupIndex={stepFilterGroupChildIndex}
/>
) : (
<WorkflowStepFilterColumn
key={stepFilterGroupChild.id}
stepFilterGroup={branchFilterGroup}
stepFilter={stepFilterGroupChild}
stepFilterIndex={stepFilterGroupChildIndex}
isIfBranch={isIfBranch}
firstFilterLabel={i18n._(branchLabel)}
/>
),
)}
</StyledFiltersContainer>
)}
</WorkflowStepFilterContext.Provider>
),
)}
</StyledFiltersContainer>

{!readonly && isDefined(branchFilterGroup) && (
<WorkflowStepFilterAddFilterRuleSelect
stepFilterGroup={branchFilterGroup}
/>
)}
</StyledBranchContainer>
</WorkflowStepFilterContext.Provider>
{!readonly && isDefined(branchFilterGroup) && (
<WorkflowStepFilterAddFilterRuleSelect
stepFilterGroup={branchFilterGroup}
/>
)}
</StyledBranchContainer>
);
};
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { isDefined } from 'twenty-shared/utils';
import { TRIGGER_STEP_ID } from 'twenty-shared/workflow';
import { TRIGGER_STEP_ID, type StepIfElseBranch } from 'twenty-shared/workflow';

import { isWorkflowEmptyAction } from 'src/modules/workflow/workflow-executor/workflow-actions/empty/guards/is-workflow-empty-action.guard';
import { isWorkflowIfElseAction } from 'src/modules/workflow/workflow-executor/workflow-actions/if-else/guards/is-workflow-if-else-action.guard';
import {
type WorkflowAction,
WorkflowActionType,
type WorkflowAction,
} from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type';
import { type WorkflowTrigger } from 'src/modules/workflow/workflow-trigger/types/workflow-trigger.type';

Expand All @@ -29,6 +31,22 @@ const computeUpdatedNextStepIds = ({
];
};

export const getEmptyChildStepIdsForIfElse = ({
branches,
allSteps,
}: {
branches: StepIfElseBranch[];
allSteps: WorkflowAction[];
}): string[] => {
const childStepIds = branches.flatMap((branch) => branch.nextStepIds);

return childStepIds.filter((childStepId) => {
const childStep = allSteps.find((step) => step.id === childStepId);

return isDefined(childStep) && isWorkflowEmptyAction(childStep);
});
};

export const removeStep = ({
existingTrigger,
existingSteps,
Expand All @@ -48,17 +66,66 @@ export const removeStep = ({
};
}

const stepToDelete = existingSteps?.find(
(step) => step.id === stepIdToDelete,
);

let emptyChildStepIds: string[] = [];

if (
isDefined(stepToDelete) &&
isWorkflowIfElseAction(stepToDelete) &&
isDefined(existingSteps)
) {
emptyChildStepIds = getEmptyChildStepIdsForIfElse({
branches: stepToDelete.settings.input.branches,
allSteps: existingSteps,
});
}

const allRemovedStepIds = [stepIdToDelete, ...emptyChildStepIds];

const allStepToDeleteChildrenIds = [
...(stepToDeleteChildrenIds || []),
...emptyChildStepIds,
];

This comment was marked as outdated.


const updatedSteps =
existingSteps
?.filter((step) => step.id !== stepIdToDelete)
?.filter((step) => !allRemovedStepIds.includes(step.id))
.map((step) => {
if (
step.type === WorkflowActionType.IF_ELSE &&
isWorkflowIfElseAction(step)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior looks huge and is not consistent with iterators. For iterators, we let the user remove the empty node. The user can delete it by himself. I think we can do that for now and see if user complain. Wdyt?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok let's keep it for now. We'll see if we do the same for iterators at some point

) {
const updatedBranches = step.settings.input.branches.map(
(branch) => ({
...branch,
nextStepIds: branch.nextStepIds.filter(
(id) => !allRemovedStepIds.includes(id),
),
}),
);

return {
...step,
settings: {
...step.settings,
input: {
...step.settings.input,
branches: updatedBranches,
},
},
};
}

if (step.nextStepIds?.includes(stepIdToDelete)) {
return {
...step,
nextStepIds: computeUpdatedNextStepIds({
existingNextStepIds: step.nextStepIds,
stepIdToRemove: stepIdToDelete,
stepToDeleteChildrenIds: stepToDeleteChildrenIds,
stepToDeleteChildrenIds: allStepToDeleteChildrenIds,
}),
};
}
Expand All @@ -77,7 +144,7 @@ export const removeStep = ({
initialLoopStepIds: computeUpdatedNextStepIds({
existingNextStepIds: step.settings.input.initialLoopStepIds,
stepIdToRemove: stepIdToDelete,
stepToDeleteChildrenIds: stepToDeleteChildrenIds,
stepToDeleteChildrenIds: allStepToDeleteChildrenIds,
}),
},
},
Expand All @@ -96,7 +163,7 @@ export const removeStep = ({
nextStepIds: computeUpdatedNextStepIds({
existingNextStepIds: existingTrigger.nextStepIds,
stepIdToRemove: stepIdToDelete,
stepToDeleteChildrenIds,
stepToDeleteChildrenIds: allStepToDeleteChildrenIds,
}),
};
}
Expand All @@ -105,6 +172,6 @@ export const removeStep = ({
return {
updatedSteps,
updatedTrigger,
removedStepIds: [stepIdToDelete],
removedStepIds: allRemovedStepIds,
};
};
Loading