-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Delete workflow step #7373
Delete workflow step #7373
Conversation
…undefined step definition that can occur when deleting a step and the drawer is opened
const workflowWithCurrentVersion = useWorkflowWithCurrentVersion(workflowId); | ||
assertWorkflowWithCurrentVersionIsDefined(workflowWithCurrentVersion); | ||
|
||
const { deleteOneStep } = useDeleteOneStep({ | ||
workflow: workflowWithCurrentVersion, | ||
stepId: id, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel it is good to subscribe to workflow changes in every Reactflow node. I only need to know what are the current trigger and steps in the deleteOneStep
function, so I think it might be better to get the data from the cache at this moment. The data are expected to be defined here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This pull request implements the functionality to delete workflow steps, enhancing the workflow management capabilities of the application.
- Added
useDeleteOneStep
hook inuseDeleteOneStep.tsx
for handling step deletion logic, including creating new workflow versions when necessary - Implemented delete button in
WorkflowDiagramStepNode.tsx
, allowing users to remove steps directly from the workflow diagram - Created
removeStep
utility function inremoveStep.ts
for safely removing steps from the workflow structure - Added unit tests in
removeStep.test.ts
to ensure proper functionality of the step removal process - Updated
RightDrawerWorkflowEditStepContent.tsx
to handle potential undefined steps after deletion, improving robustness
8 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
stepId, | ||
}: { | ||
steps: Array<WorkflowStep>; | ||
stepId: string | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: stepId is allowed to be undefined. Ensure this is handled correctly in findStepPositionOrThrow
stepId, | ||
}); | ||
|
||
parentStepPosition.steps.splice(parentStepPosition.index, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: This mutation directly affects the cloned steps. Consider returning a new array instead for better immutability
Thanks @Devessier for your contribution! |
- Allows the deletion of triggers and steps in workflows. If the workflow can not be edited right now, we create a new draft version. - The workflow right drawer can now render nothing. It's necessary to behave that way because a deleted step will still be displayed for a short amount of time in the drawer. The drawer will be filled with blank content when it disappears. https://github.com/user-attachments/assets/abd5184e-d3db-4fe7-8870-ccc78ff23d41 Closes twentyhq#7057
CleanShot.2024-10-01.at.16.28.38.mp4
Closes #7057