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: validation for variant payload number type #4671

Merged
merged 3 commits into from
Sep 13, 2023

Conversation

andreas-unleash
Copy link
Contributor

Adds validation to number type for variants

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
@vercel
Copy link

vercel bot commented Sep 12, 2023

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

Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 13, 2023 7:02am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Sep 13, 2023 7:02am

@andreas-unleash andreas-unleash changed the title fix: validation fix: validation for variant payload number type Sep 12, 2023
@@ -293,7 +293,7 @@ export const VariantForm = ({
JSON.parse(payload.value);
}
if (variantTypeNumber && payload.type === 'number') {
Number(payload.value);
return !isNaN(Number(payload.value));
Copy link
Contributor

Choose a reason for hiding this comment

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

it shouldn't matter here but in general it's safer to use Number.isNaN over global isNaN

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Copy link
Contributor

@kwasniew kwasniew left a comment

Choose a reason for hiding this comment

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

Personally if I was touching a component without a test I'd add one to test the new stuff I added

@@ -262,7 +262,7 @@ export const VariantForm = ({

const validatePayload = (payload: IPayload) => {
if (!isValidPayload(payload)) {
setError(ErrorField.PAYLOAD, 'Invalid JSON.');
setError(ErrorField.PAYLOAD, 'Invalid value.');
Copy link
Member

@ivarconr ivarconr Sep 12, 2023

Choose a reason for hiding this comment

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

Can we use the payload.type to specify "Invalid JSON" or "Invalid Number"

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
@andreas-unleash andreas-unleash merged commit 8b45208 into main Sep 13, 2023
15 checks passed
@andreas-unleash andreas-unleash deleted the fix/number_variant_validation branch September 13, 2023 07:23
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

3 participants