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 where empty webhook url causes an issue #1285

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Nov 28, 2024

Important

Fixes validation issue in RunWorkflowForm.tsx for webhookCallbackUrl by checking for both null and empty strings.

  • Bug Fix:
    • Fixes validation issue in RunWorkflowForm.tsx for webhookCallbackUrl by checking for both null and empty strings.

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

…src/'

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Fixes validation issue in `RunWorkflowForm.tsx` for empty `webhookCallbackUrl` by checking for both `null` and empty strings.
>
>   - **Bug Fix**:
>     - Fixes issue in `RunWorkflowForm.tsx` where an empty `webhookCallbackUrl` caused validation issues by updating the validation rule to check for both `null` and empty string values.
>
> <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 1278c841938be07ae64edcf61bc80388c6fee8b2. 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! Incremental review on eeb841c in 11 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/RunWorkflowForm.tsx:257
  • Draft comment:
    Consider returning a specific error message when webhookCallbackUrl is null or an empty string, similar to other validation rules.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The validation logic for webhookCallbackUrl should return a specific error message when the value is null or an empty string, to maintain consistency with other validation rules.

Workflow ID: wflow_WGvHBRBtdBkFFhon


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. Reviewed everything up to eeb841c in 40 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_IuZtGYuUGBpx4M8w


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.

@@ -253,7 +253,7 @@ function RunWorkflowForm({
name="webhookCallbackUrl"
rules={{
validate: (value) => {
if (value === null) {
if (value === null || value === "") {
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider returning a specific error message when the value is null or an empty string to maintain consistency with other validation rules.

@msalihaltun msalihaltun merged commit 28250e1 into main Nov 28, 2024
2 checks passed
@msalihaltun msalihaltun deleted the salih/webhook-callback-url-error branch November 28, 2024 14:51
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