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: constraint validation affecting disabled button #4183

Merged
merged 3 commits into from
Jul 7, 2023

Conversation

kwasniew
Copy link
Contributor

@kwasniew kwasniew commented Jul 7, 2023

About the changes

Screenshot 2023-07-07 at 14 31 02

Fixing regression introduced by #3914.
We were not disabling save strategy or add to change request button on invalid constraints because we were filtering out invalid values locally in the frontend and excluding them from checking.

This PR fixes the regression and also introduces unit tests describing the new behavior.

Important files

Discussion points

@vercel
Copy link

vercel bot commented Jul 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 7, 2023 0:44am
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 7, 2023 0:44am


return hasValues || hasValue;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is wrong. we were excluding locally invalid constraints from the remote check without saying those items are invalid

Array.isArray(constraint.values) &&
constraint.values.length > 0;
const hasValue = Boolean(constraint.value);
const invalidConstraints = constraints.find(item => !isValid(item));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

check locally invalid constraints that don't require server checks

Copy link
Member

Choose a reason for hiding this comment

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

do we highlight what's wrong? it can be confusing when button is disabled and there is no validation error

Array.isArray(constraint.values) &&
constraint.values.length > 0;
const hasValue = Boolean(constraint.value);
const invalidConstraints = constraints.find(item => !isValid(item));
Copy link
Member

Choose a reason for hiding this comment

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

do we highlight what's wrong? it can be confusing when button is disabled and there is no validation error

@kwasniew
Copy link
Contributor Author

kwasniew commented Jul 7, 2023

@Tymek previously we were not showing anything in this scenario. This is only when you add an empty constraint and try to submit it. When you try to save the constraint we have proper validation.

@kwasniew kwasniew merged commit 748bfaa into main Jul 7, 2023
15 of 16 checks passed
@kwasniew kwasniew deleted the fix-form-constraint-validation branch July 7, 2023 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants