-
Notifications
You must be signed in to change notification settings - Fork 93
fix(DATAGO-123604): add descriptions for workflow logic nodes #967
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,37 @@ const createLoopNode = (delay?: string): LayoutNode => ({ | |
| }, | ||
| }); | ||
|
|
||
| const createMapNode = (): LayoutNode => ({ | ||
| id: "process_items", | ||
| type: "map", | ||
| x: 0, | ||
| y: 0, | ||
| width: 200, | ||
| height: 60, | ||
| children: [], | ||
| data: { | ||
| label: "Process Items", | ||
| items: "{{input.items}}", | ||
| }, | ||
| }); | ||
|
|
||
| const createSwitchNode = (): LayoutNode => ({ | ||
| id: "route_request", | ||
| type: "switch", | ||
| x: 0, | ||
| y: 0, | ||
| width: 200, | ||
| height: 60, | ||
| children: [], | ||
| data: { | ||
| label: "Route Request", | ||
| cases: [ | ||
| { condition: "{{input.type}} == 'A'", node: "handle_a" }, | ||
| { condition: "{{input.type}} == 'B'", node: "handle_b" }, | ||
| ], | ||
| }, | ||
| }); | ||
|
|
||
| export const LoopNodeWithDelay: Story = { | ||
| args: { | ||
| node: createLoopNode("5s"), | ||
|
|
@@ -64,8 +95,50 @@ export const LoopNodeWithoutDelay: Story = { | |
| expect(screen.getByText("10")).toBeInTheDocument(); | ||
| expect(screen.getByText("Condition")).toBeInTheDocument(); | ||
|
|
||
| // Verify description for loop nodes is rendered | ||
| expect(screen.getByText("Repeats a node until a condition becomes false. The first iteration always runs; the condition is checked before subsequent iterations.")).toBeInTheDocument(); | ||
|
|
||
| // Verify delay is NOT rendered | ||
| const delayLabel = screen.queryByText("Delay"); | ||
| expect(delayLabel).not.toBeInTheDocument(); | ||
| }, | ||
| }; | ||
|
|
||
| export const MapNode: Story = { | ||
| args: { | ||
| node: createMapNode(), | ||
| workflowConfig: null, | ||
| agents: [], | ||
| }, | ||
| play: async () => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The pattern pre-exists in this file, but I think it would be slightly beneficial to use the canvas, like this: It just makes the tests slightly more efficient since they are pre-scoped to the story container (we do need screen for things like dialogs or toasts since they are outside the canvas). |
||
| // Verify node ID appears in the panel | ||
| const processItemsElements = screen.getAllByText("process_items"); | ||
| expect(processItemsElements.length).toBeGreaterThanOrEqual(2); | ||
|
|
||
| // Verify description for map nodes is rendered | ||
| expect(screen.getByText("Executes a node for each item in a collection. Items are processed in parallel by default.")).toBeInTheDocument(); | ||
|
|
||
| // Verify items property is rendered | ||
| expect(screen.getByText("Items")).toBeInTheDocument(); | ||
| expect(screen.getByText("{{input.items}}")).toBeInTheDocument(); | ||
| }, | ||
| }; | ||
|
|
||
| export const SwitchNode: Story = { | ||
| args: { | ||
| node: createSwitchNode(), | ||
| workflowConfig: null, | ||
| agents: [], | ||
| }, | ||
| play: async () => { | ||
| // Verify node ID appears in the panel | ||
| const routeRequestElements = screen.getAllByText("route_request"); | ||
| expect(routeRequestElements.length).toBeGreaterThanOrEqual(2); | ||
|
|
||
| // Verify description for switch nodes is rendered | ||
| expect(screen.getByText("Routes execution based on conditions. Cases are evaluated in order; the first match wins.")).toBeInTheDocument(); | ||
|
|
||
| // Verify cases are rendered | ||
| expect(screen.getByText("Cases")).toBeInTheDocument(); | ||
| }, | ||
| }; | ||
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.
Not a big deal, but could reduce the number of checks, removing
LOGIC_NODE_DESCRIPTIONS[node.type]or something from the condition since we know it's there?