chore(runway): cherry-pick fix: persist completedOnboarding state value (#15789)

- fix: cp-7.47.0 persist completedOnboarding state value (#15754)

Author: runway-github[bot]

app/store/index.ts

@@ -15,6 +15,7 @@ import getUIStartupSpan from '../core/Performance/UIStartup';
 import ReduxService, { ReduxStore } from '../core/redux';
 import { onPersistedDataLoaded } from '../actions/user';
 import { toggleBasicFunctionality } from '../actions/settings';
+import { setCompletedOnboarding } from '../actions/onboarding';
 
 // TODO: Improve type safety by using real Action types instead of `AnyAction`
 const pReducer = persistReducer<RootState, AnyAction>(
@@ -69,8 +70,22 @@ const createStoreAndPersistor = async () => {
     endTrace({ name: TraceName.StoreInit });
     // Signal that persisted data has been loaded
     store.dispatch(onPersistedDataLoaded());
+
+    const currentState = store.getState();
+
     // This sets the basic functionality value from the persisted state when the app is restarted
-    store.dispatch(toggleBasicFunctionality(store.getState().settings.basicFunctionalityEnabled));
+    store.dispatch(
+      toggleBasicFunctionality(currentState.settings.basicFunctionalityEnabled),
+    );
+
+    // This sets the completedOnboarding value based on the KeyringController state
+    // This cannot be done in a migration because `state.onboarding` was previously blacklisted in `persistConfig`
+    if (
+      !currentState.onboarding.completedOnboarding &&
+      Boolean(currentState.engine.backgroundState.KeyringController.vault)
+    ) {
+      store.dispatch(setCompletedOnboarding(true));
+    }
   };
 
   persistor = persistStore(store, null, onPersistComplete);

app/store/migrations/071.test.ts

@@ -14,75 +14,7 @@ describe('Migration 071: Set completedOnboarding based on the KeyringController
     jest.resetAllMocks();
   });
 
-  const invalidStates = [
-    {
-      state: null,
-      scenario: 'state is invalid',
-    },
-    {
-      state: merge({}, initialRootState, {
-        engine: null,
-      }),
-      scenario: 'engine state is invalid',
-    },
-    {
-      state: merge({}, initialRootState, {
-        engine: {
-          backgroundState: null,
-        },
-      }),
-      scenario: 'backgroundState is invalid',
-    },
-  ];
-
-  it.each(invalidStates)('captures exception if %s', async ({ state }) => {
-    const newState = await migrate(state);
-
-    expect(newState).toStrictEqual(state);
-    expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error));
-  });
-
-  it('sets completedOnboarding to true if vault exists', () => {
-    const state = merge({}, initialRootState, {
-      onboarding: {
-        completedOnboarding: false,
-      },
-      engine: {
-        backgroundState: {
-          KeyringController: {
-            vault: true,
-          },
-        },
-      },
-    });
-
-    migrate(state);
-
-    expect(state.onboarding.completedOnboarding).toBe(true);
-    expect(mockedCaptureException).not.toHaveBeenCalled();
-  });
-
-  it('sets completedOnboarding to false if vault does not exist', () => {
-    const state = merge({}, initialRootState, {
-      onboarding: {
-        completedOnboarding: false,
-      },
-      engine: {
-        backgroundState: {
-          KeyringController: {
-            vault: false,
-          },
-        },
-      },
-    });
-
-    migrate(state);
-
-    expect(state.onboarding.completedOnboarding).toBe(false);
-    expect(mockedCaptureException).not.toHaveBeenCalled();
-  });
-
-  it('adds the completedOnboarding property if it does not exist', () => {
+  it('returns the initial state', () => {
     const state = merge({}, initialRootState, {
       onboarding: {},
       engine: {
@@ -94,9 +26,9 @@ describe('Migration 071: Set completedOnboarding based on the KeyringController
       },
     });
 
-    migrate(state);
+    const migratedState = migrate(state);
 
-    expect(state.onboarding.completedOnboarding).toBe(true);
+    expect(migratedState).toStrictEqual(state);
     expect(mockedCaptureException).not.toHaveBeenCalled();
   });
 });

app/store/migrations/071.ts

@@ -1,40 +1,9 @@
-import { hasProperty, isObject } from '@metamask/utils';
-import { ensureValidState } from './util';
-import { captureException } from '@sentry/react-native';
-
 /**
  * Migration 71: set completedOnboarding based on the state of the KeyringController.
+ *
+ * This migration ended up never being useful, since `onboarding` was blacklisted in `persistConfig`.
+ * We're instead applying the original logic in `onPersistComplete` in `app/store/index.ts`.
  */
-const migration = (state: unknown): unknown => {
-  const migrationVersion = 71;
-
-  // Ensure the state is valid for migration
-  if (!ensureValidState(state, migrationVersion)) {
-    return state;
-  }
-
-  try {
-    const KeyringController = state.engine.backgroundState.KeyringController;
-
-    if (
-      KeyringController &&
-      hasProperty(KeyringController, 'vault') &&
-      hasProperty(state, 'onboarding') &&
-      isObject(state.onboarding)
-    ) {
-      const { vault } = KeyringController;
-      state.onboarding.completedOnboarding = Boolean(vault);
-    }
-
-    return state;
-  } catch (error) {
-    captureException(
-      new Error(
-        `Migration 071: setting completedOnboarding failed with error: ${error}`,
-      ),
-    );
-    return state;
-  }
-};
+const migration = (state: unknown): unknown => state;
 
 export default migration;

app/store/persistConfig.test.ts

@@ -113,7 +113,6 @@ describe('persistConfig', () => {
       expect(persistConfig.version).toBe(version);
       expect(persistConfig.timeout).toBe(40000);
       expect(persistConfig.blacklist).toEqual([
-        'onboarding',
         'rpcEvents',
         'accounts',
         'confirmationMetrics',
@@ -232,7 +231,7 @@ describe('persistConfig', () => {
 
   describe('transforms', () => {
     it('have engine transform configured', () => {
-      expect(persistConfig.transforms).toHaveLength(2);
+      expect(persistConfig.transforms).toHaveLength(3);
       const engineTransform = persistConfig.transforms[0] as Transform<
         unknown,
         unknown
@@ -248,6 +247,14 @@ describe('persistConfig', () => {
       expect(userTransform.whitelist).toEqual(['user']);
     });
 
+    it('has onboarding transform configured', () => {
+      const onboardingTransform = persistConfig.transforms[2] as Transform<
+        unknown,
+        unknown
+      > & { whitelist?: string[] };
+      expect(onboardingTransform.whitelist).toEqual(['onboarding']);
+    });
+
     describe('persistTransform', () => {
       const engineTransform = persistConfig.transforms[0] as Transform<
         unknown,

app/store/persistConfig.ts

@@ -130,12 +130,26 @@ const persistUserTransform = createTransform(
   { whitelist: ['user'] },
 );
 
+const persistOnboardingTransform = createTransform(
+  (inboundState: RootState['onboarding']) => {
+    const { events, ...state } = inboundState;
+    // Reconstruct data to persist
+    return state;
+  },
+  null,
+  { whitelist: ['onboarding'] },
+);
+
 const persistConfig = {
   key: 'root',
   version,
-  blacklist: ['onboarding', 'rpcEvents', 'accounts', 'confirmationMetrics'],
+  blacklist: ['rpcEvents', 'accounts', 'confirmationMetrics'],
   storage: MigratedStorage,
-  transforms: [persistTransform, persistUserTransform],
+  transforms: [
+    persistTransform,
+    persistUserTransform,
+    persistOnboardingTransform,
+  ],
   stateReconciler: autoMergeLevel2, // see "Merge Process" section for details.
   migrate: createMigrate(migrations, { debug: false }),
   timeout: TIMEOUT,