SWC-7228 #1511
SWC-7228 #1511
Conversation
| function BackButtonForPage(props: { | ||
| page: Pages | ||
| setPage: (page: Pages) => void | ||
| }) { | ||
| const { page, setPage } = props | ||
| switch (page) { | ||
| case Pages.CHOOSE_REGISTRATION: | ||
| return <BackButton to={'/authenticated/myaccount'} /> | ||
| case Pages.EMAIL_REGISTRATION: | ||
| case Pages.GOOGLE_REGISTRATION: | ||
| return <BackButton onClick={() => setPage(Pages.CHOOSE_REGISTRATION)} /> | ||
| default: | ||
| return <></> | ||
| } | ||
| } |
There was a problem hiding this comment.
Extracted nested component definition (eslint: react/no-unstable-nested-components)
| if (!aliasCheckResponse.available) { | ||
| displayToast('Sorry, that username has already been taken.', 'danger') | ||
| } else if (!aliasCheckResponse.valid) { | ||
| displayToast('Sorry, that username is not valid.', 'danger') |
There was a problem hiding this comment.
Need to check valid flag first. Extracted this check to a validateAlias utility.
| .then((data: any) => { | ||
| const authUrl = data.authorizationUrl | ||
| window.location.assign(authUrl) | ||
| }) | ||
| .catch((err: any) => { | ||
| displayToast(err.reason as string, 'danger') | ||
| }) |
There was a problem hiding this comment.
We're already using try/catch here, so change this to an await
There was a problem hiding this comment.
I'm confused by this comment. Are you referring to the await SynapseClient.isAliasAvailable()?
There was a problem hiding this comment.
I just mean SynapseClient.oAuthUrlRequest was written using .then(...).catch(...) when the error handling behavior inside the catch was identical to the error handling behavior in the try/catch that is already wrapping this code block.
So we get the same behavior with less, cleaner code by using await instead of promise callbacks
| function handleError(e: unknown) { | ||
| if (e instanceof SynapseClientError) { | ||
| displayToast(e.reason, 'danger') | ||
| } else if (e instanceof Error) { | ||
| displayToast(e.message, 'danger') | ||
| } else { | ||
| // This should never happen | ||
| console.error(e) | ||
| displayToast(JSON.stringify(e), 'danger') | ||
| } | ||
| } |
There was a problem hiding this comment.
Shared generic error handling on this page
| @@ -269,9 +286,9 @@ export const RegisterAccount1 = () => { | |||
| ) | |||
| } | |||
| value={email || ''} | |||
| onKeyPress={(e: any) => { | |||
There was a problem hiding this comment.
onKeyPress is deprecated
| {page === Pages.GOOGLE_REGISTRATION && ( | ||
| <Typography variant="body1" sx={{ marginBottom: '20px' }}> | ||
| Your <strong>Synapse</strong> account can also be used to | ||
| access many other resources from Sage Bionetworks. | ||
| {VALID_USERNAME_DESCRIPTION} | ||
| </Typography> | ||
| )} |
There was a problem hiding this comment.
Show the username requirements when prompted for a username
| it('throws if invalid and unavailable, warning about validity', () => { | ||
| expect(() => | ||
| validateAlias({ valid: false, available: false }), | ||
| ).toThrowError( | ||
| 'Sorry, that username is not valid. User names can contain letters, numbers, dot (.), dash (-) and underscore (_) and must be at least 3 characters long.', | ||
| ) | ||
| }) |
There was a problem hiding this comment.
this test validates the issue
| export function validateAlias(aliasCheckResponse: AliasCheckResponse) { | ||
| if (!aliasCheckResponse.valid) { | ||
| throw new Error( | ||
| `Sorry, that username is not valid. ${VALID_USERNAME_DESCRIPTION}`, |
There was a problem hiding this comment.
Really like the check for valid before available (invalid aliases are never available!)
| ) | ||
| }) | ||
| it('throws if alias is invalid', () => { | ||
| expect(() => validateAlias({ valid: false, available: true })).toThrowError( |
There was a problem hiding this comment.
I think technically this scenario is not possible, but fine to leave it
| type="button" | ||
| disabled={email && !isLoading ? false : true} |
There was a problem hiding this comment.
Yikes, did I really type this?
!isLoading ? false : true
| } | ||
| }} | ||
| /> | ||
| </StyledFormControl> | ||
| <Button | ||
| sx={buttonSx} | ||
| variant="contained" | ||
| onClick={onSignUpWithGoogle} | ||
| onClick={e => { | ||
| void onSignUpWithGoogle(e) |
There was a problem hiding this comment.
Should this also clear the error state before attempted signup (same as clicking Enter)?
setUsernameInvalidReason(null)
Clear the error in the handler so it happens in every code path
Fix alias check on account creation with Google