Analyze Context of Issue #1586 Using GitHub MCP

[twill-hq]

Implement "Tidy Up" Action for Workflow Diagram

Task Link: https://twill.ai/twentyhq/ENG/tasks/6

Summary

This PR adds a "Tidy Up" action to the workflow diagram interface, allowing users to automatically organize and clean up the layout of workflow nodes and connections.

Changes Made

• Added new workflow action: Created TidyUpWorkflowSingleRecordAction component to provide a UI action for tidying up workflow diagrams
• Refactored tidy-up logic: Extracted core tidy-up functionality into a reusable useTidyUp hook, separating layout logic from workflow version persistence
• Updated action menu configuration: Integrated the new tidy-up action into WorkflowActionsConfig with proper permissions and keyboard shortcuts
• Enhanced right-click menu: Updated WorkflowDiagramRightClickCommandMenu to use the refactored tidy-up hook
• Improved hook architecture: Simplified useTidyUpWorkflowVersion to delegate layout calculations to the new useTidyUp hook

Key Features

• Users can now trigger workflow diagram tidy-up from the action menu
• Consistent tidy-up behavior across both right-click menu and action menu interfaces
• Better separation of concerns between layout calculation and data persistence

📸 Screenshots

Playwright test screenshots captured during development:

command-menu-with-tidy-up-workflow.png

tidy-up-workflow-command-menu.png

tidy-up-workflow-error-toast.png

[greptile-apps]

Greptile Overview

Greptile Summary

This PR introduces a "Tidy Up" action for workflow diagrams, allowing users to automatically organize and clean up the layout of workflow nodes and connections. The implementation adds a new TidyUpWorkflowSingleRecordAction component that integrates with the existing action menu system, following the established patterns for workflow actions. The PR also includes a significant refactoring of the tidy-up functionality by extracting core logic into a reusable useTidyUp hook and simplifying the useTidyUpWorkflowVersion hook to accept explicit parameters rather than relying on internal state dependencies. This architectural improvement enables consistent tidy-up behavior across both right-click menu and action menu interfaces while maintaining better separation of concerns between layout calculation and data persistence.

Important Files Changed

Changed Files

Filename	Score	Overview
packages/twenty-front/src/modules/action-menu/actions/record-actions/single-record/workflow-actions/types/WorkflowSingleRecordActionsKeys.ts	5/5	Added new TIDY_UP enum key following established naming conventions
packages/twenty-front/src/modules/workflow/workflow-diagram/components/WorkflowDiagramRightClickCommandMenu.tsx	5/5	Refactored to use new useTidyUp hook instead of useTidyUpWorkflowVersion
packages/twenty-front/src/modules/action-menu/actions/record-actions/single-record/workflow-actions/components/TidyUpWorkflowSingleRecordAction.tsx	4/5	New component implementing tidy-up action with proper validation but lacking error handling
packages/twenty-front/src/modules/action-menu/actions/record-actions/constants/WorkflowActionsConfig.tsx	4/5	Added tidy-up action configuration and updated position numbers for all existing actions
packages/twenty-front/src/modules/workflow/workflow-version/hooks/useTidyUpWorkflowVersion.ts	4/5	Refactored hook to accept explicit parameters and improve reusability

Confidence score: 4/5

• This PR is generally safe to merge with only minor concerns around error handling
• Score reflects solid architectural improvements and consistent patterns, but points deducted for missing error handling in the new component and potential issues with position number updates across multiple actions
• Pay close attention to the TidyUpWorkflowSingleRecordAction component which lacks error handling for failed operations

Sequence Diagram

sequenceDiagram
    participant User
    participant TidyUpWorkflowSingleRecordAction
    participant useSelectedRecordIdOrThrow
    participant useWorkflowWithCurrentVersion
    participant useTidyUpWorkflowVersion
    participant getWorkflowVersionDiagram
    participant getOrganizedDiagram
    participant Apollo Client
    participant Cache

    User->>TidyUpWorkflowSingleRecordAction: "Click tidy up action"
    TidyUpWorkflowSingleRecordAction->>useSelectedRecordIdOrThrow: "Get selected record ID"
    useSelectedRecordIdOrThrow-->>TidyUpWorkflowSingleRecordAction: "Return recordId"
    
    TidyUpWorkflowSingleRecordAction->>useWorkflowWithCurrentVersion: "Get workflow with current version"
    useWorkflowWithCurrentVersion-->>TidyUpWorkflowSingleRecordAction: "Return workflowWithCurrentVersion"
    
    TidyUpWorkflowSingleRecordAction->>getWorkflowVersionDiagram: "Get workflow diagram"
    getWorkflowVersionDiagram-->>TidyUpWorkflowSingleRecordAction: "Return workflowDiagram"
    
    TidyUpWorkflowSingleRecordAction->>useTidyUpWorkflowVersion: "Call tidyUpWorkflowVersion"
    useTidyUpWorkflowVersion->>getOrganizedDiagram: "Organize diagram layout"
    getOrganizedDiagram-->>useTidyUpWorkflowVersion: "Return tidiedUpDiagram"
    
    useTidyUpWorkflowVersion->>Apollo Client: "Update workflow version positions"
    Apollo Client-->>useTidyUpWorkflowVersion: "Mutation response"
    
    useTidyUpWorkflowVersion->>Cache: "Update cached record with new positions"
    Cache-->>useTidyUpWorkflowVersion: "Cache updated"
    
    useTidyUpWorkflowVersion-->>TidyUpWorkflowSingleRecordAction: "Return tidiedUpDiagram"
    TidyUpWorkflowSingleRecordAction-->>User: "Workflow diagram tidied up"

Loading

_{5 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

style: Missing error handling for the tidyUpWorkflowVersion operation. Consider wrapping in try-catch to handle potential failures gracefully.

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/twenty-front/src/modules/action-menu/actions/record-actions/single-record/workflow-actions/components/TidyUpWorkflowSingleRecordAction.tsx
Line: 13:27

Comment:
**style:** Missing error handling for the tidyUpWorkflowVersion operation. Consider wrapping in try-catch to handle potential failures gracefully.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

style: Consider returning undefined explicitly or a default diagram structure instead of implicit undefined return to make the behavior clearer

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/twenty-front/src/modules/workflow/workflow-version/hooks/useTidyUpWorkflowVersion.ts
Line: 97:99

Comment:
**style:** Consider returning undefined explicitly or a default diagram structure instead of implicit undefined return to make the behavior clearer

How can I resolve this? If you propose a fix, please make it concise.

[github-actions]

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:63127

This environment will automatically shut down when the PR is closed or after 5 hours.

[constantinidan]

Response to Greptile Comments

Comment 1: Error Handling

Re: "Missing error handling for the tidyUpWorkflowVersion operation"

This is not necessary. The Twenty codebase uses a global error handling pattern:

• PromiseRejectionEffect catches all unhandled promise rejections and shows error snackbars
• Apollo's errorLink automatically handles GraphQL errors

I verified all other single-record action components in the codebase and none use try-catch blocks:

• TestWorkflowSingleRecordAction - no try-catch
• AddNodeWorkflowSingleRecordAction - no try-catch
• ActivateWorkflowSingleRecordAction - no try-catch
• UseAsDraftWorkflowVersionSingleRecordAction - no try-catch (even with async operations)

This implementation follows the established pattern.

---

Comment 2: Implicit Undefined Return

Re: "Consider returning undefined explicitly"

The codebase overwhelmingly prefers implicit returns:

• return; (implicit): 663 occurrences across the codebase
• return undefined; (explicit): 133 occurrences

The current implementation follows the dominant pattern (5:1 ratio).