Fix bugs in flow canvas delete handlers#2962
Conversation
📝 WalkthroughWalkthroughRefactors node deletion to use functional ChangesEdge handling in flow operations
Sequence DiagramsequenceDiagram
participant handleNodesDelete
participant setEdges as setEdges(functionalUpdater)
participant reducer as reducer(acc)
participant getIncomers as getIncomers/Outgoers
handleNodesDelete->>setEdges: call with functional updater
setEdges->>reducer: invoke with latestEdges
loop for each deleted node
reducer->>getIncomers: query using acc (evolving edges)
getIncomers->>reducer: return connected edges from acc
reducer->>reducer: accumulate reconnect edges into acc
end
reducer->>setEdges: return final edge set
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
frontend/apps/console/src/features/flows/hooks/__tests__/useDeleteExecutionResource.test.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. frontend/apps/console/src/features/flows/hooks/__tests__/useVisualFlowHandlers.test.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. frontend/apps/console/src/features/flows/hooks/useDeleteExecutionResource.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Re-create issue scenario: Screen.Recording.2026-05-25.at.11.38.54.mp4 |
|
Re-create the issue - #2644 Dilusha.Madushan_s.Video.-.May.25.2026.mp4 |
Dilusha-Madushan
force-pushed
the
fix/resolve-bugs-console-flow-builder
branch
from
f9335b3 to
6ff444e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/apps/console/src/features/flows/hooks/__tests__/useDeleteExecutionResource.test.tsx (1)
73-73: ⚡ Quick winRemove unused
onEdgeDeletemock frommockFlowPlugins.
onEdgeDeleteis no longer exercised by this hook test suite, so this leftover mock is dead test scaffolding and can be dropped to keep intent clear.Proposed cleanup
const mockFlowPlugins = { onPropertyChange: vi.fn().mockReturnValue(vi.fn()), emitPropertyChange: vi.fn().mockReturnValue(true), onPropertyPanelOpen: vi.fn().mockReturnValue(vi.fn()), emitPropertyPanelOpen: vi.fn().mockReturnValue(true), onElementFilter: vi.fn().mockReturnValue(vi.fn()), emitElementFilter: vi.fn().mockReturnValue(true), - onEdgeDelete: vi.fn().mockReturnValue(vi.fn()), emitEdgeDelete: vi.fn().mockReturnValue(true), onNodeDelete: mockOnNodeDelete,As per coding guidelines,
**/*.{go,ts,tsx,js,jsx}: Delete dead code cleanly. No// removedor// deprecatedplaceholder comments. No renaming unused variables to_prefixed names — remove them entirely unless required by an interface, callback, or framework signature.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/apps/console/src/features/flows/hooks/__tests__/useDeleteExecutionResource.test.tsx` at line 73, Remove the dead mock property onEdgeDelete from the mockFlowPlugins object in the test file useDeleteExecutionResource.test.tsx: locate the mockFlowPlugins definition and delete the line "onEdgeDelete: vi.fn().mockReturnValue(vi.fn())," (and any trailing comma adjustments) so the mock no longer contains that unused property.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@frontend/apps/console/src/features/flows/hooks/__tests__/useDeleteExecutionResource.test.tsx`:
- Line 73: Remove the dead mock property onEdgeDelete from the mockFlowPlugins
object in the test file useDeleteExecutionResource.test.tsx: locate the
mockFlowPlugins definition and delete the line "onEdgeDelete:
vi.fn().mockReturnValue(vi.fn())," (and any trailing comma adjustments) so the
mock no longer contains that unused property.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e39c31a-6adc-4e74-97c8-85c981c9903c
📒 Files selected for processing (4)
frontend/apps/console/src/features/flows/hooks/__tests__/useDeleteExecutionResource.test.tsxfrontend/apps/console/src/features/flows/hooks/__tests__/useVisualFlowHandlers.test.tsxfrontend/apps/console/src/features/flows/hooks/useDeleteExecutionResource.tsfrontend/apps/console/src/features/flows/hooks/useVisualFlowHandlers.ts
ThaminduDilshan
left a comment
ThaminduDilshan
left a comment
There was a problem hiding this comment.
Content LGTM. Not clear on the removals
3 participants
Purpose
Fixes four bugs in the flow canvas when deleting nodes and edges involving execution steps:
getEdges()snapshot instead of the live edge state, causing incorrect or missing reconnections.Approach
useVisualFlowHandlers.ts— stale edge snapshot fixSwitched
handleNodesDeletefrom a directsetEdges(reduce(..., currentEdges))call (using a snapshot captured at call time) to the functional setter formsetEdges((latestEdges) => reduce(..., latestEdges)). All graph queries (getIncomers,getOutgoers,getConnectedEdges) now operate on the running accumulatoraccinside the reducer, so each deleted node sees the already-updated edge state from prior iterations.useDeleteExecutionResource.ts— cascade and cleanup fixesRewrote
deleteComponentAndNodewith the following changes:remainingEdges(current edges minus deleted ones) upfront. An execution node is only cascade-deleted when no remaining edge still targets it, preventing premature removal of shared nodes.setNodes+setEdgesto remove both the orphaned execution node and all its incident edges, eliminating ghost edges.actionComponentIdsarray with aMap<string, string[]>keyed by source node ID, so component removal is scoped per node and cannot bleed across unrelated action nodes.sourceHandle?.endsWith('_NEXT'), preventing non-NEXT edges from triggering cascade logic.Related Issues
Summary by CodeRabbit