Fix onboarding library integration (#9835)

The bug with our onboarding library integration was introduced in #8873
because of a change in when `completeOnboarding` was called. We hadn't
realized at the time that the onboarding integration relied upon the
onboarding completing event to know when the onboarding state should
be cleared. Because onboarding is now marked as completed earlier, the
state was cleared just as it was intended to be used.

The onboarding completed event has been moved back to where it was
before: after the user exits the "end of flow" page.

The original problem that #8873 was addressing was a routing issue,
where the user would be redirected back to the seed phrase confirmation
page despite already having confirmed their seed phrase. This was fixed
in a different way here, by updating the routing in the first time flow
switch to skip straight to the end of flow page if the seed phrase has
already been confirmed.

This does involve one user-facing change in behavior; if the user opens
any MetaMask UI before navigating away from the end-of-flow screen,
they will still be considered mid-onboarding so it'll redirect to the
end-of-flow screen. But we do mark onboarding as completed if the user
closes the tab/window while on the end of flow screen, which was
another goal of #8873.

ui/app/pages/first-time-flow/create-password/import-with-seed-phrase/import-with-seed-phrase.component.js

@@ -21,7 +21,6 @@ export default class ImportWithSeedPhrase extends PureComponent {
     onSubmit: PropTypes.func.isRequired,
     setSeedPhraseBackedUp: PropTypes.func,
     initializeThreeBox: PropTypes.func,
-    completeOnboarding: PropTypes.func,
   }
 
   state = {
@@ -129,7 +128,6 @@ export default class ImportWithSeedPhrase extends PureComponent {
       onSubmit,
       setSeedPhraseBackedUp,
       initializeThreeBox,
-      completeOnboarding,
     } = this.props
 
     try {
@@ -143,7 +141,6 @@ export default class ImportWithSeedPhrase extends PureComponent {
       })
 
       setSeedPhraseBackedUp(true).then(async () => {
-        await completeOnboarding()
         initializeThreeBox()
         history.push(INITIALIZE_END_OF_FLOW_ROUTE)
       })

ui/app/pages/first-time-flow/create-password/import-with-seed-phrase/import-with-seed-phrase.container.js

@@ -2,7 +2,6 @@ import { connect } from 'react-redux'
 import {
   setSeedPhraseBackedUp,
   initializeThreeBox,
-  setCompletedOnboarding,
 } from '../../../../store/actions'
 import ImportWithSeedPhrase from './import-with-seed-phrase.component'
 
@@ -11,7 +10,6 @@ const mapDispatchToProps = (dispatch) => {
     setSeedPhraseBackedUp: (seedPhraseBackupState) =>
       dispatch(setSeedPhraseBackedUp(seedPhraseBackupState)),
     initializeThreeBox: () => dispatch(initializeThreeBox()),
-    completeOnboarding: () => dispatch(setCompletedOnboarding()),
   }
 }
 

ui/app/pages/first-time-flow/end-of-flow/end-of-flow.component.js

@@ -15,19 +15,24 @@ export default class EndOfFlowScreen extends PureComponent {
   static propTypes = {
     history: PropTypes.object,
     completionMetaMetricsName: PropTypes.string,
+    setCompletedOnboarding: PropTypes.function,
     onboardingInitiator: PropTypes.exact({
       location: PropTypes.string,
       tabId: PropTypes.number,
     }),
   }
 
-  onComplete = async () => {
-    const {
-      history,
-      completionMetaMetricsName,
-      onboardingInitiator,
-    } = this.props
+  async _beforeUnload() {
+    await this._onOnboardingComplete()
+  }
 
+  _removeBeforeUnload() {
+    window.removeEventListener('beforeunload', this._beforeUnload)
+  }
+
+  async _onOnboardingComplete() {
+    const { setCompletedOnboarding, completionMetaMetricsName } = this.props
+    await setCompletedOnboarding()
     this.context.metricsEvent({
       eventOpts: {
         category: 'Onboarding',
@@ -35,13 +40,27 @@ export default class EndOfFlowScreen extends PureComponent {
         name: completionMetaMetricsName,
       },
     })
+  }
 
+  onComplete = async () => {
+    const { history, onboardingInitiator } = this.props
+
+    this._removeBeforeUnload()
+    await this._onOnboardingComplete()
     if (onboardingInitiator) {
       await returnToOnboardingInitiator(onboardingInitiator)
     }
     history.push(DEFAULT_ROUTE)
   }
 
+  componentDidMount() {
+    window.addEventListener('beforeunload', this._beforeUnload.bind(this))
+  }
+
+  componentWillUnmount = () => {
+    this._removeBeforeUnload()
+  }
+
   render() {
     const { t } = this.context
     const { onboardingInitiator } = this.props

ui/app/pages/first-time-flow/end-of-flow/end-of-flow.container.js

@@ -1,5 +1,6 @@
 import { connect } from 'react-redux'
 import { getOnboardingInitiator } from '../../../selectors'
+import { setCompletedOnboarding } from '../../../store/actions'
 import EndOfFlow from './end-of-flow.component'
 
 const firstTimeFlowTypeNameMap = {
@@ -18,4 +19,10 @@ const mapStateToProps = (state) => {
   }
 }
 
-export default connect(mapStateToProps)(EndOfFlow)
+const mapDispatchToProps = (dispatch) => {
+  return {
+    setCompletedOnboarding: () => dispatch(setCompletedOnboarding()),
+  }
+}
+
+export default connect(mapStateToProps, mapDispatchToProps)(EndOfFlow)

ui/app/pages/first-time-flow/end-of-flow/tests/end-of-flow.test.js

@@ -12,6 +12,7 @@ describe('End of Flow Screen', function () {
     history: {
       push: sinon.spy(),
     },
+    setCompletedOnboarding: sinon.spy(),
   }
 
   beforeEach(function () {

ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.component.js

@@ -4,6 +4,7 @@ import { Redirect } from 'react-router-dom'
 import {
   DEFAULT_ROUTE,
   LOCK_ROUTE,
+  INITIALIZE_END_OF_FLOW_ROUTE,
   INITIALIZE_WELCOME_ROUTE,
   INITIALIZE_UNLOCK_ROUTE,
 } from '../../../helpers/constants/routes'
@@ -13,15 +14,25 @@ export default class FirstTimeFlowSwitch extends PureComponent {
     completedOnboarding: PropTypes.bool,
     isInitialized: PropTypes.bool,
     isUnlocked: PropTypes.bool,
+    seedPhraseBackedUp: PropTypes.bool,
   }
 
   render() {
-    const { completedOnboarding, isInitialized, isUnlocked } = this.props
+    const {
+      completedOnboarding,
+      isInitialized,
+      isUnlocked,
+      seedPhraseBackedUp,
+    } = this.props
 
     if (completedOnboarding) {
       return <Redirect to={{ pathname: DEFAULT_ROUTE }} />
     }
 
+    if (seedPhraseBackedUp !== null) {
+      return <Redirect to={{ pathname: INITIALIZE_END_OF_FLOW_ROUTE }} />
+    }
+
     if (isUnlocked) {
       return <Redirect to={{ pathname: LOCK_ROUTE }} />
     }

ui/app/pages/first-time-flow/first-time-flow-switch/first-time-flow-switch.container.js

@@ -2,12 +2,18 @@ import { connect } from 'react-redux'
 import FirstTimeFlowSwitch from './first-time-flow-switch.component'
 
 const mapStateToProps = ({ metamask }) => {
-  const { completedOnboarding, isInitialized, isUnlocked } = metamask
+  const {
+    completedOnboarding,
+    isInitialized,
+    isUnlocked,
+    seedPhraseBackedUp,
+  } = metamask
 
   return {
     completedOnboarding,
     isInitialized,
     isUnlocked,
+    seedPhraseBackedUp,
   }
 }
 

ui/app/pages/first-time-flow/first-time-flow-switch/tests/first-time-flow-switch.test.js

@@ -6,12 +6,21 @@ import {
   LOCK_ROUTE,
   INITIALIZE_WELCOME_ROUTE,
   INITIALIZE_UNLOCK_ROUTE,
+  INITIALIZE_END_OF_FLOW_ROUTE,
 } from '../../../../helpers/constants/routes'
 import FirstTimeFlowSwitch from '..'
 
 describe('FirstTimeFlowSwitch', function () {
-  it('redirects to /welcome route with no props', function () {
-    const wrapper = mountWithRouter(<FirstTimeFlowSwitch.WrappedComponent />)
+  it('redirects to /welcome route with null props', function () {
+    const props = {
+      completedOnboarding: null,
+      isInitialized: null,
+      isUnlocked: null,
+      seedPhraseBackedUp: null,
+    }
+    const wrapper = mountWithRouter(
+      <FirstTimeFlowSwitch.WrappedComponent {...props} />,
+    )
     assert.equal(
       wrapper
         .find('Lifecycle')
@@ -35,10 +44,45 @@ describe('FirstTimeFlowSwitch', function () {
     )
   })
 
+  it('redirects to end of flow route when seedPhraseBackedUp is true', function () {
+    const props = {
+      completedOnboarding: false,
+      seedPhraseBackedUp: true,
+    }
+    const wrapper = mountWithRouter(
+      <FirstTimeFlowSwitch.WrappedComponent {...props} />,
+    )
+
+    assert.equal(
+      wrapper
+        .find('Lifecycle')
+        .find({ to: { pathname: INITIALIZE_END_OF_FLOW_ROUTE } }).length,
+      1,
+    )
+  })
+
+  it('redirects to end of flow route when seedPhraseBackedUp is false', function () {
+    const props = {
+      completedOnboarding: false,
+      seedPhraseBackedUp: false,
+    }
+    const wrapper = mountWithRouter(
+      <FirstTimeFlowSwitch.WrappedComponent {...props} />,
+    )
+
+    assert.equal(
+      wrapper
+        .find('Lifecycle')
+        .find({ to: { pathname: INITIALIZE_END_OF_FLOW_ROUTE } }).length,
+      1,
+    )
+  })
+
   it('redirects to /lock route when isUnlocked is true ', function () {
     const props = {
       completedOnboarding: false,
       isUnlocked: true,
+      seedPhraseBackedUp: null,
     }
 
     const wrapper = mountWithRouter(
@@ -56,6 +100,7 @@ describe('FirstTimeFlowSwitch', function () {
       completedOnboarding: false,
       isUnlocked: false,
       isInitialized: false,
+      seedPhraseBackedUp: null,
     }
 
     const wrapper = mountWithRouter(
@@ -75,6 +120,7 @@ describe('FirstTimeFlowSwitch', function () {
       completedOnboarding: false,
       isUnlocked: false,
       isInitialized: true,
+      seedPhraseBackedUp: null,
     }
 
     const wrapper = mountWithRouter(

ui/app/pages/first-time-flow/seed-phrase/confirm-seed-phrase/confirm-seed-phrase.component.js

@@ -26,7 +26,6 @@ export default class ConfirmSeedPhrase extends PureComponent {
     seedPhrase: PropTypes.string,
     initializeThreeBox: PropTypes.func,
     setSeedPhraseBackedUp: PropTypes.func,
-    completeOnboarding: PropTypes.func,
   }
 
   state = {
@@ -70,10 +69,6 @@ export default class ConfirmSeedPhrase extends PureComponent {
     exportAsFile('', this.props.seedPhrase, 'text/plain')
   }
 
-  setOnboardingCompleted = async () => {
-    await this.props.completeOnboarding()
-  }
-
   handleSubmit = async () => {
     const { history, setSeedPhraseBackedUp, initializeThreeBox } = this.props
 
@@ -92,7 +87,6 @@ export default class ConfirmSeedPhrase extends PureComponent {
 
       setSeedPhraseBackedUp(true).then(async () => {
         initializeThreeBox()
-        this.setOnboardingCompleted()
         history.push(INITIALIZE_END_OF_FLOW_ROUTE)
       })
     } catch (error) {

ui/app/pages/first-time-flow/seed-phrase/confirm-seed-phrase/confirm-seed-phrase.container.js

@@ -2,7 +2,6 @@ import { connect } from 'react-redux'
 import {
   setSeedPhraseBackedUp,
   initializeThreeBox,
-  setCompletedOnboarding,
 } from '../../../../store/actions'
 import ConfirmSeedPhrase from './confirm-seed-phrase.component'
 
@@ -11,7 +10,6 @@ const mapDispatchToProps = (dispatch) => {
     setSeedPhraseBackedUp: (seedPhraseBackupState) =>
       dispatch(setSeedPhraseBackedUp(seedPhraseBackupState)),
     initializeThreeBox: () => dispatch(initializeThreeBox()),
-    completeOnboarding: () => dispatch(setCompletedOnboarding()),
   }
 }
 

ui/app/pages/first-time-flow/seed-phrase/tests/confirm-seed-phrase-component.test.js

@@ -154,7 +154,6 @@ describe('ConfirmSeedPhrase Component', function () {
         history: { push: pushSpy },
         setSeedPhraseBackedUp: () => Promise.resolve(),
         initializeThreeBox: initialize3BoxSpy,
-        completeOnboarding: sinon.spy(),
       },
       {
         metricsEvent: metricsEventSpy,