Migrate completedOnboarding and firstTimeFlowType state into onboardingController (#12356)

* migrate completedOnboarding state into onboardingController

* migrate firstTimeFlowType state from preferencesController to onboardingController

app/scripts/controllers/onboarding.js

@@ -4,12 +4,12 @@ import log from 'loglevel';
 /**
  * @typedef {Object} InitState
  * @property {Boolean} seedPhraseBackedUp Indicates whether the user has completed the seed phrase backup challenge
+ * @property {Boolean} completedOnboarding Indicates whether the user has completed the onboarding flow
  */
 
 /**
  * @typedef {Object} OnboardingOptions
  * @property {InitState} initState The initial controller state
- * @property {PreferencesController} preferencesController Controller for managing user perferences
  */
 
 /**
@@ -28,21 +28,12 @@ export default class OnboardingController {
     };
     const initState = {
       seedPhraseBackedUp: null,
+      firstTimeFlowType: null,
+      completedOnboarding: false,
       ...opts.initState,
       ...initialTransientState,
     };
     this.store = new ObservableStore(initState);
-    this.preferencesController = opts.preferencesController;
-    this.completedOnboarding = this.preferencesController.store.getState().completedOnboarding;
-
-    this.preferencesController.store.subscribe(({ completedOnboarding }) => {
-      if (completedOnboarding !== this.completedOnboarding) {
-        this.completedOnboarding = completedOnboarding;
-        if (completedOnboarding) {
-          this.store.updateState(initialTransientState);
-        }
-      }
-    });
   }
 
   setSeedPhraseBackedUp(newSeedPhraseBackUpState) {
@@ -51,6 +42,27 @@ export default class OnboardingController {
     });
   }
 
+  // /**
+  //  * Sets the completedOnboarding state to true, indicating that the user has completed the
+  //  * onboarding process.
+  //  */
+  completeOnboarding() {
+    this.store.updateState({
+      completedOnboarding: true,
+    });
+    return Promise.resolve(true);
+  }
+
+  /**
+   * Setter for the `firstTimeFlowType` property
+   *
+   * @param {string} type - Indicates the type of first time flow - create or import - the user wishes to follow
+   *
+   */
+  setFirstTimeFlowType(type) {
+    this.store.updateState({ firstTimeFlowType: type });
+  }
+
   /**
    * Registering a site as having initiated onboarding
    *

app/scripts/controllers/preferences.js

@@ -45,7 +45,6 @@ export default class PreferencesController {
         showIncomingTransactions: true,
       },
       knownMethodData: {},
-      firstTimeFlowType: null,
       currentLocale: opts.initLangCode,
       identities: {},
       lostIdentities: {},
@@ -56,7 +55,6 @@ export default class PreferencesController {
         useNativeCurrencyAsPrimaryCurrency: true,
         hideZeroBalanceTokens: false,
       },
-      completedOnboarding: false,
       // ENS decentralized website resolution
       ipfsGateway: 'dweb.link',
       infuraBlocked: null,
@@ -127,16 +125,6 @@ export default class PreferencesController {
     this.store.updateState({ useTokenDetection: val });
   }
 
-  /**
-   * Setter for the `firstTimeFlowType` property
-   *
-   * @param {string} type - Indicates the type of first time flow - create or import - the user wishes to follow
-   *
-   */
-  setFirstTimeFlowType(type) {
-    this.store.updateState({ firstTimeFlowType: type });
-  }
-
   /**
    * Add new methodData to state, to avoid requesting this information again through Infura
    *
@@ -509,15 +497,6 @@ export default class PreferencesController {
     return this.store.getState().preferences;
   }
 
-  /**
-   * Sets the completedOnboarding state to true, indicating that the user has completed the
-   * onboarding process.
-   */
-  completeOnboarding() {
-    this.store.updateState({ completedOnboarding: true });
-    return Promise.resolve(true);
-  }
-
   /**
    * A getter for the `ipfsGateway` property
    * @returns {string} The current IPFS gateway domain

app/scripts/metamask-controller.js

@@ -364,7 +364,6 @@ export default class MetamaskController extends EventEmitter {
 
     this.onboardingController = new OnboardingController({
       initState: initState.OnboardingController,
-      preferencesController: this.preferencesController,
     });
 
     this.tokensController.hub.on('pendingSuggestedAsset', async () => {
@@ -629,7 +628,7 @@ export default class MetamaskController extends EventEmitter {
     if (
       password &&
       !this.isUnlocked() &&
-      this.onboardingController.completedOnboarding
+      this.onboardingController.store.getState().completedOnboarding
     ) {
       this.submitPassword(password);
     }
@@ -820,7 +819,6 @@ export default class MetamaskController extends EventEmitter {
       ),
       setIpfsGateway: this.setIpfsGateway.bind(this),
       setParticipateInMetaMetrics: this.setParticipateInMetaMetrics.bind(this),
-      setFirstTimeFlowType: this.setFirstTimeFlowType.bind(this),
       setCurrentLocale: this.setCurrentLocale.bind(this),
       markPasswordForgotten: this.markPasswordForgotten.bind(this),
       unMarkPasswordForgotten: this.unMarkPasswordForgotten.bind(this),
@@ -899,10 +897,7 @@ export default class MetamaskController extends EventEmitter {
         preferencesController.setPreference,
         preferencesController,
       ),
-      completeOnboarding: nodeify(
-        preferencesController.completeOnboarding,
-        preferencesController,
-      ),
+
       addKnownMethodData: nodeify(
         preferencesController.addKnownMethodData,
         preferencesController,
@@ -1003,6 +998,14 @@ export default class MetamaskController extends EventEmitter {
         onboardingController.setSeedPhraseBackedUp,
         onboardingController,
       ),
+      completeOnboarding: nodeify(
+        onboardingController.completeOnboarding,
+        onboardingController,
+      ),
+      setFirstTimeFlowType: nodeify(
+        onboardingController.setFirstTimeFlowType,
+        onboardingController,
+      ),
 
       // alert controller
       setAlertEnabledness: nodeify(
@@ -3017,23 +3020,6 @@ export default class MetamaskController extends EventEmitter {
     }
   }
 
-  /**
-   * Sets the type of first time flow the user wishes to follow: create or import
-   * @param {string} type - Indicates the type of first time flow the user wishes to follow
-   * @param {Function} cb - A callback function called when complete.
-   */
-  setFirstTimeFlowType(type, cb) {
-    try {
-      this.preferencesController.setFirstTimeFlowType(type);
-      cb(null);
-      return;
-    } catch (err) {
-      cb(err);
-      // eslint-disable-next-line no-useless-return
-      return;
-    }
-  }
-
   /**
    * A method for setting a user's current locale, affecting the language rendered.
    * @param {string} key - Locale identifier.

app/scripts/migrations/065.js

@@ -0,0 +1,39 @@
+import { cloneDeep } from 'lodash';
+
+const version = 65;
+
+/**
+ * Removes metaMetricsSendCount from MetaMetrics controller
+ */
+export default {
+  version,
+  async migrate(originalVersionedData) {
+    const versionedData = cloneDeep(originalVersionedData);
+    versionedData.meta.version = version;
+    const state = versionedData.data;
+    const newState = transformState(state);
+    versionedData.data = newState;
+    return versionedData;
+  },
+};
+
+function transformState(state) {
+  if (state.PreferencesController) {
+    const {
+      completedOnboarding,
+      firstTimeFlowType,
+    } = state.PreferencesController;
+    state.OnboardingController = state.OnboardingController ?? {};
+
+    if (completedOnboarding !== undefined) {
+      state.OnboardingController.completedOnboarding = completedOnboarding;
+      delete state.PreferencesController.completedOnboarding;
+    }
+    if (firstTimeFlowType !== undefined) {
+      state.OnboardingController.firstTimeFlowType = firstTimeFlowType;
+      delete state.PreferencesController.firstTimeFlowType;
+    }
+  }
+
+  return state;
+}

app/scripts/migrations/065.test.js

@@ -0,0 +1,145 @@
+import migration65 from './065';
+
+describe('migration #65', () => {
+  it('should update the version metadata', async () => {
+    const oldStorage = {
+      meta: {
+        version: 64,
+      },
+      data: {},
+    };
+
+    const newStorage = await migration65.migrate(oldStorage);
+    expect(newStorage.meta).toStrictEqual({
+      version: 65,
+    });
+  });
+
+  it('should move completedOnboarding from PreferencesController to OnboardingController when completedOnboarding is true', async () => {
+    const oldStorage = {
+      meta: {},
+      data: {
+        PreferencesController: {
+          completedOnboarding: true,
+          bar: 'baz',
+        },
+        OnboardingController: {
+          foo: 'bar',
+        },
+      },
+    };
+
+    const newStorage = await migration65.migrate(oldStorage);
+    expect(newStorage.data).toStrictEqual({
+      PreferencesController: {
+        bar: 'baz',
+      },
+      OnboardingController: {
+        completedOnboarding: true,
+        foo: 'bar',
+      },
+    });
+  });
+
+  it('should move completedOnboarding from PreferencesController to OnboardingController when completedOnboarding is false', async () => {
+    const oldStorage = {
+      meta: {},
+      data: {
+        PreferencesController: {
+          completedOnboarding: false,
+          bar: 'baz',
+        },
+        OnboardingController: {
+          foo: 'bar',
+        },
+      },
+    };
+
+    const newStorage = await migration65.migrate(oldStorage);
+    expect(newStorage.data).toStrictEqual({
+      PreferencesController: {
+        bar: 'baz',
+      },
+      OnboardingController: {
+        completedOnboarding: false,
+        foo: 'bar',
+      },
+    });
+  });
+
+  it('should move firstTimeFlowType from PreferencesController to OnboardingController when firstTimeFlowType is truthy', async () => {
+    const oldStorage = {
+      meta: {},
+      data: {
+        PreferencesController: {
+          firstTimeFlowType: 'create',
+          bar: 'baz',
+        },
+        OnboardingController: {
+          foo: 'bar',
+        },
+      },
+    };
+
+    const newStorage = await migration65.migrate(oldStorage);
+    expect(newStorage.data).toStrictEqual({
+      PreferencesController: {
+        bar: 'baz',
+      },
+      OnboardingController: {
+        firstTimeFlowType: 'create',
+        foo: 'bar',
+      },
+    });
+  });
+
+  it('should move firstTimeFlowType from PreferencesController to OnboardingController when firstTimeFlowType is falsy', async () => {
+    const oldStorage = {
+      meta: {},
+      data: {
+        PreferencesController: {
+          firstTimeFlowType: null,
+          bar: 'baz',
+        },
+        OnboardingController: {
+          foo: 'bar',
+        },
+      },
+    };
+
+    const newStorage = await migration65.migrate(oldStorage);
+    expect(newStorage.data).toStrictEqual({
+      PreferencesController: {
+        bar: 'baz',
+      },
+      OnboardingController: {
+        firstTimeFlowType: null,
+        foo: 'bar',
+      },
+    });
+  });
+
+  it('should not modify PreferencesController or OnboardingController when completedOnboarding and firstTimeFlowType are undefined', async () => {
+    const oldStorage = {
+      meta: {},
+      data: {
+        PreferencesController: {
+          bar: 'baz',
+        },
+        OnboardingController: {
+          foo: 'bar',
+        },
+      },
+    };
+
+    const newStorage = await migration65.migrate(oldStorage);
+    expect(newStorage.data).toStrictEqual({
+      PreferencesController: {
+        bar: 'baz',
+      },
+      OnboardingController: {
+        foo: 'bar',
+      },
+    });
+  });
+});

app/scripts/migrations/index.js

@@ -68,6 +68,7 @@ import m061 from './061';
 import m062 from './062';
 import m063 from './063';
 import m064 from './064';
+import m065 from './065';
 
 const migrations = [
   m002,
@@ -133,6 +134,7 @@ const migrations = [
   m062,
   m063,
   m064,
+  m065,
 ];
 
 export default migrations;

ui/pages/onboarding-flow/creation-successful/creation-successful.js

@@ -1,5 +1,6 @@
 import React from 'react';
 import { useHistory } from 'react-router-dom';
+import { useDispatch, useSelector } from 'react-redux';
 import Box from '../../../components/ui/box';
 import Typography from '../../../components/ui/typography';
 import Button from '../../../components/ui/button';
@@ -13,10 +14,31 @@ import {
   ONBOARDING_PIN_EXTENSION_ROUTE,
   ONBOARDING_PRIVACY_SETTINGS_ROUTE,
 } from '../../../helpers/constants/routes';
+import { setCompletedOnboarding } from '../../../store/actions';
+import { useMetricEvent } from '../../../hooks/useMetricEvent';
+import { getFirstTimeFlowType } from '../../../selectors';
 
 export default function CreationSuccessful() {
+  const firstTimeFlowTypeNameMap = {
+    create: 'New Wallet Created',
+    import: 'New Wallet Imported',
+  };
   const history = useHistory();
   const t = useI18nContext();
+  const dispatch = useDispatch();
+  const firstTimeFlowType = useSelector(getFirstTimeFlowType);
+
+  const onboardingCompletedEvent = useMetricEvent({
+    category: 'Onboarding',
+    action: 'Onboarding Complete',
+    name: firstTimeFlowTypeNameMap[firstTimeFlowType],
+  });
+
+  const onComplete = async () => {
+    await dispatch(setCompletedOnboarding());
+    onboardingCompletedEvent();
+    history.push(ONBOARDING_PIN_EXTENSION_ROUTE);
+  };
   return (
     <div className="creation-successful">
       <Box textAlign={TEXT_ALIGN.CENTER} margin={6}>
@@ -74,12 +96,7 @@ export default function CreationSuccessful() {
         >
           {t('setAdvancedPrivacySettings')}
         </Button>
-        <Button
-          type="primary"
-          large
-          rounded
-          onClick={() => history.push(ONBOARDING_PIN_EXTENSION_ROUTE)}
-        >
+        <Button type="primary" large rounded onClick={onComplete}>
           {t('done')}
         </Button>
       </Box>

ui/pages/onboarding-flow/creation-successful/creation-successful.test.js

@@ -1,14 +1,30 @@
 import React from 'react';
 import { fireEvent } from '@testing-library/react';
 import reactRouterDom from 'react-router-dom';
+import configureMockStore from 'redux-mock-store';
+import thunk from 'redux-thunk';
+import { ONBOARDING_PRIVACY_SETTINGS_ROUTE } from '../../../helpers/constants/routes';
 import {
-  ONBOARDING_PIN_EXTENSION_ROUTE,
-  ONBOARDING_PRIVACY_SETTINGS_ROUTE,
-} from '../../../helpers/constants/routes';
-import { renderWithProvider } from '../../../../test/jest';
+  renderWithProvider,
+  setBackgroundConnection,
+} from '../../../../test/jest';
 import CreationSuccessful from './creation-successful';
 
+const completeOnboardingStub = jest
+  .fn()
+  .mockImplementation(() => Promise.resolve());
+
 describe('Creation Successful Onboarding View', () => {
+  const mockStore = {
+    metamask: {
+      provider: {
+        type: 'test',
+      },
+    },
+  };
+  const store = configureMockStore([thunk])(mockStore);
+  setBackgroundConnection({ completeOnboarding: completeOnboardingStub });
+
   const pushMock = jest.fn();
   beforeAll(() => {
     jest
@@ -17,15 +33,15 @@ describe('Creation Successful Onboarding View', () => {
       .mockReturnValue({ push: pushMock });
   });
 
-  it('should redirect to pin-extension view when "Done" button is clicked', () => {
-    const { getByText } = renderWithProvider(<CreationSuccessful />);
+  it('should call completeOnboarding in the background when "Done" button is clicked', () => {
+    const { getByText } = renderWithProvider(<CreationSuccessful />, store);
     const doneButton = getByText('Done');
     fireEvent.click(doneButton);
-    expect(pushMock).toHaveBeenCalledWith(ONBOARDING_PIN_EXTENSION_ROUTE);
+    expect(completeOnboardingStub).toHaveBeenCalledTimes(1);
   });
 
   it('should redirect to privacy-settings view when "Set advanced privacy settings" button is clicked', () => {
-    const { getByText } = renderWithProvider(<CreationSuccessful />);
+    const { getByText } = renderWithProvider(<CreationSuccessful />, store);
     const privacySettingsButton = getByText('Set advanced privacy settings');
     fireEvent.click(privacySettingsButton);
     expect(pushMock).toHaveBeenCalledWith(ONBOARDING_PRIVACY_SETTINGS_ROUTE);

ui/selectors/first-time-flow.js

@@ -25,6 +25,10 @@ export function getFirstTimeFlowTypeRoute(state) {
   return nextRoute;
 }
 
+export const getFirstTimeFlowType = (state) => {
+  return state.metamask.firstTimeFlowType;
+};
+
 export const getOnboardingInitiator = (state) => {
   const { onboardingTabs } = state.metamask;