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

Validate user allergy input #3573

Merged
merged 1 commit into from Feb 17, 2023

Conversation

ivarnakken
Copy link
Member

Description

It's surprisingly common for people to write "ingen/nei".

Result

image

Testing

  • I have thoroughly tested my changes.

Tested with and without valid inputs.

@ivarnakken ivarnakken added review-needed Pull requests that need review small-fix Pull requests that fix something small labels Feb 14, 2023
@ivarnakken ivarnakken requested a review from a team February 14, 2023 19:44
@ivarnakken ivarnakken self-assigned this Feb 14, 2023
@linear
Copy link

linear bot commented Feb 14, 2023

app/utils/validation.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@erlingfn erlingfn left a comment

Choose a reason for hiding this comment

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

Looks good!

<Field
label="Matallergier/preferanser"
parse={(value) => value}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain why this is necessary? I don't doubt that it is, but does seem a bit weird.

Copy link
Member Author

Choose a reason for hiding this comment

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

When updating your user settings, you perform a PATCH request, and react-final-form therefore removes empty values. This means that if you remove your allergies from something to nothing, it won't be patched. This overrides their default parser, so that empty values are kept.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, this is good to know!

It's surprisingly common for people to write "ingen/nei".
@ivarnakken ivarnakken force-pushed the ivarnakken/aba-234-give-user-warning-when-typing branch from 0da9d75 to e3499bc Compare February 15, 2023 13:31
@ivarnakken ivarnakken added approved Pull requests that have been approved and removed review-needed Pull requests that need review labels Feb 15, 2023
message = 'La feltet stå tomt hvis du ikke har noen allergier/preferanser'
) =>
(value: string) => {
const notValidAnswers = ['ingen', 'ingenting', 'nei', 'nope', 'nada'];
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we should also include "nay", "nah" and "negative" :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any examples of people entering them in the allergies field?

(value: string) => {
const notValidAnswers = ['ingen', 'ingenting', 'nei', 'nope', 'nada'];

return [!notValidAnswers.includes(value.toLowerCase()), message];
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain how this syntax works! It looks interesting :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure which part you're referring to. The validator needs an array with two values; a boolean and a message. The boolean indicates if the validation has passed or not, and the message is what is given to the user. What we're actually creating here is a function returning a function which returns the array.

@ivarnakken ivarnakken merged commit ca21aa0 into master Feb 17, 2023
@ivarnakken ivarnakken deleted the ivarnakken/aba-234-give-user-warning-when-typing branch February 17, 2023 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Pull requests that have been approved small-fix Pull requests that fix something small
Projects
None yet
4 participants