Skip to content

Commit c23b9d6

Browse files
j-sanaaj-sanaa
committed
Fixed review comments
1 parent 58df7ef commit c23b9d6

4 files changed

Lines changed: 249 additions & 22 deletions

File tree

‎packages/agentflow/TESTS.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ These modules carry the highest risk. Test in the same PR when modifying.
3434
| `src/infrastructure/api/client.ts` | `createApiClient` — headers, auth token, 401 interceptor | ✅ Done |
3535
| `src/infrastructure/api/chatflows.ts` | All CRUD + `generateAgentflow` + `getChatModels`, FlowData serialization | ✅ Done |
3636
| `src/infrastructure/api/nodes.ts` | `getAllNodes`, `getNodeByName`, `getNodeIconUrl` | ✅ Done |
37-
| `src/infrastructure/store/AgentflowContext.tsx` | `agentflowReducer` (all actions), `normalizeNodes`. Remaining: `deleteNode()`, `duplicateNode()`, `updateNodeData()`, `getFlowData()` | 🟡 Partial |
37+
| `src/infrastructure/store/AgentflowContext.tsx` | `agentflowReducer` (all actions), `normalizeNodes`, `deleteNode()`, `duplicateNode()`. Remaining: `updateNodeData()`, `getFlowData()` | 🟡 Partial|
3838
| `src/useAgentflow.ts` | `getFlow()`, `toJSON()`, `validate()`, `addNode()`, `clear()` | ⬜ Not yet — thin wrapper |
3939
| `src/features/canvas/hooks/useFlowHandlers.ts` | `handleConnect`, `handleNodesChange`, `handleEdgesChange`, `handleAddNode` — synchronous `onFlowChange` callbacks, dirty tracking, viewport resolution, change filtering | ✅ Done |
4040

@@ -130,6 +130,7 @@ Key features:
130130
- `./src/features/canvas/hooks/useFlowHandlers.ts`
131131
- `./src/features/node-palette/search.ts`
132132
- `./src/infrastructure/api/`
133+
- ⏳ `./src/infrastructure/store/AgentflowContext.tsx` — will be added when coverage reaches 80%
133134
- **Coverage exclusions**:
134135
- `src/__test_utils__/**` — test utilities
135136
- `src/__mocks__/**` — module mocks

‎packages/agentflow/jest.config.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ module.exports = {
4040
'./src/core/': { branches: 80, functions: 80, lines: 80, statements: 80 },
4141
'./src/features/canvas/hooks/useFlowHandlers.ts': { branches: 80, functions: 80, lines: 80, statements: 80 },
4242
'./src/features/node-palette/search.ts': { branches: 80, functions: 80, lines: 80, statements: 80 },
43-
'./src/infrastructure/api/': { branches: 80, functions: 80, lines: 80, statements: 80 }
43+
'./src/infrastructure/api/': { branches: 80, functions: 80, lines: 80, statements: 80 },
44+
'./src/infrastructure/store/AgentflowContext.tsx': { branches: 80, functions: 80, lines: 80, statements: 80 }
4445
},
4546
projects: [
4647
// .test.ts → node (fast, no DOM)

‎packages/agentflow/src/infrastructure/store/AgentflowContext.test.tsx‎

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,230 @@ describe('AgentflowContext - duplicateNode', () => {
482482
})
483483
})
484484

