diff --git a/packages/analytics-controller/CHANGELOG.md b/packages/analytics-controller/CHANGELOG.md index 88710f6ee0f..72f96dc069c 100644 --- a/packages/analytics-controller/CHANGELOG.md +++ b/packages/analytics-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Mark `analyticsId` as persisted (`persist: true`) in `AnalyticsController` state metadata so it is saved and restored with `optedIn` when using a persisted controller composition ([#8542](https://github.com/MetaMask/core/pull/8542)) - Bump `@metamask/messenger` from `^1.0.0` to `^1.1.1` ([#8364](https://github.com/MetaMask/core/pull/8364), [#8373](https://github.com/MetaMask/core/pull/8373)) - Bump `@metamask/base-controller` from `^9.0.1` to `^9.1.0` ([#8457](https://github.com/MetaMask/core/pull/8457)) diff --git a/packages/analytics-controller/README.md b/packages/analytics-controller/README.md index f4979175b7c..d60ec3f792f 100644 --- a/packages/analytics-controller/README.md +++ b/packages/analytics-controller/README.md @@ -14,36 +14,13 @@ or The AnalyticsController provides a unified interface for tracking analytics events, identifying users, and managing analytics preferences. It delegates client platform-specific implementation to an `AnalyticsPlatformAdapter` and integrates with the MetaMask messenger system for inter-controller communication. -## Client Platform-Managed Storage - -> [!NOTE] -> "Client platform" means mobile or extension - -The controller does not persist state internally. The client platform is responsible for loading and persisting analytics settings. This design enables: - -- **Early access**: The client platform can read the `analyticsId` before the controller is initialized, useful for other controllers or early startup code -- **Resilience**: Storing analytics settings separately from main state protects them from state corruption, allowing analytics to continue working even when main state is corrupted - -Load settings from storage **before** initializing the controller, then subscribe to `AnalyticsController:stateChange` events to persist any state changes. - ## State | Field | Type | Description | Persisted | | ------------- | --------- | --------------------------------------------- | --------- | -| `analyticsId` | `string` | UUIDv4 identifier (client platform-generated) | No | +| `analyticsId` | `string` | UUIDv4 identifier (client platform-generated) | Yes | | `optedIn` | `boolean` | User opt-in status | Yes | -### Why `analyticsId` Has No Default - -The `analyticsId` uniquely identifies the user. If the controller generated a new ID on each boot, the ID would be ineffective. The client platform must generate a UUID on first run, persist it, and provide it to the controller constructor. - -### Client Platform Responsibilities - -1. **Generate UUID on first run**: Use `uuid` package or client platform equivalent -2. **Load state before controller init**: Read from storage, provide to constructor -3. **Subscribe to state changes**: Persist changes to isolated storage -4. **Persist to isolated storage**: Keep analytics settings separate from main state (protects against state corruption) - ## Anonymous Events Feature When `isAnonymousEventsFeatureEnabled` is enabled in the constructor, events with sensitive properties are split into separate events: diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index e6c9a43a32b..a8bea6b3ea8 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -1,3 +1,4 @@ +import { deriveStateFromMetadata } from '@metamask/base-controller'; import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger'; import type { MockAnyNamespace } from '@metamask/messenger'; @@ -148,6 +149,88 @@ describe('AnalyticsController', () => { }); }); + describe('metadata', () => { + const metadataFixtureState: AnalyticsControllerState = { + optedIn: true, + analyticsId: '6ba7b810-9dad-41d4-80b5-0c4f5a7c1e2d', + }; + + it('includes expected state in debug snapshots', async () => { + const { controller } = await setupController({ + state: metadataFixtureState, + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInDebugSnapshot', + ), + ).toMatchInlineSnapshot(` + { + "analyticsId": "6ba7b810-9dad-41d4-80b5-0c4f5a7c1e2d", + "optedIn": true, + } + `); + }); + + it('includes expected state in state logs', async () => { + const { controller } = await setupController({ + state: metadataFixtureState, + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'includeInStateLogs', + ), + ).toMatchInlineSnapshot(` + { + "analyticsId": "6ba7b810-9dad-41d4-80b5-0c4f5a7c1e2d", + "optedIn": true, + } + `); + }); + + it('persists expected state', async () => { + const { controller } = await setupController({ + state: metadataFixtureState, + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'persist', + ), + ).toMatchInlineSnapshot(` + { + "analyticsId": "6ba7b810-9dad-41d4-80b5-0c4f5a7c1e2d", + "optedIn": true, + } + `); + }); + + it('exposes expected state to UI', async () => { + const { controller } = await setupController({ + state: metadataFixtureState, + }); + + expect( + deriveStateFromMetadata( + controller.state, + controller.metadata, + 'usedInUi', + ), + ).toMatchInlineSnapshot(` + { + "optedIn": true, + } + `); + }); + }); + describe('isValidUUIDv4', () => { it('returns true for valid UUIDv4', () => { expect(isValidUUIDv4('550e8400-e29b-41d4-a716-446655440000')).toBe(true); diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index 352b1702475..7270d16caa8 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -65,10 +65,8 @@ export function getDefaultAnalyticsControllerState(): Omit< /** * The metadata for each property in {@link AnalyticsControllerState}. * - * Note: `optedIn` is persisted by the controller (`persist: true`). - * `analyticsId` is persisted by the platform (`persist: false`) and provided - * via initial state. The platform should subscribe to `stateChange` events - * to persist any state changes. + * Both `optedIn` and `analyticsId` are persisted (`persist: true`). + * The platform must supply a valid UUIDv4 `analyticsId` on first run. */ const analyticsControllerMetadata = { optedIn: { @@ -79,7 +77,7 @@ const analyticsControllerMetadata = { }, analyticsId: { includeInStateLogs: true, - persist: false, + persist: true, includeInDebugSnapshot: true, usedInUi: false, }, @@ -151,7 +149,8 @@ export type AnalyticsControllerMessenger = Messenger< export type AnalyticsControllerOptions = { /** * Initial controller state. Must include a valid UUIDv4 `analyticsId`. - * The platform is responsible for generating and persisting the analyticsId. + * The platform is responsible for generating the ID on first run. + * It is then persisted with controller state when using a persisted store. */ state: AnalyticsControllerState; /** @@ -181,9 +180,8 @@ export type AnalyticsControllerOptions = { * messenger system to allow other controllers and components to track analytics events. * It delegates platform-specific implementation to an {@link AnalyticsPlatformAdapter}. * - * Note: The controller persists `optedIn` internally. The `analyticsId` is persisted - * by the platform and must be provided via initial state. The platform should subscribe - * to `AnalyticsController:stateChange` events to persist any state changes. + * The controller persists `optedIn` and `analyticsId` when composed with a persisted + * store. The platform must supply a valid `analyticsId` on first launch. */ export class AnalyticsController extends BaseController< 'AnalyticsController',