Add link to resend verification email. #227
Conversation
|
Blocking because ibi-group/otp-middleware#71. |
binh-dam-ibigroup
changed the title
binh-dam-ibigroup
removed
the
BLOCKED
| <Button | ||
| bsSize='large' | ||
| bsStyle='primary' | ||
| onClick={this._handleClick} |
There was a problem hiding this comment.
Since there are now 2 buttons, can this handler be renamed to something more specific?
evansiroky
left a comment
evansiroky
left a comment
There was a problem hiding this comment.
Approved assuming the 1 small change I requested is implemented.
lib/actions/user.js
Outdated
| export function resendVerificationEmail (auth0) { | ||
| return async function (dispatch, getState) { | ||
| const { apiBaseUrl, apiKey } = getMiddlewareVariables(getState()) | ||
| const accessToken = await auth0.getAccessTokenSilently() |
There was a problem hiding this comment.
Isn't accessToken stored in the reducer now? See line 123 above. My initial impression is that we should not be passing auth0 to this method.
There was a problem hiding this comment.
accessToken is not stored yet because it is set that the same time as the user, and the user didn't get set yet. Should that get split?
There was a problem hiding this comment.
Hmm, I'm not sure. Perhaps we should split the set access token functionality out from set logged in user. What do you think about having an additional check in UserLoaderScreen that checks if accessToken is available from the store? If not, it can just call auth0.getAccessTokenSilently() and store it. That would allow us to then fetch or initialize user without passing auth0.
// UserLoaderScreen
componentDidUpdate () {
const { accessToken, auth0, fetchOrInitializeUser, setAccessToken } = this.props
if (!accessToken) {
let token = await auth0.getAccessTokenSilently()
setAccessToken({accessToken: token, auth0User: auth0.user})
}
if (accessToken && this.loggedInUserIsUnfetched()) {
fetchOrInitializeUser() // can now get accessToken and auth0User from store
}
}There was a problem hiding this comment.
@landonreed let me think what you think of this: b9b0ba3. We still need the auth0 argument to initialize a default user (needs the auth0 id and email).
There was a problem hiding this comment.
That looks good to me. But resendVerificationEmail no longer needs the auth0 param now, right?
| <DivSpacer> | ||
| Please check your email inbox and follow the link in the message | ||
| to verify your email address before finishing your account setup. | ||
| </DivSpacer> |
There was a problem hiding this comment.
I'd prefer to use a p tag here. It should have some margin at the bottom.
landonreed
left a comment
landonreed
left a comment
There was a problem hiding this comment.
Looks good, but can we grab accessToken similar to the other methods?
lib/actions/user.js
Outdated
| * If absent, state.user.accessToken will be used for fetches. | ||
| * @param auth0 If provided, the auth0 login object used to initialize the default user object (with email and auth0 id). | ||
| */ | ||
| export function fetchOrInitializeUser (auth0) { |
There was a problem hiding this comment.
I think this arg should be changed to auth0User.
There was a problem hiding this comment.
How about auth0Context? Because the actual auth0 user is in auth0.user, which is passed to method createNewUser.
landonreed
left a comment
landonreed
left a comment
There was a problem hiding this comment.
Looks good, conditional approval after addressing a couple of final comments.
Remove auth0 as arg for resendVerificationEmail; change arg of fetchOrInitializeUser to auth0User; cleanup.
|
🎉 This PR is included in version 2.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR adds a link to resend an email verification message hooked to the proper OTP middleware endpoint.
To test, use the backend code from ibi-group/otp-middleware#95.