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

126 edit validation fixes #1238

Merged
merged 4 commits into from
Mar 4, 2019
Merged

Conversation

kialj876
Copy link
Collaborator

@kialj876 kialj876 commented Mar 1, 2019

Issue #:bcgov/entity#126

Description of changes:

  • added validation for name choices around 2 & 3
  • jest tests added and all pass locally

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the namex license (Apache 2.0).

Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Signed-off-by: Kial Jinnah <kialj876@gmail.com>
Copy link
Contributor

@katiemcgoff katiemcgoff left a comment

Choose a reason for hiding this comment

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

If you have time:
looks like this validation (and the existing validation on name 1) doesn't handle spaces properly, ie: if you just enter a single space, it thinks that name 1 or name 2 is filled in. This is a downfall of the vuelidate "required" checker and could be handled by adding a custom "not blank" checker as well. See the custom validation on nwpta dates for example.

If you don't get to this before you want to deploy that's fine, it can be logged as a separate task since it's half pre-existing (name 1) and half new (name 2).

Signed-off-by: Kial Jinnah <kialj876@gmail.com>
@kialj876 kialj876 requested a review from rarmitag March 4, 2019 20:01
@rarmitag rarmitag merged commit e6f6375 into bcgov:master Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants