Merge pull request #3473 from benjiwheeler/join-flow-particular-server-errors

Join flow: custom error messages, retry rules for various particular registration error causes

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
@@ -49,7 +48,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 +59,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 +226,8 @@ class EmailStep extends React.Component {
 
 EmailStep.propTypes = {
     intl: intlShape,
+    onCaptchaError: PropTypes.func,
     onNextStep: PropTypes.func,
-    onRegistrationError: PropTypes.func,
     waiting: PropTypes.bool
 };
 

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 {

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');
@@ -23,8 +24,8 @@ class JoinFlow extends React.Component {
         super(props);
         bindAll(this, [
             'handleAdvanceStep',
+            'handleCaptchaError',
             'handleErrorNext',
-            'handleRegistrationError',
             'handlePrepareToRegister',
             'handleRegistrationResponse',
             'handleSubmitRegistration'
@@ -42,15 +43,17 @@ class JoinFlow extends React.Component {
         this.state = this.initialState;
     }
     canTryAgain () {
-        return (this.state.numAttempts <= 1);
+        return (this.state.registrationError.errorAllowsTryAgain && this.state.numAttempts <= 1);
     }
-    handleRegistrationError (message) {
-        if (!message) {
-            message = this.props.intl.formatMessage({
-                id: 'registration.generalError'
-            });
-        }
-        this.setState({registrationError: message});
+    handleCaptchaError () {
+        this.setState({
+            registrationError: {
+                errorAllowsTryAgain: false,
+                errorMsg: this.props.intl.formatMessage({
+                    id: 'registration.errorCaptcha'
+                })
+            }
+        });
     }
     handlePrepareToRegister (newFormData) {
         newFormData = newFormData || {};
@@ -61,59 +64,113 @@ class JoinFlow extends React.Component {
             this.handleSubmitRegistration(this.state.formData);
         });
     }
+    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 errorsFromResponse;
+    }
+    getCustomErrMsg (errorsFromResponse) {
+        if (!errorsFromResponse || errorsFromResponse.length === 0) return null;
+        let customErrMsg = '';
+        // body can include zero or more error objects. Here we assemble
+        // all of them into a single string, customErrMsg.
+        errorsFromResponse.forEach(errorFromResponse => {
+            if (customErrMsg.length) customErrMsg += '; ';
+            customErrMsg += `${errorFromResponse.fieldName}: ${errorFromResponse.errorStr}`;
+        });
+        const problemsStr = this.props.intl.formatMessage({id: 'registration.problemsAre'});
+        return `${problemsStr}: "${customErrMsg}"`;
+    }
+    registrationIsSuccessful (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.registrationIsSuccessful(err, body, res);
+            if (success) {
+                this.props.refreshSession();
+                this.setState({step: this.state.step + 1});
+                return;
+            }
+            // 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 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 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});
                 }
             }
+            // 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: errStr ||
-                    `${this.props.intl.formatMessage({
-                        id: 'registration.generalError'
-                    })} (${res.statusCode})`
+                registrationError: {
+                    errorAllowsTryAgain: false,
+                    errorMsg: errorMsg
+                }
             });
         });
     }
     handleSubmitRegistration (formData) {
         this.setState({
+            registrationError: null, // clear any existing error
             waiting: true
         }, () => {
             api({
@@ -164,7 +221,7 @@ class JoinFlow extends React.Component {
                 {this.state.registrationError ? (
                     <RegistrationErrorStep
                         canTryAgain={this.canTryAgain()}
-                        errorMsg={this.state.registrationError}
+                        errorMsg={this.state.registrationError.errorMsg}
                         /* eslint-disable react/jsx-no-bind */
                         onSubmit={this.handleErrorNext}
                         /* eslint-enable react/jsx-no-bind */
@@ -177,8 +234,8 @@ class JoinFlow extends React.Component {
                         <GenderStep onNextStep={this.handleAdvanceStep} />
                         <EmailStep
                             waiting={this.state.waiting}
+                            onCaptchaError={this.handleCaptchaError}
                             onNextStep={this.handlePrepareToRegister}
-                            onRegistrationError={this.handleRegistrationError}
                         />
                         <WelcomeStep
                             createProjectOnComplete={this.props.createProjectOnComplete}

src/components/join-flow/registration-error-step.jsx

 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,46 @@ class RegistrationErrorStep extends React.Component {
     render () {
         return (
             <JoinFlowStep
-                description={this.props.errorMsg}
-                innerClassName="join-flow-registration-error"
+                innerClassName="join-flow-inner-error-step"
                 nextButton={this.props.canTryAgain ?
                     this.props.intl.formatMessage({id: 'general.tryAgain'}) :
                     this.props.intl.formatMessage({id: 'general.startOver'})
                 }
-                title={this.props.intl.formatMessage({id: 'registration.generalError'})}
+                title={this.props.intl.formatMessage({id: 'general.error'})}
+                titleClassName="join-flow-error-title"
                 onSubmit={this.handleSubmit}
-            />
+            >
+                <div className="join-flow-instructions">
+                    <FormattedMessage id="registration.cantCreateAccount" />
+                </div>
+                {this.props.errorMsg && (
+                    <div className="join-flow-instructions registration-error-msg">
+                        {this.props.errorMsg}
+                    </div>
+                )}
+                {this.props.canTryAgain ? (
+                    <div className="join-flow-instructions">
+                        <FormattedMessage id="registration.tryAgainInstruction" />
+                    </div>
+                ) : (
+                    <div className="join-flow-instructions">
+                        <FormattedMessage id="registration.startOverInstruction" />
+                    </div>
+                )}
+            </JoinFlowStep>
         );
     }
 }
 
 RegistrationErrorStep.propTypes = {
-    canTryAgain: PropTypes.bool,
+    canTryAgain: PropTypes.bool.isRequired,
     errorMsg: PropTypes.string,
     intl: intlShape,
-    onSubmit: PropTypes.func
+    onSubmit: PropTypes.func.isRequired
+};
+
+RegistrationErrorStep.defaultProps = {
+    canTryAgain: false
 };
 
 const IntlRegistrationErrorStep = injectIntl(RegistrationErrorStep);

src/components/join-flow/welcome-step.jsx

@@ -39,10 +39,6 @@ class WelcomeStep extends React.Component {
                     } = props;
                     return (
                         <JoinFlowStep
-                            description={this.props.intl.formatMessage({
-                                id: 'registration.welcomeStepDescriptionNonEducator'
-                            })}
-                            descriptionClassName="join-flow-welcome-description"
                             headerImgClass="welcome-step-image"
                             headerImgSrc="/images/join-flow/welcome-header.png"
                             innerClassName="join-flow-inner-welcome-step"
@@ -65,6 +61,11 @@ class WelcomeStep extends React.Component {
                             waiting={isSubmitting}
                             onSubmit={handleSubmit}
                         >
+                            <div className="join-flow-instructions">
+                                <FormattedMessage
+                                    id="registration.welcomeStepDescriptionNonEducator"
+                                />
+                            </div>
                             <div className="join-flow-instructions">
                                 <FormattedMessage
                                     id="registration.welcomeStepInstructions"

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 <a href='/educators#resources'>tips, tutorials, and guides</a>.",
     "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.",

src/lib/validate.js

@@ -101,3 +101,30 @@ 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;
+};

test/unit/components/registration-error-step.test.jsx

@@ -9,8 +9,6 @@ describe('RegistrationErrorStep', () => {
     const getRegistrationErrorStepWrapper = props => {
         const wrapper = shallowWithIntl(
             <RegistrationErrorStep
-                errorMsg={'error message'}
-                onSubmit={onSubmit}
                 {...props}
             />
         );
@@ -18,31 +16,90 @@ describe('RegistrationErrorStep', () => {
             .dive(); // unwrap injectIntl()
     };
 
+    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 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,
+            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};
+        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();
     });

test/unit/lib/validate.test.js

@@ -139,4 +139,16 @@ 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 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);
+    });
 });