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 a phone number column to the user_accounts table #2075

Conversation

plunkettgoogle
Copy link
Contributor

Add a phone number column to the user_accounts table.

Description

This change adds a phone number to the user_account table, adds the corresponding migration, and updates the swagger API.

How Can This Be Tested/Reviewed?

  1. Start a local server via yarn dev:all.
  2. Go to http://localhost:3100/docs and use the auth/login endpoint to log in as one of the test accounts.
  3. Use the auth token to set the bearer token for subsequent requests (click the lil' "lock" icon, and paste the auth token)
  4. GET the user JSON through the user endpoint
  5. Paste the user JSON into the request body of the PUT user/{id} endpoint, and modify the phone number field
  6. GET the user JSON again through the user endpoint, and verify the phone number was set.

To run the added test:
cd backend/core && yarn test:e2e:local --testPathPattern user.e2e-spec --testNamePattern "modify their phone number"

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have reviewed the changes in a desktop view
  • I have reviewed the changes in a mobile view
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have assigned reviewers
  • I have run yarn generate:client and/or created a migration if I made backend changes that require them
  • I have exported any new pieces added to ui-components
  • My commit message(s) is/are polished, and any breaking changes are indicated in the message and are well-described
  • Commits made across packages purposefully have the same commit message/version change, else are separated into different commits

Reviewer Notes:

Steps to review a PR:

  • Read and understand the issue
  • Review the code itself from a style point of view
  • Pull the changes down locally and test that the acceptance criteria is met
  • Also review the acceptance criteria on the Netlify deploy preview (noting that these do not yet include any backend changes made in the PR)
  • Either explicitly ask a clarifying question, request changes, or approve the PR if there are small remaining changes but the PR is otherwise good to go

On Merge:

If you have one commit and message, squash. If you need each message to be applied, rebase and merge.

…-user-entity

Add phone number to user accounts.
@netlify
Copy link

netlify bot commented Oct 26, 2021

👷 Deploy request for dev-partners-bloom pending review.
Visit the deploys page to approve it

🔨 Explore the source changes: c6ce525

@netlify
Copy link

netlify bot commented Oct 26, 2021

👷 Deploy request for dev-bloom pending review.
Visit the deploys page to approve it

🔨 Explore the source changes: c6ce525

@netlify
Copy link

netlify bot commented Oct 26, 2021

✔️ Deploy Preview for dev-storybook-bloom ready!

🔨 Explore the source changes: c6ce525

🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-storybook-bloom/deploys/617888f148d98c00088e04af

😎 Browse the preview: https://deploy-preview-2075--dev-storybook-bloom.netlify.app

@plunkettgoogle
Copy link
Contributor Author

Apologies, the test I added is not passing (it was passing in the downstream project). I'll address that soon.

@plunkettgoogle
Copy link
Contributor Author

Ah, actually I see that the /user/:id endpoint is admin only after #1862, which has not yet landed in the downstream CityOfDetroit project. I'll close this, and reopen another PR once we've pulled in those changes.

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.

1 participant