From 71f91568db64b0df54b4da2384e77ccaa4445499 Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Fri, 15 Oct 2021 13:52:52 -0500 Subject: [PATCH] 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 | 36 +++-- app/scripts/controllers/preferences.js | 21 --- app/scripts/metamask-controller.js | 34 ++-- app/scripts/migrations/065.js | 39 +++++ app/scripts/migrations/065.test.js | 145 ++++++++++++++++++ app/scripts/migrations/index.js | 2 + .../creation-successful.js | 29 +++- .../creation-successful.test.js | 32 +++- ui/selectors/first-time-flow.js | 4 + 9 files changed, 271 insertions(+), 71 deletions(-) create mode 100644 app/scripts/migrations/065.js create mode 100644 app/scripts/migrations/065.test.js diff --git a/app/scripts/controllers/onboarding.js b/app/scripts/controllers/onboarding.js index 5e55d0e0f..25ae2968a 100644 --- a/app/scripts/controllers/onboarding.js +++ b/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 * diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 60607d602..7f5d59ba6 100644 --- a/app/scripts/controllers/preferences.js +++ b/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 diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 58c379e34..f28c16bf3 100644 --- a/app/scripts/metamask-controller.js +++ b/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. diff --git a/app/scripts/migrations/065.js b/app/scripts/migrations/065.js new file mode 100644 index 000000000..96e9820e1 --- /dev/null +++ b/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; +} diff --git a/app/scripts/migrations/065.test.js b/app/scripts/migrations/065.test.js new file mode 100644 index 000000000..712e90e49 --- /dev/null +++ b/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', + }, + }); + }); +}); diff --git a/app/scripts/migrations/index.js b/app/scripts/migrations/index.js index 8798a0529..6ca02a881 100644 --- a/app/scripts/migrations/index.js +++ b/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; diff --git a/ui/pages/onboarding-flow/creation-successful/creation-successful.js b/ui/pages/onboarding-flow/creation-successful/creation-successful.js index 1be074057..9701b6fd4 100644 --- a/ui/pages/onboarding-flow/creation-successful/creation-successful.js +++ b/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 (
@@ -74,12 +96,7 @@ export default function CreationSuccessful() { > {t('setAdvancedPrivacySettings')} - diff --git a/ui/pages/onboarding-flow/creation-successful/creation-successful.test.js b/ui/pages/onboarding-flow/creation-successful/creation-successful.test.js index 6d828cab9..f2277f4e6 100644 --- a/ui/pages/onboarding-flow/creation-successful/creation-successful.test.js +++ b/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(); + it('should call completeOnboarding in the background when "Done" button is clicked', () => { + const { getByText } = renderWithProvider(, 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(); + const { getByText } = renderWithProvider(, store); const privacySettingsButton = getByText('Set advanced privacy settings'); fireEvent.click(privacySettingsButton); expect(pushMock).toHaveBeenCalledWith(ONBOARDING_PRIVACY_SETTINGS_ROUTE); diff --git a/ui/selectors/first-time-flow.js b/ui/selectors/first-time-flow.js index bbebfbd3a..623e1d01b 100644 --- a/ui/selectors/first-time-flow.js +++ b/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;