From 5092c58c50a5b18ea3fa6020cc7a9a55ce6a4b90 Mon Sep 17 00:00:00 2001 From: uzairr Date: Tue, 16 Feb 2021 09:11:42 +0500 Subject: [PATCH] handle api failures gracefully --- src/common-components/APIFailureMessage.jsx | 16 +- src/common-components/index.jsx | 1 + src/reset-password/ResetPasswordPage.jsx | 18 +- src/reset-password/data/actions.js | 4 +- src/reset-password/data/reducers.js | 13 +- src/reset-password/data/sagas.js | 5 +- src/reset-password/data/tests/sagas.test.js | 35 +++- src/reset-password/messages.js | 10 + .../tests/ResetPasswordPage.test.jsx | 27 +++ .../ResetPasswordPage.test.jsx.snap | 177 +++++++++++++++--- 10 files changed, 252 insertions(+), 54 deletions(-) diff --git a/src/common-components/APIFailureMessage.jsx b/src/common-components/APIFailureMessage.jsx index d4a2c783..8a7940d2 100644 --- a/src/common-components/APIFailureMessage.jsx +++ b/src/common-components/APIFailureMessage.jsx @@ -8,12 +8,16 @@ const APIFailureMessage = (props) => { const { intl, header } = props; return ( - - - {header} - - {intl.formatMessage(messages['internal.server.error.message'])} - +
+
+ + + {header} + + {intl.formatMessage(messages['internal.server.error.message'])} + +
+
); }; diff --git a/src/common-components/index.jsx b/src/common-components/index.jsx index 69194787..20894220 100644 --- a/src/common-components/index.jsx +++ b/src/common-components/index.jsx @@ -8,6 +8,7 @@ export { default as ThirdPartyAuthAlert } from './ThirdPartyAuthAlert'; export { default as InstitutionLogistration } from './InstitutionLogistration'; export { RenderInstitutionButton } from './InstitutionLogistration'; export { default as AuthnValidationFormGroup } from './AuthnValidationFormGroup'; +export { default as APIFailureMessage } from './APIFailureMessage'; export { default as reducer } from './data/reducers'; export { default as saga } from './data/sagas'; export { storeName } from './data/selectors'; diff --git a/src/reset-password/ResetPasswordPage.jsx b/src/reset-password/ResetPasswordPage.jsx index e75e4879..52f43fdc 100644 --- a/src/reset-password/ResetPasswordPage.jsx +++ b/src/reset-password/ResetPasswordPage.jsx @@ -17,8 +17,10 @@ import InvalidTokenMessage from './InvalidToken'; import ResetSuccessMessage from './ResetSuccess'; import { AuthnValidationFormGroup, + APIFailureMessage, } from '../common-components'; import Spinner from './Spinner'; +import { INTERNAL_SERVER_ERROR } from '../data/constants'; const ResetPasswordPage = (props) => { const { intl } = props; @@ -32,7 +34,7 @@ const ResetPasswordPage = (props) => { const [bannerErrorMessage, setbannerErrorMessage] = useState(''); useEffect(() => { - if (props.status === 'failure' && props.errors) { + if (props.status === 'failure' && props.errors !== INTERNAL_SERVER_ERROR) { setbannerErrorMessage(props.errors); setvalidationMessage(props.errors); setPasswordValidValue(false); @@ -95,19 +97,27 @@ const ResetPasswordPage = (props) => { props.resetPassword(formPayload, props.token, params); }; - if (props.token_status === 'pending') { + if (props.status === 'token-pending') { const { token } = props.match.params; if (token) { props.validateToken(token); return ; } - } else if (props.token_status === 'invalid') { + } else if (props.status === 'invalid' && props.errors === INTERNAL_SERVER_ERROR) { + return ( + + ); + } else if (props.status === 'invalid') { return ; } else if (props.status === 'success') { return ; } else { return ( <> + + {props.status === 'failure' && props.errors === INTERNAL_SERVER_ERROR ? ( + + ) : null}
{bannerErrorMessage ? ( @@ -168,7 +178,6 @@ const ResetPasswordPage = (props) => { ResetPasswordPage.defaultProps = { status: null, - token_status: null, token: null, match: null, errors: null, @@ -178,7 +187,6 @@ ResetPasswordPage.propTypes = { intl: intlShape.isRequired, resetPassword: PropTypes.func.isRequired, validateToken: PropTypes.func.isRequired, - token_status: PropTypes.string, token: PropTypes.string, match: PropTypes.shape({ params: PropTypes.shape({ diff --git a/src/reset-password/data/actions.js b/src/reset-password/data/actions.js index 88b3b71a..7e6b637b 100644 --- a/src/reset-password/data/actions.js +++ b/src/reset-password/data/actions.js @@ -18,9 +18,9 @@ export const validateTokenSuccess = (tokenStatus, token) => ({ payload: { tokenStatus, token }, }); -export const validateTokenFailure = (tokenStatus) => ({ +export const validateTokenFailure = errors => ({ type: VALIDATE_TOKEN.FAILURE, - payload: { tokenStatus }, + payload: { errors }, }); // Reset Password diff --git a/src/reset-password/data/reducers.js b/src/reset-password/data/reducers.js index 13b60d92..f7ce1898 100644 --- a/src/reset-password/data/reducers.js +++ b/src/reset-password/data/reducers.js @@ -1,29 +1,24 @@ import { RESET_PASSWORD, VALIDATE_TOKEN } from './actions'; export const defaultState = { - status: null, - token_status: 'pending', + status: 'token-pending', token: null, errors: null, }; const reducer = (state = defaultState, action = null) => { switch (action.type) { - case VALIDATE_TOKEN.BEGIN: - return { - ...state, - token_status: 'pending', - }; case VALIDATE_TOKEN.SUCCESS: return { ...state, - token_status: 'valid', + status: 'valid', token: action.payload.token, }; case VALIDATE_TOKEN.FAILURE: return { ...state, - token_status: 'invalid', + status: 'invalid', + errors: action.payload.errors, }; case RESET_PASSWORD.BEGIN: return { diff --git a/src/reset-password/data/sagas.js b/src/reset-password/data/sagas.js index 53185963..30f18077 100644 --- a/src/reset-password/data/sagas.js +++ b/src/reset-password/data/sagas.js @@ -14,6 +14,7 @@ import { } from './actions'; import { validateToken, resetPassword } from './service'; +import { INTERNAL_SERVER_ERROR } from '../../data/constants'; // Services export function* handleValidateToken(action) { @@ -27,7 +28,7 @@ export function* handleValidateToken(action) { yield put(validateTokenFailure(isValid)); } } catch (err) { - yield put(validateTokenFailure(err)); + yield put(validateTokenFailure(INTERNAL_SERVER_ERROR)); logError(err); } } @@ -45,7 +46,7 @@ export function* handleResetPassword(action) { yield put(resetPasswordFailure(resetErrors)); } } catch (err) { - yield put(resetPasswordFailure(err)); + yield put(resetPasswordFailure(INTERNAL_SERVER_ERROR)); logError(err); } } diff --git a/src/reset-password/data/tests/sagas.test.js b/src/reset-password/data/tests/sagas.test.js index 7c49b780..843db8ee 100644 --- a/src/reset-password/data/tests/sagas.test.js +++ b/src/reset-password/data/tests/sagas.test.js @@ -4,10 +4,13 @@ import { resetPasswordBegin, resetPasswordSuccess, resetPasswordFailure, + validateTokenBegin, + validateTokenFailure, } from '../actions'; -import { handleResetPassword } from '../sagas'; +import { handleResetPassword, handleValidateToken } from '../sagas'; import * as api from '../service'; import initializeMockLogging from '../../../setupTest'; +import { INTERNAL_SERVER_ERROR } from '../../../data/constants'; const { loggingService } = initializeMockLogging(); @@ -60,7 +63,35 @@ describe('handleResetPassword', () => { ); expect(resetPassword).toHaveBeenCalledTimes(1); - expect(dispatched).toEqual([resetPasswordBegin(), resetPasswordFailure()]); + expect(dispatched).toEqual([resetPasswordBegin(), resetPasswordFailure(INTERNAL_SERVER_ERROR)]); resetPassword.mockClear(); }); }); + +describe('handleValidateToken', () => { + const params = { + payload: { + token: 'token', + params: {}, + }, + }; + + beforeEach(() => { + loggingService.logError.mockReset(); + }); + + it('check server error on api failure', async () => { + const validateToken = jest.spyOn(api, 'validateToken') + .mockImplementation(() => Promise.reject()); + + const dispatched = []; + await runSaga( + { dispatch: (action) => dispatched.push(action) }, + handleValidateToken, + params, + ); + + expect(validateToken).toHaveBeenCalledTimes(1); + expect(dispatched).toEqual([validateTokenBegin(), validateTokenFailure(INTERNAL_SERVER_ERROR)]); + }); +}); diff --git a/src/reset-password/messages.js b/src/reset-password/messages.js index 45a642fb..32b862f0 100644 --- a/src/reset-password/messages.js +++ b/src/reset-password/messages.js @@ -61,6 +61,16 @@ const messages = defineMessages({ defaultMessage: 'We couldn\'t reset your password.', description: 'Heading that appears above error message when user submits empty form.', }, + 'reset.password.request.server.error': { + id: 'reset.password.request.server.error', + defaultMessage: 'Failed to reset password', + description: 'Failed to reset password error message heading.', + }, + 'reset.password.token.validation.sever.error': { + id: 'reset.password.token.validation.sever.error', + defaultMessage: 'Token validation failure', + description: 'Failed to validate reset password token error message.', + }, }); export default messages; diff --git a/src/reset-password/tests/ResetPasswordPage.test.jsx b/src/reset-password/tests/ResetPasswordPage.test.jsx index 4d391f55..e10694fc 100644 --- a/src/reset-password/tests/ResetPasswordPage.test.jsx +++ b/src/reset-password/tests/ResetPasswordPage.test.jsx @@ -8,8 +8,10 @@ import { IntlProvider, injectIntl } from '@edx/frontend-platform/i18n'; import CookiePolicyBanner from '@edx/frontend-component-cookie-policy-banner'; import * as auth from '@edx/frontend-platform/auth'; import { resetPassword } from '../data/actions'; +import { APIFailureMessage } from '../../common-components'; import ResetPasswordPage from '../ResetPasswordPage'; +import { INTERNAL_SERVER_ERROR } from '../../data/constants'; jest.mock('@edx/frontend-platform/auth'); @@ -241,4 +243,29 @@ describe('ResetPasswordPage', () => { const resetPasswordPage = mount(reduxWrapper()); expect(resetPasswordPage.find()).toBeTruthy(); }); + + it('should display error banner on server error', () => { + const bannerMessage = 'Failed to reset passwordAn error has occurred. Try refreshing the page, or check your Internet connection.'; + props = { + ...props, + status: 'failure', + errors: INTERNAL_SERVER_ERROR, + }; + + const resetPasswordPage = mount(reduxWrapper()); + resetPasswordPage.find('button.btn-primary').simulate('click'); + + resetPasswordPage.update(); + expect(resetPasswordPage.find('#internal-server-error').first().text()).toEqual(bannerMessage); + }); + + it('check api failure banner rendered', () => { + props = { + ...props, + status: 'invalid', + errors: INTERNAL_SERVER_ERROR, + }; + const resetPasswordPage = mount(reduxWrapper()); + expect(resetPasswordPage.find()).toBeTruthy(); + }); }); diff --git a/src/reset-password/tests/__snapshots__/ResetPasswordPage.test.jsx.snap b/src/reset-password/tests/__snapshots__/ResetPasswordPage.test.jsx.snap index 37e1cc04..f719455e 100644 --- a/src/reset-password/tests/__snapshots__/ResetPasswordPage.test.jsx.snap +++ b/src/reset-password/tests/__snapshots__/ResetPasswordPage.test.jsx.snap @@ -3,38 +3,86 @@ exports[`ResetPasswordPage should match invalid token message section snapshot 1`] = `
-
-
- - Invalid Password Reset Link -
- - This password reset link is invalid. It may have been used already. To reset your password, go to the - +

+ Enter and confirm your new password. +

+
+ + + +
+
+ + + + - sign-in - - page and select - - - Forgot Password - + Passwords do not match. - -
+
+ +
`; @@ -274,14 +322,87 @@ exports[`ResetPasswordPage should match successful reset message section snapsho exports[`ResetPasswordPage show spinner component during token validation 1`] = `
-
+
+

+ Reset your password +

+

+ Enter and confirm your new password. +

+
+ + + +
+
+ + + + + Passwords do not match. + +
+ +
`;