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(ui): Fix yup validation bugs #594

Merged
merged 3 commits into from
Jul 23, 2024
Merged

fix(ui): Fix yup validation bugs #594

merged 3 commits into from
Jul 23, 2024

Conversation

deadlycoconuts
Copy link
Contributor

Description

Similar to caraml-dev/turing#387, this PR to fixes broken validation schemas caused by the upgrade of the yup package in PR #591 - these changes can largely be grouped into the following:

  • The Schema.when function has been updated, such that when the builder object (one with the fields is, then, and sometimes otherwise defined) is passed as an argument, the then field accepts ONLY functions of the following signature (schema: Schema) => Schema instead of Schema previously.

    Existing schemas that do not follow this convention have been updated and turned into arrow functions.

    Example:

    // before
    some_field: yup.string().when("some_other_field", {
      is: "nop",
      then: yup.string().required("this is needed")
    }),
    
    // after
    some_field: yup.string().when("some_other_field", {
      is: "nop",
      then: (_) => yup.string().required("this is needed")
    }),
  • When using the same Schema.when function but passing a function directly as an argument instead of the builder object, the signature of the expected function has also been updated from (value, schema)=> Schema): Schema to (values: any[], schema) => Schema): Schema.

    Existing schemas that do not follow this convention now have been updated to reflect the new expected array:

    Example:

    // before
    some_field: yup.string().when("some_other_field", (some_other_field, schema) =>
      some_other_field ? yup.string().required("this is needed") : schema
    ),
    
    // after
    some_field: yup.string().when("some_other_field", ([some_other_field], schema) =>
      some_other_field ? yup.string().required("this is needed") : schema
    ),

Unrelated Update

This PR also bumps up the version of the caraml-dev/ui-lib library to the latest version.

Modifications

  • UI bug fixes
  • Update to caraml-dev/ui-lib library

Tests

Checklist

  • Added PR label
  • Added unit test, integration, and/or e2e tests
  • Tested locally
  • Updated documentation
  • Update Swagger spec if the PR introduce API changes
  • Regenerated Golang and Python client if the PR introduces API changes

Release Notes

NONE

@deadlycoconuts deadlycoconuts added the bug Something isn't working label Jul 23, 2024
@deadlycoconuts deadlycoconuts self-assigned this Jul 23, 2024
Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.52%. Comparing base (5b37909) to head (032dbbc).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #594   +/-   ##
=======================================
  Coverage   60.52%   60.52%           
=======================================
  Files         274      274           
  Lines       22080    22080           
=======================================
  Hits        13365    13365           
  Misses       7859     7859           
  Partials      856      856           
Flag Coverage Δ
api-test 58.48% <ø> (ø)
sdk-test-3.10 75.45% <ø> (ø)
sdk-test-3.8 75.44% <ø> (ø)
sdk-test-3.9 75.44% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@deadlycoconuts deadlycoconuts requested a review from leonlnj July 23, 2024 06:04
@deadlycoconuts
Copy link
Contributor Author

Thanks for the quick approval! Merging this now! 🚀

@deadlycoconuts deadlycoconuts merged commit 5e91806 into main Jul 23, 2024
36 checks passed
@deadlycoconuts deadlycoconuts deleted the fix_yup_upgrade_bugs branch July 23, 2024 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants