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

Member phone validator #388

Closed
wants to merge 7 commits into from
Closed

Conversation

b-d055
Copy link
Contributor

@b-d055 b-d055 commented Jun 20, 2024

Attempt to address #374

@b-d055 b-d055 changed the base branch from main to quoncc/buildingbin June 20, 2024 22:30
@b-d055 b-d055 changed the base branch from quoncc/buildingbin to main June 20, 2024 22:30
@Andrew-Dickinson
Copy link
Member

Hey @b-d055. Thanks for your contribution. Sorry we didn't look at this earlier!

This change looks good to me, but we should have a few tests just to confirm it's behaving correctly. Could you add a unit test for:

  1. Basic validation that creating a new model object and doing model.save() works with a valid phone number but fails with an invalid one
  2. Submitting the join form with an invalid phone number throws an error
  3. Attempting to change the phone number via the API fails with an invalid number

@Andrew-Dickinson
Copy link
Member

Also be sure to rebase onto main, we've made some linting changes so you'll want to double check that invoke lint is working as well

@Andrew-Dickinson
Copy link
Member

Closing this in favor of #450

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