From 6a45907dedc28bbb5c3c1783e8332112f822d4b5 Mon Sep 17 00:00:00 2001
From: picklesrus <picklesrus@users.noreply.github.com>
Date: Wed, 18 Sep 2019 10:26:37 -0400
Subject: [PATCH 1/2] Handle errors of captcha loading by setting error state
 on JoinFlow.

---
 src/components/join-flow/email-step.jsx  | 11 +++++--
 src/components/join-flow/join-flow.jsx   | 10 +++++++
 src/l10n.json                            |  1 +
 test/unit/components/email-step.test.jsx | 37 ++++++++++++++++++++++++
 test/unit/components/join-flow.test.jsx  | 13 +++++++++
 5 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx
index f2d1e42a3..15ae71872 100644
--- a/src/components/join-flow/email-step.jsx
+++ b/src/components/join-flow/email-step.jsx
@@ -56,7 +56,11 @@ class EmailStep extends React.Component {
         this.emailInput = emailInputRef;
     }
     onCaptchaError () {
-        // TODO send user to error step once we have one.
+        this.props.onMidRegistrationError(
+            this.props.intl.formatMessage({
+                id: 'registation.troubleReload'
+            })
+        );
     }
     onCaptchaLoad () {
         this.setState({captchaIsLoading: false});
@@ -64,9 +68,9 @@ class EmailStep extends React.Component {
         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.
-            // TODO: Put up the error screen when we have one.
+            this.onCaptchaError();
+            return;
         }
-        // TODO: Add in error callback for render once we have an error screen.
         this.widgetId = this.grecaptcha.render(this.captchaRef,
             {
                 callback: this.captchaSolved,
@@ -207,6 +211,7 @@ class EmailStep extends React.Component {
 
 EmailStep.propTypes = {
     intl: intlShape,
+    onMidRegistrationError: PropTypes.func,
     onNextStep: PropTypes.func,
     waiting: PropTypes.bool
 };
diff --git a/src/components/join-flow/join-flow.jsx b/src/components/join-flow/join-flow.jsx
index 6b18d6a40..5eb8641be 100644
--- a/src/components/join-flow/join-flow.jsx
+++ b/src/components/join-flow/join-flow.jsx
@@ -23,6 +23,7 @@ class JoinFlow extends React.Component {
         super(props);
         bindAll(this, [
             'handleAdvanceStep',
+            'handleMidRegistrationError',
             'handlePrepareToRegister',
             'handleRegistrationResponse',
             'handleSubmitRegistration'
@@ -34,6 +35,14 @@ class JoinFlow extends React.Component {
             waiting: false
         };
     }
+    handleMidRegistrationError (message) {
+        if (!message) {
+            message = this.props.intl.formatMessage({
+                id: 'registration.generalError'
+            });
+        }
+        this.setState({registrationError: message});
+    }
     handlePrepareToRegister (newFormData) {
         newFormData = newFormData || {};
         const newState = {
@@ -142,6 +151,7 @@ class JoinFlow extends React.Component {
                         <GenderStep onNextStep={this.handleAdvanceStep} />
                         <EmailStep
                             waiting={this.state.waiting}
+                            onMidRegistrationError={this.handleMidRegistrationError}
                             onNextStep={this.handlePrepareToRegister}
                         />
                         <WelcomeStep
diff --git a/src/l10n.json b/src/l10n.json
index 0a3b6f8bc..364be93ea 100644
--- a/src/l10n.json
+++ b/src/l10n.json
@@ -189,6 +189,7 @@
     "registration.selectCountry": "select country",
     "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.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.",
diff --git a/test/unit/components/email-step.test.jsx b/test/unit/components/email-step.test.jsx
index b49ae09c6..3cc775462 100644
--- a/test/unit/components/email-step.test.jsx
+++ b/test/unit/components/email-step.test.jsx
@@ -50,6 +50,7 @@ describe('EmailStep test', () => {
         expect(emailInputWrapper.props().onSetRef).toEqual(formikWrapper.instance().handleSetEmailRef);
         expect(emailInputWrapper.props().validate).toEqual(formikWrapper.instance().validateEmail);
     });
+
     test('props sent to FormikCheckbox for subscribe', () => {
         const wrapper = shallowWithIntl(<EmailStep />);
         // Dive to get past the intl wrapper
@@ -63,6 +64,7 @@ describe('EmailStep test', () => {
         expect(checkboxWrapper.first().props().label).toEqual('registration.receiveEmails');
         expect(checkboxWrapper.first().props().name).toEqual('subscribe');
     });
+
     test('handleValidSubmit passes formData to next step', () => {
         const formikBag = {
             setSubmitting: jest.fn()
@@ -82,6 +84,7 @@ describe('EmailStep test', () => {
         expect(formikBag.setSubmitting).toHaveBeenCalledWith(false);
         expect(global.grecaptcha.execute).toHaveBeenCalled();
     });
+
     test('captchaSolved sets token and goes to next step', () => {
         const props = {
             onNextStep: jest.fn()
@@ -116,6 +119,38 @@ describe('EmailStep test', () => {
             }));
         expect(formikBag.setSubmitting).toHaveBeenCalledWith(true);
     });
+
+    test('onCaptchaError calls error function with correct message', () => {
+        const props = {
+            onMidRegistrationError: jest.fn()
+        };
+
+        const wrapper = shallowWithIntl(
+            <EmailStep
+                {...props}
+            />);
+
+        const formikWrapper = wrapper.dive();
+        formikWrapper.instance().onCaptchaError();
+        expect(props.onMidRegistrationError).toHaveBeenCalledWith('registation.troubleReload');
+    });
+
+    test('Captcha load error calls error function', () => {
+        const props = {
+            onMidRegistrationError: jest.fn()
+        };
+        // Set this to null to force an error.
+        global.grecaptcha = null;
+        const wrapper = shallowWithIntl(
+            <EmailStep
+                {...props}
+            />);
+
+        const formikWrapper = wrapper.dive();
+        formikWrapper.instance().onCaptchaLoad();
+        expect(props.onMidRegistrationError).toHaveBeenCalledWith('registation.troubleReload');
+    });
+
     test('validateEmail test email empty', () => {
         const wrapper = shallowWithIntl(
             <EmailStep />);
@@ -123,6 +158,7 @@ describe('EmailStep test', () => {
         const val = formikWrapper.instance().validateEmail('');
         expect(val).toBe('general.required');
     });
+
     test('validateEmail test email null', () => {
         const wrapper = shallowWithIntl(
             <EmailStep />);
@@ -130,6 +166,7 @@ describe('EmailStep test', () => {
         const val = formikWrapper.instance().validateEmail(null);
         expect(val).toBe('general.required');
     });
+
     test('validateEmail test email undefined', () => {
         const wrapper = shallowWithIntl(
             <EmailStep />);
diff --git a/test/unit/components/join-flow.test.jsx b/test/unit/components/join-flow.test.jsx
index 80dfe5a14..2714ae317 100644
--- a/test/unit/components/join-flow.test.jsx
+++ b/test/unit/components/join-flow.test.jsx
@@ -126,6 +126,19 @@ describe('JoinFlow', () => {
         expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled();
         expect(joinFlowInstance.state.registrationError).toBe('registration.generalError (400)');
     });
+    test('handleMidRegistrationError with no message ', () => {
+        const joinFlowInstance = getJoinFlowWrapper().instance();
+        joinFlowInstance.setState({});
+        joinFlowInstance.handleMidRegistrationError();
+        expect(joinFlowInstance.state.registrationError).toBe('registration.generalError');
+    });
+
+    test('handleMidRegistrationError with message ', () => {
+        const joinFlowInstance = getJoinFlowWrapper().instance();
+        joinFlowInstance.setState({});
+        joinFlowInstance.handleMidRegistrationError('my message');
+        expect(joinFlowInstance.state.registrationError).toBe('my message');
+    });
 
     test('handleAdvanceStep', () => {
         const joinFlowInstance = getJoinFlowWrapper().instance();

From cdd90da423e59e825983858748617f85b29ba783 Mon Sep 17 00:00:00 2001
From: picklesrus <picklesrus@users.noreply.github.com>
Date: Thu, 19 Sep 2019 13:40:09 -0400
Subject: [PATCH 2/2] Rename error function.

---
 src/components/join-flow/email-step.jsx  | 4 ++--
 src/components/join-flow/join-flow.jsx   | 6 +++---
 test/unit/components/email-step.test.jsx | 8 ++++----
 test/unit/components/join-flow.test.jsx  | 8 ++++----
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx
index 15ae71872..803bd1bf8 100644
--- a/src/components/join-flow/email-step.jsx
+++ b/src/components/join-flow/email-step.jsx
@@ -56,7 +56,7 @@ class EmailStep extends React.Component {
         this.emailInput = emailInputRef;
     }
     onCaptchaError () {
-        this.props.onMidRegistrationError(
+        this.props.onRegistrationError(
             this.props.intl.formatMessage({
                 id: 'registation.troubleReload'
             })
@@ -211,8 +211,8 @@ class EmailStep extends React.Component {
 
 EmailStep.propTypes = {
     intl: intlShape,
-    onMidRegistrationError: 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 5eb8641be..6849531e6 100644
--- a/src/components/join-flow/join-flow.jsx
+++ b/src/components/join-flow/join-flow.jsx
@@ -23,7 +23,7 @@ class JoinFlow extends React.Component {
         super(props);
         bindAll(this, [
             'handleAdvanceStep',
-            'handleMidRegistrationError',
+            'handleRegistrationError',
             'handlePrepareToRegister',
             'handleRegistrationResponse',
             'handleSubmitRegistration'
@@ -35,7 +35,7 @@ class JoinFlow extends React.Component {
             waiting: false
         };
     }
-    handleMidRegistrationError (message) {
+    handleRegistrationError (message) {
         if (!message) {
             message = this.props.intl.formatMessage({
                 id: 'registration.generalError'
@@ -151,8 +151,8 @@ class JoinFlow extends React.Component {
                         <GenderStep onNextStep={this.handleAdvanceStep} />
                         <EmailStep
                             waiting={this.state.waiting}
-                            onMidRegistrationError={this.handleMidRegistrationError}
                             onNextStep={this.handlePrepareToRegister}
+                            onRegistrationError={this.handleRegistrationError}
                         />
                         <WelcomeStep
                             email={this.state.formData.email}
diff --git a/test/unit/components/email-step.test.jsx b/test/unit/components/email-step.test.jsx
index 3cc775462..73a84e857 100644
--- a/test/unit/components/email-step.test.jsx
+++ b/test/unit/components/email-step.test.jsx
@@ -122,7 +122,7 @@ describe('EmailStep test', () => {
 
     test('onCaptchaError calls error function with correct message', () => {
         const props = {
-            onMidRegistrationError: jest.fn()
+            onRegistrationError: jest.fn()
         };
 
         const wrapper = shallowWithIntl(
@@ -132,12 +132,12 @@ describe('EmailStep test', () => {
 
         const formikWrapper = wrapper.dive();
         formikWrapper.instance().onCaptchaError();
-        expect(props.onMidRegistrationError).toHaveBeenCalledWith('registation.troubleReload');
+        expect(props.onRegistrationError).toHaveBeenCalledWith('registation.troubleReload');
     });
 
     test('Captcha load error calls error function', () => {
         const props = {
-            onMidRegistrationError: jest.fn()
+            onRegistrationError: jest.fn()
         };
         // Set this to null to force an error.
         global.grecaptcha = null;
@@ -148,7 +148,7 @@ describe('EmailStep test', () => {
 
         const formikWrapper = wrapper.dive();
         formikWrapper.instance().onCaptchaLoad();
-        expect(props.onMidRegistrationError).toHaveBeenCalledWith('registation.troubleReload');
+        expect(props.onRegistrationError).toHaveBeenCalledWith('registation.troubleReload');
     });
 
     test('validateEmail test email empty', () => {
diff --git a/test/unit/components/join-flow.test.jsx b/test/unit/components/join-flow.test.jsx
index 2714ae317..04cb1a6dc 100644
--- a/test/unit/components/join-flow.test.jsx
+++ b/test/unit/components/join-flow.test.jsx
@@ -126,17 +126,17 @@ describe('JoinFlow', () => {
         expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled();
         expect(joinFlowInstance.state.registrationError).toBe('registration.generalError (400)');
     });
-    test('handleMidRegistrationError with no message ', () => {
+    test('handleRegistrationError with no message ', () => {
         const joinFlowInstance = getJoinFlowWrapper().instance();
         joinFlowInstance.setState({});
-        joinFlowInstance.handleMidRegistrationError();
+        joinFlowInstance.handleRegistrationError();
         expect(joinFlowInstance.state.registrationError).toBe('registration.generalError');
     });
 
-    test('handleMidRegistrationError with message ', () => {
+    test('handleRegistrationError with message ', () => {
         const joinFlowInstance = getJoinFlowWrapper().instance();
         joinFlowInstance.setState({});
-        joinFlowInstance.handleMidRegistrationError('my message');
+        joinFlowInstance.handleRegistrationError('my message');
         expect(joinFlowInstance.state.registrationError).toBe('my message');
     });