Skip to content

feat: email verification - #925

Closed
moshfeu wants to merge 8 commits into
masterfrom
email-verification
Closed

feat: email verification#925
moshfeu wants to merge 8 commits into
masterfrom
email-verification

Conversation

@moshfeu

@moshfeu moshfeu commented Apr 28, 2022

Copy link
Copy Markdown
Member

The flow

  • User register
  • A verification email sent by auth0
  • If the user still not verified, we show them a modal

Screen Shot 2022-05-04 at 12 50 18

- Clicking the button will send the user another verification email

Screen Shot 2022-05-04 at 12 51 48

Product decisions

  • User who is not verified can only do what anonymous user can do.
    • The only UI impact is that the login / register link is replaced by an empty avatar so the user can logout
  • All the users who are not verified, including existing, must verify their email (we expect to more support requests)

Server PR - Coding-Coach/find-a-mentor-api#249

todo: resend email logic
@vercel

vercel Bot commented Apr 28, 2022

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
find-a-mentor ❌ Failed (Inspect) May 6, 2022 at 6:13AM (UTC)

todo: resend verification email
todo: handle verification callback - success / error
@moshfeu
moshfeu marked this pull request as ready for review May 4, 2022 09:31
Comment thread src/context/userContext/UserContext.tsx Outdated
isMentor: boolean;
isLoading: boolean;
currentUser?: User;
emailVerifedInfo?: EmailNotVerifiedInfo;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Typo: emailVerifiedInfo

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thank you!
If this is the only issue with the changes I made in the auth0 area, I'm happy ;)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The changes made regarding Auth0 are limited I would say. But yeah, looks good!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks!
I mostly interested in a feedback about the verification email process.
If you can take a look on the server PR too it would be great.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks good 🧙

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks!

Comment thread src/api/index.ts
Comment on lines +118 to +122
ApiService.clearCurrentUserFromStorage();
}

// because we need to call it from authContext which doesn't have access to ApiService
static clearCurrentUserFromStorage = () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this static method still necessary after adding forgetUser to the auth util?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes. forgetUser calls this function.
The problem is that auth/utils not always has a reference to API so if it has reference it calls to clearCurrentUser but if not (it means that apiService is not initiated), it calls clearCurrentUserFromStorage directly.

We definitely should improve the design / structure of the auth / user area. We 2 providers and 2 services.
I did some research about auth0 react wrapper, it can help us clear our code but I didn't want to include this refactor in this PR to avoid big changes at once.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sounds good. Thanks.

@moshfeu moshfeu mentioned this pull request Apr 11, 2025
1 task
@moshfeu

moshfeu commented Apr 16, 2025

Copy link
Copy Markdown
Member Author

Done in #977

@moshfeu moshfeu closed this Apr 16, 2025
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