Skip to content
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

Fix a bug with loop block children order on save #1154

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Nov 7, 2024

Important

Fixes loop block children order bug on save by using edges in getWorkflowBlocks in workflowEditorUtils.ts.

  • Behavior:
    • Fixes bug in handleSave() in FlowRenderer.tsx by passing edges to getWorkflowBlocks to ensure correct order of loop block children.
  • Functions:
    • Adds getOrderedChildrenBlocks() in workflowEditorUtils.ts to determine the order of loop block children using edges.
    • Updates getWorkflowBlocksUtil() in workflowEditorUtils.ts to use getOrderedChildrenBlocks() for loop blocks.
  • Misc:
    • Updates getWorkflowBlocks() in workflowEditorUtils.ts to accept edges parameter.

This description was created by Ellipsis for f773255. It will automatically update as commits are pushed.

…src/'

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Fixes loop block children order bug on save by using edges in `getWorkflowBlocks` in `workflowEditorUtils.ts`.
>
>   - **Behavior**:
>     - Fixes bug in `handleSave()` in `FlowRenderer.tsx` by passing `edges` to `getWorkflowBlocks` to ensure correct order of loop block children.
>   - **Functions**:
>     - Adds `getOrderedChildrenBlocks()` in `workflowEditorUtils.ts` to determine the order of loop block children using `edges`.
>     - Updates `getWorkflowBlocksUtil()` in `workflowEditorUtils.ts` to use `getOrderedChildrenBlocks()` for loop blocks.
>   - **Misc**:
>     - Updates `getWorkflowBlocks()` in `workflowEditorUtils.ts` to accept `edges` parameter.
>
> <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for db2bb9f633252c0f84feabf005fb825979621da6. It will automatically update as commits are pushed.</sup>

<!-- ELLIPSIS_HIDDEN -->
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to f773255 in 1 minute and 7 seconds

More details
  • Looked at 103 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/workflowEditorUtils.ts:663
  • Draft comment:
    Consider adding a mechanism to detect cycles in the graph to prevent potential infinite loops when traversing nodes.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is speculative as it suggests a potential issue without strong evidence of its existence. The current implementation appears to handle node traversal in a straightforward manner, reducing the likelihood of cycles. Without evidence of cycles being a problem, the comment may not be necessary.
    I might be underestimating the complexity of the graph structure and the potential for cycles. The function's logic could be more complex than it appears, and cycles might be a real concern.
    The function's logic seems straightforward, and without evidence of cycles being an issue, the comment remains speculative. The code does not show signs of handling complex graph structures that would inherently create cycles.
    The comment is speculative and not based on strong evidence of an issue. It should be removed as it does not point to a definite problem in the code.

Workflow ID: wflow_j4TmJsYT2xkMQHOk


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@msalihaltun msalihaltun merged commit 61d1c24 into main Nov 7, 2024
2 checks passed
@msalihaltun msalihaltun deleted the salih/fix-loop-node-add-child-bug branch November 7, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants