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

@W-17458039 - Handle error states for social/passwordless login and reset password #2185

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b7cfad0
handle error states for reset password and social login
yunakim714 Jan 6, 2025
3857c96
add error handling for authorize passwordless
yunakim714 Jan 6, 2025
60726cc
add error handling of passwordless
yunakim714 Jan 7, 2025
865765c
add error handling for authorize idp
yunakim714 Jan 7, 2025
480a5f3
check error status for message
yunakim714 Jan 7, 2025
37f19c0
resolve double click bug
yunakim714 Jan 7, 2025
dd8f3e9
handle async login in useeffect correctly
yunakim714 Jan 7, 2025
4917ef3
check for invalid token error in passwordless login
yunakim714 Jan 7, 2025
2531c5c
handle invalid token error for reset password
yunakim714 Jan 7, 2025
05eabf9
fix profile tests
yunakim714 Jan 7, 2025
56b3de5
fix social login tests
yunakim714 Jan 7, 2025
79116bb
fix login form tests
yunakim714 Jan 7, 2025
812b92b
fix passwordless component tests
yunakim714 Jan 7, 2025
04e21fa
fix account tests
yunakim714 Jan 7, 2025
ee5e599
update error message for passwordless login error
yunakim714 Jan 10, 2025
b13835d
move error message to constants
yunakim714 Jan 10, 2025
ba3d8f4
add error message translations
yunakim714 Jan 13, 2025
8316458
check if reset password callback is an absolute url
yunakim714 Jan 14, 2025
d0344b6
throw error in sdk
yunakim714 Jan 14, 2025
95bea67
lint
yunakim714 Jan 14, 2025
c555280
lint
yunakim714 Jan 14, 2025
30ea366
update error handling for user not found
yunakim714 Jan 21, 2025
7f2c142
move user not found to constants
yunakim714 Jan 21, 2025
306bf87
Merge branch 'feature-passwordless-social-login' into W-17458039-hand…
yunakim714 Jan 21, 2025
9d7e556
fix account test
yunakim714 Jan 22, 2025
dde5352
Merge branch 'W-17458039-handle-error-states' of github.com:Salesforc…
yunakim714 Jan 22, 2025
e0cb754
revert change
yunakim714 Jan 22, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/commerce-sdk-react/src/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,7 @@ class Auth {
const usid = this.get('usid')
const mode = callbackURI ? 'callback' : 'sms'

await helpers.authorizePasswordless(
const res = await helpers.authorizePasswordless(
this.client,
{
clientSecret: this.clientSecret
Expand All @@ -1110,6 +1110,7 @@ class Auth {
mode
}
)
return res
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const PasswordlessLogin = ({
handleForgotPasswordClick={handleForgotPasswordClick}
hideEmail={true}
/>
)}
)}
</>
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const SocialLogin = ({form, idps}) => {
})
}, [idps])

