From e4b79c1bd24f85435ff663dcbe4cf1cc6730f040 Mon Sep 17 00:00:00 2001 From: picklesrus Date: Mon, 4 Nov 2019 10:47:04 -0500 Subject: [PATCH 1/4] Add analytics logging to join flow. Adding page views for each step in the flow. --- src/components/join-flow/birthdate-step.jsx | 7 +- src/components/join-flow/country-step.jsx | 4 +- src/components/join-flow/email-step.jsx | 6 ++ src/components/join-flow/gender-step.jsx | 6 +- src/components/join-flow/join-flow.jsx | 36 ++++++++-- .../join-flow/registration-error-step.jsx | 6 +- src/components/join-flow/username-step.jsx | 9 ++- src/components/join-flow/welcome-step.jsx | 7 ++ test/unit/components/email-step.test.jsx | 66 +++++++++++++++---- test/unit/components/join-flow.test.jsx | 13 ++++ .../registration-error-step.test.jsx | 6 ++ test/unit/components/username-step.test.jsx | 54 ++++++++++++++- 12 files changed, 194 insertions(+), 26 deletions(-) diff --git a/src/components/join-flow/birthdate-step.jsx b/src/components/join-flow/birthdate-step.jsx index 4773d6d9e..25f15e825 100644 --- a/src/components/join-flow/birthdate-step.jsx +++ b/src/components/join-flow/birthdate-step.jsx @@ -54,6 +54,10 @@ class BirthDateStep extends React.Component { 'validateSelect' ]); } + componentDidMount () { + this.props.sendAnalytics('join-birthdate'); + } + validateSelect (selection) { if (selection === 'null') { return this.props.intl.formatMessage({id: 'general.required'}); @@ -162,7 +166,8 @@ class BirthDateStep extends React.Component { BirthDateStep.propTypes = { intl: intlShape, - onNextStep: PropTypes.func + onNextStep: PropTypes.func, + sendAnalytics: PropTypes.func }; const IntlBirthDateStep = injectIntl(BirthDateStep); diff --git a/src/components/join-flow/country-step.jsx b/src/components/join-flow/country-step.jsx index d3cdc30b0..0cec8e070 100644 --- a/src/components/join-flow/country-step.jsx +++ b/src/components/join-flow/country-step.jsx @@ -23,6 +23,7 @@ class CountryStep extends React.Component { this.countryOptions = []; } componentDidMount () { + this.props.sendAnalytics('join-country'); this.setCountryOptions(); } setCountryOptions () { @@ -120,7 +121,8 @@ class CountryStep extends React.Component { CountryStep.propTypes = { intl: intlShape, - onNextStep: PropTypes.func + onNextStep: PropTypes.func, + sendAnalytics: PropTypes.func }; const IntlCountryStep = injectIntl(CountryStep); diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx index ed13f9808..0dc91a17f 100644 --- a/src/components/join-flow/email-step.jsx +++ b/src/components/join-flow/email-step.jsx @@ -35,6 +35,7 @@ class EmailStep extends React.Component { this.emailRemoteCache = {}; } componentDidMount () { + this.props.sendAnalytics('join-email'); // automatically start with focus on username field if (this.emailInput) this.emailInput.focus(); @@ -231,6 +232,11 @@ EmailStep.propTypes = { intl: intlShape, onCaptchaError: PropTypes.func, onNextStep: PropTypes.func, +<<<<<<< HEAD +======= + onRegistrationError: PropTypes.func, + sendAnalytics: PropTypes.func, +>>>>>>> Add analytics logging to join flow. Adding page views for each step in the flow. waiting: PropTypes.bool }; diff --git a/src/components/join-flow/gender-step.jsx b/src/components/join-flow/gender-step.jsx index fa450eb44..1aaec2ca8 100644 --- a/src/components/join-flow/gender-step.jsx +++ b/src/components/join-flow/gender-step.jsx @@ -61,6 +61,9 @@ class GenderStep extends React.Component { 'handleValidSubmit' ]); } + componentDidMount () { + this.props.sendAnalytics('join-gender'); + } handleSetCustomRef (customInputRef) { this.customInput = customInputRef; } @@ -180,7 +183,8 @@ class GenderStep extends React.Component { GenderStep.propTypes = { intl: intlShape, - onNextStep: PropTypes.func + onNextStep: PropTypes.func, + sendAnalytics: PropTypes.func }; module.exports = injectIntl(GenderStep); diff --git a/src/components/join-flow/join-flow.jsx b/src/components/join-flow/join-flow.jsx index 876bab9b6..193e849fb 100644 --- a/src/components/join-flow/join-flow.jsx +++ b/src/components/join-flow/join-flow.jsx @@ -215,6 +215,18 @@ class JoinFlow extends React.Component { resetState () { this.setState(this.initialState); } + sendAnalytics (path) { + const gaID = window.GA_ID; + if (!window.ga) { + return; + } + window.ga('send', { + hitType: 'pageview', + page: path, + tid: gaID + }); + } + render () { return ( @@ -228,11 +240,26 @@ class JoinFlow extends React.Component { /> ) : ( - - - - + + + + + + diff --git a/src/components/join-flow/registration-error-step.jsx b/src/components/join-flow/registration-error-step.jsx index 74c9318f3..d46f50722 100644 --- a/src/components/join-flow/registration-error-step.jsx +++ b/src/components/join-flow/registration-error-step.jsx @@ -15,6 +15,9 @@ class RegistrationErrorStep extends React.Component { 'handleSubmit' ]); } + componentDidMount () { + this.props.sendAnalytics('join-error'); + } handleSubmit (e) { // JoinFlowStep includes a
that handles a submit action. // But here, we're not really submitting, so we need to prevent @@ -60,7 +63,8 @@ RegistrationErrorStep.propTypes = { canTryAgain: PropTypes.bool.isRequired, errorMsg: PropTypes.string, intl: intlShape, - onSubmit: PropTypes.func.isRequired + onSubmit: PropTypes.func.isRequired, + sendAnalytics: PropTypes.func.isRequired }; RegistrationErrorStep.defaultProps = { diff --git a/src/components/join-flow/username-step.jsx b/src/components/join-flow/username-step.jsx index aef6d5617..918689edc 100644 --- a/src/components/join-flow/username-step.jsx +++ b/src/components/join-flow/username-step.jsx @@ -37,6 +37,12 @@ class UsernameStep extends React.Component { this.usernameRemoteCache = {}; } componentDidMount () { + // Send info to analytics when we aren't on the standalone page. + // If we are on the standalone join page, the page load will take care of this. + if (!window.location.pathname.includes('/join')) { + this.props.sendAnalytics('join-email'); + } + // automatically start with focus on username field if (this.usernameInput) this.usernameInput.focus(); } @@ -282,7 +288,8 @@ class UsernameStep extends React.Component { UsernameStep.propTypes = { intl: intlShape, - onNextStep: PropTypes.func + onNextStep: PropTypes.func, + sendAnalytics: PropTypes.func }; const IntlUsernameStep = injectIntl(UsernameStep); diff --git a/src/components/join-flow/welcome-step.jsx b/src/components/join-flow/welcome-step.jsx index f4694a1c2..7610a0fad 100644 --- a/src/components/join-flow/welcome-step.jsx +++ b/src/components/join-flow/welcome-step.jsx @@ -17,6 +17,12 @@ class WelcomeStep extends React.Component { 'validateForm' ]); } + componentDidMount () { + if (this.props.sendAnalytics) { + this.props.sendAnalytics('join-welcome'); + } + } + validateForm () { return {}; } @@ -87,6 +93,7 @@ WelcomeStep.propTypes = { email: PropTypes.string, intl: intlShape, onNextStep: PropTypes.func, + sendAnalytics: PropTypes.func, username: PropTypes.string }; diff --git a/test/unit/components/email-step.test.jsx b/test/unit/components/email-step.test.jsx index 65dc7ca91..753f253d7 100644 --- a/test/unit/components/email-step.test.jsx +++ b/test/unit/components/email-step.test.jsx @@ -1,5 +1,6 @@ const React = require('react'); const {shallowWithIntl} = require('../../helpers/intl-helpers.jsx'); +const {mountWithIntl} = require('../../helpers/intl-helpers.jsx'); const JoinFlowStep = require('../../../src/components/join-flow/join-flow-step.jsx'); const FormikInput = require('../../../src/components/formik-forms/formik-input.jsx'); const FormikCheckbox = require('../../../src/components/formik-forms/formik-checkbox.jsx'); @@ -36,13 +37,17 @@ jest.mock('../../../src/lib/validate.js', () => ( const EmailStep = require('../../../src/components/join-flow/email-step.jsx'); describe('EmailStep test', () => { + const defaultProps = () => ({ + sendAnalytics: jest.fn() + }); afterEach(() => { jest.clearAllMocks(); }); test('send correct props to formik', () => { - const intlWrapper = shallowWithIntl(); - + const intlWrapper = shallowWithIntl(); const emailStepWrapper = intlWrapper.dive(); expect(emailStepWrapper.props().initialValues.subscribe).toBe(false); expect(emailStepWrapper.props().initialValues.email).toBe(''); @@ -53,7 +58,9 @@ describe('EmailStep test', () => { }); test('props sent to JoinFlowStep', () => { - const intlWrapper = shallowWithIntl(); + const intlWrapper = shallowWithIntl(); // Dive to get past the intl wrapper const emailStepWrapper = intlWrapper.dive(); // Dive to get past the anonymous component. @@ -69,7 +76,9 @@ describe('EmailStep test', () => { }); test('props sent to FormikInput for email', () => { - const intlWrapper = shallowWithIntl(); + const intlWrapper = shallowWithIntl(); // Dive to get past the intl wrapper const emailStepWrapper = intlWrapper.dive(); // Dive to get past the anonymous component. @@ -86,7 +95,10 @@ describe('EmailStep test', () => { }); test('props sent to FormikCheckbox for subscribe', () => { - const intlWrapper = shallowWithIntl(); + const intlWrapper = shallowWithIntl(); + // Dive to get past the intl wrapper const emailStepWrapper = intlWrapper.dive(); // Dive to get past the anonymous component. @@ -109,7 +121,10 @@ describe('EmailStep test', () => { }; const formData = {item1: 'thing', item2: 'otherthing'}; const intlWrapper = shallowWithIntl( - ); + ); + const emailStepWrapper = intlWrapper.dive(); emailStepWrapper.instance().onCaptchaLoad(); // to setup catpcha state @@ -133,6 +148,7 @@ describe('EmailStep test', () => { const formData = {item1: 'thing', item2: 'otherthing'}; const intlWrapper = shallowWithIntl( ); @@ -162,6 +178,7 @@ describe('EmailStep test', () => { global.grecaptcha = null; const intlWrapper = shallowWithIntl( ); @@ -171,9 +188,20 @@ describe('EmailStep test', () => { expect(props.onCaptchaError).toHaveBeenCalled(); }); + test('Component logs analytics', () => { + const sendAnalyticsFn = jest.fn(); + mountWithIntl( + ); + expect(sendAnalyticsFn).toHaveBeenCalledWith('join-email'); + }); + test('validateEmail test email empty', () => { const intlWrapper = shallowWithIntl( - ); + ); const emailStepWrapper = intlWrapper.dive(); const val = emailStepWrapper.instance().validateEmail(''); expect(val).toBe('general.required'); @@ -181,7 +209,9 @@ describe('EmailStep test', () => { test('validateEmail test email null', () => { const intlWrapper = shallowWithIntl( - ); + ); const emailStepWrapper = intlWrapper.dive(); const val = emailStepWrapper.instance().validateEmail(null); expect(val).toBe('general.required'); @@ -189,7 +219,9 @@ describe('EmailStep test', () => { test('validateEmail test email undefined', () => { const intlWrapper = shallowWithIntl( - ); + ); const emailStepWrapper = intlWrapper.dive(); const val = emailStepWrapper.instance().validateEmail(); expect(val).toBe('general.required'); @@ -204,7 +236,9 @@ describe('validateEmailRemotelyWithCache test with successful requests', () => { test('validateEmailRemotelyWithCache calls validate.validateEmailRemotely', done => { const intlWrapper = shallowWithIntl( - ); + ); const instance = intlWrapper.dive().instance(); instance.validateEmailRemotelyWithCache('some-email@some-domain.com') @@ -218,8 +252,10 @@ describe('validateEmailRemotelyWithCache test with successful requests', () => { test('validateEmailRemotelyWithCache, called twice with different data, makes two remote requests', done => { const intlWrapper = shallowWithIntl( - - ); + ); + const instance = intlWrapper.dive().instance(); instance.validateEmailRemotelyWithCache('some-email@some-domain.com') @@ -242,8 +278,10 @@ describe('validateEmailRemotelyWithCache test with successful requests', () => { test('validateEmailRemotelyWithCache, called twice with same data, only makes one remote request', done => { const intlWrapper = shallowWithIntl( - - ); + ); + const instance = intlWrapper.dive().instance(); instance.validateEmailRemotelyWithCache('some-email@some-domain.com') diff --git a/test/unit/components/join-flow.test.jsx b/test/unit/components/join-flow.test.jsx index 66505d1b8..91d4b188d 100644 --- a/test/unit/components/join-flow.test.jsx +++ b/test/unit/components/join-flow.test.jsx @@ -67,6 +67,19 @@ describe('JoinFlow', () => { }); }); + test('sendAnalytics calls google analytics with correct params', () => { + const joinFlowInstance = getJoinFlowWrapper().instance(); + global.window.ga = jest.fn(); + global.window.GA_ID = '1234'; + joinFlowInstance.sendAnalytics('page-path'); + const obj = { + hitType: 'pageview', + page: 'page-path', + tid: '1234' + }; + expect(global.window.ga).toHaveBeenCalledWith('send', obj); + }); + test('handleAdvanceStep', () => { const joinFlowInstance = getJoinFlowWrapper().instance(); joinFlowInstance.setState({formData: {username: 'ScratchCat123'}, step: 2}); diff --git a/test/unit/components/registration-error-step.test.jsx b/test/unit/components/registration-error-step.test.jsx index 97876db95..519af9f20 100644 --- a/test/unit/components/registration-error-step.test.jsx +++ b/test/unit/components/registration-error-step.test.jsx @@ -9,6 +9,12 @@ describe('RegistrationErrorStep', () => { const getRegistrationErrorStepWrapper = props => { const wrapper = shallowWithIntl( >>>>>> Add analytics logging to join flow. Adding page views for each step in the flow. {...props} /> ); diff --git a/test/unit/components/username-step.test.jsx b/test/unit/components/username-step.test.jsx index f08b8b682..08129ecc7 100644 --- a/test/unit/components/username-step.test.jsx +++ b/test/unit/components/username-step.test.jsx @@ -1,5 +1,6 @@ const React = require('react'); const {shallowWithIntl} = require('../../helpers/intl-helpers.jsx'); +const {mountWithIntl} = require('../../helpers/intl-helpers.jsx'); const requestSuccessResponse = { requestSucceeded: true, @@ -22,6 +23,7 @@ let mockedValidateUsernameRemotely = jest.fn(() => ( /* eslint-enable no-undef */ )); + jest.mock('../../../src/lib/validate.js', () => ( { ...(jest.requireActual('../../../src/lib/validate.js')), @@ -32,10 +34,22 @@ jest.mock('../../../src/lib/validate.js', () => ( // must come after validation mocks, so validate.js will be mocked before it is required const UsernameStep = require('../../../src/components/join-flow/username-step.jsx'); +<<<<<<< HEAD describe('UsernameStep tests', () => { +======= +describe('UsernameStep test', () => { + const defaultProps = () => ({ + sendAnalytics: jest.fn() + }); + afterEach(() => { + jest.clearAllMocks(); + }); +>>>>>>> Add analytics logging to join flow. Adding page views for each step in the flow. test('send correct props to formik', () => { - const wrapper = shallowWithIntl(); + const wrapper = shallowWithIntl(); const formikWrapper = wrapper.dive(); expect(formikWrapper.props().initialValues.username).toBe(''); expect(formikWrapper.props().initialValues.password).toBe(''); @@ -47,6 +61,28 @@ describe('UsernameStep tests', () => { expect(formikWrapper.props().onSubmit).toBe(formikWrapper.instance().handleValidSubmit); }); + test('Component does not log if path is /join', () => { + const sendAnalyticsFn = jest.fn(); + + global.window.history.pushState({}, '', '/join'); + mountWithIntl( + ); + expect(sendAnalyticsFn).not.toHaveBeenCalled(); + }); + + test('Component logs analytics', () => { + // Make sure '/join' is NOT in the path + global.window.history.pushState({}, '', '/'); + const sendAnalyticsFn = jest.fn(); + mountWithIntl( + ); + expect(sendAnalyticsFn).toHaveBeenCalledWith('join-email'); + }); + test('handleValidSubmit passes formData to next step', () => { const formikBag = { setSubmitting: jest.fn() @@ -55,6 +91,7 @@ describe('UsernameStep tests', () => { const mockedOnNextStep = jest.fn(); const wrapper = shallowWithIntl( ); @@ -74,7 +111,14 @@ describe('validateUsernameRemotelyWithCache test with successful requests', () = }); test('validateUsernameRemotelyWithCache calls validate.validateUsernameRemotely', done => { +<<<<<<< HEAD const wrapper = shallowWithIntl(); +======= + const wrapper = shallowWithIntl( + ); +>>>>>>> Add analytics logging to join flow. Adding page views for each step in the flow. const instance = wrapper.dive().instance(); instance.validateUsernameRemotelyWithCache('newUniqueUsername55') @@ -89,7 +133,9 @@ describe('validateUsernameRemotelyWithCache test with successful requests', () = test('validateUsernameRemotelyWithCache, called twice with different data, makes two remote requests', done => { const wrapper = shallowWithIntl( - + ); const instance = wrapper.dive().instance(); @@ -115,7 +161,9 @@ describe('validateUsernameRemotelyWithCache test with successful requests', () = test('validateUsernameRemotelyWithCache, called twice with same data, only makes one remote request', done => { const wrapper = shallowWithIntl( - + ); const instance = wrapper.dive().instance(); From 30967a30567afa299e4a216f84b591f31022bb5e Mon Sep 17 00:00:00 2001 From: picklesrus Date: Tue, 5 Nov 2019 11:54:20 -0500 Subject: [PATCH 2/4] Set sendAnalytics to be required and send the right props to the error step. Also add a test for the error step. --- src/components/join-flow/birthdate-step.jsx | 2 +- src/components/join-flow/country-step.jsx | 2 +- src/components/join-flow/email-step.jsx | 4 ++++ src/components/join-flow/gender-step.jsx | 2 +- src/components/join-flow/join-flow.jsx | 5 +++++ src/components/join-flow/registration-error-step.jsx | 5 +++++ src/components/join-flow/username-step.jsx | 2 +- .../unit/components/registration-error-step.test.jsx | 12 ++++++++++++ 8 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/components/join-flow/birthdate-step.jsx b/src/components/join-flow/birthdate-step.jsx index 25f15e825..ae0920805 100644 --- a/src/components/join-flow/birthdate-step.jsx +++ b/src/components/join-flow/birthdate-step.jsx @@ -167,7 +167,7 @@ class BirthDateStep extends React.Component { BirthDateStep.propTypes = { intl: intlShape, onNextStep: PropTypes.func, - sendAnalytics: PropTypes.func + sendAnalytics: PropTypes.func.isRequired }; const IntlBirthDateStep = injectIntl(BirthDateStep); diff --git a/src/components/join-flow/country-step.jsx b/src/components/join-flow/country-step.jsx index 0cec8e070..a95b97536 100644 --- a/src/components/join-flow/country-step.jsx +++ b/src/components/join-flow/country-step.jsx @@ -122,7 +122,7 @@ class CountryStep extends React.Component { CountryStep.propTypes = { intl: intlShape, onNextStep: PropTypes.func, - sendAnalytics: PropTypes.func + sendAnalytics: PropTypes.func.isRequired }; const IntlCountryStep = injectIntl(CountryStep); diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx index 0dc91a17f..26cc90297 100644 --- a/src/components/join-flow/email-step.jsx +++ b/src/components/join-flow/email-step.jsx @@ -235,8 +235,12 @@ EmailStep.propTypes = { <<<<<<< HEAD ======= onRegistrationError: PropTypes.func, +<<<<<<< HEAD sendAnalytics: PropTypes.func, >>>>>>> Add analytics logging to join flow. Adding page views for each step in the flow. +======= + sendAnalytics: PropTypes.func.isRequired, +>>>>>>> Set sendAnalytics to be required and send the right props to the error step. Also add a test for the error step. waiting: PropTypes.bool }; diff --git a/src/components/join-flow/gender-step.jsx b/src/components/join-flow/gender-step.jsx index 1aaec2ca8..053034139 100644 --- a/src/components/join-flow/gender-step.jsx +++ b/src/components/join-flow/gender-step.jsx @@ -184,7 +184,7 @@ class GenderStep extends React.Component { GenderStep.propTypes = { intl: intlShape, onNextStep: PropTypes.func, - sendAnalytics: PropTypes.func + sendAnalytics: PropTypes.func.isRequired }; module.exports = injectIntl(GenderStep); diff --git a/src/components/join-flow/join-flow.jsx b/src/components/join-flow/join-flow.jsx index 193e849fb..52454e6b9 100644 --- a/src/components/join-flow/join-flow.jsx +++ b/src/components/join-flow/join-flow.jsx @@ -233,7 +233,12 @@ class JoinFlow extends React.Component { {this.state.registrationError ? ( >>>>>> Set sendAnalytics to be required and send the right props to the error step. Also add a test for the error step. /* eslint-disable react/jsx-no-bind */ onSubmit={this.handleErrorNext} /* eslint-enable react/jsx-no-bind */ diff --git a/src/components/join-flow/registration-error-step.jsx b/src/components/join-flow/registration-error-step.jsx index d46f50722..160ec1241 100644 --- a/src/components/join-flow/registration-error-step.jsx +++ b/src/components/join-flow/registration-error-step.jsx @@ -63,12 +63,17 @@ RegistrationErrorStep.propTypes = { canTryAgain: PropTypes.bool.isRequired, errorMsg: PropTypes.string, intl: intlShape, +<<<<<<< HEAD onSubmit: PropTypes.func.isRequired, sendAnalytics: PropTypes.func.isRequired }; RegistrationErrorStep.defaultProps = { canTryAgain: false +======= + onSubmit: PropTypes.func, + sendAnalytics: PropTypes.func.isRequired +>>>>>>> Set sendAnalytics to be required and send the right props to the error step. Also add a test for the error step. }; const IntlRegistrationErrorStep = injectIntl(RegistrationErrorStep); diff --git a/src/components/join-flow/username-step.jsx b/src/components/join-flow/username-step.jsx index 918689edc..5405678bd 100644 --- a/src/components/join-flow/username-step.jsx +++ b/src/components/join-flow/username-step.jsx @@ -289,7 +289,7 @@ class UsernameStep extends React.Component { UsernameStep.propTypes = { intl: intlShape, onNextStep: PropTypes.func, - sendAnalytics: PropTypes.func + sendAnalytics: PropTypes.func.isRequired }; const IntlUsernameStep = injectIntl(UsernameStep); diff --git a/test/unit/components/registration-error-step.test.jsx b/test/unit/components/registration-error-step.test.jsx index 519af9f20..f160c9b41 100644 --- a/test/unit/components/registration-error-step.test.jsx +++ b/test/unit/components/registration-error-step.test.jsx @@ -1,5 +1,6 @@ import React from 'react'; import {shallowWithIntl} from '../../helpers/intl-helpers.jsx'; +import {mountWithIntl} from '../../helpers/intl-helpers.jsx'; import JoinFlowStep from '../../../src/components/join-flow/join-flow-step'; import RegistrationErrorStep from '../../../src/components/join-flow/registration-error-step'; @@ -21,6 +22,7 @@ describe('RegistrationErrorStep', () => { return wrapper .dive(); // unwrap injectIntl() }; +<<<<<<< HEAD test('registrationError has JoinFlowStep', () => { const props = { @@ -67,6 +69,16 @@ describe('RegistrationErrorStep', () => { expect(errMsgElement).toHaveLength(0); }); +======= + test('logs to analytics', () => { + const analyticsFn = jest.fn(); + mountWithIntl( + ); + expect(analyticsFn).toHaveBeenCalledWith('join-error'); + }); +>>>>>>> Set sendAnalytics to be required and send the right props to the error step. Also add a test for the error step. test('when canTryAgain is true, show tryAgain message', () => { const props = { canTryAgain: true, From c77db390623792adb5cff78a1656109611cae3a6 Mon Sep 17 00:00:00 2001 From: picklesrus Date: Tue, 5 Nov 2019 18:17:37 -0500 Subject: [PATCH 3/4] Guard calls to componentDidMount with a check for existence. Fix tag on username step. --- src/components/join-flow/birthdate-step.jsx | 4 +++- src/components/join-flow/country-step.jsx | 4 +++- src/components/join-flow/email-step.jsx | 4 +++- src/components/join-flow/gender-step.jsx | 4 +++- src/components/join-flow/registration-error-step.jsx | 4 +++- src/components/join-flow/username-step.jsx | 4 +++- test/unit/components/username-step.test.jsx | 2 +- 7 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/components/join-flow/birthdate-step.jsx b/src/components/join-flow/birthdate-step.jsx index ae0920805..6b6a7209f 100644 --- a/src/components/join-flow/birthdate-step.jsx +++ b/src/components/join-flow/birthdate-step.jsx @@ -55,7 +55,9 @@ class BirthDateStep extends React.Component { ]); } componentDidMount () { - this.props.sendAnalytics('join-birthdate'); + if (this.props.sendAnalytics) { + this.props.sendAnalytics('join-birthdate'); + } } validateSelect (selection) { diff --git a/src/components/join-flow/country-step.jsx b/src/components/join-flow/country-step.jsx index a95b97536..5ef3f1647 100644 --- a/src/components/join-flow/country-step.jsx +++ b/src/components/join-flow/country-step.jsx @@ -23,7 +23,9 @@ class CountryStep extends React.Component { this.countryOptions = []; } componentDidMount () { - this.props.sendAnalytics('join-country'); + if (this.props.sendAnalytics) { + this.props.sendAnalytics('join-country'); + } this.setCountryOptions(); } setCountryOptions () { diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx index 26cc90297..20845e1cb 100644 --- a/src/components/join-flow/email-step.jsx +++ b/src/components/join-flow/email-step.jsx @@ -35,7 +35,9 @@ class EmailStep extends React.Component { this.emailRemoteCache = {}; } componentDidMount () { - this.props.sendAnalytics('join-email'); + if (this.props.sendAnalytics) { + this.props.sendAnalytics('join-email'); + } // automatically start with focus on username field if (this.emailInput) this.emailInput.focus(); diff --git a/src/components/join-flow/gender-step.jsx b/src/components/join-flow/gender-step.jsx index 053034139..30089899d 100644 --- a/src/components/join-flow/gender-step.jsx +++ b/src/components/join-flow/gender-step.jsx @@ -62,7 +62,9 @@ class GenderStep extends React.Component { ]); } componentDidMount () { - this.props.sendAnalytics('join-gender'); + if (this.props.sendAnalytics) { + this.props.sendAnalytics('join-gender'); + } } handleSetCustomRef (customInputRef) { this.customInput = customInputRef; diff --git a/src/components/join-flow/registration-error-step.jsx b/src/components/join-flow/registration-error-step.jsx index 160ec1241..d7eb5618c 100644 --- a/src/components/join-flow/registration-error-step.jsx +++ b/src/components/join-flow/registration-error-step.jsx @@ -16,7 +16,9 @@ class RegistrationErrorStep extends React.Component { ]); } componentDidMount () { - this.props.sendAnalytics('join-error'); + if (this.props.sendAnalytics) { + this.props.sendAnalytics('join-error'); + } } handleSubmit (e) { // JoinFlowStep includes a that handles a submit action. diff --git a/src/components/join-flow/username-step.jsx b/src/components/join-flow/username-step.jsx index 5405678bd..ed6c23c13 100644 --- a/src/components/join-flow/username-step.jsx +++ b/src/components/join-flow/username-step.jsx @@ -40,7 +40,9 @@ class UsernameStep extends React.Component { // Send info to analytics when we aren't on the standalone page. // If we are on the standalone join page, the page load will take care of this. if (!window.location.pathname.includes('/join')) { - this.props.sendAnalytics('join-email'); + if (this.props.sendAnalytics) { + this.props.sendAnalytics('join-username-modal'); + } } // automatically start with focus on username field diff --git a/test/unit/components/username-step.test.jsx b/test/unit/components/username-step.test.jsx index 08129ecc7..56e34588d 100644 --- a/test/unit/components/username-step.test.jsx +++ b/test/unit/components/username-step.test.jsx @@ -80,7 +80,7 @@ describe('UsernameStep test', () => { ); - expect(sendAnalyticsFn).toHaveBeenCalledWith('join-email'); + expect(sendAnalyticsFn).toHaveBeenCalledWith('join-username-modal'); }); test('handleValidSubmit passes formData to next step', () => { From 68fc4fe45b1b828781f078314ac2c8b909362444 Mon Sep 17 00:00:00 2001 From: picklesrus Date: Tue, 5 Nov 2019 20:01:35 -0500 Subject: [PATCH 4/4] Fix merge conflicts :( --- src/components/join-flow/email-step.jsx | 8 -------- src/components/join-flow/join-flow.jsx | 4 ---- .../join-flow/registration-error-step.jsx | 5 ----- test/unit/components/email-step.test.jsx | 17 ++++++---------- .../registration-error-step.test.jsx | 8 -------- test/unit/components/username-step.test.jsx | 20 +++---------------- 6 files changed, 9 insertions(+), 53 deletions(-) diff --git a/src/components/join-flow/email-step.jsx b/src/components/join-flow/email-step.jsx index 20845e1cb..6bcca7305 100644 --- a/src/components/join-flow/email-step.jsx +++ b/src/components/join-flow/email-step.jsx @@ -234,15 +234,7 @@ EmailStep.propTypes = { intl: intlShape, onCaptchaError: PropTypes.func, onNextStep: PropTypes.func, -<<<<<<< HEAD -======= - onRegistrationError: PropTypes.func, -<<<<<<< HEAD - sendAnalytics: PropTypes.func, ->>>>>>> Add analytics logging to join flow. Adding page views for each step in the flow. -======= sendAnalytics: PropTypes.func.isRequired, ->>>>>>> Set sendAnalytics to be required and send the right props to the error step. Also add a test for the error step. waiting: PropTypes.bool }; diff --git a/src/components/join-flow/join-flow.jsx b/src/components/join-flow/join-flow.jsx index 52454e6b9..fd6760da6 100644 --- a/src/components/join-flow/join-flow.jsx +++ b/src/components/join-flow/join-flow.jsx @@ -233,12 +233,8 @@ class JoinFlow extends React.Component { {this.state.registrationError ? ( >>>>>> Set sendAnalytics to be required and send the right props to the error step. Also add a test for the error step. /* eslint-disable react/jsx-no-bind */ onSubmit={this.handleErrorNext} /* eslint-enable react/jsx-no-bind */ diff --git a/src/components/join-flow/registration-error-step.jsx b/src/components/join-flow/registration-error-step.jsx index d7eb5618c..a7151840d 100644 --- a/src/components/join-flow/registration-error-step.jsx +++ b/src/components/join-flow/registration-error-step.jsx @@ -65,17 +65,12 @@ RegistrationErrorStep.propTypes = { canTryAgain: PropTypes.bool.isRequired, errorMsg: PropTypes.string, intl: intlShape, -<<<<<<< HEAD onSubmit: PropTypes.func.isRequired, sendAnalytics: PropTypes.func.isRequired }; RegistrationErrorStep.defaultProps = { canTryAgain: false -======= - onSubmit: PropTypes.func, - sendAnalytics: PropTypes.func.isRequired ->>>>>>> Set sendAnalytics to be required and send the right props to the error step. Also add a test for the error step. }; const IntlRegistrationErrorStep = injectIntl(RegistrationErrorStep); diff --git a/test/unit/components/email-step.test.jsx b/test/unit/components/email-step.test.jsx index 753f253d7..3f2c866d0 100644 --- a/test/unit/components/email-step.test.jsx +++ b/test/unit/components/email-step.test.jsx @@ -236,9 +236,8 @@ describe('validateEmailRemotelyWithCache test with successful requests', () => { test('validateEmailRemotelyWithCache calls validate.validateEmailRemotely', done => { const intlWrapper = shallowWithIntl( - ); + + ); const instance = intlWrapper.dive().instance(); instance.validateEmailRemotelyWithCache('some-email@some-domain.com') @@ -252,10 +251,8 @@ describe('validateEmailRemotelyWithCache test with successful requests', () => { test('validateEmailRemotelyWithCache, called twice with different data, makes two remote requests', done => { const intlWrapper = shallowWithIntl( - ); - + + ); const instance = intlWrapper.dive().instance(); instance.validateEmailRemotelyWithCache('some-email@some-domain.com') @@ -278,10 +275,8 @@ describe('validateEmailRemotelyWithCache test with successful requests', () => { test('validateEmailRemotelyWithCache, called twice with same data, only makes one remote request', done => { const intlWrapper = shallowWithIntl( - ); - + + ); const instance = intlWrapper.dive().instance(); instance.validateEmailRemotelyWithCache('some-email@some-domain.com') diff --git a/test/unit/components/registration-error-step.test.jsx b/test/unit/components/registration-error-step.test.jsx index f160c9b41..42848bf1b 100644 --- a/test/unit/components/registration-error-step.test.jsx +++ b/test/unit/components/registration-error-step.test.jsx @@ -10,19 +10,13 @@ describe('RegistrationErrorStep', () => { const getRegistrationErrorStepWrapper = props => { const wrapper = shallowWithIntl( >>>>>> Add analytics logging to join flow. Adding page views for each step in the flow. {...props} /> ); return wrapper .dive(); // unwrap injectIntl() }; -<<<<<<< HEAD test('registrationError has JoinFlowStep', () => { const props = { @@ -69,7 +63,6 @@ describe('RegistrationErrorStep', () => { expect(errMsgElement).toHaveLength(0); }); -======= test('logs to analytics', () => { const analyticsFn = jest.fn(); mountWithIntl( @@ -78,7 +71,6 @@ describe('RegistrationErrorStep', () => { />); expect(analyticsFn).toHaveBeenCalledWith('join-error'); }); ->>>>>>> Set sendAnalytics to be required and send the right props to the error step. Also add a test for the error step. test('when canTryAgain is true, show tryAgain message', () => { const props = { canTryAgain: true, diff --git a/test/unit/components/username-step.test.jsx b/test/unit/components/username-step.test.jsx index 56e34588d..873b015de 100644 --- a/test/unit/components/username-step.test.jsx +++ b/test/unit/components/username-step.test.jsx @@ -34,17 +34,14 @@ jest.mock('../../../src/lib/validate.js', () => ( // must come after validation mocks, so validate.js will be mocked before it is required const UsernameStep = require('../../../src/components/join-flow/username-step.jsx'); -<<<<<<< HEAD + describe('UsernameStep tests', () => { -======= -describe('UsernameStep test', () => { const defaultProps = () => ({ sendAnalytics: jest.fn() }); afterEach(() => { jest.clearAllMocks(); }); ->>>>>>> Add analytics logging to join flow. Adding page views for each step in the flow. test('send correct props to formik', () => { const wrapper = shallowWithIntl( { -<<<<<<< HEAD const wrapper = shallowWithIntl(); -======= - const wrapper = shallowWithIntl( - ); ->>>>>>> Add analytics logging to join flow. Adding page views for each step in the flow. const instance = wrapper.dive().instance(); instance.validateUsernameRemotelyWithCache('newUniqueUsername55') @@ -133,9 +123,7 @@ describe('validateUsernameRemotelyWithCache test with successful requests', () = test('validateUsernameRemotelyWithCache, called twice with different data, makes two remote requests', done => { const wrapper = shallowWithIntl( - + ); const instance = wrapper.dive().instance(); @@ -161,9 +149,7 @@ describe('validateUsernameRemotelyWithCache test with successful requests', () = test('validateUsernameRemotelyWithCache, called twice with same data, only makes one remote request', done => { const wrapper = shallowWithIntl( - + ); const instance = wrapper.dive().instance();