485+
describe('AgentflowContext - openEditDialog & closeEditDialog', () => {
486+
it('should open edit dialog with node data and input params', () => {
487+
const initialFlow: FlowData = {
488+
nodes: [makeNode('node-1')],
489+
edges: []
490+
}
491+
492+
const { result } = renderHook(() => useAgentflowContext(), {
493+
wrapper: createWrapper(initialFlow)
494+
})
495+
496+
// Initial state should have no editing node
497+
expect(result.current.state.editingNodeId).toBeNull()
498+
expect(result.current.state.editDialogProps).toBeNull()
499+
500+
const nodeData = {
501+
id: 'node-1',
502+
name: 'testNode',
503+
label: 'Test Node',
504+
outputAnchors: []
505+
}
506+
507+
const inputParams = [
508+
{
509+
id: 'param-1',
510+
name: 'param1',
511+
label: 'Parameter 1',
512+
type: 'string'
513+
}
514+
]
515+
516+
// Open edit dialog
517+
act(() => {
518+
result.current.openEditDialog('node-1', nodeData, inputParams)
519+
})
520+
521+
// Should set editingNodeId
522+
expect(result.current.state.editingNodeId).toBe('node-1')
523+
524+
// Should set editDialogProps
525+
expect(result.current.state.editDialogProps).toEqual({
526+
inputParams: inputParams,
527+
data: nodeData,
528+
disabled: false
529+
})
530+
})
531+
532+
it('should close edit dialog and clear state', () => {
533+
const initialFlow: FlowData = {
534+
nodes: [makeNode('node-1')],
535+
edges: []
536+
}
537+
538+
const { result } = renderHook(() => useAgentflowContext(), {
539+
wrapper: createWrapper(initialFlow)
540+
})
541+
542+
const nodeData = {
543+
id: 'node-1',
544+
name: 'testNode',
545+
label: 'Test Node',
546+
outputAnchors: []
547+
}
548+
549+
const inputParams = [
550+
{
551+
id: 'param-1',
552+
name: 'param1',
553+
label: 'Parameter 1',
554+
type: 'string'
555+
}
556+
]
557+
558+
// First open the dialog
559+
act(() => {
560+
result.current.openEditDialog('node-1', nodeData, inputParams)
561+
})
562+
563+
// Verify dialog is open
564+
expect(result.current.state.editingNodeId).toBe('node-1')
565+
expect(result.current.state.editDialogProps).not.toBeNull()
566+
567+
// Close the dialog
568+
act(() => {
569+
result.current.closeEditDialog()
570+
})
571+
572+
// Should clear editingNodeId
573+
expect(result.current.state.editingNodeId).toBeNull()
574+
575+
// Should clear editDialogProps
576+
expect(result.current.state.editDialogProps).toBeNull()
577+
})
578+
579+
it('should handle opening dialog for different nodes', () => {
580+
const initialFlow: FlowData = {
581+
nodes: [makeNode('node-1'), makeNode('node-2')],
582+
edges: []
583+
}
584+
585+
const { result } = renderHook(() => useAgentflowContext(), {
586+
wrapper: createWrapper(initialFlow)
587+
})
588+
589+
const nodeData1 = {
590+
id: 'node-1',
591+
name: 'testNode1',
592+
label: 'Test Node 1',
593+
outputAnchors: []
594+
}
595+
596+
const nodeData2 = {
597+
id: 'node-2',
598+
name: 'testNode2',
599+
label: 'Test Node 2',
600+
outputAnchors: []
601+
}
602+
603+
const inputParams = [
604+
{
605+
id: 'param-1',
606+
name: 'param1',
607+
label: 'Parameter 1',
608+
type: 'string'
609+
}
610+
]
611+
612+
// Open dialog for node-1
613+
act(() => {
614+
result.current.openEditDialog('node-1', nodeData1, inputParams)
615+
})
616+
617+
expect(result.current.state.editingNodeId).toBe('node-1')
618+
expect(result.current.state.editDialogProps).not.toBeNull()
619+
expect(result.current.state.editDialogProps!.data).toBeDefined()
620+
expect(result.current.state.editDialogProps!.data!.label).toBe('Test Node 1')
621+
622+
// Open dialog for node-2 (should replace node-1)
623+
act(() => {
624+
result.current.openEditDialog('node-2', nodeData2, inputParams)
625+
})
626+
627+
expect(result.current.state.editingNodeId).toBe('node-2')
628+
expect(result.current.state.editDialogProps).not.toBeNull()
629+
expect(result.current.state.editDialogProps!.data).toBeDefined()
630+
expect(result.current.state.editDialogProps!.data!.label).toBe('Test Node 2')
631+
})
632+
633+
it('should set disabled to false in dialog props', () => {
634+
const initialFlow: FlowData = {
635+
nodes: [makeNode('node-1')],
636+
edges: []
637+
}
638+
639+
const { result } = renderHook(() => useAgentflowContext(), {
640+
wrapper: createWrapper(initialFlow)
641+
})
642+
643+
const nodeData = {
644+
id: 'node-1',
645+
name: 'testNode',
646+
label: 'Test Node',
647+
outputAnchors: []
648+
}
649+
650+
act(() => {
651+
result.current.openEditDialog('node-1', nodeData, [])
652+
})
653+
654+
// disabled should always be false
655+
expect(result.current.state.editDialogProps?.disabled).toBe(false)
656+
})
657+
658+
it('should preserve inputParams in dialog props', () => {
659+
const initialFlow: FlowData = {
660+
nodes: [makeNode('node-1')],
661+
edges: []
662+
}
663+
664+
const { result } = renderHook(() => useAgentflowContext(), {
665+
wrapper: createWrapper(initialFlow)
666+
})
667+
668+
const nodeData = {
669+
id: 'node-1',
670+
name: 'testNode',
671+
label: 'Test Node',
672+
outputAnchors: []
673+
}
674+
675+
const inputParams = [
676+
{
677+
id: 'param-1',
678+
name: 'param1',
679+
label: 'Parameter 1',
680+
type: 'string',
681+
optional: true
682+
},
683+
{
684+
id: 'param-2',
685+
name: 'param2',
686+
label: 'Parameter 2',
687+
type: 'number',
688+
default: 42
689+
}
690+
]
691+
692+
act(() => {
693+
result.current.openEditDialog('node-1', nodeData, inputParams)
694+
})
695+
696+
// Should preserve all input params with their properties
697+
expect(result.current.state.editDialogProps).not.toBeNull()
698+
expect(result.current.state.editDialogProps!.inputParams).toEqual(inputParams)
699+
expect(result.current.state.editDialogProps!.inputParams).toHaveLength(2)
700+
701+
const params = result.current.state.editDialogProps!.inputParams!
702+
expect(params[0]).toBeDefined()
703+
expect(params[0]!.optional).toBe(true)
704+
expect(params[1]).toBeDefined()
705+
expect(params[1]!.default).toBe(42)
706+
})
707+
})
708+
485709
describe('AgentflowContext - state synchronization', () => {
486710
it('should call local state setters for setNodes', () => {
487711
const initialFlow: FlowData = {

‎packages/agentflow/src/infrastructure/store/AgentflowContext.tsx‎

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,20 @@ import type { AgentflowAction, AgentflowState, FlowConfig, FlowData, FlowEdge, F
55

66
import { agentflowReducer, initialState, normalizeNodes } from './agentflowReducer'
77

8+
// Local state setter types
9+
type NodesSetter = (nodes: FlowNode[]) => void
10+
type EdgesSetter = (edges: FlowEdge[]) => void
11+
812
// Context value interface
913
export interface AgentflowContextValue {
1014
state: AgentflowState
1115
dispatch: Dispatch<AgentflowAction>
1216

1317
// Convenience methods
14-
setNodes: (nodes: FlowNode[]) => void
15-
setEdges: (edges: FlowEdge[]) => void
16-
syncNodesFromReactFlow: (nodes: FlowNode[]) => void
17-
syncEdgesFromReactFlow: (edges: FlowEdge[]) => void
18+
setNodes: NodesSetter
19+
setEdges: EdgesSetter
20+
syncNodesFromReactFlow: NodesSetter
21+
syncEdgesFromReactFlow: EdgesSetter
1822
setChatflow: (chatflow: FlowConfig | null) => void
1923
setDirty: (dirty: boolean) => void
2024
setReactFlowInstance: (instance: ReactFlowInstance | null) => void
@@ -36,7 +40,7 @@ export interface AgentflowContextValue {
3640
closeEditDialog: () => void
3741

3842
// Register ReactFlow local state setters
39-
registerLocalStateSetters: (setLocalNodes: (nodes: FlowNode[]) => void, setLocalEdges: (edges: FlowEdge[]) => void) => void
43+
registerLocalStateSetters: (setLocalNodes: NodesSetter, setLocalEdges: EdgesSetter) => void
4044
}
4145

4246
const AgentflowContext = createContext<AgentflowContextValue | null>(null)
@@ -54,16 +58,13 @@ export function AgentflowStateProvider({ children, initialFlow }: AgentflowState
5458
})
5559

5660
// Store ReactFlow local state setters in refs which are populated by AgentflowCanvas
57-
const localNodesSetterRef = useRef<((_nodes: FlowNode[]) => void) | null>(null)
58-
const localEdgesSetterRef = useRef<((_edges: FlowEdge[]) => void) | null>(null)
61+
const localNodesSetterRef = useRef<NodesSetter | null>(null)
62+
const localEdgesSetterRef = useRef<EdgesSetter | null>(null)
5963

60-
const registerLocalStateSetters = useCallback(
61-
(setLocalNodes: (nodes: FlowNode[]) => void, setLocalEdges: (edges: FlowEdge[]) => void) => {
62-
localNodesSetterRef.current = setLocalNodes
63-
localEdgesSetterRef.current = setLocalEdges
64-
},
65-
[]
66-
)
64+
const registerLocalStateSetters = useCallback((setLocalNodes: NodesSetter, setLocalEdges: EdgesSetter) => {
65+
localNodesSetterRef.current = setLocalNodes
66+
localEdgesSetterRef.current = setLocalEdges
67+
}, [])
6768

6869
// Helper function to generate unique copy IDs
6970
const getUniqueCopyId = useCallback((baseId: string, nodes: FlowNode[]): string => {
@@ -95,25 +96,25 @@ export function AgentflowStateProvider({ children, initialFlow }: AgentflowState
9596
}, [])
9697

9798
// Convenience setters
98-
const setNodes = useCallback(
99-
(nodes: FlowNode[]) => {
99+
const setNodes = useCallback<NodesSetter>(
100+
(nodes) => {
100101
syncStateUpdate({ nodes: nodes })
101102
},
102103
[syncStateUpdate]
103104
)
104105

105-
const setEdges = useCallback(
106-
(edges: FlowEdge[]) => {
106+
const setEdges = useCallback<EdgesSetter>(
107+
(edges) => {
107108
syncStateUpdate({ edges: edges })
108109
},
109110
[syncStateUpdate]
110111
)
111112

112-
const syncNodesFromReactFlow = useCallback((nodes: FlowNode[]) => {
113+
const syncNodesFromReactFlow = useCallback<NodesSetter>((nodes) => {
113114
dispatch({ type: 'SET_NODES', payload: normalizeNodes(nodes) })
114115
}, [])
115116

116-
const syncEdgesFromReactFlow = useCallback((edges: FlowEdge[]) => {
117+
const syncEdgesFromReactFlow = useCallback<EdgesSetter>((edges) => {
117118
dispatch({ type: 'SET_EDGES', payload: edges })
118119
}, [])
119120

0 commit comments

Comments
 (0)