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

Tweak key people validation handling and heading #95

Merged
merged 3 commits into from
Mar 5, 2018

Conversation

irisfaraway
Copy link
Member

These changes are following the QA review.

The bulk of the changes are about how we handle validation messages. This commit reduces the number of validation errors which a user can get at one time:

  • Previously a totally blank form would trigger about 6 different messages - now it should just be one
  • Date validation errors have also been simplified so if all fields are missing, we only display a single message
  • We also only check that the complete date is a valid date once we've validated the individual fields to reduce the number of errors users get
  • Date fields which don't contain valid integers are no longer converted to 0, which gives us better messages too

Also changed the heading text for overseas users to match the wireframe.

These changes are following the QA review. The bulk of the changes are about how we handle validation messages. This commit reduces the number of validation errors which a user can get for submitting the form without all the required information. Previously a totally blank form would trigger about 6 different messages - now it should just be one. Date validation errors have also been simplified so if all fields are missing, we only display a single message. We also only check that the complete date is a valid date once we've validated the individual fields to reduce the number of errors users get.

Also changed the heading text for overseas users.
@irisfaraway irisfaraway self-assigned this Mar 5, 2018
@irisfaraway irisfaraway requested review from Cruikshanks and removed request for Cruikshanks March 5, 2018 12:03
@irisfaraway irisfaraway changed the title Tweak validation handling and heading Tweak key people validation handling and heading Mar 5, 2018
@irisfaraway
Copy link
Member Author

@Cruikshanks Would you mind having a quick look at this when you've got the time? I had to make some fairly big changes to how this form is validated (added another custom validator, which then calls a second custom validator) so it could probably benefit from a second pair of eyes.

return unless field_is_integer?(record, type, field)
def all_fields_empty?(record)
fields = [record.dob_day, record.dob_month, record.dob_year].compact
return false if fields.any?
Copy link
Member

Choose a reason for hiding this comment

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

🍬 👍

@irisfaraway irisfaraway merged commit 2e6964e into master Mar 5, 2018
@irisfaraway irisfaraway deleted the fix/key-person-review-changes branch March 5, 2018 13:11
@irisfaraway irisfaraway added the bug Something isn't working label Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants