Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • remove all associated edges when moving a block into a subflow, we already disallowed external edges that cross the boundary, and remove all edges when moving a block out of a subflow

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Dec 1, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Dec 1, 2025 7:10am

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 1, 2025

Greptile Overview

Greptile Summary

This PR improves subflow edge management by ensuring all connected edges are removed when moving a block into a subflow container, matching the existing behavior when removing blocks from subflows.

Key Changes:

  • Extracted edge removal logic into reusable removeEdgesForNode helper function
  • Added edge cleanup when moving blocks into subflow containers (previously only removed when moving out)
  • Updated affectedEdges tracking to include both removed and added edges for proper undo/redo support
  • Refactored remove-from-subflow handler to use the new helper function

The implementation properly handles undo/redo by batching edge operations with the parent update event, preventing individual edge recordings that would clutter the history.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are well-structured and follow established patterns. The refactoring extracts duplicate logic into a reusable helper function with proper error handling via try/finally blocks. The implementation correctly manages undo/redo state by batching edge operations with parent updates. The code maintains consistency with existing edge removal behavior when moving blocks out of subflows.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx 5/5 Refactored edge removal logic into reusable helper function and added edge cleanup when moving blocks into subflows

Sequence Diagram

sequenceDiagram
    participant User
    participant Workflow
    participant removeEdgesForNode
    participant updateNodeParent
    participant UndoRedo

    alt Moving Block INTO Subflow
        User->>Workflow: Drag block into subflow container
        Workflow->>Workflow: Filter edges connected to block
        Workflow->>removeEdgesForNode: removeEdgesForNode(blockId, edgesToRemove)
        removeEdgesForNode->>removeEdgesForNode: Dispatch skip-edge-recording (true)
        loop For each edge
            removeEdgesForNode->>Workflow: removeEdge(edge.id)
        end
        removeEdgesForNode->>removeEdgesForNode: Dispatch skip-edge-recording (false)
        Workflow->>Workflow: Compute auto-connect edges (edgesToAdd)
        Workflow->>Workflow: Combine affectedEdges = [...edgesToRemove, ...edgesToAdd]
        Workflow->>updateNodeParent: updateNodeParent(blockId, parentId, affectedEdges)
        updateNodeParent->>UndoRedo: Record parent update with all affected edges
        loop For each new edge
            Workflow->>Workflow: addEdge(edge)
        end
    end

    alt Moving Block OUT of Subflow
        User->>Workflow: Trigger remove-from-subflow event
        Workflow->>Workflow: Filter edges connected to block
        Workflow->>removeEdgesForNode: removeEdgesForNode(blockId, edgesToRemove)
        removeEdgesForNode->>removeEdgesForNode: Dispatch skip-edge-recording (true)
        loop For each edge
            removeEdgesForNode->>Workflow: removeEdge(edge.id)
        end
        removeEdgesForNode->>removeEdgesForNode: Dispatch skip-edge-recording (false)
        Workflow->>updateNodeParent: updateNodeParent(blockId, null, edgesToRemove)
        updateNodeParent->>UndoRedo: Record parent update with removed edges
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx, line 2006-2020 (link)

    logic: Missing removeEdgesForNode and edgesForDisplay in dependency array

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 3a4f130 into staging Dec 1, 2025
9 checks passed
@waleedlatif1 waleedlatif1 deleted the sim-388 branch December 1, 2025 17:06
waleedlatif1 added a commit that referenced this pull request Dec 1, 2025
… sendgrid, linkedin, more tools (#2148)

* feat(tools): added smtp, sendgrid, mailgun, linkedin, fixed permissions in context menu (#2133)

* feat(tools): added twilio sendgrid integration

* feat(tools): added smtp, sendgrid, mailgun, fixed permissions in context menu

* added top level mocks for sporadically failing tests

* incr type safety

* fix(team-plans): track departed member usage so value not lost (#2118)

* fix(team-plans): track departed member usage so value not lost

* reset usage to 0 when they leave team

* prep merge with stagig

* regen migrations

* fix org invite + ws selection'

---------

Co-authored-by: Waleed <walif6@gmail.com>

* feat(i18n): update translations (#2134)

Co-authored-by: waleedlatif1 <waleedlatif1@users.noreply.github.com>

* feat(creators): add verification for creators (#2135)

* feat(tools): added apify block/tools  (#2136)

* feat(tools): added apify

* cleanup

* feat(i18n): update translations (#2137)

Co-authored-by: waleedlatif1 <waleedlatif1@users.noreply.github.com>

* feat(env): added more optional env var examples (#2138)

* feat(statuspage): added statuspage, updated list of tools in footer, renamed routes (#2139)

* feat(statuspage): added statuspage, updated list of tools in footer, renamed routes

* ack PR comments

* feat(tools): add generic search tool (#2140)

* feat(i18n): update translations (#2141)

* fix(sdks): bump sdk versions (#2142)

* fix(webhooks): count test webhooks towards usage limit (#2143)

* fix(bill): add requestId to webhook processing (#2144)

* improvement(subflow): remove all associated edges when moving a block into a subflow (#2145)

* improvement(subflow): remove all associated edges when moving a block into a subflow

* ack PR comments

* fix(polling): mark webhook failed on webhook trigger errors (#2146)

* fix(deps): declare core transient deps explicitly (#2147)

* fix(deps): declare core transient deps explicitly

* ack PR comments

---------

Co-authored-by: Vikhyath Mondreti <vikhyathvikku@gmail.com>
Co-authored-by: waleedlatif1 <waleedlatif1@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants