From c6d45a0fe1dc3e3e4ff38460f8823b66cbac5179 Mon Sep 17 00:00:00 2001 From: Zainab Amir Date: Thu, 4 Feb 2021 13:49:22 +0500 Subject: [PATCH] Fix Password Assistance Bugs (#97) - Fixed focus issue - Added validations - Updated error message VAN-298 --- src/common-components/APIFailureMessage.jsx | 2 +- src/data/constants.js | 3 + src/forgot-password/ForgotPasswordPage.jsx | 81 +++++----- .../RequestInProgressAlert.jsx | 35 +++-- src/forgot-password/data/reducers.js | 6 +- src/forgot-password/data/tests/sagas.test.js | 57 +++++++ src/forgot-password/messages.js | 29 ++-- .../tests/ForgotPasswordPage.test.jsx | 143 +++++++----------- .../ForgotPasswordPage.test.jsx.snap | 39 ++--- src/reset-password/ResetPasswordPage.jsx | 8 +- 10 files changed, 218 insertions(+), 185 deletions(-) create mode 100644 src/forgot-password/data/tests/sagas.test.js diff --git a/src/common-components/APIFailureMessage.jsx b/src/common-components/APIFailureMessage.jsx index 52d1eace..d4a2c783 100644 --- a/src/common-components/APIFailureMessage.jsx +++ b/src/common-components/APIFailureMessage.jsx @@ -8,7 +8,7 @@ const APIFailureMessage = (props) => { const { intl, header } = props; return ( - + {header} diff --git a/src/data/constants.js b/src/data/constants.js index 583d9f21..41ad35bc 100644 --- a/src/data/constants.js +++ b/src/data/constants.js @@ -10,6 +10,9 @@ export const ENTERPRISE_LOGIN_URL = '/enterprise/login'; // Constants export const SUPPORTED_ICON_CLASSES = ['apple', 'facebook', 'google', 'microsoft']; +// Error Codes +export const INTERNAL_SERVER_ERROR = 'internal-server-error'; + // States export const DEFAULT_STATE = 'default'; export const PENDING_STATE = 'pending'; diff --git a/src/forgot-password/ForgotPasswordPage.jsx b/src/forgot-password/ForgotPasswordPage.jsx index ac7c1a37..dae73221 100644 --- a/src/forgot-password/ForgotPasswordPage.jsx +++ b/src/forgot-password/ForgotPasswordPage.jsx @@ -1,85 +1,85 @@ import React from 'react'; + +import { Formik } from 'formik'; import PropTypes from 'prop-types'; -import { sendPageEvent } from '@edx/frontend-platform/analytics'; import { connect } from 'react-redux'; import { Redirect } from 'react-router-dom'; + +import { getConfig } from '@edx/frontend-platform'; +import { sendPageEvent } from '@edx/frontend-platform/analytics'; +import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { + Alert, Form, Input, StatefulButton, ValidationFormGroup, - Alert, } from '@edx/paragon'; -import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; -import { getConfig } from '@edx/frontend-platform'; - -import { Formik } from 'formik'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { faSpinner } from '@fortawesome/free-solid-svg-icons'; -import messages from './messages'; + import { forgotPassword } from './data/actions'; import { forgotPasswordResultSelector } from './data/selectors'; import RequestInProgressAlert from './RequestInProgressAlert'; -import { LOGIN_PAGE, VALID_EMAIL_REGEX } from '../data/constants'; -import LoginHelpLinks from '../login/LoginHelpLinks'; +import messages from './messages'; import APIFailureMessage from '../common-components/APIFailureMessage'; -import { INTERNAL_SERVER_ERROR } from '../login/data/constants'; +import { INTERNAL_SERVER_ERROR, LOGIN_PAGE, VALID_EMAIL_REGEX } from '../data/constants'; +import LoginHelpLinks from '../login/LoginHelpLinks'; const ForgotPasswordPage = (props) => { const { intl, status } = props; const platformName = getConfig().SITE_NAME; - const handleOnChange = (e, setFieldValue) => { - setFieldValue('email', e.target.value); - }; + const regex = new RegExp(VALID_EMAIL_REGEX, 'i'); - const getStatusBannerifAny = (errors) => { + const getErrorMessage = (errors) => { + const header = intl.formatMessage(messages['forgot.password.request.server.error']); if (errors.email) { return ( - - {intl.formatMessage(messages['forgot.password.invalid.email.heading'])} - - {intl.formatMessage(messages['forgot.password.invalid.email.message'])} + {header} +
  • {errors.email}
 ); } if (status === INTERNAL_SERVER_ERROR) { - return ; + return ; } return status === 'forbidden' ? : null; }; + sendPageEvent('login_and_registration', 'reset'); return ( props.forgotPassword(values.email)} - validate={(values) => { - const regex = new RegExp(VALID_EMAIL_REGEX, 'i'); - if (!regex.test(values.email)) { - return { email: intl.formatMessage(messages['forgot.password.page.invalid.email.message']) }; - } - return {}; - }} - initialValues={{ - email: '', - isEmailValid: true, - }} + initialValues={{ email: '' }} validateOnChange={false} - validateOnBlur={false} + validate={(values) => { + // eslint-disable-next-line prefer-const + let errors = {}; + + if (values.email === '') { + errors.email = intl.formatMessage(messages['forgot.password.empty.email.field.error']); + } else if (!regex.test(values.email)) { + errors.email = intl.formatMessage(messages['forgot.password.page.invalid.email.message']); + } + + if (errors) { + window.scrollTo({ left: 0, top: 0, behavior: 'smooth' }); + } + return errors; + }} + onSubmit={(values) => { props.forgotPassword(values.email); }} > {({ - handleSubmit, - values, - setFieldValue, - errors, + errors, handleSubmit, setFieldValue, validateForm, values, }) => ( <> {status === 'complete' ? : null}
- { getStatusBannerifAny(errors)} + { getErrorMessage(errors) }

{intl.formatMessage(messages['forgot.password.page.heading'])}

@@ -102,19 +102,20 @@ const ForgotPasswordPage = (props) => { type="email" placeholder="username@domain.com" value={values.email} - onChange={e => handleOnChange(e, setFieldValue)} + onBlur={() => validateForm()} + onChange={e => setFieldValue('email', e.target.value)} /> }} + onClick={handleSubmit} />
diff --git a/src/forgot-password/RequestInProgressAlert.jsx b/src/forgot-password/RequestInProgressAlert.jsx index ae2ae173..7a69cd45 100644 --- a/src/forgot-password/RequestInProgressAlert.jsx +++ b/src/forgot-password/RequestInProgressAlert.jsx @@ -1,18 +1,25 @@ import React from 'react'; -import { FormattedMessage } from '@edx/frontend-platform/i18n'; -import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; -import { faExclamationTriangle } from '@fortawesome/free-solid-svg-icons'; + +import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Alert } from '@edx/paragon'; -const RequestInProgressAlert = () => ( - - - - -); +import messages from './messages'; -export default RequestInProgressAlert; +const RequestInProgressAlert = (props) => { + const { intl } = props; + + return ( + + {intl.formatMessage(messages['forgot.password.error.message.title'])} +
    +
  • {intl.formatMessage(messages['forgot.password.request.in.progress.message'])}
    • +
+ + ); +}; + +RequestInProgressAlert.propTypes = { + intl: intlShape.isRequired, +}; + +export default injectIntl(RequestInProgressAlert); diff --git a/src/forgot-password/data/reducers.js b/src/forgot-password/data/reducers.js index 24680e03..3dfa9b43 100644 --- a/src/forgot-password/data/reducers.js +++ b/src/forgot-password/data/reducers.js @@ -1,5 +1,5 @@ import { FORGOT_PASSWORD } from './actions'; -import { INTERNAL_SERVER_ERROR } from '../../login/data/constants'; +import { INTERNAL_SERVER_ERROR } from '../../data/constants'; export const defaultState = { status: null, @@ -10,23 +10,19 @@ const reducer = (state = defaultState, action = null) => { switch (action.type) { case FORGOT_PASSWORD.BEGIN: return { - ...state, status: 'pending', }; case FORGOT_PASSWORD.SUCCESS: return { - ...state, ...action.payload, status: 'complete', }; case FORGOT_PASSWORD.FORBIDDEN: return { - ...state, status: 'forbidden', }; case FORGOT_PASSWORD.FAILURE: return { - ...state, status: INTERNAL_SERVER_ERROR, }; default: diff --git a/src/forgot-password/data/tests/sagas.test.js b/src/forgot-password/data/tests/sagas.test.js new file mode 100644 index 00000000..3f2a8704 --- /dev/null +++ b/src/forgot-password/data/tests/sagas.test.js @@ -0,0 +1,57 @@ +import { runSaga } from 'redux-saga'; + +import * as actions from '../actions'; +import { handleForgotPassword } from '../sagas'; +import * as api from '../service'; + +describe('handleForgotPassword', () => { + const params = { + payload: { + formData: { + email: 'test@test.com', + }, + }, + }; + + it('should handle 500 error code', async () => { + const passwordErrorResponse = { response: { status: 500 } }; + + const forgotPasswordRequest = jest.spyOn(api, 'forgotPassword').mockImplementation( + () => Promise.reject(passwordErrorResponse), + ); + + const dispatched = []; + await runSaga( + { dispatch: (action) => dispatched.push(action) }, + handleForgotPassword, + params, + ); + + expect(dispatched).toEqual([ + actions.forgotPasswordBegin(), + actions.forgotPasswordServerError(), + ]); + forgotPasswordRequest.mockClear(); + }); + + it('should handle rate limit error', async () => { + const forbiddenErrorResponse = { response: { status: 403 } }; + + const forbiddenPasswordRequest = jest.spyOn(api, 'forgotPassword').mockImplementation( + () => Promise.reject(forbiddenErrorResponse), + ); + + const dispatched = []; + await runSaga( + { dispatch: (action) => dispatched.push(action) }, + handleForgotPassword, + params, + ); + + expect(dispatched).toEqual([ + actions.forgotPasswordBegin(), + actions.forgotPasswordForbidden(null), + ]); + forbiddenPasswordRequest.mockClear(); + }); +}); diff --git a/src/forgot-password/messages.js b/src/forgot-password/messages.js index 624974cb..07d88bc7 100644 --- a/src/forgot-password/messages.js +++ b/src/forgot-password/messages.js @@ -21,25 +21,30 @@ const messages = defineMessages({ defaultMessage: 'Email', description: 'Email field label for the forgot password page.', }, - 'forgot.password.page.email.field.help.text': { - id: 'forgot.password.page.email.field.help.text', - defaultMessage: 'The email address you used to register with edX.', - description: 'Email field help text for the forgot password page.', - }, 'forgot.password.page.submit.button': { id: 'forgot.password.page.submit.button', defaultMessage: 'Recover my password', description: 'Submit button text for the forgot password page.', }, - 'forgot.password.page.email.invalid.length.message': { - id: 'forgot.password.page.email.invalid.length.message', - defaultMessage: 'Email must have at least 3 characters.', - description: 'Invalid email address length message for the forgot password page.', - }, 'forgot.password.request.server.error': { id: 'forgot.password.request.server.error', - defaultMessage: 'Failed to Send Forgot Password Email', - description: 'Failed to Send Forgot Password Email help text heading.', + defaultMessage: 'Failed to send forgot password email.', + description: 'Failed to Send Forgot Password Email heading.', + }, + 'forgot.password.error.message.title': { + id: 'forgot.password.error.message.title', + defaultMessage: 'An error occurred.', + description: 'Title for message that appears when error occurs for password assistance page', + }, + 'forgot.password.request.in.progress.message': { + id: 'forgot.password.request.in.progress.message', + defaultMessage: 'Your previous request is in progress, please try again in a few moments.', + description: 'Message displayed when previous password reset request is still in progress.', + }, + 'forgot.password.empty.email.field.error': { + id: 'forgot.password.empty.email.field.error', + defaultMessage: 'Please enter your Email.', + description: 'Error message that appears when user tries to submit empty email field', }, 'forgot.password.invalid.email.heading': { id: 'forgot.password.invalid.email', diff --git a/src/forgot-password/tests/ForgotPasswordPage.test.jsx b/src/forgot-password/tests/ForgotPasswordPage.test.jsx index 81b91a87..1519b48b 100644 --- a/src/forgot-password/tests/ForgotPasswordPage.test.jsx +++ b/src/forgot-password/tests/ForgotPasswordPage.test.jsx @@ -10,15 +10,9 @@ import { IntlProvider, injectIntl } from '@edx/frontend-platform/i18n'; import CookiePolicyBanner from '@edx/frontend-component-cookie-policy-banner'; import * as analytics from '@edx/frontend-platform/analytics'; -import { runSaga } from 'redux-saga'; import ForgotPasswordPage from '../ForgotPasswordPage'; -import * as api from '../data/service'; +import { INTERNAL_SERVER_ERROR } from '../../data/constants'; -import { handleForgotPassword } from '../data/sagas'; -import * as actions from '../data/actions'; -import { INTERNAL_SERVER_ERROR } from '../../login/data/constants'; - -jest.mock('../data/selectors', () => jest.fn().mockImplementation(() => ({ forgotPasswordSelector: () => ({}) }))); jest.mock('@edx/frontend-platform/analytics'); analytics.sendPageEvent = jest.fn(); @@ -94,85 +88,15 @@ describe('ForgotPasswordPage', () => { it('should display email validation error message', async () => { const validationMessage = "The email address you've provided isn't formatted correctly."; const wrapper = mount(reduxWrapper()); - await act(async () => { - await wrapper.find('button.btn-primary').simulate('click', { target: { value: 'random', name: 'email' } }); - }); + + wrapper.find('input#forgot-password-input').simulate( + 'change', { target: { value: 'invalid-email', name: 'email' } }, + ); + await act(async () => { await wrapper.find('button.btn-primary').simulate('click'); }); wrapper.update(); + expect(wrapper.find('#email-invalid-feedback').text()).toEqual(validationMessage); - }); - - it('should display alert banner incase of invalid email', async () => { - const validationMessage = "An error occurred.The email address you've provided isn't formatted correctly."; - const wrapper = mount(reduxWrapper()); - await act(async () => { - await wrapper.find('button.btn-primary').simulate('click', { target: { value: 'random', name: 'email' } }); - }); - wrapper.update(); - expect(wrapper.find('.alert-danger').text()).toEqual(validationMessage); - }); - - it('should handle 500 error code', async () => { - const params = { - payload: { - formData: { - email: 'test@test.com', - }, - }, - }; - const passwordErrorResponse = { - response: { - status: 500, - data: { - errorCode: 'internal-server-error', - }, - }, - }; - - const forgotPasswordRequest = jest.spyOn(api, 'forgotPassword').mockImplementation(() => Promise.reject(passwordErrorResponse)); - const dispatched = []; - await runSaga( - { dispatch: (action) => dispatched.push(action) }, - handleForgotPassword, - params, - ); - - expect(dispatched).toEqual([ - actions.forgotPasswordBegin(), - actions.forgotPasswordServerError(), - ]); - forgotPasswordRequest.mockClear(); - }); - - it('should handle 403 error code', async () => { - const params = { - payload: { - formData: { - email: 'test@test.com', - }, - }, - }; - const forbiddenErrorResponse = { - response: { - status: 403, - data: { - msg: 'forbidden request', - }, - }, - }; - - const forbiddenPasswordRequest = jest.spyOn(api, 'forgotPassword').mockImplementation(() => Promise.reject(forbiddenErrorResponse)); - const dispatched = []; - await runSaga( - { dispatch: (action) => dispatched.push(action) }, - handleForgotPassword, - params, - ); - - expect(dispatched).toEqual([ - actions.forgotPasswordBegin(), - actions.forgotPasswordForbidden(null), - ]); - forbiddenPasswordRequest.mockClear(); + expect(wrapper.find('.alert-danger').text()).toEqual('Failed to send forgot password email.'.concat(validationMessage)); }); it('should show alert on server error', () => { @@ -180,10 +104,57 @@ describe('ForgotPasswordPage', () => { ...props, status: INTERNAL_SERVER_ERROR, }; - const expectedMessage = 'Failed to Send Forgot Password Email'; + const expectedMessage = 'Failed to send forgot password email.' + + 'An error has occurred. Try refreshing the page, or check your Internet connection.'; const wrapper = mount(reduxWrapper()); - expect(wrapper.find('div.alert-heading').text()).toEqual(expectedMessage); + expect(wrapper.find('#internal-server-error').first().text()).toEqual(expectedMessage); + }); + + it('should display empty email validation message', async () => { + const validationMessage = 'Please enter your Email.'; + const forgotPasswordPage = mount(reduxWrapper()); + + await act(async () => { await forgotPasswordPage.find('button.btn-primary').simulate('click'); }); + + forgotPasswordPage.update(); + expect(forgotPasswordPage.find('#email-invalid-feedback').text()).toEqual(validationMessage); + expect(forgotPasswordPage.find('.alert-danger').text()).toEqual( + 'Failed to send forgot password email.'.concat(validationMessage), + ); + }); + + it('should display request in progress error message', () => { + const rateLimitMessage = 'An error occurred.Your previous request is in progress, please try again in a few moments.'; + store = mockStore({ + forgotPassword: { status: 'forbidden' }, + }); + + const forgotPasswordPage = mount(reduxWrapper()); + expect(forgotPasswordPage.find('.alert-danger').text()).toEqual(rateLimitMessage); + }); + + it('should not display any error message on change event', () => { + const forgotPasswordPage = mount(reduxWrapper()); + + const emailInput = forgotPasswordPage.find('input#forgot-password-input'); + emailInput.simulate('change', { target: { value: 'invalid-email', name: 'email' } }); + forgotPasswordPage.update(); + + expect(forgotPasswordPage.find('#email-invalid-feedback').exists()).toEqual(false); + }); + + it('should display error message on blur event', async () => { + const validationMessage = 'Failed to send forgot password email.Please enter your Email.'; + const forgotPasswordPage = mount(reduxWrapper()); + const emailInput = forgotPasswordPage.find('input#forgot-password-input'); + + await act(async () => { + await emailInput.simulate('blur', { target: { value: '', name: 'email' } }); + }); + + forgotPasswordPage.update(); + expect(forgotPasswordPage.find('.alert-danger').text()).toEqual(validationMessage); }); it('check cookie rendered', () => { diff --git a/src/forgot-password/tests/__snapshots__/ForgotPasswordPage.test.jsx.snap b/src/forgot-password/tests/__snapshots__/ForgotPasswordPage.test.jsx.snap index e99da32d..122bb2c6 100644 --- a/src/forgot-password/tests/__snapshots__/ForgotPasswordPage.test.jsx.snap +++ b/src/forgot-password/tests/__snapshots__/ForgotPasswordPage.test.jsx.snap @@ -34,6 +34,7 @@ exports[`ForgotPasswordPage should match default section snapshot 1`] = ` className="form-control" id="forgot-password-input" name="email" + onBlur={[Function]} onChange={[Function]} placeholder="username@domain.com" type="email" @@ -92,7 +93,7 @@ exports[`ForgotPasswordPage should match default section snapshot 1`] = ` className="pgn__stateful-btn pgn__stateful-btn-state-null btn-primary mt-3 btn btn-primary" disabled={false} onClick={[Function]} - type="button" + type="submit" >
- - - Your previous request is still in progress, please try again in a few moments. - + An error occurred. +
+
    +
  • + Your previous request is in progress, please try again in a few moments. +
    • +

{ const { intl } = props; const params = getQueryParameters(); - const mainRef = useRef(); const [newPasswordInput, setNewPasswordValue] = useState(''); const [confirmPasswordInput, setConfirmPasswordValue] = useState(''); @@ -54,6 +53,8 @@ const ResetPasswordPage = (props) => { const handleSubmit = (e) => { e.preventDefault(); + window.scrollTo({ left: 0, top: 0, behavior: 'smooth' }); + if (newPasswordInput === '') { setEmptyFieldError(intl.formatMessage(messages['reset.password.empty.new.password.field.error'])); return; @@ -69,7 +70,6 @@ const ResetPasswordPage = (props) => { }; props.resetPassword(formPayload, props.token, params); } - mainRef.current.focus(); }; if (props.token_status === 'pending') { @@ -85,7 +85,7 @@ const ResetPasswordPage = (props) => { } else { return ( <> -
+
{(emptyFieldError || props.status) && (