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

Verification Page #194

Merged
merged 19 commits into from
Dec 3, 2023
Merged

Verification Page #194

merged 19 commits into from
Dec 3, 2023

Conversation

owen-sellner
Copy link
Contributor

@owen-sellner owen-sellner commented Nov 5, 2023

Notion task link

Login/Signup Verification

Implementation description

  • Updated AuthenticatedUser type to include if the user is verified in AuthTypes.tsx
  • Updated login and register in auth_routes.py
  • Created is_authorized_by_token in auth_service.py to check if a user is verified using the access token
  • Created isVerified in AuthAPIClient.ts for the verification page button to update the AuthenticatedUser context if the user becomes verified

Steps to test

  1. Access the database using docker exec -it SHOW-database /bin/bash -c "psql -U postgres -d show_db"
  2. Invite a new account using insert into users (first_name, last_name, role, user_status, email) values ('your first name', 'your last name', 'Admin', 'Invited', 'your email');
  3. Check if after the Signup page the user is taken to the verification page
  4. Check that pressing the button on the Verification page prompts the user with an error toast message
  5. Verify account from email
  6. Check that pressing the button on the Verification page takes the user to the homepage
  7. Logout from the account
  8. Sign in to account from Login page and check that the user is taken directly to the homepage

What should reviewers focus on?

  • Should there be a button for the user to logout from the Verification page to prevent getting stuck if the verification email expires?

Screenshot

image

Checklist

  • My PR name is descriptive and in imperative tense
  • My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
  • I have run the appropriate linter(s)
  • I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR
  • If I have made API changes, I have updated the REST API Docs
  • IF I have made changes to the db/models, I have updated the Data Models Page
  • I have documented this PR in the Production Release Page
  • I have updated other Docs as needed

Copy link

github-actions bot commented Nov 12, 2023

Visit the preview URL for this PR (updated for commit 4a08a56):

https://blueprintsupportivehousing--pr194-owen-verify-2crodrxo.web.app

(expires Sun, 10 Dec 2023 21:15:10 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f6bcdba7452bf82a6ec1a299c37d1bdff7870d09

@owen-sellner owen-sellner marked this pull request as ready for review November 23, 2023 04:50
@owen-sellner owen-sellner changed the title Owen/verify Verification Page Nov 23, 2023
Copy link
Collaborator

@connor-bechthold connor-bechthold left a comment

Choose a reason for hiding this comment

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

you did a really good job on this future pl moment fr

had a couple nits/housekeeping comments to look at but otherwise this is good to go 👨‍🍳

frontend/src/components/auth/PrivateRoute.tsx Outdated Show resolved Hide resolved
frontend/src/components/auth/Verification.tsx Outdated Show resolved Hide resolved
frontend/src/components/auth/Verification.tsx Outdated Show resolved Hide resolved
frontend/src/components/forms/Signup.tsx Outdated Show resolved Hide resolved
frontend/src/components/forms/Signup.tsx Outdated Show resolved Hide resolved
const location = useLocation();
const currentPath = location.pathname;

if (authenticatedUser === null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think it might be better to check for all possible falsey values here by simply checking for if (!authenticatedUser) for extra safety

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@kevin-pierce kevin-pierce left a comment

Choose a reason for hiding this comment

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

One minor nit, and a more general q about the exception handling for the verification endpoint ; personally I think that the exception stuff would require a bit of an overhaul, and so perhaps it could be broken out into its own ticket

Copy link
Contributor

@phamkelly17 phamkelly17 left a comment

Choose a reason for hiding this comment

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

lgtm

@owen-sellner owen-sellner merged commit 0c19df7 into main Dec 3, 2023
3 checks passed
@owen-sellner owen-sellner deleted the owen/verify branch December 3, 2023 21:32
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.

4 participants