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
Bug 1836902: validate event sources form and enable create button on yaml form #5479
Bug 1836902: validate event sources form and enable create button on yaml form #5479
Conversation
@sahil143: This pull request references Bugzilla bug 1836902, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
b35e24c
to
5275820
Compare
const handleItemChange = React.useCallback( | ||
(item: string) => { | ||
setErrors({}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, to clear any validation error once another form is selected.
/test images |
Thanks @sahil143, verified locally seems to work as expected. |
/approve /assign @rohitkrai03 |
@@ -33,6 +33,7 @@ const ApiServerSection: React.FC = () => { | |||
Ref: 'Ref', | |||
Resource: 'Resource', | |||
}; | |||
useFormikValidationFix(values); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sahil143 Instead of calling useFormikValidationFix
in each section of the form, i think it would be better to called it once in EventSourceForm
. Ultimately our goal is to trigger validateForm
whenever anything inside values
changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Will update this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the hook in EventSourceSection should work.
fix unit tests fix lint error fix unit tests update validation
5275820
to
27b25d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: invincibleJai, rohitkrai03, sahil143 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
@sahil143: All pull requests linked via external trackers have merged: openshift/console#5479. Bugzilla bug 1836902 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Fixes: https://issues.redhat.com/browse/ODC-3591
Analysis / Root cause:
create
button on the YAML form for event sources is disabled which should be enabled. yup validation was being called for other forms even when YAML form card was selected. For example: Initially ApiServeSource form is selected and when camel form(yaml form) card is clicked on form is switched to camel but the validation error from the ApiServerSource form remains because of which create button was disabled.Solution Description:
fixed the yup validation by making it manual instead of being called through
ValidationOnChange
andValidationOnBlur
.Screen shots / Gifs for design review:
Unit test coverage report:
Test setup:
Browser conformance: