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

Add range validation to number field #378

Closed
r00dgirl opened this issue Sep 28, 2020 · 12 comments · Fixed by #6575
Closed

Add range validation to number field #378

r00dgirl opened this issue Sep 28, 2020 · 12 comments · Fixed by #6575
Labels
contribute free for contributors to pick up P3

Comments

@r00dgirl
Copy link
Contributor

No description provided.

@r00dgirl r00dgirl added the P3 label Sep 28, 2020
@r00dgirl r00dgirl added the contribute free for contributors to pick up label Mar 10, 2021
@r00dgirl r00dgirl changed the title Number field should have range validation instead of number of characters Add range validation to number field May 11, 2021
@aniruddha-adhikary
Copy link
Contributor

@syan-syan Can more information be provided on the scope of this ticket? I can see that the src/app/models/field/numberField.ts descriptor already defines the ranges. What exactly needs to be done in this?

Thanks!

@mantariksh
Copy link
Contributor

mantariksh commented Jun 7, 2021

Hey @aniruddha-adhikary thanks for reaching out!

If you check out src/app/utils/field-validation/validators/numberValidator.ts, you can see that the current validation on the number field applies to the number of characters in the number rather than the actual value. We want to add range validation so users can specify that the number should only fall within a given range.

In terms of implementation, this will broadly involve the following:

  1. Add a new set of options to src/app/models/field/numberField.ts which specifies the validation range for a given field
  2. Allow these options to be updated by admins in the frontend, which will mostly involve updating src/public/modules/forms/admin/controllers/edit-fields-modal.client.controller.js
  3. Add range validation to number fields on public forms, so that the UI prevents form-fillers from submitting answers to number fields which are outside the allowed range. Most of the changes will likely be in src/public/modules/forms/base/componentViews/field-number.client.view.html.
  4. Add backend validation in src/app/utils/field-validation/validators/numberValidator.ts to ensure that submissions are validated in the backend.

Bear in mind that parts 2 and 3 might be a bit convoluted because the frontend is admittedly in a bit of a mess, so feel free to reach out if you have any trouble with it. Just FYI, we are going to rewrite the entire frontend in React before the end of this year.

And feel free to reach out if you have any more questions!

@aniruddha-adhikary
Copy link
Contributor

May I be assigned to it? I am working on it #2088

@r00dgirl
Copy link
Contributor Author

r00dgirl commented Jun 7, 2021

sure thing, have assigned you @aniruddha-adhikary

@aniruddha-adhikary
Copy link
Contributor

@syan-syan @mantariksh #2088 is ready for review 👍🏽

@pearlyong
Copy link

Designs for number validation here. Other options considered:

  1. Two different toggles to turn Length Validation and Range Validation on. This way, it is very clear what kind of validation you are choosing. But this solution might require some logic if user tries to turn both on and set conflicting values.
  2. Add another option with the current dropdown so there are 4 options (min/max/exact/range - the current proposed solution). This could be confusing to users because there are two different types of validations within the same dropdown.
  3. Therefore the proposed design is to have a Dropdown to select type of validation first (length/value), then select min/max/exact and enter number of chars. (or select Value, then enter min/max value).

@syan-syan lmk if you have other thoughts

No validation
Length validation
Value validation

@r00dgirl
Copy link
Contributor Author

@pearlyong lgtm!

@mantariksh
Copy link
Contributor

pending new React designs

@pearlyong
Copy link

updated for React

@LeonardYam
Copy link
Contributor

Hi @mantariksh, I would like to work on this! From what I understand, a working implementation would require:

  1. Updating shared/types/fields/numberField.ts to include the additional range validation options
  2. Rewriting the form builder drawer in frontend/src/features/admin-form/create/builder-and-design/BuilderAndDesignDrawer/EditFieldDrawer/edit-fieldtype/EditNumber/EditNumber.tsx by adding new components, updating the types and field names etc.
  3. Updating the backend schema in src/app/models/field/numberField.ts
  4. Updating the validators in src/app/utils/field-validation/validators/numberValidator.ts and frontend/src/utils/fieldValidation.ts

Are there any additional steps I have missed?

@mantariksh
Copy link
Contributor

hi @LeonardYam unfortunately I'm no longer on this team but feel free to check with any of the other contributors! FYI @timotheeg

@timotheeg
Copy link
Contributor

Hi @LeonardYam, just like Antariksh, I too have recently left the FormSG team. Let me direct you to a few people that can assist you. Thanks in advance for your future contributions!

cc @sebastianwzq @tshuli

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribute free for contributors to pick up P3
Projects
None yet
6 participants