From df6c369e89f46b07e3545a8b7969241f80e45edb Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 22 Oct 2019 17:16:56 -0400 Subject: [PATCH 01/19] Added error strings for particular join flow errors --- src/l10n.json | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/l10n.json b/src/l10n.json index f4d629d2c..2617df89a 100644 --- a/src/l10n.json +++ b/src/l10n.json @@ -157,6 +157,7 @@ "registration.birthDateStepInfo": "This helps us understand the age range of people who use Scratch. We use this to confirm account ownership if you contact our team. This information will not be made public on your account.", "registration.birthDateStepTitle": "When were you born?", + "registration.cantCreateAccount": "Scratch could not create your account.", "registration.checkOutResources": "Get Started with Resources", "registration.checkOutResourcesDescription": "Explore materials for educators and facilitators written by the Scratch Team, including tips, tutorials, and guides.", "registration.choosePasswordStepDescription": "Type in a new password for your account. You will use this password the next time you log into Scratch.", @@ -172,6 +173,10 @@ "registration.confirmYourEmailDescription": "If you haven't already, please click the link in the confirmation email sent to:", "registration.createAccount": "Create Your Account", "registration.createUsername": "Create a username", + "registration.errorBadUsername": "The username you chose is not allowed. Try again with a different username.", + "registration.errorCaptcha": "There was a problem with the CAPTCHA test.", + "registration.errorPasswordTooShort": "Your password is too short. It needs to be at least 6 letters long.", + "registration.errorUsernameExists": "The username you chose already exists. Try again with a different username.", "registration.genderStepTitle": "What's your gender?", "registration.genderStepDescription": "Scratch welcomes people of all genders.", "registration.genderStepInfo": "This helps us understand who uses Scratch, so that we can broaden participation. This information will not be made public on your account.", @@ -194,11 +199,14 @@ "registration.personalStepTitle": "Personal Information", "registration.personalStepDescription": "Your individual responses will not be displayed publicly, and will be kept confidential and secure", "registration.private": "We will keep this information private.", + "registration.problemsAre": "The problems are:", "registration.receiveEmails": "I'd like to receive emails from the Scratch Team about project ideas, events, and more.", "registration.selectCountry": "Select country", + "registration.startOverInstruction": "Click \"Start over.\"", "registration.studentPersonalStepDescription": "This information will not appear on the Scratch website.", "registration.showPassword": "Show password", "registration.troubleReload": "Scratch is having trouble finishing registration. Try reloading the page or try again in another browser.", + "registration.tryAgainInstruction": "Click \"Try again\".", "registration.usernameStepDescription": "Fill in the following forms to request an account. The approval process may take up to one day.", "registration.usernameStepDescriptionNonEducator": "Create projects, share ideas, make friends. It’s free!", "registration.usernameStepRealName": "Please do not use any portion of your real name in your username.", From 517d8ff7cc7252f1225f00f49c6d073b482c60a2 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 22 Oct 2019 17:36:55 -0400 Subject: [PATCH 02/19] registration error step takes different props --- .../join-flow/registration-error-step.jsx | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/components/join-flow/registration-error-step.jsx b/src/components/join-flow/registration-error-step.jsx index 983a71576..d4b41ca96 100644 --- a/src/components/join-flow/registration-error-step.jsx +++ b/src/components/join-flow/registration-error-step.jsx @@ -1,6 +1,7 @@ const bindAll = require('lodash.bindall'); const React = require('react'); const PropTypes = require('prop-types'); +const FormattedMessage = require('react-intl').FormattedMessage; const {injectIntl, intlShape} = require('react-intl'); const JoinFlowStep = require('./join-flow-step.jsx'); @@ -24,24 +25,42 @@ class RegistrationErrorStep extends React.Component { render () { return ( + > +

+ +

+ {this.props.errorMsg && ( +

+ +

+ )} + {this.props.canTryAgain ? ( +

+ +

+ ) : ( +

+ +

+ )} + ); } } RegistrationErrorStep.propTypes = { - canTryAgain: PropTypes.bool, + canTryAgain: PropTypes.bool.isRequired, errorMsg: PropTypes.string, intl: intlShape, - onSubmit: PropTypes.func + onSubmit: PropTypes.func.isRequired }; const IntlRegistrationErrorStep = injectIntl(RegistrationErrorStep); From ceb3af1c58365ffab205331da7de053b8823c53a Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 22 Oct 2019 17:37:13 -0400 Subject: [PATCH 03/19] improve formatting of welcome, error step text --- src/components/join-flow/join-flow-steps.scss | 25 ++++++++++--------- src/components/join-flow/welcome-step.jsx | 9 ++++--- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/components/join-flow/join-flow-steps.scss b/src/components/join-flow/join-flow-steps.scss index 78df6a2bc..3d7e1df30 100644 --- a/src/components/join-flow/join-flow-steps.scss +++ b/src/components/join-flow/join-flow-steps.scss @@ -33,8 +33,9 @@ .join-flow-instructions { font-size: .875rem; font-weight: bold; - line-height: 1.37500rem; - margin-bottom: 1rem; + line-height: 1.375rem; + margin-top: 1.25rem; + margin-bottom: .5rem; text-align: center; } @@ -161,6 +162,15 @@ padding-bottom: 1rem; } +.join-flow-inner-error-step { + user-select: text; /* make text selectable, so users can copy errors */ + padding-top: 5.5rem; +} + +.join-flow-error-title { + margin-bottom: 2rem; +} + .join-flow-birthdate-title { margin-bottom: 2.875rem; } @@ -177,11 +187,6 @@ background-color: $dd-medium-blue; } -.join-flow-registration-error { - user-select: text; /* make text selectable, so users can copy errors */ - padding-top: 5.5rem; -} - .join-flow-gender-description { margin-top: .625rem; margin-bottom: 1.25rem; @@ -197,11 +202,7 @@ } .join-flow-welcome-title { - margin-bottom: .25rem; -} - -.join-flow-welcome-description { - margin-bottom: 1.25rem; + margin-bottom: 1rem; } .welcome-step-image { diff --git a/src/components/join-flow/welcome-step.jsx b/src/components/join-flow/welcome-step.jsx index 24608044a..f4694a1c2 100644 --- a/src/components/join-flow/welcome-step.jsx +++ b/src/components/join-flow/welcome-step.jsx @@ -39,10 +39,6 @@ class WelcomeStep extends React.Component { } = props; return ( +
+ +
Date: Tue, 22 Oct 2019 17:38:00 -0400 Subject: [PATCH 04/19] added validation functions for error code validation --- src/lib/validate.js | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/lib/validate.js b/src/lib/validate.js index 5df20b0e4..a6c4762cf 100644 --- a/src/lib/validate.js +++ b/src/lib/validate.js @@ -101,3 +101,29 @@ module.exports.validateEmailRemotely = email => ( }); }) ); + +const responseErrorMsgs = module.exports.responseErrorMsgs = { + username: { + 'username exists': {errMsgId: 'registration.errorUsernameExists'}, + 'bad username': {errMsgId: 'registration.errorBadUsername'} + }, + password: { + 'Ensure this value has at least 6 characters (it has %d).': { + errMsgId: 'registration.errorPasswordTooShort' + } + }, + recaptcha: { + 'Incorrect, please try again.': {errMsgId: 'registration.errorCaptcha'} + } +}; + +module.exports.responseErrorMsg = (fieldName, serverRawErr) => { + if (fieldName && responseErrorMsgs[fieldName]) { + const serverErrPatterns = responseErrorMsgs[fieldName]; + // use regex compare to find matching error string in responseErrorMsgs + const matchingKey = Object.keys(serverErrPatterns).find(errPattern => ( + RegExp(errPattern).test(serverRawErr) + )); + if (matchingKey) return responseErrorMsgs[fieldName][matchingKey].errMsgId; + } + return null; From a95d17f708643d47d6182f551ee4048a3ce2f84d Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 22 Oct 2019 17:38:27 -0400 Subject: [PATCH 05/19] refactor error handling in join flow --- src/components/join-flow/join-flow.jsx | 140 +++++++++++++++++-------- 1 file changed, 96 insertions(+), 44 deletions(-) diff --git a/src/components/join-flow/join-flow.jsx b/src/components/join-flow/join-flow.jsx index b3139677c..c154034f7 100644 --- a/src/components/join-flow/join-flow.jsx +++ b/src/components/join-flow/join-flow.jsx @@ -8,6 +8,7 @@ const api = require('../../lib/api'); const injectIntl = require('../../lib/intl.jsx').injectIntl; const intlShape = require('../../lib/intl.jsx').intlShape; const sessionActions = require('../../redux/session.js'); +const validate = require('../../lib/validate'); const Progression = require('../progression/progression.jsx'); const UsernameStep = require('./username-step.jsx'); @@ -41,8 +42,8 @@ class JoinFlow extends React.Component { // reference its past fields this.state = this.initialState; } - canTryAgain () { - return (this.state.numAttempts <= 1); + canTryAgain (errorTypeAllowsTryAgain) { + return (errorTypeAllowsTryAgain && this.state.numAttempts <= 1); } handleRegistrationError (message) { if (!message) { @@ -61,54 +62,105 @@ class JoinFlow extends React.Component { this.handleSubmitRegistration(this.state.formData); }); } + getBodyErrors (err, body, res) { + return (!err && res.statusCode === 200 && body && body[0] && body[0].errors); + } + getSingleError (bodyErrors) { + if (Object.keys(bodyErrors).length === 1) { + const fieldName = Object.keys(bodyErrors)[0]; + if (bodyErrors[fieldName].length === 1) { + return {fieldName: fieldName, errorStr: bodyErrors[fieldName][0]}; + } + } + return null; + } + getCustomErrMsg (bodyErrors) { + let customErrMsg = ''; + // body can include zero or more error objects, each + // with its own key and description. Here we assemble + // all of them into a single string, customErrMsg. + const errorKeys = Object.keys(bodyErrors); + errorKeys.forEach(key => { + const vals = bodyErrors[key]; + vals.forEach(val => { + if (customErrMsg.length) customErrMsg += '; '; + customErrMsg += `${key}: ${val}`; + }); + }); + if (!customErrMsg) return null; // if no key-val pairs + const problemsStr = this.props.intl.formatMessage({id: 'registration.problemsAre'}); + return `${problemsStr} "${customErrMsg}"`; + } + getRegistrationSuccess (err, body, res) { + return !!(!err && res.statusCode === 200 && body && body[0] && body[0].success); + } + // example of failing response: + // [ + // { + // "msg": "This field is required.", + // "errors": { + // "username": ["This field is required."], + // "recaptcha": ["Incorrect, please try again."] + // }, + // "success": false + // } + // ] + // + // username messages: + // * "username": ["username exists"] + // * "username": ["invalid username"] (length, charset) + // * "username": ["bad username"] (cleanspeak) + // password messages: + // * "password": ["Ensure this value has at least 6 characters (it has LENGTH_NUM_HERE)."] + // recaptcha messages: + // * "recaptcha": ["This field is required."] + // * "recaptcha": ["Incorrect, please try again."] + // * "recaptcha": [some timeout message?] + // other messages: + // * "birth_month": ["Ensure this value is less than or equal to 12."] + // * "birth_month": ["Ensure this value is greater than or equal to 1."] handleRegistrationResponse (err, body, res) { - // example of failing response: - // [ - // { - // "msg": "This field is required.", - // "errors": { - // "username": ["This field is required."], - // "recaptcha": ["Incorrect, please try again."] - // }, - // "success": false - // } - // ] - // username: 'username exists' this.setState({ numAttempts: this.state.numAttempts + 1, waiting: false }, () => { - let errStr = ''; - if (!err && res.statusCode === 200) { - if (body && body[0]) { - if (body[0].success) { - this.props.refreshSession(); - this.setState({ - step: this.state.step + 1 - }); - return; - } - if (body[0].errors) { - // body can include zero or more error objects, each - // with its own key and description. Here we assemble - // all of them into a single string, errStr. - const errorKeys = Object.keys(body[0].errors); - errorKeys.forEach(key => { - const val = body[0].errors[key]; - if (val && val[0]) { - if (errStr.length) errStr += '; '; - errStr += `${key}: ${val[0]}`; - } - }); - } - if (!errStr.length && body[0].msg) errStr = body[0].msg; + const success = this.getRegistrationSuccess(err, body, res); + if (success) { + this.props.refreshSession(); + this.setState({step: this.state.step + 1}); + return; + } + // now we know there was some error. + // if the error was client-side, prompt user to try again + if (err || (res.statusCode >= 400 && res.statusCode < 500)) { + this.setState({registrationError: {canTryAgain: true}}); + return; + } + // now we know error was server-side. + // if server provided us info on why registration failed, + // build a summary explanation string + let errorMsg = null; + const bodyErrors = this.getBodyErrors(err, body, res); + if (bodyErrors) { + const singleError = this.getSingleError(bodyErrors); + let singleErrMsgId = null; + if (singleError) { + singleErrMsgId = validate.responseErrorMsg( + singleError.fieldName, + singleError.errorStr + ); + } + if (singleErrMsgId) { // one error that we have a predefined explanation string for + errorMsg = this.props.intl.formatMessage({id: singleErrMsgId}); + } else { // multiple errors, or unusual error; need custom message + errorMsg = this.getCustomErrMsg(bodyErrors); } } this.setState({ - registrationError: errStr || - `${this.props.intl.formatMessage({ - id: 'registration.generalError' - })} (${res.statusCode})` + registrationError: { + canTryAgain: false, + errorMsg: errorMsg + } }); }); } @@ -163,8 +215,8 @@ class JoinFlow extends React.Component { {this.state.registrationError ? ( Date: Wed, 23 Oct 2019 05:16:17 -0400 Subject: [PATCH 06/19] fix password error regex --- src/lib/validate.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/validate.js b/src/lib/validate.js index a6c4762cf..65efcc03e 100644 --- a/src/lib/validate.js +++ b/src/lib/validate.js @@ -108,7 +108,7 @@ const responseErrorMsgs = module.exports.responseErrorMsgs = { 'bad username': {errMsgId: 'registration.errorBadUsername'} }, password: { - 'Ensure this value has at least 6 characters (it has %d).': { + 'Ensure this value has at least 6 characters \\(it has \\d\\).': { errMsgId: 'registration.errorPasswordTooShort' } }, @@ -127,3 +127,4 @@ module.exports.responseErrorMsg = (fieldName, serverRawErr) => { if (matchingKey) return responseErrorMsgs[fieldName][matchingKey].errMsgId; } return null; +}; From 9a97285ac60deef7b134cea409f854bb5dc094b9 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 23 Oct 2019 05:18:03 -0400 Subject: [PATCH 07/19] rename handleRegistrationError to handleCaptchaError --- src/components/join-flow/email-step.jsx | 13 +++---------- src/components/join-flow/join-flow.jsx | 20 +++++++++++--------- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx index 6d7b8bff7..772298d81 100644 --- a/src/components/join-flow/email-step.jsx +++ b/src/components/join-flow/email-step.jsx @@ -49,7 +49,7 @@ class EmailStep extends React.Component { // Load Google ReCaptcha script. const script = document.createElement('script'); script.async = true; - script.onerror = this.onCaptchaError; + script.onerror = this.props.onCaptchaError; script.src = `https://www.recaptcha.net/recaptcha/api.js?onload=grecaptchaOnLoad&render=explicit&hl=${window._locale}`; document.body.appendChild(script); } @@ -60,20 +60,13 @@ class EmailStep extends React.Component { handleSetEmailRef (emailInputRef) { this.emailInput = emailInputRef; } - onCaptchaError () { - this.props.onRegistrationError( - this.props.intl.formatMessage({ - id: 'registration.troubleReload' - }) - ); - } onCaptchaLoad () { this.setState({captchaIsLoading: false}); this.grecaptcha = window.grecaptcha; if (!this.grecaptcha) { // According to the reCaptcha documentation, this callback shouldn't get // called unless window.grecaptcha exists. This is just here to be extra defensive. - this.onCaptchaError(); + this.props.onCaptchaError(); return; } this.widgetId = this.grecaptcha.render(this.captchaRef, @@ -234,8 +227,8 @@ class EmailStep extends React.Component { EmailStep.propTypes = { intl: intlShape, + onCaptchaError: PropTypes.func, onNextStep: PropTypes.func, - onRegistrationError: PropTypes.func, waiting: PropTypes.bool }; diff --git a/src/components/join-flow/join-flow.jsx b/src/components/join-flow/join-flow.jsx index c154034f7..4b29de4a3 100644 --- a/src/components/join-flow/join-flow.jsx +++ b/src/components/join-flow/join-flow.jsx @@ -24,8 +24,8 @@ class JoinFlow extends React.Component { super(props); bindAll(this, [ 'handleAdvanceStep', + 'handleCaptchaError', 'handleErrorNext', - 'handleRegistrationError', 'handlePrepareToRegister', 'handleRegistrationResponse', 'handleSubmitRegistration' @@ -45,13 +45,15 @@ class JoinFlow extends React.Component { canTryAgain (errorTypeAllowsTryAgain) { return (errorTypeAllowsTryAgain && this.state.numAttempts <= 1); } - handleRegistrationError (message) { - if (!message) { - message = this.props.intl.formatMessage({ - id: 'registration.generalError' - }); - } - this.setState({registrationError: message}); + handleCaptchaError () { + this.setState({ + registrationError: { + canTryAgain: false, + errorMsg: this.props.intl.formatMessage({ + id: 'registration.errorCaptcha' + }) + } + }); } handlePrepareToRegister (newFormData) { newFormData = newFormData || {}; @@ -229,8 +231,8 @@ class JoinFlow extends React.Component { Date: Wed, 23 Oct 2019 05:19:14 -0400 Subject: [PATCH 08/19] simplify canTryAgain function, reset registrationError on retry --- src/components/join-flow/join-flow.jsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/join-flow/join-flow.jsx b/src/components/join-flow/join-flow.jsx index 4b29de4a3..fec0ea000 100644 --- a/src/components/join-flow/join-flow.jsx +++ b/src/components/join-flow/join-flow.jsx @@ -42,8 +42,8 @@ class JoinFlow extends React.Component { // reference its past fields this.state = this.initialState; } - canTryAgain (errorTypeAllowsTryAgain) { - return (errorTypeAllowsTryAgain && this.state.numAttempts <= 1); + canTryAgain () { + return (this.state.registrationError.canTryAgain && this.state.numAttempts <= 1); } handleCaptchaError () { this.setState({ @@ -168,6 +168,7 @@ class JoinFlow extends React.Component { } handleSubmitRegistration (formData) { this.setState({ + registrationError: null, // clear any existing error waiting: true }, () => { api({ @@ -217,7 +218,7 @@ class JoinFlow extends React.Component { {this.state.registrationError ? ( Date: Wed, 23 Oct 2019 05:19:42 -0400 Subject: [PATCH 09/19] test registration response validation function --- test/unit/lib/validate.test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/unit/lib/validate.test.js b/test/unit/lib/validate.test.js index 6074736d5..23e3b0339 100644 --- a/test/unit/lib/validate.test.js +++ b/test/unit/lib/validate.test.js @@ -139,4 +139,11 @@ describe('unit test lib/validate.js', () => { response = validate.validateEmailLocally('much."more unusual"@example.com'); expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); }); + + test('get responseErrorMsg', () => { + let response = validate.responseErrorMsg('username', 'bad username'); + expect(response).toEqual('registration.errorBadUsername'); + response = validate.responseErrorMsg('password', 'Ensure this value has at least 6 characters (it has 3).'); + expect(response).toEqual('registration.errorPasswordTooShort'); + }); }); From 2a318af246087e4eea8baab7b73b8a524f45ecb8 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 23 Oct 2019 05:20:02 -0400 Subject: [PATCH 10/19] revise, add join flow tests --- test/unit/components/join-flow.test.jsx | 246 +++++++++++++++++++++--- 1 file changed, 220 insertions(+), 26 deletions(-) diff --git a/test/unit/components/join-flow.test.jsx b/test/unit/components/join-flow.test.jsx index 8724c8ec4..5f71a45cd 100644 --- a/test/unit/components/join-flow.test.jsx +++ b/test/unit/components/join-flow.test.jsx @@ -9,6 +9,35 @@ import RegistrationErrorStep from '../../../src/components/join-flow/registratio describe('JoinFlow', () => { const mockStore = configureStore(); let store; + const responseBodyMultipleErrs = [ + { + msg: 'This field is required.', + errors: { + username: ['This field is required.'], + recaptcha: ['Incorrect, please try again.'] + }, + success: false + } + ]; + const responseBodySingleErr = [ + { + msg: 'This field is required.', + errors: { + recaptcha: ['Incorrect, please try again.'] + }, + success: false + } + ]; + const responseBodySuccess = [ + { + msg: 'This field is required.', + errors: { + recaptcha: ['Incorrect, please try again.'] + }, + success: true + } + ]; + beforeEach(() => { store = mockStore({sessionActions: { @@ -67,7 +96,10 @@ describe('JoinFlow', () => { }; joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); - expect(joinFlowInstance.state.registrationError).toBe('username: This field is required.'); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: false, + errorMsg: 'registration.problemsAre "username: This field is required."' + }); }); test('handleRegistrationResponse with failure response, with error fields missing', () => { @@ -87,7 +119,10 @@ describe('JoinFlow', () => { }; joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); - expect(joinFlowInstance.state.registrationError).toBe('This field is required.'); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: false, + errorMsg: null + }); }); test('handleRegistrationResponse with failure response, with no text explanation', () => { @@ -106,7 +141,10 @@ describe('JoinFlow', () => { }; joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); - expect(joinFlowInstance.state.registrationError).toBe('registration.generalError (200)'); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: false, + errorMsg: null + }); }); test('handleRegistrationResponse with failure status code', () => { @@ -125,21 +163,19 @@ describe('JoinFlow', () => { }; joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); - expect(joinFlowInstance.state.registrationError).toBe('registration.generalError (400)'); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: true + }); }); - test('handleRegistrationError with no message ', () => { + test('handleCaptchaError gives state with captcha message', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.setState({}); - joinFlowInstance.handleRegistrationError(); - expect(joinFlowInstance.state.registrationError).toBe('registration.generalError'); - }); - - test('handleRegistrationError with message ', () => { - const joinFlowInstance = getJoinFlowWrapper().instance(); - joinFlowInstance.setState({}); - joinFlowInstance.handleRegistrationError('my message'); - expect(joinFlowInstance.state.registrationError).toBe('my message'); + joinFlowInstance.handleCaptchaError(); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: false, + errorMsg: 'registration.errorCaptcha' + }); }); test('handleAdvanceStep', () => { @@ -178,39 +214,64 @@ describe('JoinFlow', () => { expect(progressionWrapper).toHaveLength(1); }); - test('when numAttempts is 0, RegistrationErrorStep receives canTryAgain prop with value true', () => { + test('when numAttempts is 0 and registrationError canTryAgain is true, ' + + 'RegistrationErrorStep receives canTryAgain prop with value true', () => { const joinFlowWrapper = getJoinFlowWrapper(); joinFlowWrapper.instance().setState({ numAttempts: 0, - registrationError: 'halp there is a errors!!' + registrationError: { + canTryAgain: true, + errorMsg: 'halp there is a errors!!' + } }); const registrationErrorWrapper = joinFlowWrapper.find(RegistrationErrorStep); expect(registrationErrorWrapper.first().props().canTryAgain).toEqual(true); }); - test('when numAttempts is 1, RegistrationErrorStep receives canTryAgain prop with value true', () => { + test('when numAttempts is 1 and registrationError canTryAgain is true, ' + + 'RegistrationErrorStep receives canTryAgain prop with value true', () => { const joinFlowWrapper = getJoinFlowWrapper(); joinFlowWrapper.instance().setState({ numAttempts: 1, - registrationError: 'halp there is a errors!!' + registrationError: { + canTryAgain: true, + errorMsg: 'halp there is a errors!!' + } }); const registrationErrorWrapper = joinFlowWrapper.find(RegistrationErrorStep); expect(registrationErrorWrapper.first().props().canTryAgain).toEqual(true); }); - test('when numAttempts is 2, RegistrationErrorStep receives canTryAgain prop with value false', () => { + test('when numAttempts is 2 and registrationError canTryAgain is true, ' + + 'RegistrationErrorStep receives canTryAgain prop with value false', () => { const joinFlowWrapper = getJoinFlowWrapper(); joinFlowWrapper.instance().setState({ numAttempts: 2, - registrationError: 'halp there is a errors!!' + registrationError: { + canTryAgain: true, + errorMsg: 'halp there is a errors!!' + } + }); + const registrationErrorWrapper = joinFlowWrapper.find(RegistrationErrorStep); + expect(registrationErrorWrapper.first().props().canTryAgain).toEqual(false); + }); + + test('when numAttempts is 0 and registrationError canTryAgain is false, ' + + 'RegistrationErrorStep receives canTryAgain prop with value false', () => { + const joinFlowWrapper = getJoinFlowWrapper(); + joinFlowWrapper.instance().setState({ + numAttempts: 0, + registrationError: { + canTryAgain: false, + errorMsg: 'halp there is a errors!!' + } }); const registrationErrorWrapper = joinFlowWrapper.find(RegistrationErrorStep); expect(registrationErrorWrapper.first().props().canTryAgain).toEqual(false); }); test('resetState resets entire state, does not leave any state keys out', () => { - const joinFlowWrapper = getJoinFlowWrapper(); - const joinFlowInstance = joinFlowWrapper.instance(); + const joinFlowInstance = getJoinFlowWrapper().instance(); Object.keys(joinFlowInstance.state).forEach(key => { joinFlowInstance.setState({[key]: 'Different than the initial value'}); }); @@ -221,8 +282,7 @@ describe('JoinFlow', () => { }); test('resetState makes each state field match initial state', () => { - const joinFlowWrapper = getJoinFlowWrapper(); - const joinFlowInstance = joinFlowWrapper.instance(); + const joinFlowInstance = getJoinFlowWrapper().instance(); const stateSnapshot = {}; Object.keys(joinFlowInstance.state).forEach(key => { stateSnapshot[key] = joinFlowInstance.state[key]; @@ -234,8 +294,7 @@ describe('JoinFlow', () => { }); test('calling resetState results in state.formData which is not same reference as before', () => { - const joinFlowWrapper = getJoinFlowWrapper(); - const joinFlowInstance = joinFlowWrapper.instance(); + const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.setState({ formData: defaults({}, {username: 'abcdef'}) }); @@ -244,4 +303,139 @@ describe('JoinFlow', () => { expect(formDataReference).not.toBe(joinFlowInstance.state.formData); expect(formDataReference).not.toEqual(joinFlowInstance.state.formData); }); + + test('getBodyErrors returns object of errors', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodyMultipleErrs, {statusCode: 200}); + expect(bodyErrors).toEqual({ + username: ['This field is required.'], + recaptcha: ['Incorrect, please try again.'] + }); + }); + + test('getBodyErrors called with non-null err returns falsy', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const bodyErrors = joinFlowInstance.getBodyErrors({}, responseBodyMultipleErrs, {statusCode: 200}); + expect(bodyErrors).toBeFalsy(); + }); + + test('getBodyErrors called with non-200 status code returns falsy', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const bodyErrors = joinFlowInstance.getBodyErrors({}, responseBodyMultipleErrs, {statusCode: 400}); + expect(bodyErrors).toBeFalsy(); + }); + + test('getSingleError returns single error, when given response body with only one error', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodySingleErr, {statusCode: 200}); + const singleError = joinFlowInstance.getSingleError(bodyErrors); + expect(singleError).toEqual({ + fieldName: 'recaptcha', + errorStr: 'Incorrect, please try again.' + }); + }); + + test('getSingleError returns falsy, when given response body with multiple errors', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodyMultipleErrs, {statusCode: 200}); + const singleError = joinFlowInstance.getSingleError(bodyErrors); + expect(singleError).toBeFalsy(); + }); + + test('getCustomErrMsg string when given response body with multiple errors', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodyMultipleErrs, {statusCode: 200}); + const customErrMsg = joinFlowInstance.getCustomErrMsg(bodyErrors); + expect(customErrMsg).toEqual('registration.problemsAre "username: This field is required.; ' + + 'recaptcha: Incorrect, please try again."'); + }); + + test('getCustomErrMsg string when given response body with single error', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodySingleErr, {statusCode: 200}); + const customErrMsg = joinFlowInstance.getCustomErrMsg(bodyErrors); + expect(customErrMsg).toEqual('registration.problemsAre "recaptcha: Incorrect, please try again."'); + }); + + test('getRegistrationSuccess returns true when given response body with single error', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const success = joinFlowInstance.getRegistrationSuccess(null, responseBodySuccess, {statusCode: 200}); + expect(success).toEqual(true); + }); + + test('getRegistrationSuccess returns false when given status code not 200', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const success = joinFlowInstance.getRegistrationSuccess(null, responseBodySuccess, {statusCode: 500}); + expect(success).toEqual(false); + }); + + test('getRegistrationSuccess returns false when given body with success field false', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const success = joinFlowInstance.getRegistrationSuccess(null, responseBodySingleErr, {statusCode: 200}); + expect(success).toEqual(false); + }); + + test('getRegistrationSuccess returns false when given non null err', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + const success = joinFlowInstance.getRegistrationSuccess({}, responseBodySuccess, {statusCode: 200}); + expect(success).toEqual(false); + }); + + test('handleRegistrationResponse when passed body with success', () => { + const props = { + refreshSession: jest.fn() + }; + const joinFlowInstance = getJoinFlowWrapper(props).instance(); + joinFlowInstance.handleRegistrationResponse(null, responseBodySuccess, {statusCode: 200}); + expect(joinFlowInstance.state.registrationError).toEqual(null); + expect(joinFlowInstance.state.step).toEqual(1); + expect(joinFlowInstance.state.waiting).toBeFalsy(); + }); + + test('handleRegistrationResponse when passed body with preset server error', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + joinFlowInstance.handleRegistrationResponse(null, responseBodySingleErr, {statusCode: 200}); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: false, + errorMsg: 'registration.errorCaptcha' + }); + }); + + test('handleRegistrationResponse when passed body with unfamiliar server error', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + joinFlowInstance.handleRegistrationResponse(null, responseBodyMultipleErrs, {statusCode: 200}); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: false, + errorMsg: 'registration.problemsAre "username: This field is required.; ' + + 'recaptcha: Incorrect, please try again."' + }); + }); + + test('handleRegistrationResponse when passed non null outgoing request error', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + joinFlowInstance.handleRegistrationResponse({}, responseBodyMultipleErrs, {statusCode: 200}); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: true + }); + }); + + test('handleRegistrationResponse when passed status 400', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + joinFlowInstance.handleRegistrationResponse({}, responseBodyMultipleErrs, {statusCode: 400}); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: true + }); + }); + + test('handleRegistrationResponse when passed status 500', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + joinFlowInstance.handleRegistrationResponse(null, responseBodyMultipleErrs, {statusCode: 500}); + expect(joinFlowInstance.state.registrationError).toEqual({ + canTryAgain: false, + errorMsg: null + }); + }); + + test('handleErrorNext', () => { + }); }); From d265ad3d445ea96671566c7ce28129fdc4cf223d Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 23 Oct 2019 10:16:41 -0400 Subject: [PATCH 11/19] show errorMsg content, not formatted message with errMsg as id --- src/components/join-flow/registration-error-step.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/join-flow/registration-error-step.jsx b/src/components/join-flow/registration-error-step.jsx index d4b41ca96..058c97b4a 100644 --- a/src/components/join-flow/registration-error-step.jsx +++ b/src/components/join-flow/registration-error-step.jsx @@ -38,8 +38,8 @@ class RegistrationErrorStep extends React.Component {

{this.props.errorMsg && ( -

- +

+ {this.props.errorMsg}

)} {this.props.canTryAgain ? ( From 6e83c496c83e0baf7fcb4e75ad79e7a263a155df Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 23 Oct 2019 10:16:58 -0400 Subject: [PATCH 12/19] revise and add registration error step tests --- .../registration-error-step.test.jsx | 65 ++++++++++++++++--- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/test/unit/components/registration-error-step.test.jsx b/test/unit/components/registration-error-step.test.jsx index afad5e63a..e8a74ebd6 100644 --- a/test/unit/components/registration-error-step.test.jsx +++ b/test/unit/components/registration-error-step.test.jsx @@ -9,8 +9,6 @@ describe('RegistrationErrorStep', () => { const getRegistrationErrorStepWrapper = props => { const wrapper = shallowWithIntl( ); @@ -18,31 +16,78 @@ describe('RegistrationErrorStep', () => { .dive(); // unwrap injectIntl() }; - test('when canTryAgain is true, show tryAgain message', () => { - const props = {canTryAgain: true}; + test('registrationError has JoinFlowStep', () => { + const props = { + canTryAgain: true, + onSubmit: onSubmit + }; + const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); + expect(joinFlowStepWrapper).toHaveLength(1); + }); + + test('when errorMsg provided, registrationError shows it', () => { + const props = { + canTryAgain: true, + errorMsg: 'halp there is a errors!!', + onSubmit: onSubmit + }; + const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); + const joinFlowStepInstance = joinFlowStepWrapper.dive(); + const errMsgElement = joinFlowStepInstance.find('#registration-error-msg'); + expect(errMsgElement).toHaveLength(1); + expect(errMsgElement.text()).toEqual('halp there is a errors!!'); + }); + + test('when no errorMsg provided, registrationError does not show it', () => { + const props = { + canTryAgain: true, + onSubmit: onSubmit + }; + const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); + const joinFlowStepInstance = joinFlowStepWrapper.dive(); + const errMsgElement = joinFlowStepInstance.find('#registration-error-msg'); + expect(errMsgElement).toHaveLength(0); + }); + + test('when canTryAgain is true, show tryAgain message', () => { + const props = { + canTryAgain: true, + errorMsg: 'halp there is a errors!!', + onSubmit: onSubmit + }; const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); expect(joinFlowStepWrapper).toHaveLength(1); - expect(joinFlowStepWrapper.props().description).toBe('error message'); expect(joinFlowStepWrapper.props().nextButton).toBe('general.tryAgain'); }); test('when canTryAgain is false, show startOver message', () => { - const props = {canTryAgain: false}; + const props = { + canTryAgain: false, + errorMsg: 'halp there is a errors!!', + onSubmit: onSubmit + }; const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); expect(joinFlowStepWrapper).toHaveLength(1); - expect(joinFlowStepWrapper.props().description).toBe('error message'); expect(joinFlowStepWrapper.props().nextButton).toBe('general.startOver'); }); test('when canTryAgain is missing, show startOver message', () => { - const joinFlowStepWrapper = getRegistrationErrorStepWrapper().find(JoinFlowStep); + const props = { + errorMsg: 'halp there is a errors!!', + onSubmit: onSubmit + }; + const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); expect(joinFlowStepWrapper).toHaveLength(1); - expect(joinFlowStepWrapper.props().description).toBe('error message'); expect(joinFlowStepWrapper.props().nextButton).toBe('general.startOver'); }); test('when submitted, onSubmit is called', () => { - const joinFlowStepWrapper = getRegistrationErrorStepWrapper().find(JoinFlowStep); + const props = { + canTryAgain: true, + errorMsg: 'halp there is a errors!!', + onSubmit: onSubmit + }; + const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); joinFlowStepWrapper.props().onSubmit(new Event('event')); // eslint-disable-line no-undef expect(onSubmit).toHaveBeenCalled(); }); From f713298d37996a4391a226b5e3668cdc35ca2941 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 23 Oct 2019 10:36:20 -0400 Subject: [PATCH 13/19] use class instead of id for identifying errorMsg in test --- src/components/join-flow/registration-error-step.jsx | 2 +- test/unit/components/registration-error-step.test.jsx | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/join-flow/registration-error-step.jsx b/src/components/join-flow/registration-error-step.jsx index 058c97b4a..d79426a16 100644 --- a/src/components/join-flow/registration-error-step.jsx +++ b/src/components/join-flow/registration-error-step.jsx @@ -38,7 +38,7 @@ class RegistrationErrorStep extends React.Component {

{this.props.errorMsg && ( -

+

{this.props.errorMsg}

)} diff --git a/test/unit/components/registration-error-step.test.jsx b/test/unit/components/registration-error-step.test.jsx index e8a74ebd6..cb80e0360 100644 --- a/test/unit/components/registration-error-step.test.jsx +++ b/test/unit/components/registration-error-step.test.jsx @@ -33,7 +33,7 @@ describe('RegistrationErrorStep', () => { }; const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); const joinFlowStepInstance = joinFlowStepWrapper.dive(); - const errMsgElement = joinFlowStepInstance.find('#registration-error-msg'); + const errMsgElement = joinFlowStepInstance.find('.registration-error-msg'); expect(errMsgElement).toHaveLength(1); expect(errMsgElement.text()).toEqual('halp there is a errors!!'); }); @@ -45,7 +45,7 @@ describe('RegistrationErrorStep', () => { }; const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); const joinFlowStepInstance = joinFlowStepWrapper.dive(); - const errMsgElement = joinFlowStepInstance.find('#registration-error-msg'); + const errMsgElement = joinFlowStepInstance.find('.registration-error-msg'); expect(errMsgElement).toHaveLength(0); }); From 510906e5384028dbe62ae7c7b0660fc0bc756166 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Wed, 23 Oct 2019 15:33:31 -0400 Subject: [PATCH 14/19] removed stray onCaptchaError from bind list --- src/components/join-flow/email-step.jsx | 3 +- test/unit/components/email-step.test.jsx | 114 ++++++++++------------- 2 files changed, 51 insertions(+), 66 deletions(-) diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx index 772298d81..c70093c74 100644 --- a/src/components/join-flow/email-step.jsx +++ b/src/components/join-flow/email-step.jsx @@ -25,8 +25,7 @@ class EmailStep extends React.Component { 'validateForm', 'setCaptchaRef', 'captchaSolved', - 'onCaptchaLoad', - 'onCaptchaError' + 'onCaptchaLoad' ]); this.state = { captchaIsLoading: true diff --git a/test/unit/components/email-step.test.jsx b/test/unit/components/email-step.test.jsx index 9896d84f9..f74eebc9c 100644 --- a/test/unit/components/email-step.test.jsx +++ b/test/unit/components/email-step.test.jsx @@ -26,23 +26,23 @@ describe('EmailStep test', () => { }); test('send correct props to formik', () => { - const wrapper = shallowWithIntl(); + const intlWrapper = shallowWithIntl(); - const formikWrapper = wrapper.dive(); - expect(formikWrapper.props().initialValues.subscribe).toBe(false); - expect(formikWrapper.props().initialValues.email).toBe(''); - expect(formikWrapper.props().validateOnBlur).toBe(false); - expect(formikWrapper.props().validateOnChange).toBe(false); - expect(formikWrapper.props().validate).toBe(formikWrapper.instance().validateForm); - expect(formikWrapper.props().onSubmit).toBe(formikWrapper.instance().handleValidSubmit); + const emailStepWrapper = intlWrapper.dive(); + expect(emailStepWrapper.props().initialValues.subscribe).toBe(false); + expect(emailStepWrapper.props().initialValues.email).toBe(''); + expect(emailStepWrapper.props().validateOnBlur).toBe(false); + expect(emailStepWrapper.props().validateOnChange).toBe(false); + expect(emailStepWrapper.props().validate).toBe(emailStepWrapper.instance().validateForm); + expect(emailStepWrapper.props().onSubmit).toBe(emailStepWrapper.instance().handleValidSubmit); }); test('props sent to JoinFlowStep', () => { - const wrapper = shallowWithIntl(); + const intlWrapper = shallowWithIntl(); // Dive to get past the intl wrapper - const formikWrapper = wrapper.dive(); + const emailStepWrapper = intlWrapper.dive(); // Dive to get past the anonymous component. - const joinFlowWrapper = formikWrapper.dive().find(JoinFlowStep); + const joinFlowWrapper = emailStepWrapper.dive().find(JoinFlowStep); expect(joinFlowWrapper).toHaveLength(1); expect(joinFlowWrapper.props().footerContent.props.id).toBe('registration.acceptTermsOfUse'); expect(joinFlowWrapper.props().headerImgSrc).toBe('/images/join-flow/email-header.png'); @@ -54,11 +54,11 @@ describe('EmailStep test', () => { }); test('props sent to FormikInput for email', () => { - const wrapper = shallowWithIntl(); + const intlWrapper = shallowWithIntl(); // Dive to get past the intl wrapper - const formikWrapper = wrapper.dive(); + const emailStepWrapper = intlWrapper.dive(); // Dive to get past the anonymous component. - const joinFlowWrapper = formikWrapper.dive().find(JoinFlowStep); + const joinFlowWrapper = emailStepWrapper.dive().find(JoinFlowStep); expect(joinFlowWrapper).toHaveLength(1); const emailInputWrapper = joinFlowWrapper.find(FormikInput).first(); expect(emailInputWrapper.props().id).toEqual('email'); @@ -66,16 +66,16 @@ describe('EmailStep test', () => { expect(emailInputWrapper.props().name).toEqual('email'); expect(emailInputWrapper.props().placeholder).toEqual('general.emailAddress'); expect(emailInputWrapper.props().validationClassName).toEqual('validation-full-width-input'); - expect(emailInputWrapper.props().onSetRef).toEqual(formikWrapper.instance().handleSetEmailRef); - expect(emailInputWrapper.props().validate).toEqual(formikWrapper.instance().validateEmail); + expect(emailInputWrapper.props().onSetRef).toEqual(emailStepWrapper.instance().handleSetEmailRef); + expect(emailInputWrapper.props().validate).toEqual(emailStepWrapper.instance().validateEmail); }); test('props sent to FormikCheckbox for subscribe', () => { - const wrapper = shallowWithIntl(); + const intlWrapper = shallowWithIntl(); // Dive to get past the intl wrapper - const formikWrapper = wrapper.dive(); + const emailStepWrapper = intlWrapper.dive(); // Dive to get past the anonymous component. - const joinFlowWrapper = formikWrapper.dive().find(JoinFlowStep); + const joinFlowWrapper = emailStepWrapper.dive().find(JoinFlowStep); expect(joinFlowWrapper).toHaveLength(1); const checkboxWrapper = joinFlowWrapper.find(FormikCheckbox).first(); expect(checkboxWrapper).toHaveLength(1); @@ -93,12 +93,12 @@ describe('EmailStep test', () => { render: jest.fn() }; const formData = {item1: 'thing', item2: 'otherthing'}; - const wrapper = shallowWithIntl( + const intlWrapper = shallowWithIntl( ); - const formikWrapper = wrapper.dive(); - formikWrapper.instance().onCaptchaLoad(); // to setup catpcha state - formikWrapper.instance().handleValidSubmit(formData, formikBag); + const emailStepWrapper = intlWrapper.dive(); + emailStepWrapper.instance().onCaptchaLoad(); // to setup catpcha state + emailStepWrapper.instance().handleValidSubmit(formData, formikBag); expect(formikBag.setSubmitting).toHaveBeenCalledWith(false); expect(global.grecaptcha.execute).toHaveBeenCalled(); @@ -116,18 +116,18 @@ describe('EmailStep test', () => { render: jest.fn() }; const formData = {item1: 'thing', item2: 'otherthing'}; - const wrapper = shallowWithIntl( + const intlWrapper = shallowWithIntl( ); - const formikWrapper = wrapper.dive(); + const emailStepWrapper = intlWrapper.dive(); // Call these to setup captcha. - formikWrapper.instance().onCaptchaLoad(); // to setup catpcha state - formikWrapper.instance().handleValidSubmit(formData, formikBag); + emailStepWrapper.instance().onCaptchaLoad(); // to setup catpcha state + emailStepWrapper.instance().handleValidSubmit(formData, formikBag); const captchaToken = 'abcd'; - formikWrapper.instance().captchaSolved(captchaToken); + emailStepWrapper.instance().captchaSolved(captchaToken); // Make sure captchaSolved calls onNextStep with formData that has // a captcha token and left everything else in the object in place. expect(props.onNextStep).toHaveBeenCalledWith( @@ -139,65 +139,51 @@ describe('EmailStep test', () => { expect(formikBag.setSubmitting).toHaveBeenCalledWith(true); }); - test('onCaptchaError calls error function with correct message', () => { - const props = { - onRegistrationError: jest.fn() - }; - - const wrapper = shallowWithIntl( - ); - - const formikWrapper = wrapper.dive(); - formikWrapper.instance().onCaptchaError(); - expect(props.onRegistrationError).toHaveBeenCalledWith('registration.troubleReload'); - }); - test('Captcha load error calls error function', () => { const props = { - onRegistrationError: jest.fn() + onCaptchaError: jest.fn() }; // Set this to null to force an error. global.grecaptcha = null; - const wrapper = shallowWithIntl( + const intlWrapper = shallowWithIntl( ); + /> + ); - const formikWrapper = wrapper.dive(); - formikWrapper.instance().onCaptchaLoad(); - expect(props.onRegistrationError).toHaveBeenCalledWith('registration.troubleReload'); + const emailStepWrapper = intlWrapper.dive(); + emailStepWrapper.instance().onCaptchaLoad(); + expect(props.onCaptchaError).toHaveBeenCalled(); }); test('validateEmail test email empty', () => { - const wrapper = shallowWithIntl( + const intlWrapper = shallowWithIntl( ); - const formikWrapper = wrapper.dive(); - const val = formikWrapper.instance().validateEmail(''); + const emailStepWrapper = intlWrapper.dive(); + const val = emailStepWrapper.instance().validateEmail(''); expect(val).toBe('general.required'); }); test('validateEmail test email null', () => { - const wrapper = shallowWithIntl( + const intlWrapper = shallowWithIntl( ); - const formikWrapper = wrapper.dive(); - const val = formikWrapper.instance().validateEmail(null); + const emailStepWrapper = intlWrapper.dive(); + const val = emailStepWrapper.instance().validateEmail(null); expect(val).toBe('general.required'); }); test('validateEmail test email undefined', () => { - const wrapper = shallowWithIntl( + const intlWrapper = shallowWithIntl( ); - const formikWrapper = wrapper.dive(); - const val = formikWrapper.instance().validateEmail(); + const emailStepWrapper = intlWrapper.dive(); + const val = emailStepWrapper.instance().validateEmail(); expect(val).toBe('general.required'); }); test('validateEmailRemotelyWithCache calls validate.validateEmailRemotely', done => { - const wrapper = shallowWithIntl( + const intlWrapper = shallowWithIntl( ); - const instance = wrapper.dive().instance(); + const instance = intlWrapper.dive().instance(); instance.validateEmailRemotelyWithCache('some-email@some-domain.com') .then(response => { @@ -209,10 +195,10 @@ describe('EmailStep test', () => { }); test('validateEmailRemotelyWithCache, called twice with different data, makes two remote requests', done => { - const wrapper = shallowWithIntl( + const intlWrapper = shallowWithIntl( ); - const instance = wrapper.dive().instance(); + const instance = intlWrapper.dive().instance(); instance.validateEmailRemotelyWithCache('some-email@some-domain.com') .then(response => { @@ -233,10 +219,10 @@ describe('EmailStep test', () => { }); test('validateEmailRemotelyWithCache, called twice with same data, only makes one remote request', done => { - const wrapper = shallowWithIntl( + const intlWrapper = shallowWithIntl( ); - const instance = wrapper.dive().instance(); + const instance = intlWrapper.dive().instance(); instance.validateEmailRemotelyWithCache('some-email@some-domain.com') .then(response => { From e3d3d97c19f7cb3ecfea90aa120cb4424dedda8e Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Fri, 1 Nov 2019 15:54:30 -0400 Subject: [PATCH 15/19] change p tags to divs; set isRequired on canTryAgain --- .../join-flow/registration-error-step.jsx | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/components/join-flow/registration-error-step.jsx b/src/components/join-flow/registration-error-step.jsx index d79426a16..74c9318f3 100644 --- a/src/components/join-flow/registration-error-step.jsx +++ b/src/components/join-flow/registration-error-step.jsx @@ -34,22 +34,22 @@ class RegistrationErrorStep extends React.Component { titleClassName="join-flow-error-title" onSubmit={this.handleSubmit} > -

+

-

+
{this.props.errorMsg && ( -

+

{this.props.errorMsg} -

+
)} {this.props.canTryAgain ? ( -

+

-

+
) : ( -

+

-

+
)} ); @@ -63,6 +63,10 @@ RegistrationErrorStep.propTypes = { onSubmit: PropTypes.func.isRequired }; +RegistrationErrorStep.defaultProps = { + canTryAgain: false +}; + const IntlRegistrationErrorStep = injectIntl(RegistrationErrorStep); module.exports = IntlRegistrationErrorStep; From 7b2e75821cd34b3e3a6ba1a4b0c3acf1d645408e Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Fri, 1 Nov 2019 15:55:32 -0400 Subject: [PATCH 16/19] revise function names and handling of registration errors --- src/components/join-flow/join-flow.jsx | 90 +++++++++++++------------- 1 file changed, 46 insertions(+), 44 deletions(-) diff --git a/src/components/join-flow/join-flow.jsx b/src/components/join-flow/join-flow.jsx index fec0ea000..876bab9b6 100644 --- a/src/components/join-flow/join-flow.jsx +++ b/src/components/join-flow/join-flow.jsx @@ -43,12 +43,12 @@ class JoinFlow extends React.Component { this.state = this.initialState; } canTryAgain () { - return (this.state.registrationError.canTryAgain && this.state.numAttempts <= 1); + return (this.state.registrationError.errorAllowsTryAgain && this.state.numAttempts <= 1); } handleCaptchaError () { this.setState({ registrationError: { - canTryAgain: false, + errorAllowsTryAgain: false, errorMsg: this.props.intl.formatMessage({ id: 'registration.errorCaptcha' }) @@ -64,36 +64,34 @@ class JoinFlow extends React.Component { this.handleSubmitRegistration(this.state.formData); }); } - getBodyErrors (err, body, res) { - return (!err && res.statusCode === 200 && body && body[0] && body[0].errors); - } - getSingleError (bodyErrors) { - if (Object.keys(bodyErrors).length === 1) { - const fieldName = Object.keys(bodyErrors)[0]; - if (bodyErrors[fieldName].length === 1) { - return {fieldName: fieldName, errorStr: bodyErrors[fieldName][0]}; + getErrorsFromResponse (err, body, res) { + const errorsFromResponse = []; + if (!err && res.statusCode === 200 && body && body[0]) { + const responseBodyErrors = body[0].errors; + if (responseBodyErrors) { + Object.keys(responseBodyErrors).forEach(fieldName => { + const errorStrs = responseBodyErrors[fieldName]; + errorStrs.forEach(errorStr => { + errorsFromResponse.push({fieldName: fieldName, errorStr: errorStr}); + }); + }); } } - return null; + return errorsFromResponse; } - getCustomErrMsg (bodyErrors) { + getCustomErrMsg (errorsFromResponse) { + if (!errorsFromResponse || errorsFromResponse.length === 0) return null; let customErrMsg = ''; - // body can include zero or more error objects, each - // with its own key and description. Here we assemble + // body can include zero or more error objects. Here we assemble // all of them into a single string, customErrMsg. - const errorKeys = Object.keys(bodyErrors); - errorKeys.forEach(key => { - const vals = bodyErrors[key]; - vals.forEach(val => { - if (customErrMsg.length) customErrMsg += '; '; - customErrMsg += `${key}: ${val}`; - }); + errorsFromResponse.forEach(errorFromResponse => { + if (customErrMsg.length) customErrMsg += '; '; + customErrMsg += `${errorFromResponse.fieldName}: ${errorFromResponse.errorStr}`; }); - if (!customErrMsg) return null; // if no key-val pairs const problemsStr = this.props.intl.formatMessage({id: 'registration.problemsAre'}); - return `${problemsStr} "${customErrMsg}"`; + return `${problemsStr}: "${customErrMsg}"`; } - getRegistrationSuccess (err, body, res) { + registrationIsSuccessful (err, body, res) { return !!(!err && res.statusCode === 200 && body && body[0] && body[0].success); } // example of failing response: @@ -126,41 +124,45 @@ class JoinFlow extends React.Component { numAttempts: this.state.numAttempts + 1, waiting: false }, () => { - const success = this.getRegistrationSuccess(err, body, res); + const success = this.registrationIsSuccessful(err, body, res); if (success) { this.props.refreshSession(); this.setState({step: this.state.step + 1}); return; } - // now we know there was some error. - // if the error was client-side, prompt user to try again - if (err || (res.statusCode >= 400 && res.statusCode < 500)) { - this.setState({registrationError: {canTryAgain: true}}); + // now we know something went wrong -- either an actual error (client-side + // or server-side), or just a problem with the registration content. + + // if an actual error, prompt user to try again. + if (err || res.statusCode !== 200) { + this.setState({registrationError: {errorAllowsTryAgain: true}}); return; } - // now we know error was server-side. - // if server provided us info on why registration failed, + + // now we know there was a problem with the registration content. + // If the server provided us info on why registration failed, // build a summary explanation string let errorMsg = null; - const bodyErrors = this.getBodyErrors(err, body, res); - if (bodyErrors) { - const singleError = this.getSingleError(bodyErrors); - let singleErrMsgId = null; - if (singleError) { - singleErrMsgId = validate.responseErrorMsg( - singleError.fieldName, - singleError.errorStr - ); - } + const errorsFromResponse = this.getErrorsFromResponse(err, body, res); + // if there was exactly one error, check if we have a pre-written message + // about that precise error + if (errorsFromResponse.length === 1) { + const singleErrMsgId = validate.responseErrorMsg( + errorsFromResponse[0].fieldName, + errorsFromResponse[0].errorStr + ); if (singleErrMsgId) { // one error that we have a predefined explanation string for errorMsg = this.props.intl.formatMessage({id: singleErrMsgId}); - } else { // multiple errors, or unusual error; need custom message - errorMsg = this.getCustomErrMsg(bodyErrors); } } + // if we have more than one error, build a custom message with all of the + // server-provided error messages + if (!errorMsg && errorsFromResponse.length > 0) { + errorMsg = this.getCustomErrMsg(errorsFromResponse); + } this.setState({ registrationError: { - canTryAgain: false, + errorAllowsTryAgain: false, errorMsg: errorMsg } }); From b1d71f2a4dfb2c9f6624144350feaf4354c97b18 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Fri, 1 Nov 2019 15:56:54 -0400 Subject: [PATCH 17/19] revised join flow tests --- test/unit/components/join-flow.test.jsx | 129 ++++++++++++------------ 1 file changed, 64 insertions(+), 65 deletions(-) diff --git a/test/unit/components/join-flow.test.jsx b/test/unit/components/join-flow.test.jsx index 5f71a45cd..7e3590670 100644 --- a/test/unit/components/join-flow.test.jsx +++ b/test/unit/components/join-flow.test.jsx @@ -97,8 +97,8 @@ describe('JoinFlow', () => { joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: false, - errorMsg: 'registration.problemsAre "username: This field is required."' + errorAllowsTryAgain: false, + errorMsg: 'registration.problemsAre: "username: This field is required."' }); }); @@ -120,7 +120,7 @@ describe('JoinFlow', () => { joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: false, + errorAllowsTryAgain: false, errorMsg: null }); }); @@ -142,7 +142,7 @@ describe('JoinFlow', () => { joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: false, + errorAllowsTryAgain: false, errorMsg: null }); }); @@ -164,7 +164,7 @@ describe('JoinFlow', () => { joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: true + errorAllowsTryAgain: true }); }); @@ -173,7 +173,7 @@ describe('JoinFlow', () => { joinFlowInstance.setState({}); joinFlowInstance.handleCaptchaError(); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: false, + errorAllowsTryAgain: false, errorMsg: 'registration.errorCaptcha' }); }); @@ -214,13 +214,13 @@ describe('JoinFlow', () => { expect(progressionWrapper).toHaveLength(1); }); - test('when numAttempts is 0 and registrationError canTryAgain is true, ' + - 'RegistrationErrorStep receives canTryAgain prop with value true', () => { + test('when numAttempts is 0 and registrationError errorAllowsTryAgain is true, ' + + 'RegistrationErrorStep receives errorAllowsTryAgain prop with value true', () => { const joinFlowWrapper = getJoinFlowWrapper(); joinFlowWrapper.instance().setState({ numAttempts: 0, registrationError: { - canTryAgain: true, + errorAllowsTryAgain: true, errorMsg: 'halp there is a errors!!' } }); @@ -228,13 +228,13 @@ describe('JoinFlow', () => { expect(registrationErrorWrapper.first().props().canTryAgain).toEqual(true); }); - test('when numAttempts is 1 and registrationError canTryAgain is true, ' + - 'RegistrationErrorStep receives canTryAgain prop with value true', () => { + test('when numAttempts is 1 and registrationError errorAllowsTryAgain is true, ' + + 'RegistrationErrorStep receives errorAllowsTryAgain prop with value true', () => { const joinFlowWrapper = getJoinFlowWrapper(); joinFlowWrapper.instance().setState({ numAttempts: 1, registrationError: { - canTryAgain: true, + errorAllowsTryAgain: true, errorMsg: 'halp there is a errors!!' } }); @@ -242,13 +242,13 @@ describe('JoinFlow', () => { expect(registrationErrorWrapper.first().props().canTryAgain).toEqual(true); }); - test('when numAttempts is 2 and registrationError canTryAgain is true, ' + - 'RegistrationErrorStep receives canTryAgain prop with value false', () => { + test('when numAttempts is 2 and registrationError errorAllowsTryAgain is true, ' + + 'RegistrationErrorStep receives errorAllowsTryAgain prop with value false', () => { const joinFlowWrapper = getJoinFlowWrapper(); joinFlowWrapper.instance().setState({ numAttempts: 2, registrationError: { - canTryAgain: true, + errorAllowsTryAgain: true, errorMsg: 'halp there is a errors!!' } }); @@ -256,13 +256,13 @@ describe('JoinFlow', () => { expect(registrationErrorWrapper.first().props().canTryAgain).toEqual(false); }); - test('when numAttempts is 0 and registrationError canTryAgain is false, ' + - 'RegistrationErrorStep receives canTryAgain prop with value false', () => { + test('when numAttempts is 0 and registrationError errorAllowsTryAgain is false, ' + + 'RegistrationErrorStep receives errorAllowsTryAgain prop with value false', () => { const joinFlowWrapper = getJoinFlowWrapper(); joinFlowWrapper.instance().setState({ numAttempts: 0, registrationError: { - canTryAgain: false, + errorAllowsTryAgain: false, errorMsg: 'halp there is a errors!!' } }); @@ -304,80 +304,80 @@ describe('JoinFlow', () => { expect(formDataReference).not.toEqual(joinFlowInstance.state.formData); }); - test('getBodyErrors returns object of errors', () => { + test('getErrorsFromResponse returns object of errors', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodyMultipleErrs, {statusCode: 200}); - expect(bodyErrors).toEqual({ - username: ['This field is required.'], - recaptcha: ['Incorrect, please try again.'] - }); + const errorsFromResponse = + joinFlowInstance.getErrorsFromResponse(null, responseBodyMultipleErrs, {statusCode: 200}); + expect(errorsFromResponse).toEqual([ + { + fieldName: 'username', + errorStr: 'This field is required.' + }, { + fieldName: 'recaptcha', + errorStr: 'Incorrect, please try again.' + } + ]); }); - test('getBodyErrors called with non-null err returns falsy', () => { + test('getErrorsFromResponse called with non-null err returns empty array', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const bodyErrors = joinFlowInstance.getBodyErrors({}, responseBodyMultipleErrs, {statusCode: 200}); - expect(bodyErrors).toBeFalsy(); + const errorsFromResponse = + joinFlowInstance.getErrorsFromResponse({}, responseBodyMultipleErrs, {statusCode: 200}); + expect(errorsFromResponse).toEqual([]); }); - test('getBodyErrors called with non-200 status code returns falsy', () => { + test('getErrorsFromResponse called with non-200 status code returns empty array', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const bodyErrors = joinFlowInstance.getBodyErrors({}, responseBodyMultipleErrs, {statusCode: 400}); - expect(bodyErrors).toBeFalsy(); + const errorsFromResponse = + joinFlowInstance.getErrorsFromResponse({}, responseBodyMultipleErrs, {statusCode: 400}); + expect(errorsFromResponse).toEqual([]); }); - test('getSingleError returns single error, when given response body with only one error', () => { + test('getErrorsFromResponse gets single error, when given response body with only one error', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodySingleErr, {statusCode: 200}); - const singleError = joinFlowInstance.getSingleError(bodyErrors); - expect(singleError).toEqual({ - fieldName: 'recaptcha', - errorStr: 'Incorrect, please try again.' - }); - }); - - test('getSingleError returns falsy, when given response body with multiple errors', () => { - const joinFlowInstance = getJoinFlowWrapper().instance(); - const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodyMultipleErrs, {statusCode: 200}); - const singleError = joinFlowInstance.getSingleError(bodyErrors); - expect(singleError).toBeFalsy(); + const errorsFromResponse = + joinFlowInstance.getErrorsFromResponse(null, responseBodySingleErr, {statusCode: 200}); + expect(errorsFromResponse.length).toEqual(1); }); test('getCustomErrMsg string when given response body with multiple errors', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodyMultipleErrs, {statusCode: 200}); - const customErrMsg = joinFlowInstance.getCustomErrMsg(bodyErrors); - expect(customErrMsg).toEqual('registration.problemsAre "username: This field is required.; ' + + const errorsFromResponse = + joinFlowInstance.getErrorsFromResponse(null, responseBodyMultipleErrs, {statusCode: 200}); + const customErrMsg = joinFlowInstance.getCustomErrMsg(errorsFromResponse); + expect(customErrMsg).toEqual('registration.problemsAre: "username: This field is required.; ' + 'recaptcha: Incorrect, please try again."'); }); test('getCustomErrMsg string when given response body with single error', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const bodyErrors = joinFlowInstance.getBodyErrors(null, responseBodySingleErr, {statusCode: 200}); - const customErrMsg = joinFlowInstance.getCustomErrMsg(bodyErrors); - expect(customErrMsg).toEqual('registration.problemsAre "recaptcha: Incorrect, please try again."'); + const errorsFromResponse = + joinFlowInstance.getErrorsFromResponse(null, responseBodySingleErr, {statusCode: 200}); + const customErrMsg = joinFlowInstance.getCustomErrMsg(errorsFromResponse); + expect(customErrMsg).toEqual('registration.problemsAre: "recaptcha: Incorrect, please try again."'); }); - test('getRegistrationSuccess returns true when given response body with single error', () => { + test('registrationIsSuccessful returns true when given response body with single error', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const success = joinFlowInstance.getRegistrationSuccess(null, responseBodySuccess, {statusCode: 200}); + const success = joinFlowInstance.registrationIsSuccessful(null, responseBodySuccess, {statusCode: 200}); expect(success).toEqual(true); }); - test('getRegistrationSuccess returns false when given status code not 200', () => { + test('registrationIsSuccessful returns false when given status code not 200', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const success = joinFlowInstance.getRegistrationSuccess(null, responseBodySuccess, {statusCode: 500}); + const success = joinFlowInstance.registrationIsSuccessful(null, responseBodySuccess, {statusCode: 500}); expect(success).toEqual(false); }); - test('getRegistrationSuccess returns false when given body with success field false', () => { + test('registrationIsSuccessful returns false when given body with success field false', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const success = joinFlowInstance.getRegistrationSuccess(null, responseBodySingleErr, {statusCode: 200}); + const success = joinFlowInstance.registrationIsSuccessful(null, responseBodySingleErr, {statusCode: 200}); expect(success).toEqual(false); }); - test('getRegistrationSuccess returns false when given non null err', () => { + test('registrationIsSuccessful returns false when given non null err', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); - const success = joinFlowInstance.getRegistrationSuccess({}, responseBodySuccess, {statusCode: 200}); + const success = joinFlowInstance.registrationIsSuccessful({}, responseBodySuccess, {statusCode: 200}); expect(success).toEqual(false); }); @@ -396,7 +396,7 @@ describe('JoinFlow', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.handleRegistrationResponse(null, responseBodySingleErr, {statusCode: 200}); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: false, + errorAllowsTryAgain: false, errorMsg: 'registration.errorCaptcha' }); }); @@ -405,8 +405,8 @@ describe('JoinFlow', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.handleRegistrationResponse(null, responseBodyMultipleErrs, {statusCode: 200}); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: false, - errorMsg: 'registration.problemsAre "username: This field is required.; ' + + errorAllowsTryAgain: false, + errorMsg: 'registration.problemsAre: "username: This field is required.; ' + 'recaptcha: Incorrect, please try again."' }); }); @@ -415,7 +415,7 @@ describe('JoinFlow', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.handleRegistrationResponse({}, responseBodyMultipleErrs, {statusCode: 200}); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: true + errorAllowsTryAgain: true }); }); @@ -423,7 +423,7 @@ describe('JoinFlow', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.handleRegistrationResponse({}, responseBodyMultipleErrs, {statusCode: 400}); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: true + errorAllowsTryAgain: true }); }); @@ -431,8 +431,7 @@ describe('JoinFlow', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.handleRegistrationResponse(null, responseBodyMultipleErrs, {statusCode: 500}); expect(joinFlowInstance.state.registrationError).toEqual({ - canTryAgain: false, - errorMsg: null + errorAllowsTryAgain: true }); }); From 0305fff67036fb2d868b4ea9348787952ee3f0d7 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Fri, 1 Nov 2019 17:33:33 -0400 Subject: [PATCH 18/19] added test for error string with no message --- test/unit/lib/validate.test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/unit/lib/validate.test.js b/test/unit/lib/validate.test.js index 23e3b0339..d34eef808 100644 --- a/test/unit/lib/validate.test.js +++ b/test/unit/lib/validate.test.js @@ -140,10 +140,15 @@ describe('unit test lib/validate.js', () => { expect(response).toEqual({valid: false, errMsgId: 'registration.validationEmailInvalid'}); }); - test('get responseErrorMsg', () => { + test('get responseErrorMsg in cases where there is a dedicated string for that case', () => { let response = validate.responseErrorMsg('username', 'bad username'); expect(response).toEqual('registration.errorBadUsername'); response = validate.responseErrorMsg('password', 'Ensure this value has at least 6 characters (it has 3).'); expect(response).toEqual('registration.errorPasswordTooShort'); }); + + test('responseErrorMsg is null in case where there is no dedicated string for that case', () => { + let response = validate.responseErrorMsg('username', 'some error that is not covered'); + expect(response).toEqual(null); + }); }); From b2e7a0c9ebaccabe232c3fd85a6b49e33f685800 Mon Sep 17 00:00:00 2001 From: Ben Wheeler Date: Tue, 5 Nov 2019 08:34:33 -0500 Subject: [PATCH 19/19] revised join flow and registration error tests --- test/unit/components/join-flow.test.jsx | 166 ++++++------------ .../registration-error-step.test.jsx | 12 ++ 2 files changed, 63 insertions(+), 115 deletions(-) diff --git a/test/unit/components/join-flow.test.jsx b/test/unit/components/join-flow.test.jsx index 7e3590670..66505d1b8 100644 --- a/test/unit/components/join-flow.test.jsx +++ b/test/unit/components/join-flow.test.jsx @@ -57,117 +57,6 @@ describe('JoinFlow', () => { .dive(); // unwrap injectIntl(JoinFlow) }; - test('handleRegistrationResponse with successful response', () => { - const props = { - refreshSession: jest.fn() - }; - const joinFlowInstance = getJoinFlowWrapper(props).instance(); - const responseErr = null; - const responseBody = [ - { - success: true - } - ]; - const responseObj = { - statusCode: 200 - }; - joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); - expect(joinFlowInstance.props.refreshSession).toHaveBeenCalled(); - expect(joinFlowInstance.state.registrationError).toBe(null); - }); - - test('handleRegistrationResponse with healthy response, indicating failure', () => { - const props = { - refreshSession: jest.fn() - }; - const joinFlowInstance = getJoinFlowWrapper(props).instance(); - const responseErr = null; - const responseBody = [ - { - msg: 'This field is required.', - errors: { - username: ['This field is required.'] - }, - success: false - } - ]; - const responseObj = { - statusCode: 200 - }; - joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); - expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); - expect(joinFlowInstance.state.registrationError).toEqual({ - errorAllowsTryAgain: false, - errorMsg: 'registration.problemsAre: "username: This field is required."' - }); - }); - - test('handleRegistrationResponse with failure response, with error fields missing', () => { - const props = { - refreshSession: jest.fn() - }; - const joinFlowInstance = getJoinFlowWrapper(props).instance(); - const responseErr = null; - const responseBody = [ - { - msg: 'This field is required.', - success: false - } - ]; - const responseObj = { - statusCode: 200 - }; - joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); - expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); - expect(joinFlowInstance.state.registrationError).toEqual({ - errorAllowsTryAgain: false, - errorMsg: null - }); - }); - - test('handleRegistrationResponse with failure response, with no text explanation', () => { - const props = { - refreshSession: jest.fn() - }; - const joinFlowInstance = getJoinFlowWrapper(props).instance(); - const responseErr = null; - const responseBody = [ - { - success: false - } - ]; - const responseObj = { - statusCode: 200 - }; - joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); - expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); - expect(joinFlowInstance.state.registrationError).toEqual({ - errorAllowsTryAgain: false, - errorMsg: null - }); - }); - - test('handleRegistrationResponse with failure status code', () => { - const props = { - refreshSession: jest.fn() - }; - const joinFlowInstance = getJoinFlowWrapper(props).instance(); - const responseErr = null; - const responseBody = [ - { - success: false - } - ]; - const responseObj = { - statusCode: 400 - }; - joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); - expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); - expect(joinFlowInstance.state.registrationError).toEqual({ - errorAllowsTryAgain: true - }); - }); - test('handleCaptchaError gives state with captcha message', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.setState({}); @@ -388,6 +277,7 @@ describe('JoinFlow', () => { const joinFlowInstance = getJoinFlowWrapper(props).instance(); joinFlowInstance.handleRegistrationResponse(null, responseBodySuccess, {statusCode: 200}); expect(joinFlowInstance.state.registrationError).toEqual(null); + expect(joinFlowInstance.props.refreshSession).toHaveBeenCalled(); expect(joinFlowInstance.state.step).toEqual(1); expect(joinFlowInstance.state.waiting).toBeFalsy(); }); @@ -401,6 +291,29 @@ describe('JoinFlow', () => { }); }); + test('handleRegistrationResponse with failure response, with error fields missing', () => { + const props = { + refreshSession: jest.fn() + }; + const joinFlowInstance = getJoinFlowWrapper(props).instance(); + const responseErr = null; + const responseBody = [ + { + msg: 'This field is required.', + success: false + } + ]; + const responseObj = { + statusCode: 200 + }; + joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); + expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); + expect(joinFlowInstance.state.registrationError).toEqual({ + errorAllowsTryAgain: false, + errorMsg: null + }); + }); + test('handleRegistrationResponse when passed body with unfamiliar server error', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.handleRegistrationResponse(null, responseBodyMultipleErrs, {statusCode: 200}); @@ -411,6 +324,28 @@ describe('JoinFlow', () => { }); }); + test('handleRegistrationResponse with failure response, with no text explanation', () => { + const props = { + refreshSession: jest.fn() + }; + const joinFlowInstance = getJoinFlowWrapper(props).instance(); + const responseErr = null; + const responseBody = [ + { + success: false + } + ]; + const responseObj = { + statusCode: 200 + }; + joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj); + expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); + expect(joinFlowInstance.state.registrationError).toEqual({ + errorAllowsTryAgain: false, + errorMsg: null + }); + }); + test('handleRegistrationResponse when passed non null outgoing request error', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.handleRegistrationResponse({}, responseBodyMultipleErrs, {statusCode: 200}); @@ -420,8 +355,12 @@ describe('JoinFlow', () => { }); test('handleRegistrationResponse when passed status 400', () => { - const joinFlowInstance = getJoinFlowWrapper().instance(); + const props = { + refreshSession: jest.fn() + }; + const joinFlowInstance = getJoinFlowWrapper(props).instance(); joinFlowInstance.handleRegistrationResponse({}, responseBodyMultipleErrs, {statusCode: 400}); + expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled(); expect(joinFlowInstance.state.registrationError).toEqual({ errorAllowsTryAgain: true }); @@ -434,7 +373,4 @@ describe('JoinFlow', () => { errorAllowsTryAgain: true }); }); - - test('handleErrorNext', () => { - }); }); diff --git a/test/unit/components/registration-error-step.test.jsx b/test/unit/components/registration-error-step.test.jsx index cb80e0360..97876db95 100644 --- a/test/unit/components/registration-error-step.test.jsx +++ b/test/unit/components/registration-error-step.test.jsx @@ -38,6 +38,18 @@ describe('RegistrationErrorStep', () => { expect(errMsgElement.text()).toEqual('halp there is a errors!!'); }); + test('when errorMsg is null, registrationError does not show it', () => { + const props = { + canTryAgain: true, + errorMsg: null, + onSubmit: onSubmit + }; + const joinFlowStepWrapper = getRegistrationErrorStepWrapper(props).find(JoinFlowStep); + const joinFlowStepInstance = joinFlowStepWrapper.dive(); + const errMsgElement = joinFlowStepInstance.find('.registration-error-msg'); + expect(errMsgElement).toHaveLength(0); + }); + test('when no errorMsg provided, registrationError does not show it', () => { const props = { canTryAgain: true,