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

🪟 🐛 Update <ConditionSection /> to update values after form validation is updated #14802

Closed
wants to merge 3 commits into from

Conversation

edmundito
Copy link
Contributor

@edmundito edmundito commented Jul 18, 2022

What

Fixes #12457

This updates the ConditionSection component only to update the value after form validation has been set because otherwise, it tries to validate the fields before the validation is updated. This case is specific to when the condition has no values to fill out.

How

In the ConditionSection component, both the form value and widgetsInfo would be updated one after the other. When widgetsInfo is updated, it updates the form validation because the form shape has changed (from the previously selected condition to the new one). Now it re-runs validation after widgetsInfo updates the component flagged for re-validation.

Recommended reading order

  • One file to read

Tests

  • Tested on snowflake destination

@edmundito edmundito added area/frontend area/frontend Related to the Airbyte webapp labels Jul 18, 2022
@edmundito edmundito requested a review from a team as a code owner July 18, 2022 17:22
@github-actions github-actions bot added the area/platform issues related to the platform label Jul 18, 2022
@edmundito edmundito changed the title Update <ConditionSection /> to update values after form validation is updated 🪟 🐛 Update <ConditionSection /> to update values after form validation is updated Jul 18, 2022
Copy link
Contributor

@teallarson teallarson left a comment

Choose a reason for hiding this comment

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

Code LGTM. I can't get a Snowflake/S3 destination set up with the credentials in lastpass so have not tested the usecase from the original issue.

@timroes
Copy link
Collaborator

timroes commented Jul 28, 2022

When I reviewed this, I was thinking: Don't we have code that does exactly that revalidation of a form when the validation schema changes, and came across: https://github.com/airbytehq/airbyte/blob/master/airbyte-webapp/src/views/Connector/ServiceForm/useBuildForm.tsx#L127-L129

Digging a bit deeper, it seems this does currently not work because of a Formik bug (jaredpalmer/formik#2092) that simply always returns undefined when trying to get the validationSchema from the formik context (no matter that there is actually a proper validationSchema).

I opened a PR trying to fix that high-level code: #15109 which could superseed this PR. From what I've tested this should fix the problem as well, and gets rid of the high level non-working code anymore (in useBuildForm). Could you maybe double check if that fixes the same issue, and if we can superseed this PR by #15109?

@timroes
Copy link
Collaborator

timroes commented Jul 28, 2022

Digging into this I also created #15108, which I believe is what we should eventually trying to go for and not have validationSchemas changing over time.

@edmundito
Copy link
Contributor Author

Replaced by #15109 - Tested after merge and fixes the issue.

@edmundito edmundito closed this Aug 4, 2022
@edmundito edmundito deleted the edmundito/fix-conditional-validation branch August 4, 2022 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty config objects inside oneOfs cannot be saved
3 participants