const onSocialLoginClick = async () => {
const onSocialLoginClick = async (name) => {
try {
// Save the path where the user logged in
setSessionJSONItem('returnToPage', window.location.pathname)
Expand All @@ -93,7 +93,9 @@ const SocialLogin = ({form, idps}) => {
return (
config && (
<Button
onClick={onSocialLoginClick}
onClick={() => {
onSocialLoginClick(name)
}}
borderColor="gray.500"
color="blue.600"
variant="outline"
Expand Down
13 changes: 12 additions & 1 deletion packages/template-retail-react-app/app/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import {defineMessage} from 'react-intl'
import {noop} from '@salesforce/retail-react-app/app/utils/utils'
import {noop} from './utils/utils'
yunakim714 marked this conversation as resolved.
Show resolved Hide resolved

// Global app defaults
export const DEFAULT_LOCALE = 'en-US'
Expand Down Expand Up @@ -95,6 +95,10 @@ export const INVALID_TOKEN_ERROR_MESSAGE = defineMessage({
defaultMessage: 'Invalid token, please try again.',
id: 'global.error.invalid_token'
})
export const FEATURE_UNAVAILABLE_ERROR_MESSAGE = defineMessage({
defaultMessage: 'This feature is not currently available.',
id: 'global.error.feature_unavailable'
})

export const HOME_HREF = '/'

Expand Down Expand Up @@ -248,3 +252,10 @@ export const RESET_PASSWORD_LANDING_PATH = '/reset-password-landing'

// Constants for Passwordless Login
export const PASSWORDLESS_LOGIN_LANDING_PATH = '/passwordless-login-landing'

export const PASSWORDLESS_ERROR_MESSAGES = [
/callback_uri doesn't match/i,
/error getting user info/i,
/passwordless permissions error/i,
/client secret is not provided/i,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where do we based these messages on? I assume this is from API. What happens if the API adds/removes these messages?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes these messages are from the API. If the error message does not match any of these, the error will show the generic API_ERROR_MESSAGE, which is "Something went wrong! Please try again."

Copy link
Collaborator

Choose a reason for hiding this comment

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

my main concern is to have to maintain this list of constant. In a case where they change a bit of wording and an error will become obsolete.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the spreadsheet of error states I wonder if instead of matching by error message regex we should base it on status code? It seems like it would be possible

]
24 changes: 14 additions & 10 deletions packages/template-retail-react-app/app/hooks/use-auth-modal.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import ResetPasswordForm from '@salesforce/retail-react-app/app/components/reset
import RegisterForm from '@salesforce/retail-react-app/app/components/register'
import PasswordlessEmailConfirmation from '@salesforce/retail-react-app/app/components/email-confirmation/index'
import {noop} from '@salesforce/retail-react-app/app/utils/utils'
import {API_ERROR_MESSAGE, LOGIN_TYPES} from '@salesforce/retail-react-app/app/constants'
import {API_ERROR_MESSAGE, FEATURE_UNAVAILABLE_ERROR_MESSAGE, LOGIN_TYPES, PASSWORDLESS_ERROR_MESSAGES} from '@salesforce/retail-react-app/app/constants'
import useNavigation from '@salesforce/retail-react-app/app/hooks/use-navigation'
import {usePrevious} from '@salesforce/retail-react-app/app/hooks/use-previous'
import {usePasswordReset} from '@salesforce/retail-react-app/app/hooks/use-password-reset'
Expand Down Expand Up @@ -97,13 +97,17 @@ export const AuthModal = ({

const handlePasswordlessLogin = async (email) => {
try {
await authorizePasswordlessLogin.mutateAsync({userid: email})
const res = await authorizePasswordlessLogin.mutateAsync({userid: email})
if (res.status !== 200) {
const errorData = await res.json()
throw new Error(`${res.status} ${errorData.message}`)
}
Copy link
Collaborator

@hajinsuha1 hajinsuha1 Jan 9, 2025

Choose a reason for hiding this comment

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

Would we consider moving the logic to throw the error message into commerce-sdk-react?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could... Is there a specific reason to move this to the sdk?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess it would be to simplify the code within the retail react app, if we give the responsibility of throwing the error to the sdk the retail react app doesn't have to worry about checking the status code and can just catch the error when it occurs.

it'll also reduce code duplication, for example for passwordless checkout we wouldn't need to duplicate this logic

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1 on moving it to the sdk

setCurrentView(EMAIL_VIEW)
} catch (error) {
form.setError('global', {
type: 'manual',
message: formatMessage(API_ERROR_MESSAGE)
})
const message = PASSWORDLESS_ERROR_MESSAGES.some(msg => msg.test(error.message))
? formatMessage(FEATURE_UNAVAILABLE_ERROR_MESSAGE)
: formatMessage(API_ERROR_MESSAGE)
form.setError('global', { type: 'manual', message })
}
}

Expand Down Expand Up @@ -169,10 +173,10 @@ export const AuthModal = ({
try {
await getPasswordResetToken(data.email)
} catch (e) {
form.setError('global', {
type: 'manual',
message: formatMessage(API_ERROR_MESSAGE)
})
const message = /^400/.test(e.message)
yunakim714 marked this conversation as resolved.
Show resolved Hide resolved
? formatMessage(FEATURE_UNAVAILABLE_ERROR_MESSAGE)
: formatMessage(API_ERROR_MESSAGE)
form.setError('global', { type: 'manual', message });
}
},
email: async () => {
Expand Down
19 changes: 13 additions & 6 deletions packages/template-retail-react-app/app/pages/login/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ import PasswordlessEmailConfirmation from '@salesforce/retail-react-app/app/comp
import {
API_ERROR_MESSAGE,
INVALID_TOKEN_ERROR_MESSAGE,
FEATURE_UNAVAILABLE_ERROR_MESSAGE,
LOGIN_TYPES,
PASSWORDLESS_LOGIN_LANDING_PATH
PASSWORDLESS_LOGIN_LANDING_PATH,
PASSWORDLESS_ERROR_MESSAGES
} from '@salesforce/retail-react-app/app/constants'
import {usePrevious} from '@salesforce/retail-react-app/app/hooks/use-previous'
import {isServer} from '@salesforce/retail-react-app/app/utils/utils'
Expand Down Expand Up @@ -104,12 +106,17 @@ const Login = ({initialView = LOGIN_VIEW}) => {

const handlePasswordlessLogin = async (email) => {
try {
await authorizePasswordlessLogin.mutateAsync({userid: email})
const res = await authorizePasswordlessLogin.mutateAsync({userid: email})
if (res.status !== 200) {
const errorData = await res.json()
throw new Error(`${res.status} ${errorData.message}`)
}
setCurrentView(EMAIL_VIEW)
} catch (error) {
form.setError('global', {
type: 'manual',
message: formatMessage(API_ERROR_MESSAGE)
})
const message = PASSWORDLESS_ERROR_MESSAGES.some(msg => msg.test(error.message))
? formatMessage(FEATURE_UNAVAILABLE_ERROR_MESSAGE)
: formatMessage(API_ERROR_MESSAGE)
form.setError('global', { type: 'manual', message })
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import React, {useEffect} from 'react'
import {useIntl} from 'react-intl'
import PropTypes from 'prop-types'
import {Box, Container} from '@salesforce/retail-react-app/app/components/shared/ui'
import {useForm} from 'react-hook-form'
Expand All @@ -17,9 +18,10 @@ import useEinstein from '@salesforce/retail-react-app/app/hooks/use-einstein'
import {useLocation} from 'react-router-dom'
import {useRouteMatch} from 'react-router'
import {usePasswordReset} from '@salesforce/retail-react-app/app/hooks/use-password-reset'
import {RESET_PASSWORD_LANDING_PATH} from '@salesforce/retail-react-app/app/constants'
import {RESET_PASSWORD_LANDING_PATH, API_ERROR_MESSAGE, FEATURE_UNAVAILABLE_ERROR_MESSAGE} from '@salesforce/retail-react-app/app/constants'

const ResetPassword = () => {
const {formatMessage} = useIntl()
const form = useForm()
const navigate = useNavigation()
const einstein = useEinstein()
Expand All @@ -31,7 +33,10 @@ const ResetPassword = () => {
try {
await getPasswordResetToken(email)
} catch (error) {
form.setError('global', {type: 'manual', message: error.message})
const message = /^400/.test(error.message)
? formatMessage(FEATURE_UNAVAILABLE_ERROR_MESSAGE)
: formatMessage(API_ERROR_MESSAGE)
form.setError('global', { type: 'manual', message });
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ const SocialLoginRedirect = () => {
const [error, setError] = useState('')

// Runs after successful 3rd-party IDP authorization, processing query parameters
useEffect(() => {
useEffect(async () => {
if (!searchParams.code) {
return
}
try {
loginIDPUser.mutateAsync({
await loginIDPUser.mutateAsync({
code: searchParams.code,
redirectURI: redirectURI,
...(searchParams.usid && {usid: searchParams.usid})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,12 @@
"value": "Wishlist"
}
],
"global.error.feature_unavailable": [
{
"type": 0,
"value": "This feature is not currently available."
}
],
"global.error.invalid_token": [
{
"type": 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,12 @@
"value": "Wishlist"
}
],
"global.error.feature_unavailable": [
{
"type": 0,
"value": "This feature is not currently available."
}
],
"global.error.invalid_token": [
{
"type": 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3185,6 +3185,20 @@
"value": "]"
}
],
"global.error.feature_unavailable": [
{
"type": 0,
"value": "["
},
{
"type": 0,
"value": "Ŧħīş ƒḗḗȧȧŧŭŭřḗḗ īş ƞǿǿŧ ƈŭŭřřḗḗƞŧŀẏ ȧȧṽȧȧīŀȧȧƀŀḗḗ."
},
{
"type": 0,
"value": "]"
}
],
"global.error.invalid_token": [
{
"type": 0,
Expand Down
3 changes: 3 additions & 0 deletions packages/template-retail-react-app/translations/en-GB.json
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,9 @@
"global.account.link.wishlist": {
"defaultMessage": "Wishlist"
},
"global.error.feature_unavailable": {
"defaultMessage": "This feature is not currently available."
},
"global.error.invalid_token": {
"defaultMessage": "Invalid token, please try again."
},
Expand Down
3 changes: 3 additions & 0 deletions packages/template-retail-react-app/translations/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,9 @@
"global.account.link.wishlist": {
"defaultMessage": "Wishlist"
},
"global.error.feature_unavailable": {
"defaultMessage": "This feature is not currently available."
},
"global.error.invalid_token": {
"defaultMessage": "Invalid token, please try again."
},
Expand Down
Loading