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

add completion criteria to navigation block as well #1385

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Dec 13, 2024

Important

Add completion and termination criteria to NavigationNode and LoginNode, updating types and utility functions.

  • Behavior:
    • Add completeCriterion and terminateCriterion to NavigationNode and LoginNode in NavigationNode.tsx and LoginNode.tsx.
    • Update convertToNode() and getWorkflowBlock() in workflowEditorUtils.ts to handle new criteria.
  • Types:
    • Add completeCriterion and terminateCriterion to NavigationNodeData and LoginNodeData in types.ts.
    • Update NavigationBlock, LoginBlock, NavigationBlockYAML, and LoginBlockYAML in workflowTypes.ts and workflowYamlTypes.ts to include new criteria.

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

…src/'

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Add completion and termination criteria to `NavigationNode` and `LoginNode`, updating types and utility functions.
>
>   - **Behavior**:
>     - Add `completeCriterion` and `terminateCriterion` to `NavigationNode` and `LoginNode` in `NavigationNode.tsx` and `LoginNode.tsx`.
>     - Update `convertToNode()` and `getWorkflowBlock()` in `workflowEditorUtils.ts` to handle new criteria.
>   - **Types**:
>     - Add `completeCriterion` and `terminateCriterion` to `NavigationNodeData` and `LoginNodeData` in `types.ts`.
>     - Update `NavigationBlock`, `LoginBlock`, `NavigationBlockYAML`, and `LoginBlockYAML` in `workflowTypes.ts` and `workflowYamlTypes.ts` to include new criteria.
>
> <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 a267b3843943cb666f0a14748ec2d03c0e0b4077. 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 a1617e6 in 30 seconds

More details
  • Looked at 220 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/workflowEditorUtils.ts:922
  • Draft comment:
    Consider adding validation for completeCriterion and terminateCriterion in getWorkflowErrors for NavigationNode and LoginNode to ensure at least one is provided.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_nRKxjB2SmU0gNY8t


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

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.

❌ Changes requested. Incremental review on a1617e6 in 53 seconds

More details
  • Looked at 220 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/editor/nodes/NavigationNode/NavigationNode.tsx:144
  • Draft comment:
    terminateCriterion is added to the state but not used in the UI. Consider adding a UI component for it, similar to completeCriterion.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_5rClEatydx2jSg9H


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -41,6 +41,8 @@ function LoginNode({ id, data }: NodeProps<LoginNode>) {
cacheActions: data.cacheActions,
totpVerificationUrl: data.totpVerificationUrl,
totpIdentifier: data.totpIdentifier,
completeCriterion: data.completeCriterion,
terminateCriterion: data.terminateCriterion,
});
const deleteNodeCallback = useDeleteNodeCallback();
Copy link
Contributor

Choose a reason for hiding this comment

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

terminateCriterion is added to the state but not used in the UI. Consider adding a UI component for it, similar to completeCriterion.

@msalihaltun msalihaltun merged commit e61f663 into main Dec 13, 2024
2 checks passed
@msalihaltun msalihaltun deleted the salih/complete-criterion-on-task-based-blocks branch December 13, 2024 16:13
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