Skip to content

Conversation

@liammoyn
Copy link
Contributor

Why

To verify an email a user needs to create an account and then visit the link sent to them by the backend. This PR includes the necessary changes to make this work with the backend scaffold.

This PR

  1. Removed username field from signup form because it is not in the data model we're using.
  2. Changed types so that the confirmPassword key is not sent to the backend on signup since the backend is not expecting that field
  3. Duplicated the verifyEmail route into the protected routes as well, typically a user will verify their email directly after signing up which means they'll have a valid JWT token in their local storage

Verification Steps

Tested sending a verification email to myself locally running the backend scaffold and the frontend scaffold from creating a user to clicking the verify link.

@liammoyn liammoyn requested a review from jackblanc March 19, 2021 13:59
Comment on lines +31 to +34
// confirmPassword is checked in the form but should not be sent in the request
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { confirmPassword, ...request } = values;
dispatch(signup(request));
Copy link
Member

Choose a reason for hiding this comment

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

interesting... okay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah pretty weird. We could change the backend to not fail with extra parameters in the request but having it there will probably save us from bugs in the long term. If you have any cleaner way of getting rid of unwanted parameters in a JS object then we can do that here instead of the destructing call

@liammoyn liammoyn merged commit 198d258 into master Apr 2, 2021
@liammoyn liammoyn deleted the verify-email branch April 2, 2021 15:00
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