From cb9ef22e97dd270f8a9c1e3f5816e403e9bf86c1 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Thu, 6 Nov 2025 21:48:49 +0100 Subject: [PATCH 01/20] feat(analytics-controller): add onSetupCompleted lifecycle hook - Add onSetupCompleted lifecycle hook to AnalyticsPlatformAdapter interface - Remove add method from interface (breaking change) - Change AnalyticsPlatformAdapter from type alias to interface - Add UUIDv4 validation for analyticsId with error handling - Add AnalyticsUserTraits type for better semantic separation - Rename trackEvent to track and trackPage to view in adapter interface - Update controller to call onSetupCompleted after initialization - Add comprehensive tests for lifecycle hook and validation - Update documentation with lifecycle hook usage examples --- packages/analytics-controller/README.md | 62 +++++- ...AnalyticsController-method-action-types.ts | 12 +- .../src/AnalyticsController.test.ts | 178 ++++++++++++++++-- .../src/AnalyticsController.ts | 37 +++- .../src/AnalyticsPlatformAdapter.types.ts | 59 ++++-- packages/analytics-controller/src/index.ts | 3 +- 6 files changed, 300 insertions(+), 51 deletions(-) diff --git a/packages/analytics-controller/README.md b/packages/analytics-controller/README.md index 241043d5a0a..b87d4d08e48 100644 --- a/packages/analytics-controller/README.md +++ b/packages/analytics-controller/README.md @@ -30,15 +30,24 @@ The controller delegates platform-specific analytics implementation to a `Analyt import type { AnalyticsPlatformAdapter } from '@metamask/analytics-controller'; const platformAdapter: AnalyticsPlatformAdapter = { - trackEvent: (eventName: string, properties: Record) => { + track: (eventName: string, properties: Record) => { // Platform-specific implementation (e.g., Segment, Mixpanel, etc.) segment.track(eventName, properties); }, identify: (userId: string, traits?: Record) => { segment.identify(userId, traits); }, - trackPage: (pageName: string, properties?: Record) => { - segment.page(pageName, properties); + view: (name: string, properties?: Record) => { + segment.page(name, properties); + }, + onSetupCompleted: (analyticsId: string) => { + // Lifecycle hook called after controller initialization + // The analyticsId is guaranteed to be set when this method is called + // Use this for platform-specific setup that requires the analytics ID + // For example, adding plugins that need the analytics ID: + segment.add({ + plugin: new PrivacyPlugin(analyticsId), + }); }, }; ``` @@ -112,7 +121,7 @@ console.log(controller.state.analyticsId); // '550e8400-e29b-41d4-a716-446655440 ### 5. Track Page Views ```typescript -controller.trackPage('home', { +controller.trackView('home', { referrer: 'google', campaign: 'summer-2024', }); @@ -193,6 +202,51 @@ const defaultState = getDefaultAnalyticsControllerState(); **Analytics ID:** The `analyticsId` is a UUIDv4 string. If not provided in the `state` parameter, the controller automatically generates one on initialization. This ID is persisted in state and remains consistent across restarts. If you provide an `analyticsId` in the `state` parameter, it will be used instead (useful for migrations). +## Lifecycle Hooks + +### `onSetupCompleted` + +The `onSetupCompleted` lifecycle hook is called once after the `AnalyticsController` is fully initialized. This hook allows platform-specific adapters to perform setup that requires access to the controller's state (e.g., `analyticsId`). + +**When it's called:** +- After the controller is fully initialized +- Only when `analyticsId` is set in controller state (the presence of `analyticsId` is the definition of "completed" setup) +- The `analyticsId` parameter is guaranteed to be set and be a valid UUIDv4 when this method is called + +**What it's used for:** + +Platform-specific setup that requires access to adapter implementation like adding plugins or middleware that need the `analyticsId` +Any initialization that depends on the controller being ready + +**Example usage:** + +```typescript +const platformAdapter: AnalyticsPlatformAdapter = { + // ... other methods ... + onSetupCompleted: (analyticsId: string) => { + // Add platform-specific plugins that require analyticsId + client.add({ + plugin: new PrivacyPlugin(analyticsId), + }); + }, +}; +``` + +**Error handling:** +- Errors thrown in `onSetupCompleted` are caught and logged + +**Best practices:** +- Use `onSetupCompleted` for setup that requires controller state +- Keep setup logic minimal and focused +- Handle errors gracefully within the hook +- If you don't need setup, provide a no-op implementation: + +```typescript +onSetupCompleted: (_analyticsId: string) => { + // No-op: this adapter doesn't need setup +}, +``` + ## Debugging To display analytics-controller logs in the mobile app, you can add the following to your `.js.env` file: diff --git a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts index 5a59d78c718..e589311fbbb 100644 --- a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts +++ b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts @@ -32,12 +32,12 @@ export type AnalyticsControllerIdentifyAction = { /** * Track a page view. * - * @param pageName - The name of the page - * @param properties - Page properties + * @param name - The name of the UI item being viewed (pages for web, screen for mobile) + * @param properties - UI item properties */ -export type AnalyticsControllerTrackPageAction = { - type: `AnalyticsController:trackPage`; - handler: AnalyticsController['trackPage']; +export type AnalyticsControllerTrackViewAction = { + type: `AnalyticsController:trackView`; + handler: AnalyticsController['trackView']; }; /** @@ -78,7 +78,7 @@ export type AnalyticsControllerOptOutAction = { export type AnalyticsControllerMethodActions = | AnalyticsControllerTrackEventAction | AnalyticsControllerIdentifyAction - | AnalyticsControllerTrackPageAction + | AnalyticsControllerTrackViewAction | AnalyticsControllerEnableAction | AnalyticsControllerDisableAction | AnalyticsControllerOptInAction diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index 131fe84af43..fcd36ea91a4 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -40,9 +40,10 @@ function setupController( const adapter = platformAdapter ?? ({ - trackEvent: jest.fn(), + track: jest.fn(), identify: jest.fn(), - trackPage: jest.fn(), + view: jest.fn(), + onSetupCompleted: jest.fn(), } satisfies AnalyticsPlatformAdapter); const rootMessenger = new Messenger< @@ -85,9 +86,10 @@ function isValidUUIDv4(value: string): boolean { describe('AnalyticsController', () => { const createMockAdapter = (): AnalyticsPlatformAdapter => ({ - trackEvent: jest.fn(), + track: jest.fn(), identify: jest.fn(), - trackPage: jest.fn(), + view: jest.fn(), + onSetupCompleted: jest.fn(), }); describe('constructor', () => { @@ -231,6 +233,150 @@ describe('AnalyticsController', () => { }); }); + describe('onSetupCompleted lifecycle hook', () => { + it('calls onSetupCompleted after initialization when analyticsId is set', () => { + const mockAdapter = createMockAdapter(); + const customId = '550e8400-e29b-41d4-a716-446655440000'; + + setupController({ + state: { analyticsId: customId }, + platformAdapter: mockAdapter, + }); + + expect(mockAdapter.onSetupCompleted).toHaveBeenCalledTimes(1); + expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith(customId); + }); + + it('calls onSetupCompleted with auto-generated analyticsId when not provided', () => { + const mockAdapter = createMockAdapter(); + + const { controller } = setupController({ + platformAdapter: mockAdapter, + }); + + expect(mockAdapter.onSetupCompleted).toHaveBeenCalledTimes(1); + expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith( + controller.state.analyticsId, + ); + expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); + }); + + it('throws error when analyticsId is empty string', () => { + const mockAdapter = createMockAdapter(); + + expect(() => { + setupController({ + state: { analyticsId: '' }, + platformAdapter: mockAdapter, + }); + }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got ""'); + + expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); + }); + + it('throws error when analyticsId is undefined', () => { + const mockAdapter = createMockAdapter(); + + expect(() => { + setupController({ + state: { analyticsId: undefined as unknown as string }, + platformAdapter: mockAdapter, + }); + }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got undefined'); + + expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); + }); + + it('throws error when analyticsId is null', () => { + const mockAdapter = createMockAdapter(); + + expect(() => { + setupController({ + state: { analyticsId: null as unknown as string }, + platformAdapter: mockAdapter, + }); + }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got null'); + + expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); + }); + + it('throws error when analyticsId is not a valid UUIDv4', () => { + const mockAdapter = createMockAdapter(); + + expect(() => { + setupController({ + state: { analyticsId: 'not-a-uuid' }, + platformAdapter: mockAdapter, + }); + }).toThrow( + 'Invalid analyticsId: expected a valid UUIDv4, but got "not-a-uuid"', + ); + + expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); + }); + + it('throws error when analyticsId is a valid UUID but not v4', () => { + const mockAdapter = createMockAdapter(); + // UUIDv1 example + const uuidV1 = '6ba7b810-9dad-11d1-80b4-00c04fd430c8'; + + expect(() => { + setupController({ + state: { analyticsId: uuidV1 }, + platformAdapter: mockAdapter, + }); + }).toThrow( + `Invalid analyticsId: expected a valid UUIDv4, but got "${uuidV1}"`, + ); + + expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); + }); + + it('handles errors in onSetupCompleted without breaking controller initialization', () => { + const mockAdapter = createMockAdapter(); + const error = new Error('Setup failed'); + mockAdapter.onSetupCompleted = jest.fn(() => { + throw error; + }); + + const projectLoggerSpy = jest + .spyOn(require('./AnalyticsLogger'), 'projectLogger') + .mockImplementation(); + + const { controller } = setupController({ + state: { analyticsId: '550e8400-e29b-41d4-a716-446655440000' }, + platformAdapter: mockAdapter, + }); + + // Controller should still be initialized + expect(controller).toBeDefined(); + expect(controller.state.analyticsId).toBe( + '550e8400-e29b-41d4-a716-446655440000', + ); + + // Error should be logged + expect(projectLoggerSpy).toHaveBeenCalledWith( + 'Error calling platformAdapter.onSetupCompleted', + error, + ); + + projectLoggerSpy.mockRestore(); + }); + + it('calls onSetupCompleted with the correct analyticsId from state', () => { + const mockAdapter = createMockAdapter(); + const customId = '550e8400-e29b-41d4-a716-446655440000'; + + const { controller } = setupController({ + state: { analyticsId: customId }, + platformAdapter: mockAdapter, + }); + + expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith(customId); + expect(controller.state.analyticsId).toBe(customId); + }); + }); + describe('trackEvent', () => { it('tracks event when enabled', () => { const mockAdapter = createMockAdapter(); @@ -238,7 +384,7 @@ describe('AnalyticsController', () => { controller.trackEvent('test_event', { prop: 'value' }); - expect(mockAdapter.trackEvent).toHaveBeenCalledWith('test_event', { + expect(mockAdapter.track).toHaveBeenCalledWith('test_event', { prop: 'value', }); }); @@ -249,7 +395,7 @@ describe('AnalyticsController', () => { controller.trackEvent('test_event'); - expect(mockAdapter.trackEvent).toHaveBeenCalledWith('test_event', {}); + expect(mockAdapter.track).toHaveBeenCalledWith('test_event', {}); }); it('does not track event when disabled', () => { @@ -261,7 +407,7 @@ describe('AnalyticsController', () => { controller.trackEvent('test_event', { prop: 'value' }); - expect(mockAdapter.trackEvent).not.toHaveBeenCalled(); + expect(mockAdapter.track).not.toHaveBeenCalled(); }); }); @@ -291,14 +437,14 @@ describe('AnalyticsController', () => { }); }); - describe('trackPage', () => { + describe('trackView', () => { it('tracks page view', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ platformAdapter: mockAdapter }); - controller.trackPage('home', { referrer: 'test' }); + controller.trackView('home', { referrer: 'test' }); - expect(mockAdapter.trackPage).toHaveBeenCalledWith('home', { + expect(mockAdapter.view).toHaveBeenCalledWith('home', { referrer: 'test', }); }); @@ -310,9 +456,9 @@ describe('AnalyticsController', () => { platformAdapter: mockAdapter, }); - controller.trackPage('home'); + controller.trackView('home'); - expect(mockAdapter.trackPage).not.toHaveBeenCalled(); + expect(mockAdapter.view).not.toHaveBeenCalled(); }); }); @@ -381,7 +527,7 @@ describe('AnalyticsController', () => { prop: 'value', }); - expect(mockAdapter.trackEvent).toHaveBeenCalled(); + expect(mockAdapter.track).toHaveBeenCalled(); }); it('handles identify action', () => { @@ -398,13 +544,13 @@ describe('AnalyticsController', () => { expect(controller.state.analyticsId).toBe('user-123'); }); - it('handles trackPage action', () => { + it('handles trackView action', () => { const mockAdapter = createMockAdapter(); const { messenger } = setupController({ platformAdapter: mockAdapter }); - messenger.call('AnalyticsController:trackPage', 'home', {}); + messenger.call('AnalyticsController:trackView', 'home', {}); - expect(mockAdapter.trackPage).toHaveBeenCalled(); + expect(mockAdapter.view).toHaveBeenCalled(); }); it('handles enable action', () => { diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index ecac411d3af..e5ddafbd0cb 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -5,13 +5,14 @@ import type { } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; import type { Messenger } from '@metamask/messenger'; -import { v4 as uuidv4 } from 'uuid'; +import { v4 as uuidv4, validate as uuidValidate, version as uuidVersion } from 'uuid'; import type { AnalyticsControllerMethodActions } from './AnalyticsController-method-action-types'; import { projectLogger } from './AnalyticsLogger'; import type { AnalyticsPlatformAdapter, AnalyticsEventProperties, + AnalyticsUserTraits, } from './AnalyticsPlatformAdapter.types'; // === GENERAL === @@ -90,7 +91,7 @@ export function getDefaultAnalyticsControllerState(): AnalyticsControllerState { const MESSENGER_EXPOSED_METHODS = [ 'trackEvent', 'identify', - 'trackPage', + 'trackView', 'enable', 'disable', 'optIn', @@ -211,6 +212,26 @@ export class AnalyticsController extends BaseController< optedIn: this.state.optedIn, analyticsId: this.state.analyticsId, }); + + // Call onSetupCompleted lifecycle hook after initialization + // Only call if analyticsId is set and is a valid UUIDv4 (this is the definition of "completed" setup) + if ( + this.state.analyticsId && + uuidValidate(this.state.analyticsId) && + uuidVersion(this.state.analyticsId) === 4 + ) { + try { + this.#platformAdapter.onSetupCompleted(this.state.analyticsId); + } catch (error) { + // Log error but don't throw - adapter setup failure shouldn't break controller + projectLogger('Error calling platformAdapter.onSetupCompleted', error); + } + } else { + // analyticsId is undefined, null, empty string, or not a valid UUIDv4 + throw new Error( + `Invalid analyticsId: expected a valid UUIDv4, but got ${JSON.stringify(this.state.analyticsId)}`, + ); + } } /** @@ -231,7 +252,7 @@ export class AnalyticsController extends BaseController< } // Delegate to platform adapter - this.#platformAdapter.trackEvent(eventName, properties); + this.#platformAdapter.track(eventName, properties); } /** @@ -240,7 +261,7 @@ export class AnalyticsController extends BaseController< * @param userId - The user identifier (e.g., metametrics ID) * @param traits - User traits/properties */ - identify(userId: string, traits?: AnalyticsEventProperties): void { + identify(userId: string, traits?: AnalyticsUserTraits): void { if (!this.state.enabled) { return; } @@ -262,15 +283,13 @@ export class AnalyticsController extends BaseController< * @param pageName - The name of the page * @param properties - Page properties */ - trackPage(pageName: string, properties?: AnalyticsEventProperties): void { + trackView(name: string, properties?: AnalyticsEventProperties): void { if (!this.state.enabled) { return; } - // Delegate to platform adapter if supported - if (this.#platformAdapter.trackPage) { - this.#platformAdapter.trackPage(pageName, properties); - } + // Delegate to platform adapter + this.#platformAdapter.view(name, properties); } /** diff --git a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts index 29e1cb7b6e7..a7b56aa83ab 100644 --- a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts +++ b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts @@ -5,38 +5,67 @@ import type { Json } from '@metamask/utils'; */ export type AnalyticsEventProperties = Record; +/** + * User traits/properties for analytics identification + */ +export type AnalyticsUserTraits = Record; + /** * Platform adapter interface for analytics tracking * Implementations should handle platform-specific details (Segment SDK, etc.) - * - * @todo This type is work in progress and will be updated as we - * integrate with the new analytics system on mobile. - * We have this draft type to help us iterate on the implementation. - * It will be updated with proper types as we create the mobile adapter - * And the controller package will be released only when this is completed. */ -export type AnalyticsPlatformAdapter = { +export interface AnalyticsPlatformAdapter { /** - * Track an analytics event + * Track an analytics event. + * + * This is the same as trackEvent in the old analytics system * * @param eventName - The name of the event * @param properties - Event properties */ - trackEvent(eventName: string, properties: AnalyticsEventProperties): void; + track(eventName: string, properties: AnalyticsEventProperties): void; /** - * Identify a user + * Identify a user with traits. * * @param userId - The user identifier (e.g., metametrics ID) * @param traits - User traits/properties */ - identify?(userId: string, traits?: AnalyticsEventProperties): void; + identify(userId: string, traits?: AnalyticsUserTraits): void; /** - * Track a page view + * Track a UI unit (page or screen) view depending on the platform + * + * This is the same as page in Segment web SDK and screen in Segment mobile SDK. + * Each platform adapter should implement this method to track UI views + * using the appropriate method for the platform: "pages" for web, "screen" for mobile. * - * @param pageName - The name of the page + * @param name - The name of the UI item being viewed (pages for web, screen for mobile) * @param properties - Page properties */ - trackPage?(pageName: string, properties?: AnalyticsEventProperties): void; -}; + view(name: string, properties?: AnalyticsEventProperties): void; + + /** + * Lifecycle hook called after the AnalyticsController is fully initialized. + * + * This hook allows platform-specific adapters to perform setup that requires + * access to the controller's state (e.g., analyticsId). + * + * The controller calls this method once after initialization, passing the + * analyticsId from controller state. The analyticsId is guaranteed to be set + * when this method is called - this is the definition of "completed" setup. + * + * @param analyticsId - The analytics ID from controller state. Always set (never empty). + * + * @example + * ```typescript + * onSetupCompleted(analyticsId: string): void { + * // Add platform-specific plugins that require analyticsId + * client.add({ + * plugin: new PrivacyPlugin(analyticsId), + * }); + * } + * ``` + */ + onSetupCompleted(analyticsId: string): void; +} diff --git a/packages/analytics-controller/src/index.ts b/packages/analytics-controller/src/index.ts index 54af2dfeb57..02e06243ecc 100644 --- a/packages/analytics-controller/src/index.ts +++ b/packages/analytics-controller/src/index.ts @@ -5,6 +5,7 @@ export type { AnalyticsControllerOptions } from './AnalyticsController'; // Export types export type { AnalyticsEventProperties, + AnalyticsUserTraits, AnalyticsPlatformAdapter, } from './AnalyticsPlatformAdapter.types'; @@ -26,7 +27,7 @@ export type { export type { AnalyticsControllerTrackEventAction, AnalyticsControllerIdentifyAction, - AnalyticsControllerTrackPageAction, + AnalyticsControllerTrackViewAction, AnalyticsControllerEnableAction, AnalyticsControllerDisableAction, AnalyticsControllerOptInAction, From 34e635746b2ce2dbe86b64e303d34114575d00d4 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Fri, 7 Nov 2025 11:49:23 +0100 Subject: [PATCH 02/20] feat(analytics-controller): add isOptedIn action and refactor identify method - Add isOptedIn method and corresponding action type - Refactor identify method to use analyticsId from state instead of userId parameter - Update tests to use realistic traits (ENABLE_OPENSEA_API, NFT_AUTODETECTION) instead of email - Update UUID test example from v1 to v5 - Remove obsolete tests - Fix formatting issues --- ...AnalyticsController-method-action-types.ts | 38 ++++- .../src/AnalyticsController.test.ts | 160 ++++++++---------- .../src/AnalyticsController.ts | 57 +++++-- .../src/AnalyticsPlatformAdapter.types.ts | 12 +- packages/analytics-controller/src/index.ts | 2 + 5 files changed, 156 insertions(+), 113 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts index e589311fbbb..9397bff2f39 100644 --- a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts +++ b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts @@ -21,7 +21,6 @@ export type AnalyticsControllerTrackEventAction = { /** * Identify a user for analytics. * - * @param userId - The user identifier (e.g., metametrics ID) * @param traits - User traits/properties */ export type AnalyticsControllerIdentifyAction = { @@ -33,7 +32,7 @@ export type AnalyticsControllerIdentifyAction = { * Track a page view. * * @param name - The name of the UI item being viewed (pages for web, screen for mobile) - * @param properties - UI item properties + * @param properties - UI item properties */ export type AnalyticsControllerTrackViewAction = { type: `AnalyticsController:trackView`; @@ -72,6 +71,36 @@ export type AnalyticsControllerOptOutAction = { handler: AnalyticsController['optOut']; }; +/** + * Get the analytics ID from the controller state. + * + * @returns The current analytics ID. + */ +export type AnalyticsControllerGetAnalyticsIdAction = { + type: `AnalyticsController:getAnalyticsId`; + handler: AnalyticsController['getAnalyticsId']; +}; + +/** + * Get the enabled status from the controller state. + * + * @returns The current enabled status. + */ +export type AnalyticsControllerIsEnabledAction = { + type: `AnalyticsController:isEnabled`; + handler: AnalyticsController['isEnabled']; +}; + +/** + * Get the opted in status from the controller state. + * + * @returns The current opted in status. + */ +export type AnalyticsControllerIsOptedInAction = { + type: `AnalyticsController:isOptedIn`; + handler: AnalyticsController['isOptedIn']; +}; + /** * Union of all AnalyticsController action types. */ @@ -82,4 +111,7 @@ export type AnalyticsControllerMethodActions = | AnalyticsControllerEnableAction | AnalyticsControllerDisableAction | AnalyticsControllerOptInAction - | AnalyticsControllerOptOutAction; + | AnalyticsControllerOptOutAction + | AnalyticsControllerGetAnalyticsIdAction + | AnalyticsControllerIsEnabledAction + | AnalyticsControllerIsOptedInAction; diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index fcd36ea91a4..1b2f41b5e4c 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -130,17 +130,6 @@ describe('AnalyticsController', () => { expect(controller.state.analyticsId).toBe(customId); }); - it('preserves provided analyticsId and does not auto-generate', () => { - const customId = '550e8400-e29b-41d4-a716-446655440000'; - const { controller } = setupController({ - state: { - analyticsId: customId, - }, - }); - - expect(controller.state.analyticsId).toBe(customId); - }); - it('restores analyticsId from persisted state', () => { // First, create a controller (generates UUID) const { controller: firstController } = setupController(); @@ -190,17 +179,6 @@ describe('AnalyticsController', () => { expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); }); - it('uses provided analyticsId when passed in state at initialization', () => { - const customId = '550e8400-e29b-41d4-a716-446655440000'; - const { controller } = setupController({ - state: { - analyticsId: customId, - }, - }); - - expect(controller.state.analyticsId).toBe(customId); - }); - it('initializes with default values when options are undefined', () => { const mockAdapter = createMockAdapter(); const rootMessenger = new Messenger< @@ -282,7 +260,9 @@ describe('AnalyticsController', () => { state: { analyticsId: undefined as unknown as string }, platformAdapter: mockAdapter, }); - }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got undefined'); + }).toThrow( + 'Invalid analyticsId: expected a valid UUIDv4, but got undefined', + ); expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); }); @@ -317,22 +297,22 @@ describe('AnalyticsController', () => { it('throws error when analyticsId is a valid UUID but not v4', () => { const mockAdapter = createMockAdapter(); - // UUIDv1 example - const uuidV1 = '6ba7b810-9dad-11d1-80b4-00c04fd430c8'; + // UUIDv5 example + const uuidV5 = 'c87ee674-4ddc-5efe-bd81-0b5a0b4d45b6'; expect(() => { setupController({ - state: { analyticsId: uuidV1 }, + state: { analyticsId: uuidV5 }, platformAdapter: mockAdapter, }); }).toThrow( - `Invalid analyticsId: expected a valid UUIDv4, but got "${uuidV1}"`, + `Invalid analyticsId: expected a valid UUIDv4, but got "${uuidV5}"`, ); expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); }); - it('handles errors in onSetupCompleted without breaking controller initialization', () => { + it('continues controller initialization when onSetupCompleted throws error', () => { const mockAdapter = createMockAdapter(); const error = new Error('Setup failed'); mockAdapter.onSetupCompleted = jest.fn(() => { @@ -363,7 +343,7 @@ describe('AnalyticsController', () => { projectLoggerSpy.mockRestore(); }); - it('calls onSetupCompleted with the correct analyticsId from state', () => { + it('calls onSetupCompleted with analyticsId', () => { const mockAdapter = createMockAdapter(); const customId = '550e8400-e29b-41d4-a716-446655440000'; @@ -412,16 +392,23 @@ describe('AnalyticsController', () => { }); describe('identify', () => { - it('identifies user and updates state', () => { + it('identifies user with traits using current analytics ID', () => { const mockAdapter = createMockAdapter(); - const { controller } = setupController({ platformAdapter: mockAdapter }); + const customId = '550e8400-e29b-41d4-a716-446655440000'; + const { controller } = setupController({ + state: { analyticsId: customId }, + platformAdapter: mockAdapter, + }); - controller.identify('user-123', { email: 'test@example.com' }); + const traits = { + ENABLE_OPENSEA_API: 'ON', + NFT_AUTODETECTION: 'ON', + }; - expect(controller.state.analyticsId).toBe('user-123'); - expect(mockAdapter.identify).toHaveBeenCalledWith('user-123', { - email: 'test@example.com', - }); + controller.identify(traits); + + expect(controller.state.analyticsId).toBe(customId); + expect(mockAdapter.identify).toHaveBeenCalledWith(customId, traits); }); it('does not identify when disabled', () => { @@ -431,14 +418,19 @@ describe('AnalyticsController', () => { platformAdapter: mockAdapter, }); - controller.identify('user-123'); + const traits = { + ENABLE_OPENSEA_API: 'ON', + NFT_AUTODETECTION: 'ON', + }; + + controller.identify(traits); expect(mockAdapter.identify).not.toHaveBeenCalled(); }); }); describe('trackView', () => { - it('tracks page view', () => { + it('tracks view', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ platformAdapter: mockAdapter }); @@ -449,7 +441,7 @@ describe('AnalyticsController', () => { }); }); - it('does not track page when disabled', () => { + it('does not track view when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { enabled: false }, @@ -516,87 +508,75 @@ describe('AnalyticsController', () => { }); }); - describe('messenger actions', () => { - it('handles trackEvent action', () => { - const mockAdapter = createMockAdapter(); - const { messenger } = setupController({ - platformAdapter: mockAdapter, + describe('getAnalyticsId', () => { + it('returns analytics ID', () => { + const customId = '550e8400-e29b-41d4-a716-446655440000'; + const { controller, messenger } = setupController({ + state: { analyticsId: customId }, }); - messenger.call('AnalyticsController:trackEvent', 'test_event', { - prop: 'value', - }); + const analyticsId = messenger.call('AnalyticsController:getAnalyticsId'); - expect(mockAdapter.track).toHaveBeenCalled(); + expect(analyticsId).toBe(customId); + expect(analyticsId).toBe(controller.state.analyticsId); }); + }); - it('handles identify action', () => { - const mockAdapter = createMockAdapter(); + describe('isEnabled', () => { + it('returns enabled status from state', () => { const { controller, messenger } = setupController({ - platformAdapter: mockAdapter, + state: { enabled: true }, }); - messenger.call('AnalyticsController:identify', 'user-123', { - email: 'test', - }); + const isEnabled = messenger.call('AnalyticsController:isEnabled'); - expect(mockAdapter.identify).toHaveBeenCalled(); - expect(controller.state.analyticsId).toBe('user-123'); + expect(isEnabled).toBe(true); + expect(isEnabled).toBe(controller.state.enabled); }); - it('handles trackView action', () => { - const mockAdapter = createMockAdapter(); - const { messenger } = setupController({ platformAdapter: mockAdapter }); - - messenger.call('AnalyticsController:trackView', 'home', {}); - - expect(mockAdapter.view).toHaveBeenCalled(); - }); - - it('handles enable action', () => { + it('returns updated value when state changes', () => { const { controller, messenger } = setupController({ state: { enabled: false }, }); - expect(controller.state.enabled).toBe(false); - - messenger.call('AnalyticsController:enable'); + expect(messenger.call('AnalyticsController:isEnabled')).toBe(false); - expect(controller.state.enabled).toBe(true); - }); - - it('handles disable action', () => { - const { controller, messenger } = setupController({ - state: { enabled: true }, - }); + controller.enable(); - expect(controller.state.enabled).toBe(true); + expect(messenger.call('AnalyticsController:isEnabled')).toBe(true); - messenger.call('AnalyticsController:disable'); + controller.disable(); - expect(controller.state.enabled).toBe(false); + expect(messenger.call('AnalyticsController:isEnabled')).toBe(false); }); + }); - it('handles optIn action', () => { - const { controller, messenger } = setupController(); - - expect(controller.state.optedIn).toBe(false); + describe('isOptedIn', () => { + it('returns opted in status from state', () => { + const { controller, messenger } = setupController({ + state: { optedIn: true }, + }); - messenger.call('AnalyticsController:optIn'); + const isOptedIn = messenger.call('AnalyticsController:isOptedIn'); - expect(controller.state.optedIn).toBe(true); + expect(isOptedIn).toBe(true); + expect(isOptedIn).toBe(controller.state.optedIn); }); - it('handles optOut action', () => { + it('returns updated value when state changes', () => { const { controller, messenger } = setupController({ - state: { optedIn: true }, + state: { optedIn: false }, }); - expect(controller.state.optedIn).toBe(true); + expect(messenger.call('AnalyticsController:isOptedIn')).toBe(false); - messenger.call('AnalyticsController:optOut'); + controller.optIn(); - expect(controller.state.optedIn).toBe(false); + expect(messenger.call('AnalyticsController:isOptedIn')).toBe(true); + + controller.optOut(); + + expect(messenger.call('AnalyticsController:isOptedIn')).toBe(false); }); }); }); diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index e5ddafbd0cb..f7345dc33d7 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -5,7 +5,11 @@ import type { } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; import type { Messenger } from '@metamask/messenger'; -import { v4 as uuidv4, validate as uuidValidate, version as uuidVersion } from 'uuid'; +import { + v4 as uuidv4, + validate as uuidValidate, + version as uuidVersion, +} from 'uuid'; import type { AnalyticsControllerMethodActions } from './AnalyticsController-method-action-types'; import { projectLogger } from './AnalyticsLogger'; @@ -96,6 +100,9 @@ const MESSENGER_EXPOSED_METHODS = [ 'disable', 'optIn', 'optOut', + 'getAnalyticsId', + 'isEnabled', + 'isOptedIn', ] as const; /** @@ -181,7 +188,8 @@ export class AnalyticsController extends BaseController< * Constructs an AnalyticsController instance. * * @param options - Controller options - * @param options.state - Initial controller state (defaults from getDefaultAnalyticsControllerState) + * @param options.state - Initial controller state (defaults from getDefaultAnalyticsControllerState). + * For migration from a previous system, pass the existing analytics ID via state.analyticsId. * @param options.messenger - Messenger used to communicate with BaseController * @param options.platformAdapter - Platform adapter implementation for tracking */ @@ -231,7 +239,7 @@ export class AnalyticsController extends BaseController< throw new Error( `Invalid analyticsId: expected a valid UUIDv4, but got ${JSON.stringify(this.state.analyticsId)}`, ); - } + } } /** @@ -258,30 +266,24 @@ export class AnalyticsController extends BaseController< /** * Identify a user for analytics. * - * @param userId - The user identifier (e.g., metametrics ID) * @param traits - User traits/properties */ - identify(userId: string, traits?: AnalyticsUserTraits): void { + identify(traits?: AnalyticsUserTraits): void { if (!this.state.enabled) { return; } - // Update state with analytics ID - this.update((state) => { - state.analyticsId = userId; - }); - - // Delegate to platform adapter if supported + // Delegate to platform adapter if supported, using the current analytics ID if (this.#platformAdapter.identify) { - this.#platformAdapter.identify(userId, traits); + this.#platformAdapter.identify(this.state.analyticsId, traits); } } /** * Track a page view. * - * @param pageName - The name of the page - * @param properties - Page properties + * @param name - The name of the UI item being viewed (pages for web, screen for mobile) + * @param properties - UI item properties */ trackView(name: string, properties?: AnalyticsEventProperties): void { if (!this.state.enabled) { @@ -327,4 +329,31 @@ export class AnalyticsController extends BaseController< state.optedIn = false; }); } + + /** + * Get the analytics ID from the controller state. + * + * @returns The current analytics ID. + */ + getAnalyticsId(): string { + return this.state.analyticsId; + } + + /** + * Get the enabled status from the controller state. + * + * @returns The current enabled status. + */ + isEnabled(): boolean { + return this.state.enabled; + } + + /** + * Get the opted in status from the controller state. + * + * @returns The current opted in status. + */ + isOptedIn(): boolean { + return this.state.optedIn; + } } diff --git a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts index a7b56aa83ab..a49af34efb5 100644 --- a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts +++ b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts @@ -17,7 +17,7 @@ export type AnalyticsUserTraits = Record; export interface AnalyticsPlatformAdapter { /** * Track an analytics event. - * + * * This is the same as trackEvent in the old analytics system * * @param eventName - The name of the event @@ -35,7 +35,7 @@ export interface AnalyticsPlatformAdapter { /** * Track a UI unit (page or screen) view depending on the platform - * + * * This is the same as page in Segment web SDK and screen in Segment mobile SDK. * Each platform adapter should implement this method to track UI views * using the appropriate method for the platform: "pages" for web, "screen" for mobile. @@ -47,16 +47,16 @@ export interface AnalyticsPlatformAdapter { /** * Lifecycle hook called after the AnalyticsController is fully initialized. - * + * * This hook allows platform-specific adapters to perform setup that requires * access to the controller's state (e.g., analyticsId). - * + * * The controller calls this method once after initialization, passing the * analyticsId from controller state. The analyticsId is guaranteed to be set * when this method is called - this is the definition of "completed" setup. - * + * * @param analyticsId - The analytics ID from controller state. Always set (never empty). - * + * * @example * ```typescript * onSetupCompleted(analyticsId: string): void { diff --git a/packages/analytics-controller/src/index.ts b/packages/analytics-controller/src/index.ts index 02e06243ecc..ca42718d256 100644 --- a/packages/analytics-controller/src/index.ts +++ b/packages/analytics-controller/src/index.ts @@ -32,5 +32,7 @@ export type { AnalyticsControllerDisableAction, AnalyticsControllerOptInAction, AnalyticsControllerOptOutAction, + AnalyticsControllerGetAnalyticsIdAction, + AnalyticsControllerIsEnabledAction, AnalyticsControllerMethodActions, } from './AnalyticsController-method-action-types'; From 18659a990617633cdcd745eb58c71ae2ab771f5a Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Fri, 7 Nov 2025 14:28:44 +0100 Subject: [PATCH 03/20] feat(analytics-controller): add error type and fix linter errors --- packages/analytics-controller/README.md | 3 ++ .../src/AnalyticsController.test.ts | 29 +++++++++---------- .../src/AnalyticsPlatformAdapter.types.ts | 7 +++-- .../src/AnalyticsPlatformAdapterSetupError.ts | 14 +++++++++ packages/analytics-controller/src/index.ts | 3 ++ 5 files changed, 39 insertions(+), 17 deletions(-) create mode 100644 packages/analytics-controller/src/AnalyticsPlatformAdapterSetupError.ts diff --git a/packages/analytics-controller/README.md b/packages/analytics-controller/README.md index b87d4d08e48..f4942363a92 100644 --- a/packages/analytics-controller/README.md +++ b/packages/analytics-controller/README.md @@ -209,6 +209,7 @@ const defaultState = getDefaultAnalyticsControllerState(); The `onSetupCompleted` lifecycle hook is called once after the `AnalyticsController` is fully initialized. This hook allows platform-specific adapters to perform setup that requires access to the controller's state (e.g., `analyticsId`). **When it's called:** + - After the controller is fully initialized - Only when `analyticsId` is set in controller state (the presence of `analyticsId` is the definition of "completed" setup) - The `analyticsId` parameter is guaranteed to be set and be a valid UUIDv4 when this method is called @@ -233,9 +234,11 @@ const platformAdapter: AnalyticsPlatformAdapter = { ``` **Error handling:** + - Errors thrown in `onSetupCompleted` are caught and logged **Best practices:** + - Use `onSetupCompleted` for setup that requires controller state - Keep setup logic minimal and focused - Handle errors gracefully within the hook diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index 1b2f41b5e4c..d5c920a40e9 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -11,6 +11,7 @@ import { type AnalyticsControllerActions, type AnalyticsControllerEvents, type AnalyticsPlatformAdapter, + AnalyticsPlatformAdapterSetupError, getDefaultAnalyticsControllerState, } from '.'; import type { AnalyticsControllerState } from '.'; @@ -314,33 +315,31 @@ describe('AnalyticsController', () => { it('continues controller initialization when onSetupCompleted throws error', () => { const mockAdapter = createMockAdapter(); - const error = new Error('Setup failed'); - mockAdapter.onSetupCompleted = jest.fn(() => { + // Simulate a fake Segment SDK plugin configuration error + const cause = new Error( + 'MetaMetricsPrivacySegmentPlugin configure failed', + ); + const error = new AnalyticsPlatformAdapterSetupError( + 'Failed to add privacy plugin to Segment client', + cause, + ); + jest.spyOn(mockAdapter, 'onSetupCompleted').mockImplementation(() => { throw error; }); - const projectLoggerSpy = jest - .spyOn(require('./AnalyticsLogger'), 'projectLogger') - .mockImplementation(); - + // Controller should initialize successfully despite the error const { controller } = setupController({ state: { analyticsId: '550e8400-e29b-41d4-a716-446655440000' }, platformAdapter: mockAdapter, }); - // Controller should still be initialized + // Controller should still be initialized with correct state expect(controller).toBeDefined(); expect(controller.state.analyticsId).toBe( '550e8400-e29b-41d4-a716-446655440000', ); - - // Error should be logged - expect(projectLoggerSpy).toHaveBeenCalledWith( - 'Error calling platformAdapter.onSetupCompleted', - error, - ); - - projectLoggerSpy.mockRestore(); + expect(controller.state.enabled).toBe(true); + expect(controller.state.optedIn).toBe(false); }); it('calls onSetupCompleted with analyticsId', () => { diff --git a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts index a49af34efb5..38a20845c1b 100644 --- a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts +++ b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts @@ -14,7 +14,7 @@ export type AnalyticsUserTraits = Record; * Platform adapter interface for analytics tracking * Implementations should handle platform-specific details (Segment SDK, etc.) */ -export interface AnalyticsPlatformAdapter { +export type AnalyticsPlatformAdapter = { /** * Track an analytics event. * @@ -56,6 +56,9 @@ export interface AnalyticsPlatformAdapter { * when this method is called - this is the definition of "completed" setup. * * @param analyticsId - The analytics ID from controller state. Always set (never empty). + * @throws {AnalyticsPlatformAdapterSetupError} May throw errors during setup (e.g., configuration errors, network failures). + * Errors thrown by this method are caught and logged by the controller, but do not prevent + * controller initialization from completing successfully. * * @example * ```typescript @@ -68,4 +71,4 @@ export interface AnalyticsPlatformAdapter { * ``` */ onSetupCompleted(analyticsId: string): void; -} +}; diff --git a/packages/analytics-controller/src/AnalyticsPlatformAdapterSetupError.ts b/packages/analytics-controller/src/AnalyticsPlatformAdapterSetupError.ts new file mode 100644 index 00000000000..79552228525 --- /dev/null +++ b/packages/analytics-controller/src/AnalyticsPlatformAdapterSetupError.ts @@ -0,0 +1,14 @@ +/** + * Error thrown when platform adapter setup fails during the onSetupCompleted lifecycle hook. + */ +export class AnalyticsPlatformAdapterSetupError extends Error { + cause?: Error; + + constructor(message: string, cause?: Error) { + super(message); + this.name = this.constructor.name; + if (cause) { + this.cause = cause; + } + } +} diff --git a/packages/analytics-controller/src/index.ts b/packages/analytics-controller/src/index.ts index ca42718d256..98d9c1bdb67 100644 --- a/packages/analytics-controller/src/index.ts +++ b/packages/analytics-controller/src/index.ts @@ -2,6 +2,9 @@ export { AnalyticsController } from './AnalyticsController'; export type { AnalyticsControllerOptions } from './AnalyticsController'; +// Export errors +export { AnalyticsPlatformAdapterSetupError } from './AnalyticsPlatformAdapterSetupError'; + // Export types export type { AnalyticsEventProperties, From d6488c192b9d9c3feced8df8aa1b2d6a39138dc2 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 17 Nov 2025 16:47:56 +0100 Subject: [PATCH 04/20] refactor(analytics-controller): update trackEvent to accept AnalyticsTrackingEvent object - Add AnalyticsJsonValue and AnalyticsJsonMap types similar to legacy types for easier migration - Add AnalyticsTrackingEvent type matching legacy ITrackingEvent structure - Update trackEvent method to accept AnalyticsTrackingEvent instead of separate eventName and properties - Implement same logic as legacy trackEvent (handles hasProperties, isAnonymous, sensitiveProperties) - Remove redundant saveDataRecording parameter (now part of event object) - Convert interfaces to type aliases for consistency - Update tests --- ...AnalyticsController-method-action-types.ts | 54 ++- .../src/AnalyticsController.test.ts | 385 +++++++++++++----- .../src/AnalyticsController.ts | 146 ++++--- .../src/AnalyticsPlatformAdapter.types.ts | 35 ++ .../src/analyticsStateComputer.test.ts | 88 ++++ .../src/analyticsStateComputer.ts | 47 +++ packages/analytics-controller/src/index.ts | 7 +- 7 files changed, 588 insertions(+), 174 deletions(-) create mode 100644 packages/analytics-controller/src/analyticsStateComputer.test.ts create mode 100644 packages/analytics-controller/src/analyticsStateComputer.ts diff --git a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts index 9397bff2f39..ecff7a59e6d 100644 --- a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts +++ b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts @@ -39,24 +39,9 @@ export type AnalyticsControllerTrackViewAction = { handler: AnalyticsController['trackView']; }; -/** - * Enable analytics tracking. - */ -export type AnalyticsControllerEnableAction = { - type: `AnalyticsController:enable`; - handler: AnalyticsController['enable']; -}; - -/** - * Disable analytics tracking. - */ -export type AnalyticsControllerDisableAction = { - type: `AnalyticsController:disable`; - handler: AnalyticsController['disable']; -}; - /** * Opt in to analytics. + * This updates the user's opt-in status. */ export type AnalyticsControllerOptInAction = { type: `AnalyticsController:optIn`; @@ -65,12 +50,31 @@ export type AnalyticsControllerOptInAction = { /** * Opt out of analytics. + * This updates the user's opt-in status. */ export type AnalyticsControllerOptOutAction = { type: `AnalyticsController:optOut`; handler: AnalyticsController['optOut']; }; +/** + * Opt in to analytics through social account. + * This updates the user's social opt-in status. + */ +export type AnalyticsControllerSocialOptInAction = { + type: `AnalyticsController:socialOptIn`; + handler: AnalyticsController['socialOptIn']; +}; + +/** + * Opt out of analytics through social account. + * This updates the user's social opt-in status. + */ +export type AnalyticsControllerSocialOptOutAction = { + type: `AnalyticsController:socialOptOut`; + handler: AnalyticsController['socialOptOut']; +}; + /** * Get the analytics ID from the controller state. * @@ -83,6 +87,7 @@ export type AnalyticsControllerGetAnalyticsIdAction = { /** * Get the enabled status from the controller state. + * This is computed from user state via the state machine. * * @returns The current enabled status. */ @@ -101,6 +106,16 @@ export type AnalyticsControllerIsOptedInAction = { handler: AnalyticsController['isOptedIn']; }; +/** + * Get the social opted in status from the controller state. + * + * @returns The current social opted in status. + */ +export type AnalyticsControllerIsSocialOptedInAction = { + type: `AnalyticsController:isSocialOptedIn`; + handler: AnalyticsController['isSocialOptedIn']; +}; + /** * Union of all AnalyticsController action types. */ @@ -108,10 +123,11 @@ export type AnalyticsControllerMethodActions = | AnalyticsControllerTrackEventAction | AnalyticsControllerIdentifyAction | AnalyticsControllerTrackViewAction - | AnalyticsControllerEnableAction - | AnalyticsControllerDisableAction | AnalyticsControllerOptInAction | AnalyticsControllerOptOutAction + | AnalyticsControllerSocialOptInAction + | AnalyticsControllerSocialOptOutAction | AnalyticsControllerGetAnalyticsIdAction | AnalyticsControllerIsEnabledAction - | AnalyticsControllerIsOptedInAction; + | AnalyticsControllerIsOptedInAction + | AnalyticsControllerIsSocialOptedInAction; diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index d5c920a40e9..ecc7dbffec7 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -13,6 +13,7 @@ import { type AnalyticsPlatformAdapter, AnalyticsPlatformAdapterSetupError, getDefaultAnalyticsControllerState, + type AnalyticsTrackingEvent, } from '.'; import type { AnalyticsControllerState } from '.'; @@ -85,6 +86,36 @@ function isValidUUIDv4(value: string): boolean { return uuidValidate(value) && uuidVersion(value) === 4; } +/** + * Creates a test analytics tracking event. + * + * @param name - Event name + * @param properties - Event properties + * @param sensitiveProperties - Sensitive properties + * @param saveDataRecording - Whether to save data recording flag + * @returns A test tracking event + */ +function createTestEvent( + name: string, + properties: Record = {}, + sensitiveProperties: Record = {}, + saveDataRecording = true, +): AnalyticsTrackingEvent { + const hasProps = + Object.keys(properties).length > 0 || + Object.keys(sensitiveProperties).length > 0; + const isAnon = Object.keys(sensitiveProperties).length > 0; + + return { + name, + properties: properties as AnalyticsTrackingEvent['properties'], + sensitiveProperties: sensitiveProperties as AnalyticsTrackingEvent['sensitiveProperties'], + saveDataRecording, + isAnonymous: isAnon, + hasProperties: hasProps, + }; +} + describe('AnalyticsController', () => { const createMockAdapter = (): AnalyticsPlatformAdapter => ({ track: jest.fn(), @@ -99,9 +130,12 @@ describe('AnalyticsController', () => { const defaultState = getDefaultAnalyticsControllerState(); - expect(controller.state.enabled).toBe(defaultState.enabled); - expect(controller.state.optedIn).toBe(defaultState.optedIn); - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); + expect(controller.isEnabled()).toBe(false); + expect(controller.state.user_optedIn).toBe(defaultState.user_optedIn); + expect(controller.state.user_socialOptedIn).toBe( + defaultState.user_socialOptedIn, + ); + expect(isValidUUIDv4(controller.state.user_analyticsId)).toBe(true); }); it('generates a new UUID when no state is provided (first-time initialization)', () => { @@ -109,10 +143,10 @@ describe('AnalyticsController', () => { const { controller: controller1 } = setupController(); const { controller: controller2 } = setupController(); - expect(isValidUUIDv4(controller1.state.analyticsId)).toBe(true); - expect(isValidUUIDv4(controller2.state.analyticsId)).toBe(true); - expect(controller1.state.analyticsId).not.toBe( - controller2.state.analyticsId, + expect(isValidUUIDv4(controller1.state.user_analyticsId)).toBe(true); + expect(isValidUUIDv4(controller2.state.user_analyticsId)).toBe(true); + expect(controller1.state.user_analyticsId).not.toBe( + controller2.state.user_analyticsId, ); }); @@ -120,64 +154,64 @@ describe('AnalyticsController', () => { const customId = '550e8400-e29b-41d4-a716-446655440000'; const { controller } = setupController({ state: { - enabled: false, - optedIn: true, - analyticsId: customId, + user_optedIn: true, + user_socialOptedIn: false, + user_analyticsId: customId, }, }); - expect(controller.state.enabled).toBe(false); - expect(controller.state.optedIn).toBe(true); - expect(controller.state.analyticsId).toBe(customId); + expect(controller.isEnabled()).toBe(true); + expect(controller.state.user_optedIn).toBe(true); + expect(controller.state.user_socialOptedIn).toBe(false); + expect(controller.state.user_analyticsId).toBe(customId); }); it('restores analyticsId from persisted state', () => { // First, create a controller (generates UUID) const { controller: firstController } = setupController(); - const originalAnalyticsId = firstController.state.analyticsId; + const originalAnalyticsId = firstController.state.user_analyticsId; expect(isValidUUIDv4(originalAnalyticsId)).toBe(true); // Simulate restoration: create a new controller with the previous state const { controller: restoredController } = setupController({ state: { - enabled: firstController.state.enabled, - optedIn: firstController.state.optedIn, - analyticsId: firstController.state.analyticsId, + user_optedIn: firstController.state.user_optedIn, + user_socialOptedIn: firstController.state.user_socialOptedIn, + user_analyticsId: firstController.state.user_analyticsId, }, }); // The restored controller should have the same state as the original controller - expect(restoredController.state.analyticsId).toBe(originalAnalyticsId); - expect(restoredController.state.enabled).toBe( - firstController.state.enabled, - ); - expect(restoredController.state.optedIn).toBe( - firstController.state.optedIn, + expect(restoredController.state.user_analyticsId).toBe(originalAnalyticsId); + expect(restoredController.isEnabled()).toBe(firstController.isEnabled()); + expect(restoredController.state.user_optedIn).toBe( + firstController.state.user_optedIn, ); }); - it('initializes with custom enabled/optedIn', () => { + it('initializes with custom optedIn', () => { const { controller } = setupController({ state: { - enabled: false, - optedIn: true, + user_optedIn: true, + user_socialOptedIn: false, }, }); - expect(controller.state.enabled).toBe(false); - expect(controller.state.optedIn).toBe(true); + expect(controller.isEnabled()).toBe(true); + expect(controller.state.user_optedIn).toBe(true); + expect(controller.state.user_socialOptedIn).toBe(false); }); it('uses default analyticsId when not provided in partial state', () => { const { controller } = setupController({ state: { - enabled: false, - optedIn: true, + user_optedIn: true, + user_socialOptedIn: false, }, }); - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); + expect(isValidUUIDv4(controller.state.user_analyticsId)).toBe(true); }); it('initializes with default values when options are undefined', () => { @@ -204,11 +238,14 @@ describe('AnalyticsController', () => { }); const defaultState = getDefaultAnalyticsControllerState(); - expect(controller.state.enabled).toBe(defaultState.enabled); - expect(controller.state.optedIn).toBe(defaultState.optedIn); + expect(controller.isEnabled()).toBe(false); + expect(controller.state.user_optedIn).toBe(defaultState.user_optedIn); + expect(controller.state.user_socialOptedIn).toBe( + defaultState.user_socialOptedIn, + ); // analyticsId is auto-generated (UUIDv4) - expect(typeof controller.state.analyticsId).toBe('string'); - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); + expect(typeof controller.state.user_analyticsId).toBe('string'); + expect(isValidUUIDv4(controller.state.user_analyticsId)).toBe(true); }); }); @@ -218,7 +255,7 @@ describe('AnalyticsController', () => { const customId = '550e8400-e29b-41d4-a716-446655440000'; setupController({ - state: { analyticsId: customId }, + state: { user_analyticsId: customId }, platformAdapter: mockAdapter, }); @@ -235,9 +272,9 @@ describe('AnalyticsController', () => { expect(mockAdapter.onSetupCompleted).toHaveBeenCalledTimes(1); expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith( - controller.state.analyticsId, + controller.state.user_analyticsId, ); - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); + expect(isValidUUIDv4(controller.state.user_analyticsId)).toBe(true); }); it('throws error when analyticsId is empty string', () => { @@ -245,7 +282,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { analyticsId: '' }, + state: { user_analyticsId: '' }, platformAdapter: mockAdapter, }); }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got ""'); @@ -258,7 +295,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { analyticsId: undefined as unknown as string }, + state: { user_analyticsId: undefined as unknown as string }, platformAdapter: mockAdapter, }); }).toThrow( @@ -273,7 +310,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { analyticsId: null as unknown as string }, + state: { user_analyticsId: null as unknown as string }, platformAdapter: mockAdapter, }); }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got null'); @@ -286,7 +323,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { analyticsId: 'not-a-uuid' }, + state: { user_analyticsId: 'not-a-uuid' }, platformAdapter: mockAdapter, }); }).toThrow( @@ -303,7 +340,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { analyticsId: uuidV5 }, + state: { user_analyticsId: uuidV5 }, platformAdapter: mockAdapter, }); }).toThrow( @@ -329,17 +366,18 @@ describe('AnalyticsController', () => { // Controller should initialize successfully despite the error const { controller } = setupController({ - state: { analyticsId: '550e8400-e29b-41d4-a716-446655440000' }, + state: { user_analyticsId: '550e8400-e29b-41d4-a716-446655440000' }, platformAdapter: mockAdapter, }); // Controller should still be initialized with correct state expect(controller).toBeDefined(); - expect(controller.state.analyticsId).toBe( + expect(controller.state.user_analyticsId).toBe( '550e8400-e29b-41d4-a716-446655440000', ); - expect(controller.state.enabled).toBe(true); - expect(controller.state.optedIn).toBe(false); + expect(controller.isEnabled()).toBe(false); + expect(controller.state.user_optedIn).toBe(false); + expect(controller.state.user_socialOptedIn).toBe(false); }); it('calls onSetupCompleted with analyticsId', () => { @@ -347,44 +385,98 @@ describe('AnalyticsController', () => { const customId = '550e8400-e29b-41d4-a716-446655440000'; const { controller } = setupController({ - state: { analyticsId: customId }, + state: { user_analyticsId: customId }, platformAdapter: mockAdapter, }); expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith(customId); - expect(controller.state.analyticsId).toBe(customId); + expect(controller.state.user_analyticsId).toBe(customId); }); }); describe('trackEvent', () => { - it('tracks event when enabled', () => { + it('tracks event when enabled via regular opt-in', () => { const mockAdapter = createMockAdapter(); - const { controller } = setupController({ platformAdapter: mockAdapter }); + const { controller } = setupController({ + state: { user_optedIn: true, user_socialOptedIn: false }, + platformAdapter: mockAdapter, + }); - controller.trackEvent('test_event', { prop: 'value' }); + const event = createTestEvent('test_event', { prop: 'value' }); + controller.trackEvent(event); expect(mockAdapter.track).toHaveBeenCalledWith('test_event', { + anonymous: false, prop: 'value', }); }); - it('tracks event with default empty properties when properties not provided', () => { + it('tracks event when enabled via social opt-in', () => { const mockAdapter = createMockAdapter(); - const { controller } = setupController({ platformAdapter: mockAdapter }); + const { controller } = setupController({ + state: { user_optedIn: false, user_socialOptedIn: true }, + platformAdapter: mockAdapter, + }); - controller.trackEvent('test_event'); + const event = createTestEvent('test_event', { prop: 'value' }); + controller.trackEvent(event); - expect(mockAdapter.track).toHaveBeenCalledWith('test_event', {}); + expect(mockAdapter.track).toHaveBeenCalledWith('test_event', { + anonymous: false, + prop: 'value', + }); + }); + + it('tracks event with no properties when hasProperties is false', () => { + const mockAdapter = createMockAdapter(); + const { controller } = setupController({ + state: { user_optedIn: true, user_socialOptedIn: false }, + platformAdapter: mockAdapter, + }); + + const event = createTestEvent('test_event', {}, {}, true); + controller.trackEvent(event); + + expect(mockAdapter.track).toHaveBeenCalledWith('test_event', { + anonymous: false, + }); + }); + + it('tracks anonymous event when isAnonymous is true', () => { + const mockAdapter = createMockAdapter(); + const { controller } = setupController({ + state: { user_optedIn: true, user_socialOptedIn: false }, + platformAdapter: mockAdapter, + }); + + const event = createTestEvent( + 'test_event', + { prop: 'value' }, + { sensitive: 'data' }, + ); + controller.trackEvent(event); + + expect(mockAdapter.track).toHaveBeenCalledTimes(2); + expect(mockAdapter.track).toHaveBeenNthCalledWith(1, 'test_event', { + anonymous: false, + prop: 'value', + }); + expect(mockAdapter.track).toHaveBeenNthCalledWith(2, 'test_event', { + anonymous: true, + prop: 'value', + sensitive: 'data', + }); }); it('does not track event when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { enabled: false }, + state: { user_optedIn: false, user_socialOptedIn: false }, platformAdapter: mockAdapter, }); - controller.trackEvent('test_event', { prop: 'value' }); + const event = createTestEvent('test_event', { prop: 'value' }); + controller.trackEvent(event); expect(mockAdapter.track).not.toHaveBeenCalled(); }); @@ -395,7 +487,11 @@ describe('AnalyticsController', () => { const mockAdapter = createMockAdapter(); const customId = '550e8400-e29b-41d4-a716-446655440000'; const { controller } = setupController({ - state: { analyticsId: customId }, + state: { + user_analyticsId: customId, + user_optedIn: true, + user_socialOptedIn: false, + }, platformAdapter: mockAdapter, }); @@ -406,14 +502,14 @@ describe('AnalyticsController', () => { controller.identify(traits); - expect(controller.state.analyticsId).toBe(customId); + expect(controller.state.user_analyticsId).toBe(customId); expect(mockAdapter.identify).toHaveBeenCalledWith(customId, traits); }); it('does not identify when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { enabled: false }, + state: { user_optedIn: false, user_socialOptedIn: false }, platformAdapter: mockAdapter, }); @@ -431,7 +527,10 @@ describe('AnalyticsController', () => { describe('trackView', () => { it('tracks view', () => { const mockAdapter = createMockAdapter(); - const { controller } = setupController({ platformAdapter: mockAdapter }); + const { controller } = setupController({ + state: { user_optedIn: true, user_socialOptedIn: false }, + platformAdapter: mockAdapter, + }); controller.trackView('home', { referrer: 'test' }); @@ -443,7 +542,7 @@ describe('AnalyticsController', () => { it('does not track view when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { enabled: false }, + state: { user_optedIn: false, user_socialOptedIn: false }, platformAdapter: mockAdapter, }); @@ -453,57 +552,91 @@ describe('AnalyticsController', () => { }); }); - describe('enable', () => { - it('enables analytics tracking', () => { + describe('optIn', () => { + it('opts in to analytics and enables tracking', () => { + const { controller } = setupController(); + + expect(controller.state.user_optedIn).toBe(false); + expect(controller.isEnabled()).toBe(false); + + controller.optIn(); + + expect(controller.state.user_optedIn).toBe(true); + expect(controller.isEnabled()).toBe(true); + }); + }); + + describe('optOut', () => { + it('opts out of analytics and disables tracking when social opt-in is false', () => { const { controller } = setupController({ - state: { enabled: false }, + state: { user_optedIn: true, user_socialOptedIn: false }, }); - expect(controller.state.enabled).toBe(false); + expect(controller.state.user_optedIn).toBe(true); + expect(controller.isEnabled()).toBe(true); - controller.enable(); + controller.optOut(); - expect(controller.state.enabled).toBe(true); + expect(controller.state.user_optedIn).toBe(false); + expect(controller.isEnabled()).toBe(false); }); - }); - describe('disable', () => { - it('disables analytics tracking', () => { + it('opts out of analytics but keeps tracking enabled when social opt-in is true', () => { const { controller } = setupController({ - state: { enabled: true }, + state: { user_optedIn: true, user_socialOptedIn: true }, }); - expect(controller.state.enabled).toBe(true); + expect(controller.state.user_optedIn).toBe(true); + expect(controller.isEnabled()).toBe(true); - controller.disable(); + controller.optOut(); - expect(controller.state.enabled).toBe(false); + expect(controller.state.user_optedIn).toBe(false); + expect(controller.isEnabled()).toBe(true); }); }); - describe('optIn', () => { - it('opts in to analytics', () => { + describe('socialOptIn', () => { + it('opts in to analytics through social account and enables tracking', () => { const { controller } = setupController(); - expect(controller.state.optedIn).toBe(false); + expect(controller.state.user_socialOptedIn).toBe(false); + expect(controller.isEnabled()).toBe(false); - controller.optIn(); + controller.socialOptIn(); - expect(controller.state.optedIn).toBe(true); + expect(controller.state.user_socialOptedIn).toBe(true); + expect(controller.isEnabled()).toBe(true); }); }); - describe('optOut', () => { - it('opts out of analytics', () => { + describe('socialOptOut', () => { + it('opts out of analytics through social account and disables tracking when regular opt-in is false', () => { const { controller } = setupController({ - state: { optedIn: true }, + state: { user_optedIn: false, user_socialOptedIn: true }, }); - expect(controller.state.optedIn).toBe(true); + expect(controller.state.user_socialOptedIn).toBe(true); + expect(controller.isEnabled()).toBe(true); - controller.optOut(); + controller.socialOptOut(); - expect(controller.state.optedIn).toBe(false); + expect(controller.state.user_socialOptedIn).toBe(false); + expect(controller.isEnabled()).toBe(false); + }); + + it('opts out of analytics through social account but keeps tracking enabled when regular opt-in is true', () => { + const { controller } = setupController({ + state: { user_optedIn: true, user_socialOptedIn: true }, + }); + + expect(controller.state.user_socialOptedIn).toBe(true); + expect(controller.isEnabled()).toBe(true); + + controller.socialOptOut(); + + expect(controller.state.user_socialOptedIn).toBe(false); + expect(controller.isEnabled()).toBe(true); }); }); @@ -511,40 +644,59 @@ describe('AnalyticsController', () => { it('returns analytics ID', () => { const customId = '550e8400-e29b-41d4-a716-446655440000'; const { controller, messenger } = setupController({ - state: { analyticsId: customId }, + state: { user_analyticsId: customId }, }); const analyticsId = messenger.call('AnalyticsController:getAnalyticsId'); expect(analyticsId).toBe(customId); - expect(analyticsId).toBe(controller.state.analyticsId); + expect(analyticsId).toBe(controller.state.user_analyticsId); }); }); describe('isEnabled', () => { - it('returns enabled status from state', () => { + it('returns enabled status computed from user state', () => { const { controller, messenger } = setupController({ - state: { enabled: true }, + state: { user_optedIn: true, user_socialOptedIn: false }, }); const isEnabled = messenger.call('AnalyticsController:isEnabled'); expect(isEnabled).toBe(true); - expect(isEnabled).toBe(controller.state.enabled); + expect(isEnabled).toBe(controller.isEnabled()); }); - it('returns updated value when state changes', () => { + it('returns true when only social opt-in is true', () => { const { controller, messenger } = setupController({ - state: { enabled: false }, + state: { user_optedIn: false, user_socialOptedIn: true }, }); + const isEnabled = messenger.call('AnalyticsController:isEnabled'); + + expect(isEnabled).toBe(true); + expect(isEnabled).toBe(controller.isEnabled()); + }); + + it('returns updated value when user state changes', () => { + const { controller, messenger } = setupController({ + state: { user_optedIn: false, user_socialOptedIn: false }, + }); + + expect(messenger.call('AnalyticsController:isEnabled')).toBe(false); + + controller.optIn(); + + expect(messenger.call('AnalyticsController:isEnabled')).toBe(true); + + controller.optOut(); + expect(messenger.call('AnalyticsController:isEnabled')).toBe(false); - controller.enable(); + controller.socialOptIn(); expect(messenger.call('AnalyticsController:isEnabled')).toBe(true); - controller.disable(); + controller.socialOptOut(); expect(messenger.call('AnalyticsController:isEnabled')).toBe(false); }); @@ -553,18 +705,18 @@ describe('AnalyticsController', () => { describe('isOptedIn', () => { it('returns opted in status from state', () => { const { controller, messenger } = setupController({ - state: { optedIn: true }, + state: { user_optedIn: true, user_socialOptedIn: false }, }); const isOptedIn = messenger.call('AnalyticsController:isOptedIn'); expect(isOptedIn).toBe(true); - expect(isOptedIn).toBe(controller.state.optedIn); + expect(isOptedIn).toBe(controller.state.user_optedIn); }); it('returns updated value when state changes', () => { const { controller, messenger } = setupController({ - state: { optedIn: false }, + state: { user_optedIn: false, user_socialOptedIn: false }, }); expect(messenger.call('AnalyticsController:isOptedIn')).toBe(false); @@ -578,4 +730,39 @@ describe('AnalyticsController', () => { expect(messenger.call('AnalyticsController:isOptedIn')).toBe(false); }); }); + + describe('isSocialOptedIn', () => { + it('returns social opted in status from state', () => { + const { controller, messenger } = setupController({ + state: { user_optedIn: false, user_socialOptedIn: true }, + }); + + const isSocialOptedIn = messenger.call( + 'AnalyticsController:isSocialOptedIn', + ); + + expect(isSocialOptedIn).toBe(true); + expect(isSocialOptedIn).toBe(controller.state.user_socialOptedIn); + }); + + it('returns updated value when state changes', () => { + const { controller, messenger } = setupController({ + state: { user_optedIn: false, user_socialOptedIn: false }, + }); + + expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe( + false, + ); + + controller.socialOptIn(); + + expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe(true); + + controller.socialOptOut(); + + expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe( + false, + ); + }); + }); }); diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index f7345dc33d7..875381eadcb 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -17,7 +17,9 @@ import type { AnalyticsPlatformAdapter, AnalyticsEventProperties, AnalyticsUserTraits, + AnalyticsTrackingEvent, } from './AnalyticsPlatformAdapter.types'; +import { computeEnabledState } from './analyticsStateComputer'; // === GENERAL === @@ -35,38 +37,38 @@ export const controllerName = 'AnalyticsController'; */ export type AnalyticsControllerState = { /** - * Whether analytics tracking is enabled + * User domain: Whether the user has opted in to analytics. */ - enabled: boolean; + user_optedIn: boolean; /** - * Whether the user has opted in to analytics + * User domain: Whether the user has opted in to analytics through social account. */ - optedIn: boolean; + user_socialOptedIn: boolean; /** - * User's UUIDv4 analytics identifier + * User domain: User's UUIDv4 analytics identifier. */ - analyticsId: string; + user_analyticsId: string; }; /** * The metadata for each property in {@link AnalyticsControllerState}. */ const analyticsControllerMetadata = { - enabled: { + user_optedIn: { includeInStateLogs: true, persist: true, includeInDebugSnapshot: true, usedInUi: true, }, - optedIn: { + user_socialOptedIn: { includeInStateLogs: true, persist: true, includeInDebugSnapshot: true, usedInUi: true, }, - analyticsId: { + user_analyticsId: { includeInStateLogs: true, persist: true, includeInDebugSnapshot: true, @@ -84,9 +86,9 @@ const analyticsControllerMetadata = { */ export function getDefaultAnalyticsControllerState(): AnalyticsControllerState { return { - enabled: true, - optedIn: false, - analyticsId: uuidv4(), + user_optedIn: false, + user_socialOptedIn: false, + user_analyticsId: uuidv4(), }; } @@ -96,13 +98,14 @@ const MESSENGER_EXPOSED_METHODS = [ 'trackEvent', 'identify', 'trackView', - 'enable', - 'disable', 'optIn', 'optOut', + 'socialOptIn', + 'socialOptOut', 'getAnalyticsId', 'isEnabled', 'isOptedIn', + 'isSocialOptedIn', ] as const; /** @@ -189,7 +192,7 @@ export class AnalyticsController extends BaseController< * * @param options - Controller options * @param options.state - Initial controller state (defaults from getDefaultAnalyticsControllerState). - * For migration from a previous system, pass the existing analytics ID via state.analyticsId. + * For migration from a previous system, pass the existing analytics ID via state.user_analyticsId. * @param options.messenger - Messenger used to communicate with BaseController * @param options.platformAdapter - Platform adapter implementation for tracking */ @@ -198,13 +201,15 @@ export class AnalyticsController extends BaseController< messenger, platformAdapter, }: AnalyticsControllerOptions) { + const initialState = { + ...getDefaultAnalyticsControllerState(), + ...state, + }; + super({ name: controllerName, metadata: analyticsControllerMetadata, - state: { - ...getDefaultAnalyticsControllerState(), - ...state, - }, + state: initialState, messenger, }); @@ -216,20 +221,21 @@ export class AnalyticsController extends BaseController< ); projectLogger('AnalyticsController initialized and ready', { - enabled: this.state.enabled, - optedIn: this.state.optedIn, - analyticsId: this.state.analyticsId, + enabled: computeEnabledState(this.state), + optedIn: this.state.user_optedIn, + socialOptedIn: this.state.user_socialOptedIn, + analyticsId: this.state.user_analyticsId, }); // Call onSetupCompleted lifecycle hook after initialization // Only call if analyticsId is set and is a valid UUIDv4 (this is the definition of "completed" setup) if ( - this.state.analyticsId && - uuidValidate(this.state.analyticsId) && - uuidVersion(this.state.analyticsId) === 4 + this.state.user_analyticsId && + uuidValidate(this.state.user_analyticsId) && + uuidVersion(this.state.user_analyticsId) === 4 ) { try { - this.#platformAdapter.onSetupCompleted(this.state.analyticsId); + this.#platformAdapter.onSetupCompleted(this.state.user_analyticsId); } catch (error) { // Log error but don't throw - adapter setup failure shouldn't break controller projectLogger('Error calling platformAdapter.onSetupCompleted', error); @@ -237,7 +243,7 @@ export class AnalyticsController extends BaseController< } else { // analyticsId is undefined, null, empty string, or not a valid UUIDv4 throw new Error( - `Invalid analyticsId: expected a valid UUIDv4, but got ${JSON.stringify(this.state.analyticsId)}`, + `Invalid analyticsId: expected a valid UUIDv4, but got ${JSON.stringify(this.state.user_analyticsId)}`, ); } } @@ -247,20 +253,38 @@ export class AnalyticsController extends BaseController< * * Events are only tracked if analytics is enabled. * - * @param eventName - The name of the event - * @param properties - Event properties + * @param event - Analytics event with properties and sensitive properties */ - trackEvent( - eventName: string, - properties: AnalyticsEventProperties = {}, - ): void { + trackEvent(event: AnalyticsTrackingEvent): void { // Don't track if analytics is disabled - if (!this.state.enabled) { + if (!computeEnabledState(this.state)) { return; } - // Delegate to platform adapter - this.#platformAdapter.track(eventName, properties); + // if event does not have properties, only send the non-anonymous empty event + // and return to prevent any additional processing + if (!event.hasProperties) { + this.#platformAdapter.track(event.name, { + anonymous: false, + } as AnalyticsEventProperties); + + return; + } + + // Log all non-anonymous properties, or an empty event if there's no non-anon props. + this.#platformAdapter.track(event.name, { + anonymous: false, + ...event.properties, + } as AnalyticsEventProperties); + + // Track all sensitive properties in an anonymous event + if (event.isAnonymous) { + this.#platformAdapter.track(event.name, { + anonymous: true, + ...event.properties, + ...event.sensitiveProperties, + } as AnalyticsEventProperties); + } } /** @@ -269,13 +293,13 @@ export class AnalyticsController extends BaseController< * @param traits - User traits/properties */ identify(traits?: AnalyticsUserTraits): void { - if (!this.state.enabled) { + if (!computeEnabledState(this.state)) { return; } // Delegate to platform adapter if supported, using the current analytics ID if (this.#platformAdapter.identify) { - this.#platformAdapter.identify(this.state.analyticsId, traits); + this.#platformAdapter.identify(this.state.user_analyticsId, traits); } } @@ -286,7 +310,7 @@ export class AnalyticsController extends BaseController< * @param properties - UI item properties */ trackView(name: string, properties?: AnalyticsEventProperties): void { - if (!this.state.enabled) { + if (!computeEnabledState(this.state)) { return; } @@ -295,38 +319,42 @@ export class AnalyticsController extends BaseController< } /** - * Enable analytics tracking. + * Opt in to analytics. + * This updates the user's opt-in status. */ - enable(): void { + optIn(): void { this.update((state) => { - state.enabled = true; + state.user_optedIn = true; }); } /** - * Disable analytics tracking. + * Opt out of analytics. + * This updates the user's opt-in status. */ - disable(): void { + optOut(): void { this.update((state) => { - state.enabled = false; + state.user_optedIn = false; }); } /** - * Opt in to analytics. + * Opt in to analytics through social account. + * This updates the user's social opt-in status. */ - optIn(): void { + socialOptIn(): void { this.update((state) => { - state.optedIn = true; + state.user_socialOptedIn = true; }); } /** - * Opt out of analytics. + * Opt out of analytics through social account. + * This updates the user's social opt-in status. */ - optOut(): void { + socialOptOut(): void { this.update((state) => { - state.optedIn = false; + state.user_socialOptedIn = false; }); } @@ -336,16 +364,17 @@ export class AnalyticsController extends BaseController< * @returns The current analytics ID. */ getAnalyticsId(): string { - return this.state.analyticsId; + return this.state.user_analyticsId; } /** * Get the enabled status from the controller state. + * This is computed from user state via the state machine. * * @returns The current enabled status. */ isEnabled(): boolean { - return this.state.enabled; + return computeEnabledState(this.state); } /** @@ -354,6 +383,15 @@ export class AnalyticsController extends BaseController< * @returns The current opted in status. */ isOptedIn(): boolean { - return this.state.optedIn; + return this.state.user_optedIn; + } + + /** + * Get the social opted in status from the controller state. + * + * @returns The current social opted in status. + */ + isSocialOptedIn(): boolean { + return this.state.user_socialOptedIn; } } diff --git a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts index 38a20845c1b..1e3a7d1dd86 100644 --- a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts +++ b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts @@ -1,5 +1,27 @@ import type { Json } from '@metamask/utils'; +/** + * Represents values that can be passed as properties to the event tracking function. + * Similar to JsonValue from Segment SDK but decoupled for platform agnosticism. + */ +type AnalyticsJsonValue = + | boolean + | number + | string + | null + | AnalyticsJsonValue[] + | AnalyticsJsonMap + | undefined; + +/** + * Represents the map object used to pass properties to the event tracking function. + * Similar to JsonMap from Segment SDK but decoupled for platform agnosticism. + */ +type AnalyticsJsonMap = { + [key: string]: AnalyticsJsonValue; + [index: number]: AnalyticsJsonValue; +}; + /** * Analytics event properties */ @@ -10,6 +32,19 @@ export type AnalyticsEventProperties = Record; */ export type AnalyticsUserTraits = Record; +/** + * Event properties structure with two distinct properties lists for anonymous and non-anonymous data. + * Similar to ITrackingEvent from legacy analytics but decoupled for platform agnosticism. + */ +export type AnalyticsTrackingEvent = { + readonly name: string; + properties: AnalyticsJsonMap; + sensitiveProperties: AnalyticsJsonMap; + saveDataRecording: boolean; + readonly isAnonymous: boolean; + readonly hasProperties: boolean; +}; + /** * Platform adapter interface for analytics tracking * Implementations should handle platform-specific details (Segment SDK, etc.) diff --git a/packages/analytics-controller/src/analyticsStateComputer.test.ts b/packages/analytics-controller/src/analyticsStateComputer.test.ts new file mode 100644 index 00000000000..07c04c9ab72 --- /dev/null +++ b/packages/analytics-controller/src/analyticsStateComputer.test.ts @@ -0,0 +1,88 @@ +import { computeEnabledState } from './analyticsStateComputer'; +import type { AnalyticsControllerState } from './AnalyticsController'; + +describe('analyticsStateComputer', () => { + describe('computeEnabledState', () => { + const defaultAnalyticsId = '550e8400-e29b-41d4-a716-446655440000'; + + it('returns true when optIn=true and socialOptIn=true', () => { + const state: AnalyticsControllerState = { + user_optedIn: true, + user_socialOptedIn: true, + user_analyticsId: defaultAnalyticsId, + }; + + const result = computeEnabledState(state); + + expect(result).toBe(true); + }); + + it('returns true when optIn=false and socialOptIn=true', () => { + const state: AnalyticsControllerState = { + user_optedIn: false, + user_socialOptedIn: true, + user_analyticsId: defaultAnalyticsId, + }; + + const result = computeEnabledState(state); + + expect(result).toBe(true); + }); + + it('returns true when optIn=true and socialOptIn=false', () => { + const state: AnalyticsControllerState = { + user_optedIn: true, + user_socialOptedIn: false, + user_analyticsId: defaultAnalyticsId, + }; + + const result = computeEnabledState(state); + + expect(result).toBe(true); + }); + + it('returns false when optIn=false and socialOptIn=false', () => { + const state: AnalyticsControllerState = { + user_optedIn: false, + user_socialOptedIn: false, + user_analyticsId: defaultAnalyticsId, + }; + + const result = computeEnabledState(state); + + expect(result).toBe(false); + }); + + it('computes enabled state based on user_optedIn OR user_socialOptedIn', () => { + const bothOptedIn: AnalyticsControllerState = { + user_optedIn: true, + user_socialOptedIn: true, + user_analyticsId: defaultAnalyticsId, + }; + + const onlyRegularOptedIn: AnalyticsControllerState = { + user_optedIn: true, + user_socialOptedIn: false, + user_analyticsId: defaultAnalyticsId, + }; + + const onlySocialOptedIn: AnalyticsControllerState = { + user_optedIn: false, + user_socialOptedIn: true, + user_analyticsId: defaultAnalyticsId, + }; + + const neitherOptedIn: AnalyticsControllerState = { + user_optedIn: false, + user_socialOptedIn: false, + user_analyticsId: defaultAnalyticsId, + }; + + expect(computeEnabledState(bothOptedIn)).toBe(true); + expect(computeEnabledState(onlyRegularOptedIn)).toBe(true); + expect(computeEnabledState(onlySocialOptedIn)).toBe(true); + expect(computeEnabledState(neitherOptedIn)).toBe(false); + }); + }); +}); + diff --git a/packages/analytics-controller/src/analyticsStateComputer.ts b/packages/analytics-controller/src/analyticsStateComputer.ts new file mode 100644 index 00000000000..5c0f4b4b5af --- /dev/null +++ b/packages/analytics-controller/src/analyticsStateComputer.ts @@ -0,0 +1,47 @@ +import type { AnalyticsControllerState } from './AnalyticsController'; + +/** + * State computer that computes controller values from controller state. + * + * This module provides functions that compute derived controller values from the + * controller's state. Currently, it computes the `controller_enabled` state, but + * can be extended to compute other values in the future. + * + * **State Computer Computations:** + * + * 1. **Enabled State** (`computeEnabledState`): + * - Determines whether analytics tracking is active + * - Rules: `controller_enabled = user_optedIn || user_socialOptedIn` + * - Analytics is enabled if the user has opted in through regular account OR social account + * + * 2. **Future computations** (e.g., feature flags, permissions, etc.) + * + * **Usage:** + * These functions are called: + * - During controller initialization to set initial values + * - Whenever user state changes (e.g., in `optIn()`, `optOut()`) + * + * **Extensibility:** + * To add new computations, add new functions that take `AnalyticsControllerState` as input. + * To add new user state properties, update the `AnalyticsControllerState` type with `user_` prefix + * and all computation functions that need to consider them. + */ + +/** + * Computes the `controller_enabled` state from controller state. + * + * @param state - The current controller state + * @returns `true` if analytics tracking should be enabled, `false` otherwise + */ +export function computeEnabledState( + state: AnalyticsControllerState, +): boolean { + // Analytics is enabled if user has opted in through regular account OR social account + // Rules: + // - optIn==true && socialOptIn==true -> enabled=true + // - optIn==false && socialOptIn==true -> enabled=true + // - optIn==true && socialOptIn==false -> enabled=true + // - optIn==false && socialOptIn==false -> enabled=false + return state.user_optedIn || state.user_socialOptedIn; +} + diff --git a/packages/analytics-controller/src/index.ts b/packages/analytics-controller/src/index.ts index 98d9c1bdb67..7608de0d185 100644 --- a/packages/analytics-controller/src/index.ts +++ b/packages/analytics-controller/src/index.ts @@ -10,6 +10,7 @@ export type { AnalyticsEventProperties, AnalyticsUserTraits, AnalyticsPlatformAdapter, + AnalyticsTrackingEvent, } from './AnalyticsPlatformAdapter.types'; // Export state types and utilities @@ -31,11 +32,13 @@ export type { AnalyticsControllerTrackEventAction, AnalyticsControllerIdentifyAction, AnalyticsControllerTrackViewAction, - AnalyticsControllerEnableAction, - AnalyticsControllerDisableAction, AnalyticsControllerOptInAction, AnalyticsControllerOptOutAction, + AnalyticsControllerSocialOptInAction, + AnalyticsControllerSocialOptOutAction, AnalyticsControllerGetAnalyticsIdAction, AnalyticsControllerIsEnabledAction, + AnalyticsControllerIsOptedInAction, + AnalyticsControllerIsSocialOptedInAction, AnalyticsControllerMethodActions, } from './AnalyticsController-method-action-types'; From b19d7ee89d4a834514ce601be37f2db5d653cd55 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 17 Nov 2025 17:06:28 +0100 Subject: [PATCH 05/20] format: fix lint --- .../src/AnalyticsController.test.ts | 15 +++++++-------- .../src/analyticsStateComputer.test.ts | 3 +-- .../src/analyticsStateComputer.ts | 11 ++++------- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index ecc7dbffec7..b55ba3add09 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -109,7 +109,8 @@ function createTestEvent( return { name, properties: properties as AnalyticsTrackingEvent['properties'], - sensitiveProperties: sensitiveProperties as AnalyticsTrackingEvent['sensitiveProperties'], + sensitiveProperties: + sensitiveProperties as AnalyticsTrackingEvent['sensitiveProperties'], saveDataRecording, isAnonymous: isAnon, hasProperties: hasProps, @@ -183,7 +184,9 @@ describe('AnalyticsController', () => { }); // The restored controller should have the same state as the original controller - expect(restoredController.state.user_analyticsId).toBe(originalAnalyticsId); + expect(restoredController.state.user_analyticsId).toBe( + originalAnalyticsId, + ); expect(restoredController.isEnabled()).toBe(firstController.isEnabled()); expect(restoredController.state.user_optedIn).toBe( firstController.state.user_optedIn, @@ -750,9 +753,7 @@ describe('AnalyticsController', () => { state: { user_optedIn: false, user_socialOptedIn: false }, }); - expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe( - false, - ); + expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe(false); controller.socialOptIn(); @@ -760,9 +761,7 @@ describe('AnalyticsController', () => { controller.socialOptOut(); - expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe( - false, - ); + expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe(false); }); }); }); diff --git a/packages/analytics-controller/src/analyticsStateComputer.test.ts b/packages/analytics-controller/src/analyticsStateComputer.test.ts index 07c04c9ab72..dd13a1a189b 100644 --- a/packages/analytics-controller/src/analyticsStateComputer.test.ts +++ b/packages/analytics-controller/src/analyticsStateComputer.test.ts @@ -1,5 +1,5 @@ -import { computeEnabledState } from './analyticsStateComputer'; import type { AnalyticsControllerState } from './AnalyticsController'; +import { computeEnabledState } from './analyticsStateComputer'; describe('analyticsStateComputer', () => { describe('computeEnabledState', () => { @@ -85,4 +85,3 @@ describe('analyticsStateComputer', () => { }); }); }); - diff --git a/packages/analytics-controller/src/analyticsStateComputer.ts b/packages/analytics-controller/src/analyticsStateComputer.ts index 5c0f4b4b5af..eff35ec3ece 100644 --- a/packages/analytics-controller/src/analyticsStateComputer.ts +++ b/packages/analytics-controller/src/analyticsStateComputer.ts @@ -10,9 +10,9 @@ import type { AnalyticsControllerState } from './AnalyticsController'; * **State Computer Computations:** * * 1. **Enabled State** (`computeEnabledState`): - * - Determines whether analytics tracking is active - * - Rules: `controller_enabled = user_optedIn || user_socialOptedIn` - * - Analytics is enabled if the user has opted in through regular account OR social account + * - Determines whether analytics tracking is active + * - Rules: `controller_enabled = user_optedIn || user_socialOptedIn` + * - Analytics is enabled if the user has opted in through regular account OR social account * * 2. **Future computations** (e.g., feature flags, permissions, etc.) * @@ -33,9 +33,7 @@ import type { AnalyticsControllerState } from './AnalyticsController'; * @param state - The current controller state * @returns `true` if analytics tracking should be enabled, `false` otherwise */ -export function computeEnabledState( - state: AnalyticsControllerState, -): boolean { +export function computeEnabledState(state: AnalyticsControllerState): boolean { // Analytics is enabled if user has opted in through regular account OR social account // Rules: // - optIn==true && socialOptIn==true -> enabled=true @@ -44,4 +42,3 @@ export function computeEnabledState( // - optIn==false && socialOptIn==false -> enabled=false return state.user_optedIn || state.user_socialOptedIn; } - From 8f93f3ff480b310d2c5919e3cedecfb708387b29 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 17 Nov 2025 17:57:04 +0100 Subject: [PATCH 06/20] fix: update AnalyticsController-method-action-types.ts --- .../src/AnalyticsController-method-action-types.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts index ecff7a59e6d..8fac3e2eb1b 100644 --- a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts +++ b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts @@ -10,8 +10,7 @@ import type { AnalyticsController } from './AnalyticsController'; * * Events are only tracked if analytics is enabled. * - * @param eventName - The name of the event - * @param properties - Event properties + * @param event - Analytics event with properties and sensitive properties */ export type AnalyticsControllerTrackEventAction = { type: `AnalyticsController:trackEvent`; From 2195150df3b709e9087f03c50d87fa90ee01578d Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Tue, 18 Nov 2025 15:57:06 +0100 Subject: [PATCH 07/20] refactor(analytics-controller): replace isAnonymous with isSensitive and extract validation - Remove isAnonymous field, derive sensitivity from sensitiveProperties presence - Extract state validation to analyticsStateValidator module (called before super) - Add selectors module for state access - Add comprehensive unit tests for validator and selectors - Update documentation for clarity --- packages/analytics-controller/README.md | 18 +- ...AnalyticsController-method-action-types.ts | 93 ++--- .../src/AnalyticsController.test.ts | 338 +++++++++--------- .../src/AnalyticsController.ts | 176 ++++----- .../src/AnalyticsPlatformAdapter.types.ts | 45 +-- .../src/analyticsStateComputer.test.ts | 58 +-- .../src/analyticsStateComputer.ts | 18 +- .../src/analyticsStateValidator.test.ts | 123 +++++++ .../src/analyticsStateValidator.ts | 25 ++ packages/analytics-controller/src/index.ts | 15 +- .../src/selectors.test.ts | 192 ++++++++++ .../analytics-controller/src/selectors.ts | 51 +++ 12 files changed, 711 insertions(+), 441 deletions(-) create mode 100644 packages/analytics-controller/src/analyticsStateValidator.test.ts create mode 100644 packages/analytics-controller/src/analyticsStateValidator.ts create mode 100644 packages/analytics-controller/src/selectors.test.ts create mode 100644 packages/analytics-controller/src/selectors.ts diff --git a/packages/analytics-controller/README.md b/packages/analytics-controller/README.md index f4942363a92..21494c58cd7 100644 --- a/packages/analytics-controller/README.md +++ b/packages/analytics-controller/README.md @@ -134,9 +134,13 @@ controller.trackView('home', { controller.enable(); controller.disable(); -// Opt in/out -controller.optIn(); -controller.optOut(); +// Opt in/out for regular account +controller.optInForRegularAccount(); +controller.optOutForRegularAccount(); + +// Opt in/out for social account +controller.optInForSocialAccount(); +controller.optOutForSocialAccount(); ``` ### 7. Use Messenger Actions @@ -157,10 +161,10 @@ messenger.call( }, ); -messenger.call('AnalyticsController:enable'); -messenger.call('AnalyticsController:disable'); -messenger.call('AnalyticsController:optIn'); -messenger.call('AnalyticsController:optOut'); +messenger.call('AnalyticsController:optInForRegularAccount'); +messenger.call('AnalyticsController:optOutForRegularAccount'); +messenger.call('AnalyticsController:optInForSocialAccount'); +messenger.call('AnalyticsController:optOutForSocialAccount'); ``` ### 8. Subscribe to State Changes diff --git a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts index 8fac3e2eb1b..b41d3401a5c 100644 --- a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts +++ b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts @@ -39,80 +39,39 @@ export type AnalyticsControllerTrackViewAction = { }; /** - * Opt in to analytics. - * This updates the user's opt-in status. + * Opt in to analytics for regular account. + * This updates the user's opt-in status for regular account. */ -export type AnalyticsControllerOptInAction = { - type: `AnalyticsController:optIn`; - handler: AnalyticsController['optIn']; +export type AnalyticsControllerOptInForRegularAccountAction = { + type: `AnalyticsController:optInForRegularAccount`; + handler: AnalyticsController['optInForRegularAccount']; }; /** - * Opt out of analytics. - * This updates the user's opt-in status. + * Opt out of analytics for regular account. + * This updates the user's opt-in status for regular account. */ -export type AnalyticsControllerOptOutAction = { - type: `AnalyticsController:optOut`; - handler: AnalyticsController['optOut']; +export type AnalyticsControllerOptOutForRegularAccountAction = { + type: `AnalyticsController:optOutForRegularAccount`; + handler: AnalyticsController['optOutForRegularAccount']; }; /** - * Opt in to analytics through social account. - * This updates the user's social opt-in status. + * Opt in to analytics for social account. + * This updates the user's opt-in status for social account. */ -export type AnalyticsControllerSocialOptInAction = { - type: `AnalyticsController:socialOptIn`; - handler: AnalyticsController['socialOptIn']; +export type AnalyticsControllerOptInForSocialAccountAction = { + type: `AnalyticsController:optInForSocialAccount`; + handler: AnalyticsController['optInForSocialAccount']; }; /** - * Opt out of analytics through social account. - * This updates the user's social opt-in status. + * Opt out of analytics for social account. + * This updates the user's opt-in status for social account. */ -export type AnalyticsControllerSocialOptOutAction = { - type: `AnalyticsController:socialOptOut`; - handler: AnalyticsController['socialOptOut']; -}; - -/** - * Get the analytics ID from the controller state. - * - * @returns The current analytics ID. - */ -export type AnalyticsControllerGetAnalyticsIdAction = { - type: `AnalyticsController:getAnalyticsId`; - handler: AnalyticsController['getAnalyticsId']; -}; - -/** - * Get the enabled status from the controller state. - * This is computed from user state via the state machine. - * - * @returns The current enabled status. - */ -export type AnalyticsControllerIsEnabledAction = { - type: `AnalyticsController:isEnabled`; - handler: AnalyticsController['isEnabled']; -}; - -/** - * Get the opted in status from the controller state. - * - * @returns The current opted in status. - */ -export type AnalyticsControllerIsOptedInAction = { - type: `AnalyticsController:isOptedIn`; - handler: AnalyticsController['isOptedIn']; -}; - -/** - * Get the social opted in status from the controller state. - * - * @returns The current social opted in status. - */ -export type AnalyticsControllerIsSocialOptedInAction = { - type: `AnalyticsController:isSocialOptedIn`; - handler: AnalyticsController['isSocialOptedIn']; +export type AnalyticsControllerOptOutForSocialAccountAction = { + type: `AnalyticsController:optOutForSocialAccount`; + handler: AnalyticsController['optOutForSocialAccount']; }; /** @@ -122,11 +81,7 @@ export type AnalyticsControllerMethodActions = | AnalyticsControllerTrackEventAction | AnalyticsControllerIdentifyAction | AnalyticsControllerTrackViewAction - | AnalyticsControllerOptInAction - | AnalyticsControllerOptOutAction - | AnalyticsControllerSocialOptInAction - | AnalyticsControllerSocialOptOutAction - | AnalyticsControllerGetAnalyticsIdAction - | AnalyticsControllerIsEnabledAction - | AnalyticsControllerIsOptedInAction - | AnalyticsControllerIsSocialOptedInAction; + | AnalyticsControllerOptInForRegularAccountAction + | AnalyticsControllerOptOutForRegularAccountAction + | AnalyticsControllerOptInForSocialAccountAction + | AnalyticsControllerOptOutForSocialAccountAction; diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index b55ba3add09..6fc83ac1dd4 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -3,7 +3,7 @@ import { MOCK_ANY_NAMESPACE, type MockAnyNamespace, } from '@metamask/messenger'; -import { validate as uuidValidate, version as uuidVersion } from 'uuid'; +import { validate as validateUuid, version as getUuidVersion } from 'uuid'; import { AnalyticsController, @@ -14,6 +14,7 @@ import { AnalyticsPlatformAdapterSetupError, getDefaultAnalyticsControllerState, type AnalyticsTrackingEvent, + analyticsControllerSelectors, } from '.'; import type { AnalyticsControllerState } from '.'; @@ -83,7 +84,7 @@ function setupController( * @returns True if the string matches UUIDv4 format */ function isValidUUIDv4(value: string): boolean { - return uuidValidate(value) && uuidVersion(value) === 4; + return validateUuid(value) && getUuidVersion(value) === 4; } /** @@ -104,7 +105,6 @@ function createTestEvent( const hasProps = Object.keys(properties).length > 0 || Object.keys(sensitiveProperties).length > 0; - const isAnon = Object.keys(sensitiveProperties).length > 0; return { name, @@ -112,7 +112,6 @@ function createTestEvent( sensitiveProperties: sensitiveProperties as AnalyticsTrackingEvent['sensitiveProperties'], saveDataRecording, - isAnonymous: isAnon, hasProperties: hasProps, }; } @@ -131,12 +130,12 @@ describe('AnalyticsController', () => { const defaultState = getDefaultAnalyticsControllerState(); - expect(controller.isEnabled()).toBe(false); - expect(controller.state.user_optedIn).toBe(defaultState.user_optedIn); - expect(controller.state.user_socialOptedIn).toBe( - defaultState.user_socialOptedIn, + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); + expect(controller.state.optedInForRegularAccount).toBe(defaultState.optedInForRegularAccount); + expect(controller.state.optedInForSocialAccount).toBe( + defaultState.optedInForSocialAccount, ); - expect(isValidUUIDv4(controller.state.user_analyticsId)).toBe(true); + expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); }); it('generates a new UUID when no state is provided (first-time initialization)', () => { @@ -144,10 +143,10 @@ describe('AnalyticsController', () => { const { controller: controller1 } = setupController(); const { controller: controller2 } = setupController(); - expect(isValidUUIDv4(controller1.state.user_analyticsId)).toBe(true); - expect(isValidUUIDv4(controller2.state.user_analyticsId)).toBe(true); - expect(controller1.state.user_analyticsId).not.toBe( - controller2.state.user_analyticsId, + expect(isValidUUIDv4(controller1.state.analyticsId)).toBe(true); + expect(isValidUUIDv4(controller2.state.analyticsId)).toBe(true); + expect(controller1.state.analyticsId).not.toBe( + controller2.state.analyticsId, ); }); @@ -155,66 +154,68 @@ describe('AnalyticsController', () => { const customId = '550e8400-e29b-41d4-a716-446655440000'; const { controller } = setupController({ state: { - user_optedIn: true, - user_socialOptedIn: false, - user_analyticsId: customId, + optedInForRegularAccount: true, + optedInForSocialAccount: false, + analyticsId: customId, }, }); - expect(controller.isEnabled()).toBe(true); - expect(controller.state.user_optedIn).toBe(true); - expect(controller.state.user_socialOptedIn).toBe(false); - expect(controller.state.user_analyticsId).toBe(customId); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); + expect(controller.state.optedInForRegularAccount).toBe(true); + expect(controller.state.optedInForSocialAccount).toBe(false); + expect(controller.state.analyticsId).toBe(customId); }); it('restores analyticsId from persisted state', () => { // First, create a controller (generates UUID) const { controller: firstController } = setupController(); - const originalAnalyticsId = firstController.state.user_analyticsId; + const originalAnalyticsId = firstController.state.analyticsId; expect(isValidUUIDv4(originalAnalyticsId)).toBe(true); // Simulate restoration: create a new controller with the previous state const { controller: restoredController } = setupController({ state: { - user_optedIn: firstController.state.user_optedIn, - user_socialOptedIn: firstController.state.user_socialOptedIn, - user_analyticsId: firstController.state.user_analyticsId, + optedInForRegularAccount: firstController.state.optedInForRegularAccount, + optedInForSocialAccount: firstController.state.optedInForSocialAccount, + analyticsId: firstController.state.analyticsId, }, }); // The restored controller should have the same state as the original controller - expect(restoredController.state.user_analyticsId).toBe( + expect(restoredController.state.analyticsId).toBe( originalAnalyticsId, ); - expect(restoredController.isEnabled()).toBe(firstController.isEnabled()); - expect(restoredController.state.user_optedIn).toBe( - firstController.state.user_optedIn, + expect(analyticsControllerSelectors.selectEnabled(restoredController.state)).toBe( + analyticsControllerSelectors.selectEnabled(firstController.state), + ); + expect(restoredController.state.optedInForRegularAccount).toBe( + firstController.state.optedInForRegularAccount, ); }); it('initializes with custom optedIn', () => { const { controller } = setupController({ state: { - user_optedIn: true, - user_socialOptedIn: false, + optedInForRegularAccount: true, + optedInForSocialAccount: false, }, }); - expect(controller.isEnabled()).toBe(true); - expect(controller.state.user_optedIn).toBe(true); - expect(controller.state.user_socialOptedIn).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); + expect(controller.state.optedInForRegularAccount).toBe(true); + expect(controller.state.optedInForSocialAccount).toBe(false); }); it('uses default analyticsId when not provided in partial state', () => { const { controller } = setupController({ state: { - user_optedIn: true, - user_socialOptedIn: false, + optedInForRegularAccount: true, + optedInForSocialAccount: false, }, }); - expect(isValidUUIDv4(controller.state.user_analyticsId)).toBe(true); + expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); }); it('initializes with default values when options are undefined', () => { @@ -241,14 +242,14 @@ describe('AnalyticsController', () => { }); const defaultState = getDefaultAnalyticsControllerState(); - expect(controller.isEnabled()).toBe(false); - expect(controller.state.user_optedIn).toBe(defaultState.user_optedIn); - expect(controller.state.user_socialOptedIn).toBe( - defaultState.user_socialOptedIn, + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); + expect(controller.state.optedInForRegularAccount).toBe(defaultState.optedInForRegularAccount); + expect(controller.state.optedInForSocialAccount).toBe( + defaultState.optedInForSocialAccount, ); // analyticsId is auto-generated (UUIDv4) - expect(typeof controller.state.user_analyticsId).toBe('string'); - expect(isValidUUIDv4(controller.state.user_analyticsId)).toBe(true); + expect(typeof controller.state.analyticsId).toBe('string'); + expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); }); }); @@ -258,7 +259,7 @@ describe('AnalyticsController', () => { const customId = '550e8400-e29b-41d4-a716-446655440000'; setupController({ - state: { user_analyticsId: customId }, + state: { analyticsId: customId }, platformAdapter: mockAdapter, }); @@ -275,9 +276,9 @@ describe('AnalyticsController', () => { expect(mockAdapter.onSetupCompleted).toHaveBeenCalledTimes(1); expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith( - controller.state.user_analyticsId, + controller.state.analyticsId, ); - expect(isValidUUIDv4(controller.state.user_analyticsId)).toBe(true); + expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); }); it('throws error when analyticsId is empty string', () => { @@ -285,7 +286,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { user_analyticsId: '' }, + state: { analyticsId: '' }, platformAdapter: mockAdapter, }); }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got ""'); @@ -298,7 +299,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { user_analyticsId: undefined as unknown as string }, + state: { analyticsId: undefined as unknown as string }, platformAdapter: mockAdapter, }); }).toThrow( @@ -313,7 +314,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { user_analyticsId: null as unknown as string }, + state: { analyticsId: null as unknown as string }, platformAdapter: mockAdapter, }); }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got null'); @@ -326,7 +327,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { user_analyticsId: 'not-a-uuid' }, + state: { analyticsId: 'not-a-uuid' }, platformAdapter: mockAdapter, }); }).toThrow( @@ -343,7 +344,7 @@ describe('AnalyticsController', () => { expect(() => { setupController({ - state: { user_analyticsId: uuidV5 }, + state: { analyticsId: uuidV5 }, platformAdapter: mockAdapter, }); }).toThrow( @@ -369,18 +370,18 @@ describe('AnalyticsController', () => { // Controller should initialize successfully despite the error const { controller } = setupController({ - state: { user_analyticsId: '550e8400-e29b-41d4-a716-446655440000' }, + state: { analyticsId: '550e8400-e29b-41d4-a716-446655440000' }, platformAdapter: mockAdapter, }); // Controller should still be initialized with correct state expect(controller).toBeDefined(); - expect(controller.state.user_analyticsId).toBe( + expect(controller.state.analyticsId).toBe( '550e8400-e29b-41d4-a716-446655440000', ); - expect(controller.isEnabled()).toBe(false); - expect(controller.state.user_optedIn).toBe(false); - expect(controller.state.user_socialOptedIn).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); + expect(controller.state.optedInForRegularAccount).toBe(false); + expect(controller.state.optedInForSocialAccount).toBe(false); }); it('calls onSetupCompleted with analyticsId', () => { @@ -388,12 +389,12 @@ describe('AnalyticsController', () => { const customId = '550e8400-e29b-41d4-a716-446655440000'; const { controller } = setupController({ - state: { user_analyticsId: customId }, + state: { analyticsId: customId }, platformAdapter: mockAdapter, }); expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith(customId); - expect(controller.state.user_analyticsId).toBe(customId); + expect(controller.state.analyticsId).toBe(customId); }); }); @@ -401,7 +402,7 @@ describe('AnalyticsController', () => { it('tracks event when enabled via regular opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { user_optedIn: true, user_socialOptedIn: false }, + state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, platformAdapter: mockAdapter, }); @@ -409,7 +410,6 @@ describe('AnalyticsController', () => { controller.trackEvent(event); expect(mockAdapter.track).toHaveBeenCalledWith('test_event', { - anonymous: false, prop: 'value', }); }); @@ -417,7 +417,7 @@ describe('AnalyticsController', () => { it('tracks event when enabled via social opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: true }, + state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, platformAdapter: mockAdapter, }); @@ -425,7 +425,6 @@ describe('AnalyticsController', () => { controller.trackEvent(event); expect(mockAdapter.track).toHaveBeenCalledWith('test_event', { - anonymous: false, prop: 'value', }); }); @@ -433,22 +432,20 @@ describe('AnalyticsController', () => { it('tracks event with no properties when hasProperties is false', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { user_optedIn: true, user_socialOptedIn: false }, + state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, platformAdapter: mockAdapter, }); const event = createTestEvent('test_event', {}, {}, true); controller.trackEvent(event); - expect(mockAdapter.track).toHaveBeenCalledWith('test_event', { - anonymous: false, - }); + expect(mockAdapter.track).toHaveBeenCalledWith('test_event'); }); - it('tracks anonymous event when isAnonymous is true', () => { + it('tracks sensitive event when sensitiveProperties are present', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { user_optedIn: true, user_socialOptedIn: false }, + state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, platformAdapter: mockAdapter, }); @@ -461,11 +458,10 @@ describe('AnalyticsController', () => { expect(mockAdapter.track).toHaveBeenCalledTimes(2); expect(mockAdapter.track).toHaveBeenNthCalledWith(1, 'test_event', { - anonymous: false, prop: 'value', }); expect(mockAdapter.track).toHaveBeenNthCalledWith(2, 'test_event', { - anonymous: true, + isSensitive: true, prop: 'value', sensitive: 'data', }); @@ -474,7 +470,7 @@ describe('AnalyticsController', () => { it('does not track event when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: false }, + state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, platformAdapter: mockAdapter, }); @@ -491,9 +487,9 @@ describe('AnalyticsController', () => { const customId = '550e8400-e29b-41d4-a716-446655440000'; const { controller } = setupController({ state: { - user_analyticsId: customId, - user_optedIn: true, - user_socialOptedIn: false, + analyticsId: customId, + optedInForRegularAccount: true, + optedInForSocialAccount: false, }, platformAdapter: mockAdapter, }); @@ -505,14 +501,14 @@ describe('AnalyticsController', () => { controller.identify(traits); - expect(controller.state.user_analyticsId).toBe(customId); + expect(controller.state.analyticsId).toBe(customId); expect(mockAdapter.identify).toHaveBeenCalledWith(customId, traits); }); it('does not identify when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: false }, + state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, platformAdapter: mockAdapter, }); @@ -531,7 +527,7 @@ describe('AnalyticsController', () => { it('tracks view', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { user_optedIn: true, user_socialOptedIn: false }, + state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, platformAdapter: mockAdapter, }); @@ -545,7 +541,7 @@ describe('AnalyticsController', () => { it('does not track view when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: false }, + state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, platformAdapter: mockAdapter, }); @@ -555,213 +551,211 @@ describe('AnalyticsController', () => { }); }); - describe('optIn', () => { - it('opts in to analytics and enables tracking', () => { + describe('optInForRegularAccount', () => { + it('opts in to analytics via regular account and enables tracking', () => { const { controller } = setupController(); - expect(controller.state.user_optedIn).toBe(false); - expect(controller.isEnabled()).toBe(false); + expect(controller.state.optedInForRegularAccount).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - controller.optIn(); + controller.optInForRegularAccount(); - expect(controller.state.user_optedIn).toBe(true); - expect(controller.isEnabled()).toBe(true); + expect(controller.state.optedInForRegularAccount).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); - describe('optOut', () => { - it('opts out of analytics and disables tracking when social opt-in is false', () => { + describe('optOutForRegularAccount', () => { + it('opts out of analytics via regular account and disables tracking when social opt-in is false', () => { const { controller } = setupController({ - state: { user_optedIn: true, user_socialOptedIn: false }, + state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, }); - expect(controller.state.user_optedIn).toBe(true); - expect(controller.isEnabled()).toBe(true); + expect(controller.state.optedInForRegularAccount).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - controller.optOut(); + controller.optOutForRegularAccount(); - expect(controller.state.user_optedIn).toBe(false); - expect(controller.isEnabled()).toBe(false); + expect(controller.state.optedInForRegularAccount).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); }); - it('opts out of analytics but keeps tracking enabled when social opt-in is true', () => { + it('opts out of analytics via regular account but keeps tracking enabled when social opt-in is true', () => { const { controller } = setupController({ - state: { user_optedIn: true, user_socialOptedIn: true }, + state: { optedInForRegularAccount: true, optedInForSocialAccount: true }, }); - expect(controller.state.user_optedIn).toBe(true); - expect(controller.isEnabled()).toBe(true); + expect(controller.state.optedInForRegularAccount).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - controller.optOut(); + controller.optOutForRegularAccount(); - expect(controller.state.user_optedIn).toBe(false); - expect(controller.isEnabled()).toBe(true); + expect(controller.state.optedInForRegularAccount).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); - describe('socialOptIn', () => { - it('opts in to analytics through social account and enables tracking', () => { + describe('optInForSocialAccount', () => { + it('opts in to analytics via social account and enables tracking', () => { const { controller } = setupController(); - expect(controller.state.user_socialOptedIn).toBe(false); - expect(controller.isEnabled()).toBe(false); + expect(controller.state.optedInForSocialAccount).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - controller.socialOptIn(); + controller.optInForSocialAccount(); - expect(controller.state.user_socialOptedIn).toBe(true); - expect(controller.isEnabled()).toBe(true); + expect(controller.state.optedInForSocialAccount).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); - describe('socialOptOut', () => { - it('opts out of analytics through social account and disables tracking when regular opt-in is false', () => { + describe('optOutForSocialAccount', () => { + it('opts out of analytics via social account and disables tracking when regular opt-in is false', () => { const { controller } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: true }, + state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, }); - expect(controller.state.user_socialOptedIn).toBe(true); - expect(controller.isEnabled()).toBe(true); + expect(controller.state.optedInForSocialAccount).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - controller.socialOptOut(); + controller.optOutForSocialAccount(); - expect(controller.state.user_socialOptedIn).toBe(false); - expect(controller.isEnabled()).toBe(false); + expect(controller.state.optedInForSocialAccount).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); }); - it('opts out of analytics through social account but keeps tracking enabled when regular opt-in is true', () => { + it('opts out of analytics via social account but keeps tracking enabled when regular opt-in is true', () => { const { controller } = setupController({ - state: { user_optedIn: true, user_socialOptedIn: true }, + state: { optedInForRegularAccount: true, optedInForSocialAccount: true }, }); - expect(controller.state.user_socialOptedIn).toBe(true); - expect(controller.isEnabled()).toBe(true); + expect(controller.state.optedInForSocialAccount).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - controller.socialOptOut(); + controller.optOutForSocialAccount(); - expect(controller.state.user_socialOptedIn).toBe(false); - expect(controller.isEnabled()).toBe(true); + expect(controller.state.optedInForSocialAccount).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); - describe('getAnalyticsId', () => { + describe('selectAnalyticsId', () => { it('returns analytics ID', () => { const customId = '550e8400-e29b-41d4-a716-446655440000'; - const { controller, messenger } = setupController({ - state: { user_analyticsId: customId }, + const { controller } = setupController({ + state: { analyticsId: customId }, }); - const analyticsId = messenger.call('AnalyticsController:getAnalyticsId'); + const analyticsId = analyticsControllerSelectors.selectAnalyticsId(controller.state); expect(analyticsId).toBe(customId); - expect(analyticsId).toBe(controller.state.user_analyticsId); + expect(analyticsId).toBe(controller.state.analyticsId); }); }); - describe('isEnabled', () => { + describe('selectEnabled', () => { it('returns enabled status computed from user state', () => { - const { controller, messenger } = setupController({ - state: { user_optedIn: true, user_socialOptedIn: false }, + const { controller } = setupController({ + state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, }); - const isEnabled = messenger.call('AnalyticsController:isEnabled'); + const isEnabled = analyticsControllerSelectors.selectEnabled(controller.state); expect(isEnabled).toBe(true); - expect(isEnabled).toBe(controller.isEnabled()); + expect(isEnabled).toBe(analyticsControllerSelectors.selectEnabled(controller.state)); }); it('returns true when only social opt-in is true', () => { - const { controller, messenger } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: true }, + const { controller } = setupController({ + state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, }); - const isEnabled = messenger.call('AnalyticsController:isEnabled'); + const isEnabled = analyticsControllerSelectors.selectEnabled(controller.state); expect(isEnabled).toBe(true); - expect(isEnabled).toBe(controller.isEnabled()); + expect(isEnabled).toBe(analyticsControllerSelectors.selectEnabled(controller.state)); }); it('returns updated value when user state changes', () => { - const { controller, messenger } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: false }, + const { controller } = setupController({ + state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, }); - expect(messenger.call('AnalyticsController:isEnabled')).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - controller.optIn(); + controller.optInForRegularAccount(); - expect(messenger.call('AnalyticsController:isEnabled')).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - controller.optOut(); + controller.optOutForRegularAccount(); - expect(messenger.call('AnalyticsController:isEnabled')).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - controller.socialOptIn(); + controller.optInForSocialAccount(); - expect(messenger.call('AnalyticsController:isEnabled')).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - controller.socialOptOut(); + controller.optOutForSocialAccount(); - expect(messenger.call('AnalyticsController:isEnabled')).toBe(false); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); }); }); - describe('isOptedIn', () => { + describe('selectOptedIn', () => { it('returns opted in status from state', () => { - const { controller, messenger } = setupController({ - state: { user_optedIn: true, user_socialOptedIn: false }, + const { controller } = setupController({ + state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, }); - const isOptedIn = messenger.call('AnalyticsController:isOptedIn'); + const isOptedIn = analyticsControllerSelectors.selectOptedIn(controller.state); expect(isOptedIn).toBe(true); - expect(isOptedIn).toBe(controller.state.user_optedIn); + expect(isOptedIn).toBe(controller.state.optedInForRegularAccount); }); it('returns updated value when state changes', () => { - const { controller, messenger } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: false }, + const { controller } = setupController({ + state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, }); - expect(messenger.call('AnalyticsController:isOptedIn')).toBe(false); + expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe(false); - controller.optIn(); + controller.optInForRegularAccount(); - expect(messenger.call('AnalyticsController:isOptedIn')).toBe(true); + expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe(true); - controller.optOut(); + controller.optOutForRegularAccount(); - expect(messenger.call('AnalyticsController:isOptedIn')).toBe(false); + expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe(false); }); }); - describe('isSocialOptedIn', () => { + describe('selectSocialOptedIn', () => { it('returns social opted in status from state', () => { - const { controller, messenger } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: true }, + const { controller } = setupController({ + state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, }); - const isSocialOptedIn = messenger.call( - 'AnalyticsController:isSocialOptedIn', - ); + const isSocialOptedIn = analyticsControllerSelectors.selectSocialOptedIn(controller.state); expect(isSocialOptedIn).toBe(true); - expect(isSocialOptedIn).toBe(controller.state.user_socialOptedIn); + expect(isSocialOptedIn).toBe(controller.state.optedInForSocialAccount); }); it('returns updated value when state changes', () => { - const { controller, messenger } = setupController({ - state: { user_optedIn: false, user_socialOptedIn: false }, + const { controller } = setupController({ + state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, }); - expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe(false); + expect(analyticsControllerSelectors.selectSocialOptedIn(controller.state)).toBe(false); - controller.socialOptIn(); + controller.optInForSocialAccount(); - expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe(true); + expect(analyticsControllerSelectors.selectSocialOptedIn(controller.state)).toBe(true); - controller.socialOptOut(); + controller.optOutForSocialAccount(); - expect(messenger.call('AnalyticsController:isSocialOptedIn')).toBe(false); + expect(analyticsControllerSelectors.selectSocialOptedIn(controller.state)).toBe(false); }); }); }); diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index 875381eadcb..7c22d689f3f 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -5,11 +5,7 @@ import type { } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; import type { Messenger } from '@metamask/messenger'; -import { - v4 as uuidv4, - validate as uuidValidate, - version as uuidVersion, -} from 'uuid'; +import { v4 as uuidv4 } from 'uuid'; import type { AnalyticsControllerMethodActions } from './AnalyticsController-method-action-types'; import { projectLogger } from './AnalyticsLogger'; @@ -20,6 +16,7 @@ import type { AnalyticsTrackingEvent, } from './AnalyticsPlatformAdapter.types'; import { computeEnabledState } from './analyticsStateComputer'; +import { validateAnalyticsState } from './analyticsStateValidator'; // === GENERAL === @@ -37,38 +34,38 @@ export const controllerName = 'AnalyticsController'; */ export type AnalyticsControllerState = { /** - * User domain: Whether the user has opted in to analytics. + * Whether the user has opted in to analytics for regular account. */ - user_optedIn: boolean; + optedInForRegularAccount: boolean; /** - * User domain: Whether the user has opted in to analytics through social account. + * Whether the user has opted in to analytics for social account. */ - user_socialOptedIn: boolean; + optedInForSocialAccount: boolean; /** - * User domain: User's UUIDv4 analytics identifier. + * User's UUIDv4 analytics identifier. */ - user_analyticsId: string; + analyticsId: string; }; /** * The metadata for each property in {@link AnalyticsControllerState}. */ const analyticsControllerMetadata = { - user_optedIn: { + optedInForRegularAccount: { includeInStateLogs: true, persist: true, includeInDebugSnapshot: true, usedInUi: true, }, - user_socialOptedIn: { + optedInForSocialAccount: { includeInStateLogs: true, persist: true, includeInDebugSnapshot: true, usedInUi: true, }, - user_analyticsId: { + analyticsId: { includeInStateLogs: true, persist: true, includeInDebugSnapshot: true, @@ -86,9 +83,9 @@ const analyticsControllerMetadata = { */ export function getDefaultAnalyticsControllerState(): AnalyticsControllerState { return { - user_optedIn: false, - user_socialOptedIn: false, - user_analyticsId: uuidv4(), + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: uuidv4(), }; } @@ -98,14 +95,10 @@ const MESSENGER_EXPOSED_METHODS = [ 'trackEvent', 'identify', 'trackView', - 'optIn', - 'optOut', - 'socialOptIn', - 'socialOptOut', - 'getAnalyticsId', - 'isEnabled', - 'isOptedIn', - 'isSocialOptedIn', + 'optInForRegularAccount', + 'optOutForRegularAccount', + 'optInForSocialAccount', + 'optOutForSocialAccount', ] as const; /** @@ -192,7 +185,7 @@ export class AnalyticsController extends BaseController< * * @param options - Controller options * @param options.state - Initial controller state (defaults from getDefaultAnalyticsControllerState). - * For migration from a previous system, pass the existing analytics ID via state.user_analyticsId. + * For migration from a previous system, pass the existing analytics ID via state.analyticsId. * @param options.messenger - Messenger used to communicate with BaseController * @param options.platformAdapter - Platform adapter implementation for tracking */ @@ -206,6 +199,8 @@ export class AnalyticsController extends BaseController< ...state, }; + validateAnalyticsState(initialState); + super({ name: controllerName, metadata: analyticsControllerMetadata, @@ -222,29 +217,18 @@ export class AnalyticsController extends BaseController< projectLogger('AnalyticsController initialized and ready', { enabled: computeEnabledState(this.state), - optedIn: this.state.user_optedIn, - socialOptedIn: this.state.user_socialOptedIn, - analyticsId: this.state.user_analyticsId, + optedIn: this.state.optedInForRegularAccount, + socialOptedIn: this.state.optedInForSocialAccount, + analyticsId: this.state.analyticsId, }); // Call onSetupCompleted lifecycle hook after initialization - // Only call if analyticsId is set and is a valid UUIDv4 (this is the definition of "completed" setup) - if ( - this.state.user_analyticsId && - uuidValidate(this.state.user_analyticsId) && - uuidVersion(this.state.user_analyticsId) === 4 - ) { - try { - this.#platformAdapter.onSetupCompleted(this.state.user_analyticsId); - } catch (error) { - // Log error but don't throw - adapter setup failure shouldn't break controller - projectLogger('Error calling platformAdapter.onSetupCompleted', error); - } - } else { - // analyticsId is undefined, null, empty string, or not a valid UUIDv4 - throw new Error( - `Invalid analyticsId: expected a valid UUIDv4, but got ${JSON.stringify(this.state.user_analyticsId)}`, - ); + // State is already validated, so analyticsId is guaranteed to be a valid UUIDv4 + try { + this.#platformAdapter.onSetupCompleted(this.state.analyticsId); + } catch (error) { + // Log error but don't throw - adapter setup failure shouldn't break controller + projectLogger('Error calling platformAdapter.onSetupCompleted', error); } } @@ -261,29 +245,29 @@ export class AnalyticsController extends BaseController< return; } - // if event does not have properties, only send the non-anonymous empty event + // Derive sensitivity from presence of sensitiveProperties + const hasSensitiveProperties = + Object.keys(event.sensitiveProperties).length > 0; + + // if event does not have properties, send event without properties // and return to prevent any additional processing if (!event.hasProperties) { - this.#platformAdapter.track(event.name, { - anonymous: false, - } as AnalyticsEventProperties); - + this.#platformAdapter.track(event.name); return; } - // Log all non-anonymous properties, or an empty event if there's no non-anon props. + // Track regular properties (without isSensitive flag - it's the default) this.#platformAdapter.track(event.name, { - anonymous: false, ...event.properties, - } as AnalyticsEventProperties); + }); - // Track all sensitive properties in an anonymous event - if (event.isAnonymous) { + // Track sensitive properties in a separate event with isSensitive flag + if (hasSensitiveProperties) { this.#platformAdapter.track(event.name, { - anonymous: true, + isSensitive: true, ...event.properties, ...event.sensitiveProperties, - } as AnalyticsEventProperties); + }); } } @@ -299,15 +283,15 @@ export class AnalyticsController extends BaseController< // Delegate to platform adapter if supported, using the current analytics ID if (this.#platformAdapter.identify) { - this.#platformAdapter.identify(this.state.user_analyticsId, traits); + this.#platformAdapter.identify(this.state.analyticsId, traits); } } /** - * Track a page view. + * Track a page or screen view. * - * @param name - The name of the UI item being viewed (pages for web, screen for mobile) - * @param properties - UI item properties + * @param name - The identifier/name of the page or screen being viewed (e.g., "home", "settings", "wallet") + * @param properties - Optional properties associated with the view */ trackView(name: string, properties?: AnalyticsEventProperties): void { if (!computeEnabledState(this.state)) { @@ -319,79 +303,43 @@ export class AnalyticsController extends BaseController< } /** - * Opt in to analytics. - * This updates the user's opt-in status. + * Opt in to analytics for regular account. + * This updates the user's opt-in status for regular account. */ - optIn(): void { + optInForRegularAccount(): void { this.update((state) => { - state.user_optedIn = true; + state.optedInForRegularAccount = true; }); } /** - * Opt out of analytics. - * This updates the user's opt-in status. + * Opt out of analytics for regular account. + * This updates the user's opt-in status for regular account. */ - optOut(): void { + optOutForRegularAccount(): void { this.update((state) => { - state.user_optedIn = false; + state.optedInForRegularAccount = false; }); } /** - * Opt in to analytics through social account. - * This updates the user's social opt-in status. + * Opt in to analytics for social account. + * This updates the user's opt-in status for social account. */ - socialOptIn(): void { + optInForSocialAccount(): void { this.update((state) => { - state.user_socialOptedIn = true; + state.optedInForSocialAccount = true; }); } /** - * Opt out of analytics through social account. - * This updates the user's social opt-in status. + * Opt out of analytics for social account. + * This updates the user's opt-in status for social account. */ - socialOptOut(): void { + optOutForSocialAccount(): void { this.update((state) => { - state.user_socialOptedIn = false; + state.optedInForSocialAccount = false; }); } - /** - * Get the analytics ID from the controller state. - * - * @returns The current analytics ID. - */ - getAnalyticsId(): string { - return this.state.user_analyticsId; - } - - /** - * Get the enabled status from the controller state. - * This is computed from user state via the state machine. - * - * @returns The current enabled status. - */ - isEnabled(): boolean { - return computeEnabledState(this.state); - } - - /** - * Get the opted in status from the controller state. - * - * @returns The current opted in status. - */ - isOptedIn(): boolean { - return this.state.user_optedIn; - } - - /** - * Get the social opted in status from the controller state. - * - * @returns The current social opted in status. - */ - isSocialOptedIn(): boolean { - return this.state.user_socialOptedIn; - } } diff --git a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts index 1e3a7d1dd86..2fca86a9793 100644 --- a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts +++ b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts @@ -1,27 +1,5 @@ import type { Json } from '@metamask/utils'; -/** - * Represents values that can be passed as properties to the event tracking function. - * Similar to JsonValue from Segment SDK but decoupled for platform agnosticism. - */ -type AnalyticsJsonValue = - | boolean - | number - | string - | null - | AnalyticsJsonValue[] - | AnalyticsJsonMap - | undefined; - -/** - * Represents the map object used to pass properties to the event tracking function. - * Similar to JsonMap from Segment SDK but decoupled for platform agnosticism. - */ -type AnalyticsJsonMap = { - [key: string]: AnalyticsJsonValue; - [index: number]: AnalyticsJsonValue; -}; - /** * Analytics event properties */ @@ -33,15 +11,15 @@ export type AnalyticsEventProperties = Record; export type AnalyticsUserTraits = Record; /** - * Event properties structure with two distinct properties lists for anonymous and non-anonymous data. + * Event properties structure with two distinct properties lists for regular and sensitive data. * Similar to ITrackingEvent from legacy analytics but decoupled for platform agnosticism. + * Sensitivity is derived from the presence of sensitiveProperties (if sensitiveProperties has keys, the event is sensitive). */ export type AnalyticsTrackingEvent = { readonly name: string; - properties: AnalyticsJsonMap; - sensitiveProperties: AnalyticsJsonMap; + properties: AnalyticsEventProperties; + sensitiveProperties: AnalyticsEventProperties; saveDataRecording: boolean; - readonly isAnonymous: boolean; readonly hasProperties: boolean; }; @@ -56,9 +34,10 @@ export type AnalyticsPlatformAdapter = { * This is the same as trackEvent in the old analytics system * * @param eventName - The name of the event - * @param properties - Event properties + * @param properties - Event properties. If not provided, the event has no properties. + * The privacy plugin should check for `isSensitive === true` to determine if an event contains sensitive data. */ - track(eventName: string, properties: AnalyticsEventProperties): void; + track(eventName: string, properties?: AnalyticsEventProperties): void; /** * Identify a user with traits. @@ -71,12 +50,12 @@ export type AnalyticsPlatformAdapter = { /** * Track a UI unit (page or screen) view depending on the platform * - * This is the same as page in Segment web SDK and screen in Segment mobile SDK. - * Each platform adapter should implement this method to track UI views - * using the appropriate method for the platform: "pages" for web, "screen" for mobile. + * This method delegates to platform-specific Segment SDK methods: + * - Web adapters should call `analytics.page(name, properties)` + * - Mobile adapters should call `analytics.screen(name, properties)` * - * @param name - The name of the UI item being viewed (pages for web, screen for mobile) - * @param properties - Page properties + * @param name - The identifier/name of the page or screen being viewed (e.g., "home", "settings", "wallet") + * @param properties - Optional properties associated with the view */ view(name: string, properties?: AnalyticsEventProperties): void; diff --git a/packages/analytics-controller/src/analyticsStateComputer.test.ts b/packages/analytics-controller/src/analyticsStateComputer.test.ts index dd13a1a189b..4cd58c85bee 100644 --- a/packages/analytics-controller/src/analyticsStateComputer.test.ts +++ b/packages/analytics-controller/src/analyticsStateComputer.test.ts @@ -5,11 +5,11 @@ describe('analyticsStateComputer', () => { describe('computeEnabledState', () => { const defaultAnalyticsId = '550e8400-e29b-41d4-a716-446655440000'; - it('returns true when optIn=true and socialOptIn=true', () => { + it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=true', () => { const state: AnalyticsControllerState = { - user_optedIn: true, - user_socialOptedIn: true, - user_analyticsId: defaultAnalyticsId, + optedInForRegularAccount: true, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, }; const result = computeEnabledState(state); @@ -17,11 +17,11 @@ describe('analyticsStateComputer', () => { expect(result).toBe(true); }); - it('returns true when optIn=false and socialOptIn=true', () => { + it('returns true when optedInForRegularAccount=false and optedInForSocialAccount=true', () => { const state: AnalyticsControllerState = { - user_optedIn: false, - user_socialOptedIn: true, - user_analyticsId: defaultAnalyticsId, + optedInForRegularAccount: false, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, }; const result = computeEnabledState(state); @@ -29,11 +29,11 @@ describe('analyticsStateComputer', () => { expect(result).toBe(true); }); - it('returns true when optIn=true and socialOptIn=false', () => { + it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=false', () => { const state: AnalyticsControllerState = { - user_optedIn: true, - user_socialOptedIn: false, - user_analyticsId: defaultAnalyticsId, + optedInForRegularAccount: true, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, }; const result = computeEnabledState(state); @@ -41,11 +41,11 @@ describe('analyticsStateComputer', () => { expect(result).toBe(true); }); - it('returns false when optIn=false and socialOptIn=false', () => { + it('returns false when optedInForRegularAccount=false and optedInForSocialAccount=false', () => { const state: AnalyticsControllerState = { - user_optedIn: false, - user_socialOptedIn: false, - user_analyticsId: defaultAnalyticsId, + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, }; const result = computeEnabledState(state); @@ -53,29 +53,29 @@ describe('analyticsStateComputer', () => { expect(result).toBe(false); }); - it('computes enabled state based on user_optedIn OR user_socialOptedIn', () => { + it('computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount', () => { const bothOptedIn: AnalyticsControllerState = { - user_optedIn: true, - user_socialOptedIn: true, - user_analyticsId: defaultAnalyticsId, + optedInForRegularAccount: true, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, }; const onlyRegularOptedIn: AnalyticsControllerState = { - user_optedIn: true, - user_socialOptedIn: false, - user_analyticsId: defaultAnalyticsId, + optedInForRegularAccount: true, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, }; const onlySocialOptedIn: AnalyticsControllerState = { - user_optedIn: false, - user_socialOptedIn: true, - user_analyticsId: defaultAnalyticsId, + optedInForRegularAccount: false, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, }; const neitherOptedIn: AnalyticsControllerState = { - user_optedIn: false, - user_socialOptedIn: false, - user_analyticsId: defaultAnalyticsId, + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, }; expect(computeEnabledState(bothOptedIn)).toBe(true); diff --git a/packages/analytics-controller/src/analyticsStateComputer.ts b/packages/analytics-controller/src/analyticsStateComputer.ts index eff35ec3ece..c84d08f735b 100644 --- a/packages/analytics-controller/src/analyticsStateComputer.ts +++ b/packages/analytics-controller/src/analyticsStateComputer.ts @@ -11,15 +11,15 @@ import type { AnalyticsControllerState } from './AnalyticsController'; * * 1. **Enabled State** (`computeEnabledState`): * - Determines whether analytics tracking is active - * - Rules: `controller_enabled = user_optedIn || user_socialOptedIn` - * - Analytics is enabled if the user has opted in through regular account OR social account + * - Rules: `controller_enabled = optedInForRegularAccount || optedInForSocialAccount` + * - Analytics is enabled if the user has opted in for regular account OR social account * * 2. **Future computations** (e.g., feature flags, permissions, etc.) * * **Usage:** * These functions are called: * - During controller initialization to set initial values - * - Whenever user state changes (e.g., in `optIn()`, `optOut()`) + * - Whenever user state changes (e.g., in `optInForRegularAccount()`, `optOutForRegularAccount()`, `optInForSocialAccount()`, `optOutForSocialAccount()`) * * **Extensibility:** * To add new computations, add new functions that take `AnalyticsControllerState` as input. @@ -34,11 +34,11 @@ import type { AnalyticsControllerState } from './AnalyticsController'; * @returns `true` if analytics tracking should be enabled, `false` otherwise */ export function computeEnabledState(state: AnalyticsControllerState): boolean { - // Analytics is enabled if user has opted in through regular account OR social account + // Analytics is enabled if user has opted in for regular account OR social account // Rules: - // - optIn==true && socialOptIn==true -> enabled=true - // - optIn==false && socialOptIn==true -> enabled=true - // - optIn==true && socialOptIn==false -> enabled=true - // - optIn==false && socialOptIn==false -> enabled=false - return state.user_optedIn || state.user_socialOptedIn; + // - optedInForRegularAccount==true && optedInForSocialAccount==true -> enabled=true + // - optedInForRegularAccount==false && optedInForSocialAccount==true -> enabled=true + // - optedInForRegularAccount==true && optedInForSocialAccount==false -> enabled=true + // - optedInForRegularAccount==false && optedInForSocialAccount==false -> enabled=false + return state.optedInForRegularAccount || state.optedInForSocialAccount; } diff --git a/packages/analytics-controller/src/analyticsStateValidator.test.ts b/packages/analytics-controller/src/analyticsStateValidator.test.ts new file mode 100644 index 00000000000..6bcb43d1a25 --- /dev/null +++ b/packages/analytics-controller/src/analyticsStateValidator.test.ts @@ -0,0 +1,123 @@ +import type { AnalyticsControllerState } from './AnalyticsController'; +import { validateAnalyticsState } from './analyticsStateValidator'; + +describe('analyticsStateValidator', () => { + describe('validateAnalyticsState', () => { + const validUUIDv4 = '550e8400-e29b-41d4-a716-446655440000'; + + it('does not throw when analyticsId is a valid UUIDv4', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: validUUIDv4, + }; + + expect(() => validateAnalyticsState(state)).not.toThrow(); + }); + + it('throws when analyticsId is undefined', () => { + const state = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: undefined, + } as unknown as AnalyticsControllerState; + + expect(() => validateAnalyticsState(state)).toThrow( + 'Invalid analyticsId: expected a valid UUIDv4, but got undefined', + ); + }); + + it('throws when analyticsId is null', () => { + const state = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: null, + } as unknown as AnalyticsControllerState; + + expect(() => validateAnalyticsState(state)).toThrow( + 'Invalid analyticsId: expected a valid UUIDv4, but got null', + ); + }); + + it('throws when analyticsId is an empty string', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: '', + }; + + expect(() => validateAnalyticsState(state)).toThrow( + 'Invalid analyticsId: expected a valid UUIDv4, but got ""', + ); + }); + + it('throws when analyticsId is not a valid UUID', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: 'not-a-uuid', + }; + + expect(() => validateAnalyticsState(state)).toThrow( + 'Invalid analyticsId: expected a valid UUIDv4, but got "not-a-uuid"', + ); + }); + + it('throws when analyticsId is a valid UUID but not version 4', () => { + // UUIDv1 example + const uuidV1 = 'c232ab00-9414-11e8-8eb2-f2801f1b9fd1'; + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: uuidV1, + }; + + expect(() => validateAnalyticsState(state)).toThrow( + `Invalid analyticsId: expected a valid UUIDv4, but got "${uuidV1}"`, + ); + }); + + it('throws with correct error message format for various invalid inputs', () => { + const testCases = [ + { analyticsId: undefined, expectedMessage: 'undefined' }, + { analyticsId: null, expectedMessage: 'null' }, + { analyticsId: '', expectedMessage: '""' }, + { analyticsId: 'invalid', expectedMessage: '"invalid"' }, + { analyticsId: '12345', expectedMessage: '"12345"' }, + { analyticsId: '550e8400-e29b-41d4-a716', expectedMessage: '"550e8400-e29b-41d4-a716"' }, + ]; + + testCases.forEach(({ analyticsId, expectedMessage }) => { + const state = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId, + } as unknown as AnalyticsControllerState; + + expect(() => validateAnalyticsState(state)).toThrow( + `Invalid analyticsId: expected a valid UUIDv4, but got ${expectedMessage}`, + ); + }); + }); + + it('validates different valid UUIDv4 formats', () => { + const validUUIDs = [ + '550e8400-e29b-41d4-a716-446655440000', + '123e4567-e89b-42d3-a456-426614174000', + '00000000-0000-4000-8000-000000000000', + 'ffffffff-ffff-4fff-8fff-ffffffffffff', + ]; + + validUUIDs.forEach((uuid) => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: uuid, + }; + + expect(() => validateAnalyticsState(state)).not.toThrow(); + }); + }); + }); +}); + diff --git a/packages/analytics-controller/src/analyticsStateValidator.ts b/packages/analytics-controller/src/analyticsStateValidator.ts new file mode 100644 index 00000000000..59ff42091ff --- /dev/null +++ b/packages/analytics-controller/src/analyticsStateValidator.ts @@ -0,0 +1,25 @@ +import { + validate as validateUuid, + version as getUuidVersion, +} from 'uuid'; + +import type { AnalyticsControllerState } from './AnalyticsController'; + +/** + * Validates that the analytics state has a valid UUIDv4 analyticsId. + * + * @param state - The analytics controller state to validate + * @throws {Error} If analyticsId is missing, invalid, or not a UUIDv4 + */ +export function validateAnalyticsState(state: AnalyticsControllerState): void { + if ( + !state.analyticsId || + !validateUuid(state.analyticsId) || + getUuidVersion(state.analyticsId) !== 4 + ) { + throw new Error( + `Invalid analyticsId: expected a valid UUIDv4, but got ${JSON.stringify(state.analyticsId)}`, + ); + } +} + diff --git a/packages/analytics-controller/src/index.ts b/packages/analytics-controller/src/index.ts index 7608de0d185..e477ffe1490 100644 --- a/packages/analytics-controller/src/index.ts +++ b/packages/analytics-controller/src/index.ts @@ -17,6 +17,9 @@ export type { export type { AnalyticsControllerState } from './AnalyticsController'; export { getDefaultAnalyticsControllerState } from './AnalyticsController'; +// Export selectors +export { analyticsControllerSelectors } from './selectors'; + // Export messenger types export type { AnalyticsControllerMessenger } from './AnalyticsController'; @@ -32,13 +35,9 @@ export type { AnalyticsControllerTrackEventAction, AnalyticsControllerIdentifyAction, AnalyticsControllerTrackViewAction, - AnalyticsControllerOptInAction, - AnalyticsControllerOptOutAction, - AnalyticsControllerSocialOptInAction, - AnalyticsControllerSocialOptOutAction, - AnalyticsControllerGetAnalyticsIdAction, - AnalyticsControllerIsEnabledAction, - AnalyticsControllerIsOptedInAction, - AnalyticsControllerIsSocialOptedInAction, + AnalyticsControllerOptInForRegularAccountAction, + AnalyticsControllerOptOutForRegularAccountAction, + AnalyticsControllerOptInForSocialAccountAction, + AnalyticsControllerOptOutForSocialAccountAction, AnalyticsControllerMethodActions, } from './AnalyticsController-method-action-types'; diff --git a/packages/analytics-controller/src/selectors.test.ts b/packages/analytics-controller/src/selectors.test.ts new file mode 100644 index 00000000000..5649fa122ed --- /dev/null +++ b/packages/analytics-controller/src/selectors.test.ts @@ -0,0 +1,192 @@ +import type { AnalyticsControllerState } from './AnalyticsController'; +import { analyticsControllerSelectors } from './selectors'; + +describe('analyticsControllerSelectors', () => { + const defaultAnalyticsId = '550e8400-e29b-41d4-a716-446655440000'; + + describe('selectAnalyticsId', () => { + it('returns the analyticsId from state', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectAnalyticsId(state); + + expect(result).toBe(defaultAnalyticsId); + }); + + it('returns different analyticsId when state has different value', () => { + const differentId = '123e4567-e89b-42d3-a456-426614174000'; + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: differentId, + }; + + const result = analyticsControllerSelectors.selectAnalyticsId(state); + + expect(result).toBe(differentId); + expect(result).not.toBe(defaultAnalyticsId); + }); + }); + + describe('selectOptedIn', () => { + it('returns true when optedInForRegularAccount is true', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectOptedIn(state); + + expect(result).toBe(true); + }); + + it('returns false when optedInForRegularAccount is false', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectOptedIn(state); + + expect(result).toBe(false); + }); + + it('returns false even when optedInForSocialAccount is true', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectOptedIn(state); + + expect(result).toBe(false); + }); + }); + + describe('selectSocialOptedIn', () => { + it('returns true when optedInForSocialAccount is true', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectSocialOptedIn(state); + + expect(result).toBe(true); + }); + + it('returns false when optedInForSocialAccount is false', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectSocialOptedIn(state); + + expect(result).toBe(false); + }); + + it('returns false even when optedInForRegularAccount is true', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectSocialOptedIn(state); + + expect(result).toBe(false); + }); + }); + + describe('selectEnabled', () => { + it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=true', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: true, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectEnabled(state); + + expect(result).toBe(true); + }); + + it('returns true when optedInForRegularAccount=false and optedInForSocialAccount=true', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectEnabled(state); + + expect(result).toBe(true); + }); + + it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=false', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectEnabled(state); + + expect(result).toBe(true); + }); + + it('returns false when optedInForRegularAccount=false and optedInForSocialAccount=false', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectEnabled(state); + + expect(result).toBe(false); + }); + + it('computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount', () => { + const bothOptedIn: AnalyticsControllerState = { + optedInForRegularAccount: true, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, + }; + + const onlyRegularOptedIn: AnalyticsControllerState = { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const onlySocialOptedIn: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: true, + analyticsId: defaultAnalyticsId, + }; + + const neitherOptedIn: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + expect(analyticsControllerSelectors.selectEnabled(bothOptedIn)).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(onlyRegularOptedIn)).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(onlySocialOptedIn)).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(neitherOptedIn)).toBe(false); + }); + }); +}); + diff --git a/packages/analytics-controller/src/selectors.ts b/packages/analytics-controller/src/selectors.ts new file mode 100644 index 00000000000..a742db03cf3 --- /dev/null +++ b/packages/analytics-controller/src/selectors.ts @@ -0,0 +1,51 @@ +import type { AnalyticsControllerState } from './AnalyticsController'; +import { computeEnabledState } from './analyticsStateComputer'; + +/** + * Selects the analytics ID from the controller state. + * + * @param state - The controller state + * @returns The analytics ID + */ +const selectAnalyticsId = (state: AnalyticsControllerState): string => + state.analyticsId; + +/** + * Selects the opted-in status from the controller state. + * + * @param state - The controller state + * @returns Whether the user has opted in for regular account + */ +const selectOptedIn = (state: AnalyticsControllerState): boolean => + state.optedInForRegularAccount; + +/** + * Selects the social opted-in status from the controller state. + * + * @param state - The controller state + * @returns Whether the user has opted in for social account + */ +const selectSocialOptedIn = (state: AnalyticsControllerState): boolean => + state.optedInForSocialAccount; + +/** + * Selects the enabled status from the controller state. + * This is computed from user state via the state computer. + * + * @param state - The controller state + * @returns Whether analytics tracking is enabled + */ +const selectEnabled = (state: AnalyticsControllerState): boolean => + computeEnabledState(state); + +/** + * Selectors for the AnalyticsController state. + * These can be used with Redux or directly with controller state. + */ +export const analyticsControllerSelectors = { + selectAnalyticsId, + selectOptedIn, + selectSocialOptedIn, + selectEnabled, +}; + From bc3f7ddb80672064d5caeb4d089d0b0b3de8f189 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Tue, 18 Nov 2025 18:31:08 +0100 Subject: [PATCH 08/20] refactor(analytics-controller): improve test quality and reduce redundancy --- .../src/AnalyticsController.test.ts | 331 +++--------------- .../src/analyticsStateComputer.test.ts | 101 ++---- .../src/analyticsStateValidator.test.ts | 123 ++----- .../src/selectors.test.ts | 159 ++------- 4 files changed, 149 insertions(+), 565 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index 6fc83ac1dd4..320c974bfe1 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -17,6 +17,7 @@ import { analyticsControllerSelectors, } from '.'; import type { AnalyticsControllerState } from '.'; +import * as analyticsStateComputer from './analyticsStateComputer'; type SetupControllerOptions = { state?: Partial; @@ -37,7 +38,7 @@ type SetupControllerReturn = { function setupController( options: SetupControllerOptions = {}, ): SetupControllerReturn { - const { state = {}, platformAdapter } = options; + const { state, platformAdapter } = options; // Create default mock adapter if not provided const adapter = @@ -125,7 +126,7 @@ describe('AnalyticsController', () => { }); describe('constructor', () => { - it('initializes with default state including auto-generated analyticsId', () => { + it('initializes with default state', () => { const { controller } = setupController(); const defaultState = getDefaultAnalyticsControllerState(); @@ -138,7 +139,7 @@ describe('AnalyticsController', () => { expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); }); - it('generates a new UUID when no state is provided (first-time initialization)', () => { + it('generates a new analyticsId when no state is provided', () => { // Create two controllers without providing state const { controller: controller1 } = setupController(); const { controller: controller2 } = setupController(); @@ -166,35 +167,7 @@ describe('AnalyticsController', () => { expect(controller.state.analyticsId).toBe(customId); }); - it('restores analyticsId from persisted state', () => { - // First, create a controller (generates UUID) - const { controller: firstController } = setupController(); - const originalAnalyticsId = firstController.state.analyticsId; - - expect(isValidUUIDv4(originalAnalyticsId)).toBe(true); - - // Simulate restoration: create a new controller with the previous state - const { controller: restoredController } = setupController({ - state: { - optedInForRegularAccount: firstController.state.optedInForRegularAccount, - optedInForSocialAccount: firstController.state.optedInForSocialAccount, - analyticsId: firstController.state.analyticsId, - }, - }); - - // The restored controller should have the same state as the original controller - expect(restoredController.state.analyticsId).toBe( - originalAnalyticsId, - ); - expect(analyticsControllerSelectors.selectEnabled(restoredController.state)).toBe( - analyticsControllerSelectors.selectEnabled(firstController.state), - ); - expect(restoredController.state.optedInForRegularAccount).toBe( - firstController.state.optedInForRegularAccount, - ); - }); - - it('initializes with custom optedIn', () => { + it('initializes with custom opt-in settings', () => { const { controller } = setupController({ state: { optedInForRegularAccount: true, @@ -206,71 +179,15 @@ describe('AnalyticsController', () => { expect(controller.state.optedInForRegularAccount).toBe(true); expect(controller.state.optedInForSocialAccount).toBe(false); }); - - it('uses default analyticsId when not provided in partial state', () => { - const { controller } = setupController({ - state: { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - }, - }); - - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); - }); - - it('initializes with default values when options are undefined', () => { - const mockAdapter = createMockAdapter(); - const rootMessenger = new Messenger< - MockAnyNamespace, - AnalyticsControllerActions, - AnalyticsControllerEvents - >({ namespace: MOCK_ANY_NAMESPACE }); - - const analyticsControllerMessenger = new Messenger< - 'AnalyticsController', - AnalyticsControllerActions, - AnalyticsControllerEvents, - typeof rootMessenger - >({ - namespace: 'AnalyticsController', - parent: rootMessenger, - }); - - const controller = new AnalyticsController({ - messenger: analyticsControllerMessenger, - platformAdapter: mockAdapter, - }); - - const defaultState = getDefaultAnalyticsControllerState(); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - expect(controller.state.optedInForRegularAccount).toBe(defaultState.optedInForRegularAccount); - expect(controller.state.optedInForSocialAccount).toBe( - defaultState.optedInForSocialAccount, - ); - // analyticsId is auto-generated (UUIDv4) - expect(typeof controller.state.analyticsId).toBe('string'); - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); - }); }); describe('onSetupCompleted lifecycle hook', () => { - it('calls onSetupCompleted after initialization when analyticsId is set', () => { + it('calls onSetupCompleted with analyticsId after initialization', () => { const mockAdapter = createMockAdapter(); const customId = '550e8400-e29b-41d4-a716-446655440000'; - setupController({ - state: { analyticsId: customId }, - platformAdapter: mockAdapter, - }); - - expect(mockAdapter.onSetupCompleted).toHaveBeenCalledTimes(1); - expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith(customId); - }); - - it('calls onSetupCompleted with auto-generated analyticsId when not provided', () => { - const mockAdapter = createMockAdapter(); - const { controller } = setupController({ + state: { analyticsId: customId }, platformAdapter: mockAdapter, }); @@ -278,85 +195,10 @@ describe('AnalyticsController', () => { expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith( controller.state.analyticsId, ); - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); - }); - - it('throws error when analyticsId is empty string', () => { - const mockAdapter = createMockAdapter(); - - expect(() => { - setupController({ - state: { analyticsId: '' }, - platformAdapter: mockAdapter, - }); - }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got ""'); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); }); - it('throws error when analyticsId is undefined', () => { + it('returns controller when onSetupCompleted throws error', () => { const mockAdapter = createMockAdapter(); - - expect(() => { - setupController({ - state: { analyticsId: undefined as unknown as string }, - platformAdapter: mockAdapter, - }); - }).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got undefined', - ); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); - }); - - it('throws error when analyticsId is null', () => { - const mockAdapter = createMockAdapter(); - - expect(() => { - setupController({ - state: { analyticsId: null as unknown as string }, - platformAdapter: mockAdapter, - }); - }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got null'); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); - }); - - it('throws error when analyticsId is not a valid UUIDv4', () => { - const mockAdapter = createMockAdapter(); - - expect(() => { - setupController({ - state: { analyticsId: 'not-a-uuid' }, - platformAdapter: mockAdapter, - }); - }).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got "not-a-uuid"', - ); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); - }); - - it('throws error when analyticsId is a valid UUID but not v4', () => { - const mockAdapter = createMockAdapter(); - // UUIDv5 example - const uuidV5 = 'c87ee674-4ddc-5efe-bd81-0b5a0b4d45b6'; - - expect(() => { - setupController({ - state: { analyticsId: uuidV5 }, - platformAdapter: mockAdapter, - }); - }).toThrow( - `Invalid analyticsId: expected a valid UUIDv4, but got "${uuidV5}"`, - ); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); - }); - - it('continues controller initialization when onSetupCompleted throws error', () => { - const mockAdapter = createMockAdapter(); - // Simulate a fake Segment SDK plugin configuration error const cause = new Error( 'MetaMetricsPrivacySegmentPlugin configure failed', ); @@ -368,33 +210,13 @@ describe('AnalyticsController', () => { throw error; }); - // Controller should initialize successfully despite the error const { controller } = setupController({ state: { analyticsId: '550e8400-e29b-41d4-a716-446655440000' }, platformAdapter: mockAdapter, }); - // Controller should still be initialized with correct state expect(controller).toBeDefined(); - expect(controller.state.analyticsId).toBe( - '550e8400-e29b-41d4-a716-446655440000', - ); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - expect(controller.state.optedInForRegularAccount).toBe(false); - expect(controller.state.optedInForSocialAccount).toBe(false); - }); - - it('calls onSetupCompleted with analyticsId', () => { - const mockAdapter = createMockAdapter(); - const customId = '550e8400-e29b-41d4-a716-446655440000'; - - const { controller } = setupController({ - state: { analyticsId: customId }, - platformAdapter: mockAdapter, - }); - - expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith(customId); - expect(controller.state.analyticsId).toBe(customId); + expect(mockAdapter.onSetupCompleted).toHaveBeenCalledTimes(1); }); }); @@ -429,7 +251,7 @@ describe('AnalyticsController', () => { }); }); - it('tracks event with no properties when hasProperties is false', () => { + it('tracks event without properties when event has no properties', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, @@ -452,7 +274,7 @@ describe('AnalyticsController', () => { const event = createTestEvent( 'test_event', { prop: 'value' }, - { sensitive: 'data' }, + { sensitive_prop: 'sensitive value' }, ); controller.trackEvent(event); @@ -463,7 +285,7 @@ describe('AnalyticsController', () => { expect(mockAdapter.track).toHaveBeenNthCalledWith(2, 'test_event', { isSensitive: true, prop: 'value', - sensitive: 'data', + sensitive_prop: 'sensitive value', }); }); @@ -505,6 +327,23 @@ describe('AnalyticsController', () => { expect(mockAdapter.identify).toHaveBeenCalledWith(customId, traits); }); + it('identifies user without traits', () => { + const mockAdapter = createMockAdapter(); + const customId = '550e8400-e29b-41d4-a716-446655440000'; + const { controller } = setupController({ + state: { + analyticsId: customId, + optedInForRegularAccount: true, + optedInForSocialAccount: false, + }, + platformAdapter: mockAdapter, + }); + + controller.identify(); + + expect(mockAdapter.identify).toHaveBeenCalledWith(customId, undefined); + }); + it('does not identify when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ @@ -552,90 +391,54 @@ describe('AnalyticsController', () => { }); describe('optInForRegularAccount', () => { - it('opts in to analytics via regular account and enables tracking', () => { + it('sets optedInForRegularAccount to true', () => { const { controller } = setupController(); expect(controller.state.optedInForRegularAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); controller.optInForRegularAccount(); expect(controller.state.optedInForRegularAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); describe('optOutForRegularAccount', () => { - it('opts out of analytics via regular account and disables tracking when social opt-in is false', () => { + it('sets optedInForRegularAccount to false', () => { const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, + state: { optedInForRegularAccount: true }, }); expect(controller.state.optedInForRegularAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - - controller.optOutForRegularAccount(); - - expect(controller.state.optedInForRegularAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - }); - - it('opts out of analytics via regular account but keeps tracking enabled when social opt-in is true', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: true }, - }); - - expect(controller.state.optedInForRegularAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); controller.optOutForRegularAccount(); expect(controller.state.optedInForRegularAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); describe('optInForSocialAccount', () => { - it('opts in to analytics via social account and enables tracking', () => { + it('sets optedInForSocialAccount to true', () => { const { controller } = setupController(); expect(controller.state.optedInForSocialAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); controller.optInForSocialAccount(); expect(controller.state.optedInForSocialAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); describe('optOutForSocialAccount', () => { - it('opts out of analytics via social account and disables tracking when regular opt-in is false', () => { + it('sets optedInForSocialAccount to false', () => { const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, + state: { optedInForSocialAccount: true }, }); expect(controller.state.optedInForSocialAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); controller.optOutForSocialAccount(); expect(controller.state.optedInForSocialAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - }); - - it('opts out of analytics via social account but keeps tracking enabled when regular opt-in is true', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: true }, - }); - - expect(controller.state.optedInForSocialAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - - controller.optOutForSocialAccount(); - - expect(controller.state.optedInForSocialAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); @@ -654,55 +457,29 @@ describe('AnalyticsController', () => { }); describe('selectEnabled', () => { - it('returns enabled status computed from user state', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, - }); - - const isEnabled = analyticsControllerSelectors.selectEnabled(controller.state); - - expect(isEnabled).toBe(true); - expect(isEnabled).toBe(analyticsControllerSelectors.selectEnabled(controller.state)); - }); - - it('returns true when only social opt-in is true', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, - }); - - const isEnabled = analyticsControllerSelectors.selectEnabled(controller.state); - - expect(isEnabled).toBe(true); - expect(isEnabled).toBe(analyticsControllerSelectors.selectEnabled(controller.state)); - }); - - it('returns updated value when user state changes', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, - }); - - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - - controller.optInForRegularAccount(); - - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - - controller.optOutForRegularAccount(); - - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - - controller.optInForSocialAccount(); - - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); + it.each([[true], [false]])( + 'returns %s from computeEnabledState', + (expectedValue) => { + jest + .spyOn(analyticsStateComputer, 'computeEnabledState') + .mockReturnValue(expectedValue); + + const { controller } = setupController({ + state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, + }); - controller.optOutForSocialAccount(); + const isEnabled = analyticsControllerSelectors.selectEnabled(controller.state); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - }); + expect(analyticsStateComputer.computeEnabledState).toHaveBeenCalledWith( + controller.state, + ); + expect(isEnabled).toBe(expectedValue); + }, + ); }); - describe('selectOptedIn', () => { - it('returns opted in status from state', () => { + describe('selectRegularAccountOptedIn', () => { + it('returns regular account opted in status from state', () => { const { controller } = setupController({ state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, }); diff --git a/packages/analytics-controller/src/analyticsStateComputer.test.ts b/packages/analytics-controller/src/analyticsStateComputer.test.ts index 4cd58c85bee..7bf44eba293 100644 --- a/packages/analytics-controller/src/analyticsStateComputer.test.ts +++ b/packages/analytics-controller/src/analyticsStateComputer.test.ts @@ -5,83 +5,44 @@ describe('analyticsStateComputer', () => { describe('computeEnabledState', () => { const defaultAnalyticsId = '550e8400-e29b-41d4-a716-446655440000'; - it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=true', () => { - const state: AnalyticsControllerState = { + it.each([ + { optedInForRegularAccount: true, optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = computeEnabledState(state); - - expect(result).toBe(true); - }); - - it('returns true when optedInForRegularAccount=false and optedInForSocialAccount=true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = computeEnabledState(state); - - expect(result).toBe(true); - }); - - it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=false', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = computeEnabledState(state); - - expect(result).toBe(true); - }); - - it('returns false when optedInForRegularAccount=false and optedInForSocialAccount=false', () => { - const state: AnalyticsControllerState = { + expectedEnabled: true, + description: 'both opt-ins are true', + }, + { optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = computeEnabledState(state); - - expect(result).toBe(false); - }); - - it('computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount', () => { - const bothOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: true, optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const onlyRegularOptedIn: AnalyticsControllerState = { + expectedEnabled: true, + description: 'only social opt-in is true', + }, + { optedInForRegularAccount: true, optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const onlySocialOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const neitherOptedIn: AnalyticsControllerState = { + expectedEnabled: true, + description: 'only regular opt-in is true', + }, + { optedInForRegularAccount: false, optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - expect(computeEnabledState(bothOptedIn)).toBe(true); - expect(computeEnabledState(onlyRegularOptedIn)).toBe(true); - expect(computeEnabledState(onlySocialOptedIn)).toBe(true); - expect(computeEnabledState(neitherOptedIn)).toBe(false); - }); + expectedEnabled: false, + description: 'both opt-ins are false', + }, + ])( + 'computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount when $description', + ({ optedInForRegularAccount, optedInForSocialAccount, expectedEnabled }) => { + const state: AnalyticsControllerState = { + optedInForRegularAccount, + optedInForSocialAccount, + analyticsId: defaultAnalyticsId, + }; + + const result = computeEnabledState(state); + + expect(result).toBe(expectedEnabled); + }, + ); }); }); diff --git a/packages/analytics-controller/src/analyticsStateValidator.test.ts b/packages/analytics-controller/src/analyticsStateValidator.test.ts index 6bcb43d1a25..ca0debd5778 100644 --- a/packages/analytics-controller/src/analyticsStateValidator.test.ts +++ b/packages/analytics-controller/src/analyticsStateValidator.test.ts @@ -15,79 +15,24 @@ describe('analyticsStateValidator', () => { expect(() => validateAnalyticsState(state)).not.toThrow(); }); - it('throws when analyticsId is undefined', () => { - const state = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: undefined, - } as unknown as AnalyticsControllerState; - - expect(() => validateAnalyticsState(state)).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got undefined', - ); - }); - - it('throws when analyticsId is null', () => { - const state = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: null, - } as unknown as AnalyticsControllerState; - - expect(() => validateAnalyticsState(state)).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got null', - ); - }); - - it('throws when analyticsId is an empty string', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: '', - }; - - expect(() => validateAnalyticsState(state)).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got ""', - ); - }); - - it('throws when analyticsId is not a valid UUID', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: 'not-a-uuid', - }; + it.each([ + [undefined], + [null], + [''], + ['not-a-uuid'], + ['12345'], + ['550e8400-e29b-41d4-a716'], + ['c232ab00-9414-11e8-8eb2-f2801f1b9fd1'], + ])( + 'throws with correct error message format for invalid input: %s', + (analyticsId) => { + const expectedMessage = + analyticsId === undefined + ? 'undefined' + : analyticsId === null + ? 'null' + : JSON.stringify(analyticsId); - expect(() => validateAnalyticsState(state)).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got "not-a-uuid"', - ); - }); - - it('throws when analyticsId is a valid UUID but not version 4', () => { - // UUIDv1 example - const uuidV1 = 'c232ab00-9414-11e8-8eb2-f2801f1b9fd1'; - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: uuidV1, - }; - - expect(() => validateAnalyticsState(state)).toThrow( - `Invalid analyticsId: expected a valid UUIDv4, but got "${uuidV1}"`, - ); - }); - - it('throws with correct error message format for various invalid inputs', () => { - const testCases = [ - { analyticsId: undefined, expectedMessage: 'undefined' }, - { analyticsId: null, expectedMessage: 'null' }, - { analyticsId: '', expectedMessage: '""' }, - { analyticsId: 'invalid', expectedMessage: '"invalid"' }, - { analyticsId: '12345', expectedMessage: '"12345"' }, - { analyticsId: '550e8400-e29b-41d4-a716', expectedMessage: '"550e8400-e29b-41d4-a716"' }, - ]; - - testCases.forEach(({ analyticsId, expectedMessage }) => { const state = { optedInForRegularAccount: false, optedInForSocialAccount: false, @@ -97,26 +42,22 @@ describe('analyticsStateValidator', () => { expect(() => validateAnalyticsState(state)).toThrow( `Invalid analyticsId: expected a valid UUIDv4, but got ${expectedMessage}`, ); - }); - }); - - it('validates different valid UUIDv4 formats', () => { - const validUUIDs = [ - '550e8400-e29b-41d4-a716-446655440000', - '123e4567-e89b-42d3-a456-426614174000', - '00000000-0000-4000-8000-000000000000', - 'ffffffff-ffff-4fff-8fff-ffffffffffff', - ]; - - validUUIDs.forEach((uuid) => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: uuid, - }; + }, + ); + + it.each([ + ['550e8400-e29b-41d4-a716-446655440000'], + ['123e4567-e89b-42d3-a456-426614174000'], + ['00000000-0000-4000-8000-000000000000'], + ['ffffffff-ffff-4fff-8fff-ffffffffffff'], + ])('validates valid UUIDv4 format: %s', (uuid) => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: uuid, + }; - expect(() => validateAnalyticsState(state)).not.toThrow(); - }); + expect(() => validateAnalyticsState(state)).not.toThrow(); }); }); }); diff --git a/packages/analytics-controller/src/selectors.test.ts b/packages/analytics-controller/src/selectors.test.ts index 5649fa122ed..10b24b8edc0 100644 --- a/packages/analytics-controller/src/selectors.test.ts +++ b/packages/analytics-controller/src/selectors.test.ts @@ -1,5 +1,6 @@ import type { AnalyticsControllerState } from './AnalyticsController'; import { analyticsControllerSelectors } from './selectors'; +import * as analyticsStateComputer from './analyticsStateComputer'; describe('analyticsControllerSelectors', () => { const defaultAnalyticsId = '550e8400-e29b-41d4-a716-446655440000'; @@ -16,20 +17,6 @@ describe('analyticsControllerSelectors', () => { expect(result).toBe(defaultAnalyticsId); }); - - it('returns different analyticsId when state has different value', () => { - const differentId = '123e4567-e89b-42d3-a456-426614174000'; - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: differentId, - }; - - const result = analyticsControllerSelectors.selectAnalyticsId(state); - - expect(result).toBe(differentId); - expect(result).not.toBe(defaultAnalyticsId); - }); }); describe('selectOptedIn', () => { @@ -71,122 +58,40 @@ describe('analyticsControllerSelectors', () => { }); describe('selectSocialOptedIn', () => { - it('returns true when optedInForSocialAccount is true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectSocialOptedIn(state); - - expect(result).toBe(true); - }); - - it('returns false when optedInForSocialAccount is false', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectSocialOptedIn(state); - - expect(result).toBe(false); - }); - - it('returns false even when optedInForRegularAccount is true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectSocialOptedIn(state); - - expect(result).toBe(false); - }); + it.each([[true], [false]])( + 'returns %s when optedInForSocialAccount is %s', + (optedInForSocialAccount) => { + const state: AnalyticsControllerState = { + optedInForSocialAccount, + }; + + const result = analyticsControllerSelectors.selectSocialOptedIn(state); + + expect(result).toBe(optedInForSocialAccount); + }, + ); }); describe('selectEnabled', () => { - it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectEnabled(state); - - expect(result).toBe(true); - }); - - it('returns true when optedInForRegularAccount=false and optedInForSocialAccount=true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectEnabled(state); - - expect(result).toBe(true); - }); - - it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=false', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectEnabled(state); - - expect(result).toBe(true); - }); - - it('returns false when optedInForRegularAccount=false and optedInForSocialAccount=false', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectEnabled(state); - - expect(result).toBe(false); - }); - - it('computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount', () => { - const bothOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const onlyRegularOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const onlySocialOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const neitherOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - expect(analyticsControllerSelectors.selectEnabled(bothOptedIn)).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(onlyRegularOptedIn)).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(onlySocialOptedIn)).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(neitherOptedIn)).toBe(false); - }); + it.each([[true], [false]])( + 'returns %s from computeEnabledState', + (expectedValue) => { + jest + .spyOn(analyticsStateComputer, 'computeEnabledState') + .mockReturnValue(expectedValue); + + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectEnabled(state); + + expect(analyticsStateComputer.computeEnabledState).toHaveBeenCalledWith(state); + expect(result).toBe(expectedValue); + }, + ); }); }); From 01e0924ece7e58fd8edb491f76d9527b356ff9c6 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Tue, 18 Nov 2025 18:31:08 +0100 Subject: [PATCH 09/20] refactor(analytics-controller): improve test quality and reduce redundancy --- .../src/AnalyticsController.test.ts | 331 +++--------------- .../src/analyticsStateComputer.test.ts | 101 ++---- .../src/analyticsStateValidator.test.ts | 123 ++----- .../src/selectors.test.ts | 161 ++------- 4 files changed, 151 insertions(+), 565 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index 6fc83ac1dd4..320c974bfe1 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -17,6 +17,7 @@ import { analyticsControllerSelectors, } from '.'; import type { AnalyticsControllerState } from '.'; +import * as analyticsStateComputer from './analyticsStateComputer'; type SetupControllerOptions = { state?: Partial; @@ -37,7 +38,7 @@ type SetupControllerReturn = { function setupController( options: SetupControllerOptions = {}, ): SetupControllerReturn { - const { state = {}, platformAdapter } = options; + const { state, platformAdapter } = options; // Create default mock adapter if not provided const adapter = @@ -125,7 +126,7 @@ describe('AnalyticsController', () => { }); describe('constructor', () => { - it('initializes with default state including auto-generated analyticsId', () => { + it('initializes with default state', () => { const { controller } = setupController(); const defaultState = getDefaultAnalyticsControllerState(); @@ -138,7 +139,7 @@ describe('AnalyticsController', () => { expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); }); - it('generates a new UUID when no state is provided (first-time initialization)', () => { + it('generates a new analyticsId when no state is provided', () => { // Create two controllers without providing state const { controller: controller1 } = setupController(); const { controller: controller2 } = setupController(); @@ -166,35 +167,7 @@ describe('AnalyticsController', () => { expect(controller.state.analyticsId).toBe(customId); }); - it('restores analyticsId from persisted state', () => { - // First, create a controller (generates UUID) - const { controller: firstController } = setupController(); - const originalAnalyticsId = firstController.state.analyticsId; - - expect(isValidUUIDv4(originalAnalyticsId)).toBe(true); - - // Simulate restoration: create a new controller with the previous state - const { controller: restoredController } = setupController({ - state: { - optedInForRegularAccount: firstController.state.optedInForRegularAccount, - optedInForSocialAccount: firstController.state.optedInForSocialAccount, - analyticsId: firstController.state.analyticsId, - }, - }); - - // The restored controller should have the same state as the original controller - expect(restoredController.state.analyticsId).toBe( - originalAnalyticsId, - ); - expect(analyticsControllerSelectors.selectEnabled(restoredController.state)).toBe( - analyticsControllerSelectors.selectEnabled(firstController.state), - ); - expect(restoredController.state.optedInForRegularAccount).toBe( - firstController.state.optedInForRegularAccount, - ); - }); - - it('initializes with custom optedIn', () => { + it('initializes with custom opt-in settings', () => { const { controller } = setupController({ state: { optedInForRegularAccount: true, @@ -206,71 +179,15 @@ describe('AnalyticsController', () => { expect(controller.state.optedInForRegularAccount).toBe(true); expect(controller.state.optedInForSocialAccount).toBe(false); }); - - it('uses default analyticsId when not provided in partial state', () => { - const { controller } = setupController({ - state: { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - }, - }); - - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); - }); - - it('initializes with default values when options are undefined', () => { - const mockAdapter = createMockAdapter(); - const rootMessenger = new Messenger< - MockAnyNamespace, - AnalyticsControllerActions, - AnalyticsControllerEvents - >({ namespace: MOCK_ANY_NAMESPACE }); - - const analyticsControllerMessenger = new Messenger< - 'AnalyticsController', - AnalyticsControllerActions, - AnalyticsControllerEvents, - typeof rootMessenger - >({ - namespace: 'AnalyticsController', - parent: rootMessenger, - }); - - const controller = new AnalyticsController({ - messenger: analyticsControllerMessenger, - platformAdapter: mockAdapter, - }); - - const defaultState = getDefaultAnalyticsControllerState(); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - expect(controller.state.optedInForRegularAccount).toBe(defaultState.optedInForRegularAccount); - expect(controller.state.optedInForSocialAccount).toBe( - defaultState.optedInForSocialAccount, - ); - // analyticsId is auto-generated (UUIDv4) - expect(typeof controller.state.analyticsId).toBe('string'); - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); - }); }); describe('onSetupCompleted lifecycle hook', () => { - it('calls onSetupCompleted after initialization when analyticsId is set', () => { + it('calls onSetupCompleted with analyticsId after initialization', () => { const mockAdapter = createMockAdapter(); const customId = '550e8400-e29b-41d4-a716-446655440000'; - setupController({ - state: { analyticsId: customId }, - platformAdapter: mockAdapter, - }); - - expect(mockAdapter.onSetupCompleted).toHaveBeenCalledTimes(1); - expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith(customId); - }); - - it('calls onSetupCompleted with auto-generated analyticsId when not provided', () => { - const mockAdapter = createMockAdapter(); - const { controller } = setupController({ + state: { analyticsId: customId }, platformAdapter: mockAdapter, }); @@ -278,85 +195,10 @@ describe('AnalyticsController', () => { expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith( controller.state.analyticsId, ); - expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); - }); - - it('throws error when analyticsId is empty string', () => { - const mockAdapter = createMockAdapter(); - - expect(() => { - setupController({ - state: { analyticsId: '' }, - platformAdapter: mockAdapter, - }); - }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got ""'); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); }); - it('throws error when analyticsId is undefined', () => { + it('returns controller when onSetupCompleted throws error', () => { const mockAdapter = createMockAdapter(); - - expect(() => { - setupController({ - state: { analyticsId: undefined as unknown as string }, - platformAdapter: mockAdapter, - }); - }).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got undefined', - ); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); - }); - - it('throws error when analyticsId is null', () => { - const mockAdapter = createMockAdapter(); - - expect(() => { - setupController({ - state: { analyticsId: null as unknown as string }, - platformAdapter: mockAdapter, - }); - }).toThrow('Invalid analyticsId: expected a valid UUIDv4, but got null'); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); - }); - - it('throws error when analyticsId is not a valid UUIDv4', () => { - const mockAdapter = createMockAdapter(); - - expect(() => { - setupController({ - state: { analyticsId: 'not-a-uuid' }, - platformAdapter: mockAdapter, - }); - }).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got "not-a-uuid"', - ); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); - }); - - it('throws error when analyticsId is a valid UUID but not v4', () => { - const mockAdapter = createMockAdapter(); - // UUIDv5 example - const uuidV5 = 'c87ee674-4ddc-5efe-bd81-0b5a0b4d45b6'; - - expect(() => { - setupController({ - state: { analyticsId: uuidV5 }, - platformAdapter: mockAdapter, - }); - }).toThrow( - `Invalid analyticsId: expected a valid UUIDv4, but got "${uuidV5}"`, - ); - - expect(mockAdapter.onSetupCompleted).not.toHaveBeenCalled(); - }); - - it('continues controller initialization when onSetupCompleted throws error', () => { - const mockAdapter = createMockAdapter(); - // Simulate a fake Segment SDK plugin configuration error const cause = new Error( 'MetaMetricsPrivacySegmentPlugin configure failed', ); @@ -368,33 +210,13 @@ describe('AnalyticsController', () => { throw error; }); - // Controller should initialize successfully despite the error const { controller } = setupController({ state: { analyticsId: '550e8400-e29b-41d4-a716-446655440000' }, platformAdapter: mockAdapter, }); - // Controller should still be initialized with correct state expect(controller).toBeDefined(); - expect(controller.state.analyticsId).toBe( - '550e8400-e29b-41d4-a716-446655440000', - ); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - expect(controller.state.optedInForRegularAccount).toBe(false); - expect(controller.state.optedInForSocialAccount).toBe(false); - }); - - it('calls onSetupCompleted with analyticsId', () => { - const mockAdapter = createMockAdapter(); - const customId = '550e8400-e29b-41d4-a716-446655440000'; - - const { controller } = setupController({ - state: { analyticsId: customId }, - platformAdapter: mockAdapter, - }); - - expect(mockAdapter.onSetupCompleted).toHaveBeenCalledWith(customId); - expect(controller.state.analyticsId).toBe(customId); + expect(mockAdapter.onSetupCompleted).toHaveBeenCalledTimes(1); }); }); @@ -429,7 +251,7 @@ describe('AnalyticsController', () => { }); }); - it('tracks event with no properties when hasProperties is false', () => { + it('tracks event without properties when event has no properties', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, @@ -452,7 +274,7 @@ describe('AnalyticsController', () => { const event = createTestEvent( 'test_event', { prop: 'value' }, - { sensitive: 'data' }, + { sensitive_prop: 'sensitive value' }, ); controller.trackEvent(event); @@ -463,7 +285,7 @@ describe('AnalyticsController', () => { expect(mockAdapter.track).toHaveBeenNthCalledWith(2, 'test_event', { isSensitive: true, prop: 'value', - sensitive: 'data', + sensitive_prop: 'sensitive value', }); }); @@ -505,6 +327,23 @@ describe('AnalyticsController', () => { expect(mockAdapter.identify).toHaveBeenCalledWith(customId, traits); }); + it('identifies user without traits', () => { + const mockAdapter = createMockAdapter(); + const customId = '550e8400-e29b-41d4-a716-446655440000'; + const { controller } = setupController({ + state: { + analyticsId: customId, + optedInForRegularAccount: true, + optedInForSocialAccount: false, + }, + platformAdapter: mockAdapter, + }); + + controller.identify(); + + expect(mockAdapter.identify).toHaveBeenCalledWith(customId, undefined); + }); + it('does not identify when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ @@ -552,90 +391,54 @@ describe('AnalyticsController', () => { }); describe('optInForRegularAccount', () => { - it('opts in to analytics via regular account and enables tracking', () => { + it('sets optedInForRegularAccount to true', () => { const { controller } = setupController(); expect(controller.state.optedInForRegularAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); controller.optInForRegularAccount(); expect(controller.state.optedInForRegularAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); describe('optOutForRegularAccount', () => { - it('opts out of analytics via regular account and disables tracking when social opt-in is false', () => { + it('sets optedInForRegularAccount to false', () => { const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, + state: { optedInForRegularAccount: true }, }); expect(controller.state.optedInForRegularAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - - controller.optOutForRegularAccount(); - - expect(controller.state.optedInForRegularAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - }); - - it('opts out of analytics via regular account but keeps tracking enabled when social opt-in is true', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: true }, - }); - - expect(controller.state.optedInForRegularAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); controller.optOutForRegularAccount(); expect(controller.state.optedInForRegularAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); describe('optInForSocialAccount', () => { - it('opts in to analytics via social account and enables tracking', () => { + it('sets optedInForSocialAccount to true', () => { const { controller } = setupController(); expect(controller.state.optedInForSocialAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); controller.optInForSocialAccount(); expect(controller.state.optedInForSocialAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); describe('optOutForSocialAccount', () => { - it('opts out of analytics via social account and disables tracking when regular opt-in is false', () => { + it('sets optedInForSocialAccount to false', () => { const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, + state: { optedInForSocialAccount: true }, }); expect(controller.state.optedInForSocialAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); controller.optOutForSocialAccount(); expect(controller.state.optedInForSocialAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - }); - - it('opts out of analytics via social account but keeps tracking enabled when regular opt-in is true', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: true }, - }); - - expect(controller.state.optedInForSocialAccount).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - - controller.optOutForSocialAccount(); - - expect(controller.state.optedInForSocialAccount).toBe(false); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); }); }); @@ -654,55 +457,29 @@ describe('AnalyticsController', () => { }); describe('selectEnabled', () => { - it('returns enabled status computed from user state', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, - }); - - const isEnabled = analyticsControllerSelectors.selectEnabled(controller.state); - - expect(isEnabled).toBe(true); - expect(isEnabled).toBe(analyticsControllerSelectors.selectEnabled(controller.state)); - }); - - it('returns true when only social opt-in is true', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, - }); - - const isEnabled = analyticsControllerSelectors.selectEnabled(controller.state); - - expect(isEnabled).toBe(true); - expect(isEnabled).toBe(analyticsControllerSelectors.selectEnabled(controller.state)); - }); - - it('returns updated value when user state changes', () => { - const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, - }); - - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - - controller.optInForRegularAccount(); - - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); - - controller.optOutForRegularAccount(); - - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - - controller.optInForSocialAccount(); - - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); + it.each([[true], [false]])( + 'returns %s from computeEnabledState', + (expectedValue) => { + jest + .spyOn(analyticsStateComputer, 'computeEnabledState') + .mockReturnValue(expectedValue); + + const { controller } = setupController({ + state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, + }); - controller.optOutForSocialAccount(); + const isEnabled = analyticsControllerSelectors.selectEnabled(controller.state); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - }); + expect(analyticsStateComputer.computeEnabledState).toHaveBeenCalledWith( + controller.state, + ); + expect(isEnabled).toBe(expectedValue); + }, + ); }); - describe('selectOptedIn', () => { - it('returns opted in status from state', () => { + describe('selectRegularAccountOptedIn', () => { + it('returns regular account opted in status from state', () => { const { controller } = setupController({ state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, }); diff --git a/packages/analytics-controller/src/analyticsStateComputer.test.ts b/packages/analytics-controller/src/analyticsStateComputer.test.ts index 4cd58c85bee..7bf44eba293 100644 --- a/packages/analytics-controller/src/analyticsStateComputer.test.ts +++ b/packages/analytics-controller/src/analyticsStateComputer.test.ts @@ -5,83 +5,44 @@ describe('analyticsStateComputer', () => { describe('computeEnabledState', () => { const defaultAnalyticsId = '550e8400-e29b-41d4-a716-446655440000'; - it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=true', () => { - const state: AnalyticsControllerState = { + it.each([ + { optedInForRegularAccount: true, optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = computeEnabledState(state); - - expect(result).toBe(true); - }); - - it('returns true when optedInForRegularAccount=false and optedInForSocialAccount=true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = computeEnabledState(state); - - expect(result).toBe(true); - }); - - it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=false', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = computeEnabledState(state); - - expect(result).toBe(true); - }); - - it('returns false when optedInForRegularAccount=false and optedInForSocialAccount=false', () => { - const state: AnalyticsControllerState = { + expectedEnabled: true, + description: 'both opt-ins are true', + }, + { optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = computeEnabledState(state); - - expect(result).toBe(false); - }); - - it('computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount', () => { - const bothOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: true, optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const onlyRegularOptedIn: AnalyticsControllerState = { + expectedEnabled: true, + description: 'only social opt-in is true', + }, + { optedInForRegularAccount: true, optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const onlySocialOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const neitherOptedIn: AnalyticsControllerState = { + expectedEnabled: true, + description: 'only regular opt-in is true', + }, + { optedInForRegularAccount: false, optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - expect(computeEnabledState(bothOptedIn)).toBe(true); - expect(computeEnabledState(onlyRegularOptedIn)).toBe(true); - expect(computeEnabledState(onlySocialOptedIn)).toBe(true); - expect(computeEnabledState(neitherOptedIn)).toBe(false); - }); + expectedEnabled: false, + description: 'both opt-ins are false', + }, + ])( + 'computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount when $description', + ({ optedInForRegularAccount, optedInForSocialAccount, expectedEnabled }) => { + const state: AnalyticsControllerState = { + optedInForRegularAccount, + optedInForSocialAccount, + analyticsId: defaultAnalyticsId, + }; + + const result = computeEnabledState(state); + + expect(result).toBe(expectedEnabled); + }, + ); }); }); diff --git a/packages/analytics-controller/src/analyticsStateValidator.test.ts b/packages/analytics-controller/src/analyticsStateValidator.test.ts index 6bcb43d1a25..ca0debd5778 100644 --- a/packages/analytics-controller/src/analyticsStateValidator.test.ts +++ b/packages/analytics-controller/src/analyticsStateValidator.test.ts @@ -15,79 +15,24 @@ describe('analyticsStateValidator', () => { expect(() => validateAnalyticsState(state)).not.toThrow(); }); - it('throws when analyticsId is undefined', () => { - const state = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: undefined, - } as unknown as AnalyticsControllerState; - - expect(() => validateAnalyticsState(state)).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got undefined', - ); - }); - - it('throws when analyticsId is null', () => { - const state = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: null, - } as unknown as AnalyticsControllerState; - - expect(() => validateAnalyticsState(state)).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got null', - ); - }); - - it('throws when analyticsId is an empty string', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: '', - }; - - expect(() => validateAnalyticsState(state)).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got ""', - ); - }); - - it('throws when analyticsId is not a valid UUID', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: 'not-a-uuid', - }; + it.each([ + [undefined], + [null], + [''], + ['not-a-uuid'], + ['12345'], + ['550e8400-e29b-41d4-a716'], + ['c232ab00-9414-11e8-8eb2-f2801f1b9fd1'], + ])( + 'throws with correct error message format for invalid input: %s', + (analyticsId) => { + const expectedMessage = + analyticsId === undefined + ? 'undefined' + : analyticsId === null + ? 'null' + : JSON.stringify(analyticsId); - expect(() => validateAnalyticsState(state)).toThrow( - 'Invalid analyticsId: expected a valid UUIDv4, but got "not-a-uuid"', - ); - }); - - it('throws when analyticsId is a valid UUID but not version 4', () => { - // UUIDv1 example - const uuidV1 = 'c232ab00-9414-11e8-8eb2-f2801f1b9fd1'; - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: uuidV1, - }; - - expect(() => validateAnalyticsState(state)).toThrow( - `Invalid analyticsId: expected a valid UUIDv4, but got "${uuidV1}"`, - ); - }); - - it('throws with correct error message format for various invalid inputs', () => { - const testCases = [ - { analyticsId: undefined, expectedMessage: 'undefined' }, - { analyticsId: null, expectedMessage: 'null' }, - { analyticsId: '', expectedMessage: '""' }, - { analyticsId: 'invalid', expectedMessage: '"invalid"' }, - { analyticsId: '12345', expectedMessage: '"12345"' }, - { analyticsId: '550e8400-e29b-41d4-a716', expectedMessage: '"550e8400-e29b-41d4-a716"' }, - ]; - - testCases.forEach(({ analyticsId, expectedMessage }) => { const state = { optedInForRegularAccount: false, optedInForSocialAccount: false, @@ -97,26 +42,22 @@ describe('analyticsStateValidator', () => { expect(() => validateAnalyticsState(state)).toThrow( `Invalid analyticsId: expected a valid UUIDv4, but got ${expectedMessage}`, ); - }); - }); - - it('validates different valid UUIDv4 formats', () => { - const validUUIDs = [ - '550e8400-e29b-41d4-a716-446655440000', - '123e4567-e89b-42d3-a456-426614174000', - '00000000-0000-4000-8000-000000000000', - 'ffffffff-ffff-4fff-8fff-ffffffffffff', - ]; - - validUUIDs.forEach((uuid) => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: uuid, - }; + }, + ); + + it.each([ + ['550e8400-e29b-41d4-a716-446655440000'], + ['123e4567-e89b-42d3-a456-426614174000'], + ['00000000-0000-4000-8000-000000000000'], + ['ffffffff-ffff-4fff-8fff-ffffffffffff'], + ])('validates valid UUIDv4 format: %s', (uuid) => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: uuid, + }; - expect(() => validateAnalyticsState(state)).not.toThrow(); - }); + expect(() => validateAnalyticsState(state)).not.toThrow(); }); }); }); diff --git a/packages/analytics-controller/src/selectors.test.ts b/packages/analytics-controller/src/selectors.test.ts index 5649fa122ed..f8c8c41eec4 100644 --- a/packages/analytics-controller/src/selectors.test.ts +++ b/packages/analytics-controller/src/selectors.test.ts @@ -1,5 +1,6 @@ import type { AnalyticsControllerState } from './AnalyticsController'; import { analyticsControllerSelectors } from './selectors'; +import * as analyticsStateComputer from './analyticsStateComputer'; describe('analyticsControllerSelectors', () => { const defaultAnalyticsId = '550e8400-e29b-41d4-a716-446655440000'; @@ -16,20 +17,6 @@ describe('analyticsControllerSelectors', () => { expect(result).toBe(defaultAnalyticsId); }); - - it('returns different analyticsId when state has different value', () => { - const differentId = '123e4567-e89b-42d3-a456-426614174000'; - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: differentId, - }; - - const result = analyticsControllerSelectors.selectAnalyticsId(state); - - expect(result).toBe(differentId); - expect(result).not.toBe(defaultAnalyticsId); - }); }); describe('selectOptedIn', () => { @@ -71,122 +58,42 @@ describe('analyticsControllerSelectors', () => { }); describe('selectSocialOptedIn', () => { - it('returns true when optedInForSocialAccount is true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectSocialOptedIn(state); - - expect(result).toBe(true); - }); - - it('returns false when optedInForSocialAccount is false', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectSocialOptedIn(state); - - expect(result).toBe(false); - }); - - it('returns false even when optedInForRegularAccount is true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectSocialOptedIn(state); - - expect(result).toBe(false); - }); + it.each([[true], [false]])( + 'returns %s when optedInForSocialAccount is %s', + (optedInForSocialAccount) => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectSocialOptedIn(state); + + expect(result).toBe(optedInForSocialAccount); + }, + ); }); describe('selectEnabled', () => { - it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectEnabled(state); - - expect(result).toBe(true); - }); - - it('returns true when optedInForRegularAccount=false and optedInForSocialAccount=true', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectEnabled(state); - - expect(result).toBe(true); - }); - - it('returns true when optedInForRegularAccount=true and optedInForSocialAccount=false', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectEnabled(state); - - expect(result).toBe(true); - }); - - it('returns false when optedInForRegularAccount=false and optedInForSocialAccount=false', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const result = analyticsControllerSelectors.selectEnabled(state); - - expect(result).toBe(false); - }); - - it('computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount', () => { - const bothOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const onlyRegularOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - const onlySocialOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - analyticsId: defaultAnalyticsId, - }; - - const neitherOptedIn: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: defaultAnalyticsId, - }; - - expect(analyticsControllerSelectors.selectEnabled(bothOptedIn)).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(onlyRegularOptedIn)).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(onlySocialOptedIn)).toBe(true); - expect(analyticsControllerSelectors.selectEnabled(neitherOptedIn)).toBe(false); - }); + it.each([[true], [false]])( + 'returns %s from computeEnabledState', + (expectedValue) => { + jest + .spyOn(analyticsStateComputer, 'computeEnabledState') + .mockReturnValue(expectedValue); + + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: defaultAnalyticsId, + }; + + const result = analyticsControllerSelectors.selectEnabled(state); + + expect(analyticsStateComputer.computeEnabledState).toHaveBeenCalledWith(state); + expect(result).toBe(expectedValue); + }, + ); }); }); From 087fc5e99cae414bda7ee3145d68a59b4bb10750 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Wed, 19 Nov 2025 11:28:22 +0100 Subject: [PATCH 10/20] format(analytics-controller): fix lint --- .../src/AnalyticsController.test.ts | 121 ++++++++++++++---- .../src/AnalyticsController.ts | 1 - .../src/analyticsStateComputer.test.ts | 6 +- .../src/analyticsStateValidator.test.ts | 15 ++- .../src/analyticsStateValidator.ts | 6 +- .../src/selectors.test.ts | 7 +- .../analytics-controller/src/selectors.ts | 1 - 7 files changed, 112 insertions(+), 45 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index 320c974bfe1..518e3d2ae35 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -131,8 +131,12 @@ describe('AnalyticsController', () => { const defaultState = getDefaultAnalyticsControllerState(); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(false); - expect(controller.state.optedInForRegularAccount).toBe(defaultState.optedInForRegularAccount); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe( + false, + ); + expect(controller.state.optedInForRegularAccount).toBe( + defaultState.optedInForRegularAccount, + ); expect(controller.state.optedInForSocialAccount).toBe( defaultState.optedInForSocialAccount, ); @@ -161,7 +165,9 @@ describe('AnalyticsController', () => { }, }); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe( + true, + ); expect(controller.state.optedInForRegularAccount).toBe(true); expect(controller.state.optedInForSocialAccount).toBe(false); expect(controller.state.analyticsId).toBe(customId); @@ -175,7 +181,9 @@ describe('AnalyticsController', () => { }, }); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe(true); + expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe( + true, + ); expect(controller.state.optedInForRegularAccount).toBe(true); expect(controller.state.optedInForSocialAccount).toBe(false); }); @@ -224,7 +232,10 @@ describe('AnalyticsController', () => { it('tracks event when enabled via regular opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + }, platformAdapter: mockAdapter, }); @@ -239,7 +250,10 @@ describe('AnalyticsController', () => { it('tracks event when enabled via social opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, + state: { + optedInForRegularAccount: false, + optedInForSocialAccount: true, + }, platformAdapter: mockAdapter, }); @@ -254,7 +268,10 @@ describe('AnalyticsController', () => { it('tracks event without properties when event has no properties', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + }, platformAdapter: mockAdapter, }); @@ -267,7 +284,10 @@ describe('AnalyticsController', () => { it('tracks sensitive event when sensitiveProperties are present', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + }, platformAdapter: mockAdapter, }); @@ -292,7 +312,10 @@ describe('AnalyticsController', () => { it('does not track event when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + }, platformAdapter: mockAdapter, }); @@ -347,7 +370,10 @@ describe('AnalyticsController', () => { it('does not identify when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + }, platformAdapter: mockAdapter, }); @@ -366,7 +392,10 @@ describe('AnalyticsController', () => { it('tracks view', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + }, platformAdapter: mockAdapter, }); @@ -380,7 +409,10 @@ describe('AnalyticsController', () => { it('does not track view when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + }, platformAdapter: mockAdapter, }); @@ -449,7 +481,9 @@ describe('AnalyticsController', () => { state: { analyticsId: customId }, }); - const analyticsId = analyticsControllerSelectors.selectAnalyticsId(controller.state); + const analyticsId = analyticsControllerSelectors.selectAnalyticsId( + controller.state, + ); expect(analyticsId).toBe(customId); expect(analyticsId).toBe(controller.state.analyticsId); @@ -465,10 +499,15 @@ describe('AnalyticsController', () => { .mockReturnValue(expectedValue); const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + }, }); - const isEnabled = analyticsControllerSelectors.selectEnabled(controller.state); + const isEnabled = analyticsControllerSelectors.selectEnabled( + controller.state, + ); expect(analyticsStateComputer.computeEnabledState).toHaveBeenCalledWith( controller.state, @@ -481,10 +520,15 @@ describe('AnalyticsController', () => { describe('selectRegularAccountOptedIn', () => { it('returns regular account opted in status from state', () => { const { controller } = setupController({ - state: { optedInForRegularAccount: true, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + }, }); - const isOptedIn = analyticsControllerSelectors.selectOptedIn(controller.state); + const isOptedIn = analyticsControllerSelectors.selectOptedIn( + controller.state, + ); expect(isOptedIn).toBe(true); expect(isOptedIn).toBe(controller.state.optedInForRegularAccount); @@ -492,28 +536,42 @@ describe('AnalyticsController', () => { it('returns updated value when state changes', () => { const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + }, }); - expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe(false); + expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe( + false, + ); controller.optInForRegularAccount(); - expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe(true); + expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe( + true, + ); controller.optOutForRegularAccount(); - expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe(false); + expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe( + false, + ); }); }); describe('selectSocialOptedIn', () => { it('returns social opted in status from state', () => { const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: true }, + state: { + optedInForRegularAccount: false, + optedInForSocialAccount: true, + }, }); - const isSocialOptedIn = analyticsControllerSelectors.selectSocialOptedIn(controller.state); + const isSocialOptedIn = analyticsControllerSelectors.selectSocialOptedIn( + controller.state, + ); expect(isSocialOptedIn).toBe(true); expect(isSocialOptedIn).toBe(controller.state.optedInForSocialAccount); @@ -521,18 +579,27 @@ describe('AnalyticsController', () => { it('returns updated value when state changes', () => { const { controller } = setupController({ - state: { optedInForRegularAccount: false, optedInForSocialAccount: false }, + state: { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + }, }); - expect(analyticsControllerSelectors.selectSocialOptedIn(controller.state)).toBe(false); + expect( + analyticsControllerSelectors.selectSocialOptedIn(controller.state), + ).toBe(false); controller.optInForSocialAccount(); - expect(analyticsControllerSelectors.selectSocialOptedIn(controller.state)).toBe(true); + expect( + analyticsControllerSelectors.selectSocialOptedIn(controller.state), + ).toBe(true); controller.optOutForSocialAccount(); - expect(analyticsControllerSelectors.selectSocialOptedIn(controller.state)).toBe(false); + expect( + analyticsControllerSelectors.selectSocialOptedIn(controller.state), + ).toBe(false); }); }); }); diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index 7c22d689f3f..9025dd5a8dc 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -341,5 +341,4 @@ export class AnalyticsController extends BaseController< state.optedInForSocialAccount = false; }); } - } diff --git a/packages/analytics-controller/src/analyticsStateComputer.test.ts b/packages/analytics-controller/src/analyticsStateComputer.test.ts index 7bf44eba293..e99928b3486 100644 --- a/packages/analytics-controller/src/analyticsStateComputer.test.ts +++ b/packages/analytics-controller/src/analyticsStateComputer.test.ts @@ -32,7 +32,11 @@ describe('analyticsStateComputer', () => { }, ])( 'computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount when $description', - ({ optedInForRegularAccount, optedInForSocialAccount, expectedEnabled }) => { + ({ + optedInForRegularAccount, + optedInForSocialAccount, + expectedEnabled, + }) => { const state: AnalyticsControllerState = { optedInForRegularAccount, optedInForSocialAccount, diff --git a/packages/analytics-controller/src/analyticsStateValidator.test.ts b/packages/analytics-controller/src/analyticsStateValidator.test.ts index ca0debd5778..2d05882d0ce 100644 --- a/packages/analytics-controller/src/analyticsStateValidator.test.ts +++ b/packages/analytics-controller/src/analyticsStateValidator.test.ts @@ -26,12 +26,14 @@ describe('analyticsStateValidator', () => { ])( 'throws with correct error message format for invalid input: %s', (analyticsId) => { - const expectedMessage = - analyticsId === undefined - ? 'undefined' - : analyticsId === null - ? 'null' - : JSON.stringify(analyticsId); + let expectedMessage: string; + if (analyticsId === undefined) { + expectedMessage = 'undefined'; + } else if (analyticsId === null) { + expectedMessage = 'null'; + } else { + expectedMessage = JSON.stringify(analyticsId); + } const state = { optedInForRegularAccount: false, @@ -61,4 +63,3 @@ describe('analyticsStateValidator', () => { }); }); }); - diff --git a/packages/analytics-controller/src/analyticsStateValidator.ts b/packages/analytics-controller/src/analyticsStateValidator.ts index 59ff42091ff..35c27b3fb61 100644 --- a/packages/analytics-controller/src/analyticsStateValidator.ts +++ b/packages/analytics-controller/src/analyticsStateValidator.ts @@ -1,7 +1,4 @@ -import { - validate as validateUuid, - version as getUuidVersion, -} from 'uuid'; +import { validate as validateUuid, version as getUuidVersion } from 'uuid'; import type { AnalyticsControllerState } from './AnalyticsController'; @@ -22,4 +19,3 @@ export function validateAnalyticsState(state: AnalyticsControllerState): void { ); } } - diff --git a/packages/analytics-controller/src/selectors.test.ts b/packages/analytics-controller/src/selectors.test.ts index f8c8c41eec4..f9a094cf553 100644 --- a/packages/analytics-controller/src/selectors.test.ts +++ b/packages/analytics-controller/src/selectors.test.ts @@ -1,6 +1,6 @@ import type { AnalyticsControllerState } from './AnalyticsController'; -import { analyticsControllerSelectors } from './selectors'; import * as analyticsStateComputer from './analyticsStateComputer'; +import { analyticsControllerSelectors } from './selectors'; describe('analyticsControllerSelectors', () => { const defaultAnalyticsId = '550e8400-e29b-41d4-a716-446655440000'; @@ -90,10 +90,11 @@ describe('analyticsControllerSelectors', () => { const result = analyticsControllerSelectors.selectEnabled(state); - expect(analyticsStateComputer.computeEnabledState).toHaveBeenCalledWith(state); + expect(analyticsStateComputer.computeEnabledState).toHaveBeenCalledWith( + state, + ); expect(result).toBe(expectedValue); }, ); }); }); - diff --git a/packages/analytics-controller/src/selectors.ts b/packages/analytics-controller/src/selectors.ts index a742db03cf3..9dc55272c43 100644 --- a/packages/analytics-controller/src/selectors.ts +++ b/packages/analytics-controller/src/selectors.ts @@ -48,4 +48,3 @@ export const analyticsControllerSelectors = { selectSocialOptedIn, selectEnabled, }; - From 21a5ddac5c45f18b553d6069b9e5bdd8fee0e7d7 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Wed, 19 Nov 2025 12:30:31 +0100 Subject: [PATCH 11/20] fix: regenerate action types --- .../src/AnalyticsController-method-action-types.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts index b41d3401a5c..ecbcb53d3bf 100644 --- a/packages/analytics-controller/src/AnalyticsController-method-action-types.ts +++ b/packages/analytics-controller/src/AnalyticsController-method-action-types.ts @@ -28,10 +28,10 @@ export type AnalyticsControllerIdentifyAction = { }; /** - * Track a page view. + * Track a page or screen view. * - * @param name - The name of the UI item being viewed (pages for web, screen for mobile) - * @param properties - UI item properties + * @param name - The identifier/name of the page or screen being viewed (e.g., "home", "settings", "wallet") + * @param properties - Optional properties associated with the view */ export type AnalyticsControllerTrackViewAction = { type: `AnalyticsController:trackView`; From dfb14f38b9bf99e184f57ad05b14391202062054 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 24 Nov 2025 11:32:24 +0100 Subject: [PATCH 12/20] refactor(analytics-controller): inline state computer and remove default state factory - Remove analyticsStateComputer module and inline enabled state logic into selectors - Remove getDefaultAnalyticsControllerState() and inline defaults in constructor - Rename selectors to be more explicit (selectOptedIn -> selectOptedInForRegularAccount, selectSocialOptedIn -> selectOptedInForSocialAccount) - Add missing test for trackView when enabled via social opt-in - Update documentation to reflect internal default state initialization --- packages/analytics-controller/README.md | 34 +++--- .../src/AnalyticsController.test.ts | 106 +++++++++--------- .../src/AnalyticsController.ts | 35 ++---- .../src/analyticsStateComputer.test.ts | 52 --------- .../src/analyticsStateComputer.ts | 44 -------- packages/analytics-controller/src/index.ts | 1 - .../src/selectors.test.ts | 48 ++++---- .../analytics-controller/src/selectors.ts | 23 ++-- 8 files changed, 125 insertions(+), 218 deletions(-) delete mode 100644 packages/analytics-controller/src/analyticsStateComputer.test.ts delete mode 100644 packages/analytics-controller/src/analyticsStateComputer.ts diff --git a/packages/analytics-controller/README.md b/packages/analytics-controller/README.md index 21494c58cd7..644d9b75154 100644 --- a/packages/analytics-controller/README.md +++ b/packages/analytics-controller/README.md @@ -90,7 +90,7 @@ const controller = new AnalyticsController({ }); ``` -**Important:** The `state` parameter is the single source of truth for initial values. Any properties you provide will override the defaults from `getDefaultAnalyticsControllerState()`. +**Important:** The `state` parameter is the single source of truth for initial values. Any properties you provide will override the controller's internal defaults. ### 3. Track Events @@ -183,22 +183,14 @@ messenger.subscribe('AnalyticsController:stateChange', (state, prevState) => { ### Default State -The default state is provided by `getDefaultAnalyticsControllerState()`: - -```typescript -import { getDefaultAnalyticsControllerState } from '@metamask/analytics-controller'; - -const defaultState = getDefaultAnalyticsControllerState(); -// { -// enabled: true, -// optedIn: false, -// analyticsId: auto-generated UUIDv4 -// } -``` +The controller initializes with default state values internally. The default state includes: +- `optedInForRegularAccount`: `false` +- `optedInForSocialAccount`: `false` +- `analyticsId`: Auto-generated UUIDv4 ### Initialization Strategy -- **No `state` parameter**: Uses defaults from `getDefaultAnalyticsControllerState()` and auto-generates `analyticsId` as UUIDv4 +- **No `state` parameter**: Uses internal defaults and auto-generates `analyticsId` as UUIDv4 - **Partial `state`**: Merges with defaults (user-provided values override defaults); `analyticsId` is auto-generated if not provided - **Complete `state`**: Full control for migrations and advanced use cases @@ -264,6 +256,20 @@ export DEBUG="metamask:analytics-controller" This will enable debug logging for the analytics-controller, allowing you to see detailed logs of analytics events, state changes, and controller operations. +## Development + +### Build + +```bash +yarn install && yarn workspace @metamask/analytics-controller build +``` + +### Test + +```bash +yarn install && yarn workspace @metamask/analytics-controller test +``` + ## Contributing This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index 518e3d2ae35..1156841a23a 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -12,12 +12,10 @@ import { type AnalyticsControllerEvents, type AnalyticsPlatformAdapter, AnalyticsPlatformAdapterSetupError, - getDefaultAnalyticsControllerState, type AnalyticsTrackingEvent, analyticsControllerSelectors, } from '.'; import type { AnalyticsControllerState } from '.'; -import * as analyticsStateComputer from './analyticsStateComputer'; type SetupControllerOptions = { state?: Partial; @@ -129,17 +127,11 @@ describe('AnalyticsController', () => { it('initializes with default state', () => { const { controller } = setupController(); - const defaultState = getDefaultAnalyticsControllerState(); - expect(analyticsControllerSelectors.selectEnabled(controller.state)).toBe( false, ); - expect(controller.state.optedInForRegularAccount).toBe( - defaultState.optedInForRegularAccount, - ); - expect(controller.state.optedInForSocialAccount).toBe( - defaultState.optedInForSocialAccount, - ); + expect(controller.state.optedInForRegularAccount).toBe(false); + expect(controller.state.optedInForSocialAccount).toBe(false); expect(isValidUUIDv4(controller.state.analyticsId)).toBe(true); }); @@ -229,7 +221,7 @@ describe('AnalyticsController', () => { }); describe('trackEvent', () => { - it('tracks event when enabled via regular opt-in', () => { + it('calls platform adapter to track event when enabled via regular opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -247,7 +239,7 @@ describe('AnalyticsController', () => { }); }); - it('tracks event when enabled via social opt-in', () => { + it('tracks event via platform adapter when enabled via social opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -309,7 +301,7 @@ describe('AnalyticsController', () => { }); }); - it('does not track event when disabled', () => { + it('does not call platform adapter when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -327,7 +319,7 @@ describe('AnalyticsController', () => { }); describe('identify', () => { - it('identifies user with traits using current analytics ID', () => { + it('identifies user via platform adapter with traits using current analytics ID', () => { const mockAdapter = createMockAdapter(); const customId = '550e8400-e29b-41d4-a716-446655440000'; const { controller } = setupController({ @@ -389,7 +381,7 @@ describe('AnalyticsController', () => { }); describe('trackView', () => { - it('tracks view', () => { + it('calls platform adapter to track view when enabled via regular opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -406,7 +398,24 @@ describe('AnalyticsController', () => { }); }); - it('does not track view when disabled', () => { + it('calls platform adapter to track view when enabled via social opt-in', () => { + const mockAdapter = createMockAdapter(); + const { controller } = setupController({ + state: { + optedInForRegularAccount: false, + optedInForSocialAccount: true, + }, + platformAdapter: mockAdapter, + }); + + controller.trackView('home', { referrer: 'test' }); + + expect(mockAdapter.view).toHaveBeenCalledWith('home', { + referrer: 'test', + }); + }); + + it('does not call platform adapter when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -490,33 +499,6 @@ describe('AnalyticsController', () => { }); }); - describe('selectEnabled', () => { - it.each([[true], [false]])( - 'returns %s from computeEnabledState', - (expectedValue) => { - jest - .spyOn(analyticsStateComputer, 'computeEnabledState') - .mockReturnValue(expectedValue); - - const { controller } = setupController({ - state: { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - }, - }); - - const isEnabled = analyticsControllerSelectors.selectEnabled( - controller.state, - ); - - expect(analyticsStateComputer.computeEnabledState).toHaveBeenCalledWith( - controller.state, - ); - expect(isEnabled).toBe(expectedValue); - }, - ); - }); - describe('selectRegularAccountOptedIn', () => { it('returns regular account opted in status from state', () => { const { controller } = setupController({ @@ -526,7 +508,8 @@ describe('AnalyticsController', () => { }, }); - const isOptedIn = analyticsControllerSelectors.selectOptedIn( + const isOptedIn = + analyticsControllerSelectors.selectOptedInForRegularAccount( controller.state, ); @@ -542,25 +525,37 @@ describe('AnalyticsController', () => { }, }); - expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe( + expect( + analyticsControllerSelectors.selectOptedInForRegularAccount( + controller.state, + ), + ).toBe( false, ); controller.optInForRegularAccount(); - expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe( + expect( + analyticsControllerSelectors.selectOptedInForRegularAccount( + controller.state, + ), + ).toBe( true, ); controller.optOutForRegularAccount(); - expect(analyticsControllerSelectors.selectOptedIn(controller.state)).toBe( + expect( + analyticsControllerSelectors.selectOptedInForRegularAccount( + controller.state, + ), + ).toBe( false, ); }); }); - describe('selectSocialOptedIn', () => { + describe('selectOptedInForSocialAccount', () => { it('returns social opted in status from state', () => { const { controller } = setupController({ state: { @@ -569,7 +564,8 @@ describe('AnalyticsController', () => { }, }); - const isSocialOptedIn = analyticsControllerSelectors.selectSocialOptedIn( + const isSocialOptedIn = + analyticsControllerSelectors.selectOptedInForSocialAccount( controller.state, ); @@ -586,19 +582,25 @@ describe('AnalyticsController', () => { }); expect( - analyticsControllerSelectors.selectSocialOptedIn(controller.state), + analyticsControllerSelectors.selectOptedInForSocialAccount( + controller.state, + ), ).toBe(false); controller.optInForSocialAccount(); expect( - analyticsControllerSelectors.selectSocialOptedIn(controller.state), + analyticsControllerSelectors.selectOptedInForSocialAccount( + controller.state, + ), ).toBe(true); controller.optOutForSocialAccount(); expect( - analyticsControllerSelectors.selectSocialOptedIn(controller.state), + analyticsControllerSelectors.selectOptedInForSocialAccount( + controller.state, + ), ).toBe(false); }); }); diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index 9025dd5a8dc..3c75b13ea5b 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -15,7 +15,7 @@ import type { AnalyticsUserTraits, AnalyticsTrackingEvent, } from './AnalyticsPlatformAdapter.types'; -import { computeEnabledState } from './analyticsStateComputer'; +import { analyticsControllerSelectors } from './selectors'; import { validateAnalyticsState } from './analyticsStateValidator'; // === GENERAL === @@ -73,22 +73,6 @@ const analyticsControllerMetadata = { }, } satisfies StateMetadata; -/** - * Constructs the default {@link AnalyticsController} state. This allows - * consumers to provide a partial state object when initializing the controller - * and also helps in constructing complete state objects for this controller in - * tests. - * - * @returns The default {@link AnalyticsController} state. - */ -export function getDefaultAnalyticsControllerState(): AnalyticsControllerState { - return { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: uuidv4(), - }; -} - // === MESSENGER === const MESSENGER_EXPOSED_METHODS = [ @@ -184,7 +168,8 @@ export class AnalyticsController extends BaseController< * Constructs an AnalyticsController instance. * * @param options - Controller options - * @param options.state - Initial controller state (defaults from getDefaultAnalyticsControllerState). + * @param options.state - Initial controller state. Defaults: optedInForRegularAccount=false, + * optedInForSocialAccount=false, analyticsId=auto-generated UUIDv4. * For migration from a previous system, pass the existing analytics ID via state.analyticsId. * @param options.messenger - Messenger used to communicate with BaseController * @param options.platformAdapter - Platform adapter implementation for tracking @@ -194,8 +179,10 @@ export class AnalyticsController extends BaseController< messenger, platformAdapter, }: AnalyticsControllerOptions) { - const initialState = { - ...getDefaultAnalyticsControllerState(), + const initialState: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: uuidv4(), ...state, }; @@ -216,7 +203,7 @@ export class AnalyticsController extends BaseController< ); projectLogger('AnalyticsController initialized and ready', { - enabled: computeEnabledState(this.state), + enabled: analyticsControllerSelectors.selectEnabled(this.state), optedIn: this.state.optedInForRegularAccount, socialOptedIn: this.state.optedInForSocialAccount, analyticsId: this.state.analyticsId, @@ -241,7 +228,7 @@ export class AnalyticsController extends BaseController< */ trackEvent(event: AnalyticsTrackingEvent): void { // Don't track if analytics is disabled - if (!computeEnabledState(this.state)) { + if (!analyticsControllerSelectors.selectEnabled(this.state)) { return; } @@ -277,7 +264,7 @@ export class AnalyticsController extends BaseController< * @param traits - User traits/properties */ identify(traits?: AnalyticsUserTraits): void { - if (!computeEnabledState(this.state)) { + if (!analyticsControllerSelectors.selectEnabled(this.state)) { return; } @@ -294,7 +281,7 @@ export class AnalyticsController extends BaseController< * @param properties - Optional properties associated with the view */ trackView(name: string, properties?: AnalyticsEventProperties): void { - if (!computeEnabledState(this.state)) { + if (!analyticsControllerSelectors.selectEnabled(this.state)) { return; } diff --git a/packages/analytics-controller/src/analyticsStateComputer.test.ts b/packages/analytics-controller/src/analyticsStateComputer.test.ts deleted file mode 100644 index e99928b3486..00000000000 --- a/packages/analytics-controller/src/analyticsStateComputer.test.ts +++ /dev/null @@ -1,52 +0,0 @@ -import type { AnalyticsControllerState } from './AnalyticsController'; -import { computeEnabledState } from './analyticsStateComputer'; - -describe('analyticsStateComputer', () => { - describe('computeEnabledState', () => { - const defaultAnalyticsId = '550e8400-e29b-41d4-a716-446655440000'; - - it.each([ - { - optedInForRegularAccount: true, - optedInForSocialAccount: true, - expectedEnabled: true, - description: 'both opt-ins are true', - }, - { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - expectedEnabled: true, - description: 'only social opt-in is true', - }, - { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - expectedEnabled: true, - description: 'only regular opt-in is true', - }, - { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - expectedEnabled: false, - description: 'both opt-ins are false', - }, - ])( - 'computes enabled state based on optedInForRegularAccount OR optedInForSocialAccount when $description', - ({ - optedInForRegularAccount, - optedInForSocialAccount, - expectedEnabled, - }) => { - const state: AnalyticsControllerState = { - optedInForRegularAccount, - optedInForSocialAccount, - analyticsId: defaultAnalyticsId, - }; - - const result = computeEnabledState(state); - - expect(result).toBe(expectedEnabled); - }, - ); - }); -}); diff --git a/packages/analytics-controller/src/analyticsStateComputer.ts b/packages/analytics-controller/src/analyticsStateComputer.ts deleted file mode 100644 index c84d08f735b..00000000000 --- a/packages/analytics-controller/src/analyticsStateComputer.ts +++ /dev/null @@ -1,44 +0,0 @@ -import type { AnalyticsControllerState } from './AnalyticsController'; - -/** - * State computer that computes controller values from controller state. - * - * This module provides functions that compute derived controller values from the - * controller's state. Currently, it computes the `controller_enabled` state, but - * can be extended to compute other values in the future. - * - * **State Computer Computations:** - * - * 1. **Enabled State** (`computeEnabledState`): - * - Determines whether analytics tracking is active - * - Rules: `controller_enabled = optedInForRegularAccount || optedInForSocialAccount` - * - Analytics is enabled if the user has opted in for regular account OR social account - * - * 2. **Future computations** (e.g., feature flags, permissions, etc.) - * - * **Usage:** - * These functions are called: - * - During controller initialization to set initial values - * - Whenever user state changes (e.g., in `optInForRegularAccount()`, `optOutForRegularAccount()`, `optInForSocialAccount()`, `optOutForSocialAccount()`) - * - * **Extensibility:** - * To add new computations, add new functions that take `AnalyticsControllerState` as input. - * To add new user state properties, update the `AnalyticsControllerState` type with `user_` prefix - * and all computation functions that need to consider them. - */ - -/** - * Computes the `controller_enabled` state from controller state. - * - * @param state - The current controller state - * @returns `true` if analytics tracking should be enabled, `false` otherwise - */ -export function computeEnabledState(state: AnalyticsControllerState): boolean { - // Analytics is enabled if user has opted in for regular account OR social account - // Rules: - // - optedInForRegularAccount==true && optedInForSocialAccount==true -> enabled=true - // - optedInForRegularAccount==false && optedInForSocialAccount==true -> enabled=true - // - optedInForRegularAccount==true && optedInForSocialAccount==false -> enabled=true - // - optedInForRegularAccount==false && optedInForSocialAccount==false -> enabled=false - return state.optedInForRegularAccount || state.optedInForSocialAccount; -} diff --git a/packages/analytics-controller/src/index.ts b/packages/analytics-controller/src/index.ts index e477ffe1490..1ae87759a98 100644 --- a/packages/analytics-controller/src/index.ts +++ b/packages/analytics-controller/src/index.ts @@ -15,7 +15,6 @@ export type { // Export state types and utilities export type { AnalyticsControllerState } from './AnalyticsController'; -export { getDefaultAnalyticsControllerState } from './AnalyticsController'; // Export selectors export { analyticsControllerSelectors } from './selectors'; diff --git a/packages/analytics-controller/src/selectors.test.ts b/packages/analytics-controller/src/selectors.test.ts index f9a094cf553..f6c6995ea77 100644 --- a/packages/analytics-controller/src/selectors.test.ts +++ b/packages/analytics-controller/src/selectors.test.ts @@ -1,5 +1,4 @@ import type { AnalyticsControllerState } from './AnalyticsController'; -import * as analyticsStateComputer from './analyticsStateComputer'; import { analyticsControllerSelectors } from './selectors'; describe('analyticsControllerSelectors', () => { @@ -19,7 +18,7 @@ describe('analyticsControllerSelectors', () => { }); }); - describe('selectOptedIn', () => { + describe('selectOptedInForRegularAccount', () => { it('returns true when optedInForRegularAccount is true', () => { const state: AnalyticsControllerState = { optedInForRegularAccount: true, @@ -27,7 +26,8 @@ describe('analyticsControllerSelectors', () => { analyticsId: defaultAnalyticsId, }; - const result = analyticsControllerSelectors.selectOptedIn(state); + const result = + analyticsControllerSelectors.selectOptedInForRegularAccount(state); expect(result).toBe(true); }); @@ -39,7 +39,8 @@ describe('analyticsControllerSelectors', () => { analyticsId: defaultAnalyticsId, }; - const result = analyticsControllerSelectors.selectOptedIn(state); + const result = + analyticsControllerSelectors.selectOptedInForRegularAccount(state); expect(result).toBe(false); }); @@ -51,13 +52,14 @@ describe('analyticsControllerSelectors', () => { analyticsId: defaultAnalyticsId, }; - const result = analyticsControllerSelectors.selectOptedIn(state); + const result = + analyticsControllerSelectors.selectOptedInForRegularAccount(state); expect(result).toBe(false); }); }); - describe('selectSocialOptedIn', () => { + describe('selectOptedInForSocialAccount', () => { it.each([[true], [false]])( 'returns %s when optedInForSocialAccount is %s', (optedInForSocialAccount) => { @@ -67,7 +69,8 @@ describe('analyticsControllerSelectors', () => { analyticsId: defaultAnalyticsId, }; - const result = analyticsControllerSelectors.selectSocialOptedIn(state); + const result = + analyticsControllerSelectors.selectOptedInForSocialAccount(state); expect(result).toBe(optedInForSocialAccount); }, @@ -75,25 +78,30 @@ describe('analyticsControllerSelectors', () => { }); describe('selectEnabled', () => { - it.each([[true], [false]])( - 'returns %s from computeEnabledState', - (expectedValue) => { - jest - .spyOn(analyticsStateComputer, 'computeEnabledState') - .mockReturnValue(expectedValue); - + /** + * Tests all combinations of opt-in states: + * 1. Neither account opted in -> analytics disabled + * 2. Only regular account opted in -> analytics enabled + * 3. Only social account opted in -> analytics enabled + * 4. Both accounts opted in -> analytics enabled + */ + it.each([ + [false, false, false], + [true, false, true], + [false, true, true], + [true, true, true], + ])( + 'when optedInForRegularAccount=%s and optedInForSocialAccount=%s, returns %s', + (optedInForRegularAccount, optedInForSocialAccount, expected) => { const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, + optedInForRegularAccount, + optedInForSocialAccount, analyticsId: defaultAnalyticsId, }; const result = analyticsControllerSelectors.selectEnabled(state); - expect(analyticsStateComputer.computeEnabledState).toHaveBeenCalledWith( - state, - ); - expect(result).toBe(expectedValue); + expect(result).toBe(expected); }, ); }); diff --git a/packages/analytics-controller/src/selectors.ts b/packages/analytics-controller/src/selectors.ts index 9dc55272c43..2b8af585d55 100644 --- a/packages/analytics-controller/src/selectors.ts +++ b/packages/analytics-controller/src/selectors.ts @@ -1,5 +1,4 @@ import type { AnalyticsControllerState } from './AnalyticsController'; -import { computeEnabledState } from './analyticsStateComputer'; /** * Selects the analytics ID from the controller state. @@ -11,32 +10,34 @@ const selectAnalyticsId = (state: AnalyticsControllerState): string => state.analyticsId; /** - * Selects the opted-in status from the controller state. + * Selects the opted-in status for regular account from the controller state. * * @param state - The controller state * @returns Whether the user has opted in for regular account */ -const selectOptedIn = (state: AnalyticsControllerState): boolean => - state.optedInForRegularAccount; +const selectOptedInForRegularAccount = ( + state: AnalyticsControllerState, +): boolean => state.optedInForRegularAccount; /** - * Selects the social opted-in status from the controller state. + * Selects the opted-in status for social account from the controller state. * * @param state - The controller state * @returns Whether the user has opted in for social account */ -const selectSocialOptedIn = (state: AnalyticsControllerState): boolean => - state.optedInForSocialAccount; +const selectOptedInForSocialAccount = ( + state: AnalyticsControllerState, +): boolean => state.optedInForSocialAccount; /** * Selects the enabled status from the controller state. - * This is computed from user state via the state computer. + * Analytics is enabled if the user has opted in for regular account OR social account. * * @param state - The controller state * @returns Whether analytics tracking is enabled */ const selectEnabled = (state: AnalyticsControllerState): boolean => - computeEnabledState(state); + state.optedInForRegularAccount || state.optedInForSocialAccount; /** * Selectors for the AnalyticsController state. @@ -44,7 +45,7 @@ const selectEnabled = (state: AnalyticsControllerState): boolean => */ export const analyticsControllerSelectors = { selectAnalyticsId, - selectOptedIn, - selectSocialOptedIn, + selectOptedInForRegularAccount, + selectOptedInForSocialAccount, selectEnabled, }; From 50c97eb7f4ad1d050d8f0958f8a5f89eff977ad2 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 24 Nov 2025 11:40:37 +0100 Subject: [PATCH 13/20] refactor(analytics-controller): rename validateAnalyticsState to validateAnalyticsControllerState - Rename function from validateAnalyticsState to validateAnalyticsControllerState - Rename file from analyticsStateValidator.ts to analyticsControllerStateValidator.ts - Update all imports and usages across codebase - Improve test description for onSetupCompleted error handling --- .../src/AnalyticsController.test.ts | 3 +- .../src/AnalyticsController.ts | 4 +- .../analyticsControllerStateValidator.test.ts | 65 +++++++++++++++++++ .../src/analyticsControllerStateValidator.ts | 21 ++++++ .../src/analyticsStateValidator.test.ts | 12 ++-- .../src/analyticsStateValidator.ts | 2 +- 6 files changed, 96 insertions(+), 11 deletions(-) create mode 100644 packages/analytics-controller/src/analyticsControllerStateValidator.test.ts create mode 100644 packages/analytics-controller/src/analyticsControllerStateValidator.ts diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index 1156841a23a..e041f7e02fd 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -197,8 +197,7 @@ describe('AnalyticsController', () => { ); }); - it('returns controller when onSetupCompleted throws error', () => { - const mockAdapter = createMockAdapter(); + it('ignores errors thrown by onSetupCompleted', () => { const mockAdapter = createMockAdapter(); const cause = new Error( 'MetaMetricsPrivacySegmentPlugin configure failed', ); diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index 3c75b13ea5b..c309ba9b48b 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -16,7 +16,7 @@ import type { AnalyticsTrackingEvent, } from './AnalyticsPlatformAdapter.types'; import { analyticsControllerSelectors } from './selectors'; -import { validateAnalyticsState } from './analyticsStateValidator'; +import { validateAnalyticsControllerState } from './analyticsControllerStateValidator'; // === GENERAL === @@ -186,7 +186,7 @@ export class AnalyticsController extends BaseController< ...state, }; - validateAnalyticsState(initialState); + validateAnalyticsControllerState(initialState); super({ name: controllerName, diff --git a/packages/analytics-controller/src/analyticsControllerStateValidator.test.ts b/packages/analytics-controller/src/analyticsControllerStateValidator.test.ts new file mode 100644 index 00000000000..08c98a3dcd1 --- /dev/null +++ b/packages/analytics-controller/src/analyticsControllerStateValidator.test.ts @@ -0,0 +1,65 @@ +import type { AnalyticsControllerState } from './AnalyticsController'; +import { validateAnalyticsControllerState } from './analyticsControllerStateValidator'; + +describe('analyticsControllerStateValidator', () => { + describe('validateAnalyticsControllerState', () => { + const validUUIDv4 = '550e8400-e29b-41d4-a716-446655440000'; + + it('does not throw when analyticsId is a valid UUIDv4', () => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: validUUIDv4, + }; + + expect(() => validateAnalyticsControllerState(state)).not.toThrow(); + }); + + it.each([ + [undefined], + [null], + [''], + ['not-a-uuid'], + ['12345'], + ['550e8400-e29b-41d4-a716'], + ['c232ab00-9414-11e8-8eb2-f2801f1b9fd1'], + ])( + 'throws with correct error message format for invalid input: %s', + (analyticsId) => { + let expectedMessage: string; + if (analyticsId === undefined) { + expectedMessage = 'undefined'; + } else if (analyticsId === null) { + expectedMessage = 'null'; + } else { + expectedMessage = JSON.stringify(analyticsId); + } + + const state = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId, + } as unknown as AnalyticsControllerState; + + expect(() => validateAnalyticsControllerState(state)).toThrow( + `Invalid analyticsId: expected a valid UUIDv4, but got ${expectedMessage}`, + ); + }, + ); + + it.each([ + ['550e8400-e29b-41d4-a716-446655440000'], + ['123e4567-e89b-42d3-a456-426614174000'], + ['00000000-0000-4000-8000-000000000000'], + ['ffffffff-ffff-4fff-8fff-ffffffffffff'], + ])('validates valid UUIDv4 format: %s', (uuid) => { + const state: AnalyticsControllerState = { + optedInForRegularAccount: false, + optedInForSocialAccount: false, + analyticsId: uuid, + }; + + expect(() => validateAnalyticsControllerState(state)).not.toThrow(); + }); + }); +}); diff --git a/packages/analytics-controller/src/analyticsControllerStateValidator.ts b/packages/analytics-controller/src/analyticsControllerStateValidator.ts new file mode 100644 index 00000000000..0216f6127fb --- /dev/null +++ b/packages/analytics-controller/src/analyticsControllerStateValidator.ts @@ -0,0 +1,21 @@ +import { validate as validateUuid, version as getUuidVersion } from 'uuid'; + +import type { AnalyticsControllerState } from './AnalyticsController'; + +/** + * Validates that the analytics state has a valid UUIDv4 analyticsId. + * + * @param state - The analytics controller state to validate + * @throws {Error} If analyticsId is missing, invalid, or not a UUIDv4 + */ +export function validateAnalyticsControllerState(state: AnalyticsControllerState): void { + if ( + !state.analyticsId || + !validateUuid(state.analyticsId) || + getUuidVersion(state.analyticsId) !== 4 + ) { + throw new Error( + `Invalid analyticsId: expected a valid UUIDv4, but got ${JSON.stringify(state.analyticsId)}`, + ); + } +} diff --git a/packages/analytics-controller/src/analyticsStateValidator.test.ts b/packages/analytics-controller/src/analyticsStateValidator.test.ts index 2d05882d0ce..08c98a3dcd1 100644 --- a/packages/analytics-controller/src/analyticsStateValidator.test.ts +++ b/packages/analytics-controller/src/analyticsStateValidator.test.ts @@ -1,8 +1,8 @@ import type { AnalyticsControllerState } from './AnalyticsController'; -import { validateAnalyticsState } from './analyticsStateValidator'; +import { validateAnalyticsControllerState } from './analyticsControllerStateValidator'; -describe('analyticsStateValidator', () => { - describe('validateAnalyticsState', () => { +describe('analyticsControllerStateValidator', () => { + describe('validateAnalyticsControllerState', () => { const validUUIDv4 = '550e8400-e29b-41d4-a716-446655440000'; it('does not throw when analyticsId is a valid UUIDv4', () => { @@ -12,7 +12,7 @@ describe('analyticsStateValidator', () => { analyticsId: validUUIDv4, }; - expect(() => validateAnalyticsState(state)).not.toThrow(); + expect(() => validateAnalyticsControllerState(state)).not.toThrow(); }); it.each([ @@ -41,7 +41,7 @@ describe('analyticsStateValidator', () => { analyticsId, } as unknown as AnalyticsControllerState; - expect(() => validateAnalyticsState(state)).toThrow( + expect(() => validateAnalyticsControllerState(state)).toThrow( `Invalid analyticsId: expected a valid UUIDv4, but got ${expectedMessage}`, ); }, @@ -59,7 +59,7 @@ describe('analyticsStateValidator', () => { analyticsId: uuid, }; - expect(() => validateAnalyticsState(state)).not.toThrow(); + expect(() => validateAnalyticsControllerState(state)).not.toThrow(); }); }); }); diff --git a/packages/analytics-controller/src/analyticsStateValidator.ts b/packages/analytics-controller/src/analyticsStateValidator.ts index 35c27b3fb61..0216f6127fb 100644 --- a/packages/analytics-controller/src/analyticsStateValidator.ts +++ b/packages/analytics-controller/src/analyticsStateValidator.ts @@ -8,7 +8,7 @@ import type { AnalyticsControllerState } from './AnalyticsController'; * @param state - The analytics controller state to validate * @throws {Error} If analyticsId is missing, invalid, or not a UUIDv4 */ -export function validateAnalyticsState(state: AnalyticsControllerState): void { +export function validateAnalyticsControllerState(state: AnalyticsControllerState): void { if ( !state.analyticsId || !validateUuid(state.analyticsId) || From fd9f4ab750becb3681009f6a3670a388e5d20b3c Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 24 Nov 2025 11:47:22 +0100 Subject: [PATCH 14/20] test(analytics-controller): remove redundant test assertions and selector tests - Remove redundant initial state assertions from optIn/optOut tests - Remove selector tests that are already covered in selectors.test.ts - Focus AnalyticsController tests on controller behavior rather than selector implementation --- .../src/AnalyticsController.test.ts | 130 ------------------ 1 file changed, 130 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index e041f7e02fd..6d931db1beb 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -434,8 +434,6 @@ describe('AnalyticsController', () => { it('sets optedInForRegularAccount to true', () => { const { controller } = setupController(); - expect(controller.state.optedInForRegularAccount).toBe(false); - controller.optInForRegularAccount(); expect(controller.state.optedInForRegularAccount).toBe(true); @@ -448,8 +446,6 @@ describe('AnalyticsController', () => { state: { optedInForRegularAccount: true }, }); - expect(controller.state.optedInForRegularAccount).toBe(true); - controller.optOutForRegularAccount(); expect(controller.state.optedInForRegularAccount).toBe(false); @@ -460,8 +456,6 @@ describe('AnalyticsController', () => { it('sets optedInForSocialAccount to true', () => { const { controller } = setupController(); - expect(controller.state.optedInForSocialAccount).toBe(false); - controller.optInForSocialAccount(); expect(controller.state.optedInForSocialAccount).toBe(true); @@ -474,133 +468,9 @@ describe('AnalyticsController', () => { state: { optedInForSocialAccount: true }, }); - expect(controller.state.optedInForSocialAccount).toBe(true); - controller.optOutForSocialAccount(); expect(controller.state.optedInForSocialAccount).toBe(false); }); }); - - describe('selectAnalyticsId', () => { - it('returns analytics ID', () => { - const customId = '550e8400-e29b-41d4-a716-446655440000'; - const { controller } = setupController({ - state: { analyticsId: customId }, - }); - - const analyticsId = analyticsControllerSelectors.selectAnalyticsId( - controller.state, - ); - - expect(analyticsId).toBe(customId); - expect(analyticsId).toBe(controller.state.analyticsId); - }); - }); - - describe('selectRegularAccountOptedIn', () => { - it('returns regular account opted in status from state', () => { - const { controller } = setupController({ - state: { - optedInForRegularAccount: true, - optedInForSocialAccount: false, - }, - }); - - const isOptedIn = - analyticsControllerSelectors.selectOptedInForRegularAccount( - controller.state, - ); - - expect(isOptedIn).toBe(true); - expect(isOptedIn).toBe(controller.state.optedInForRegularAccount); - }); - - it('returns updated value when state changes', () => { - const { controller } = setupController({ - state: { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - }, - }); - - expect( - analyticsControllerSelectors.selectOptedInForRegularAccount( - controller.state, - ), - ).toBe( - false, - ); - - controller.optInForRegularAccount(); - - expect( - analyticsControllerSelectors.selectOptedInForRegularAccount( - controller.state, - ), - ).toBe( - true, - ); - - controller.optOutForRegularAccount(); - - expect( - analyticsControllerSelectors.selectOptedInForRegularAccount( - controller.state, - ), - ).toBe( - false, - ); - }); - }); - - describe('selectOptedInForSocialAccount', () => { - it('returns social opted in status from state', () => { - const { controller } = setupController({ - state: { - optedInForRegularAccount: false, - optedInForSocialAccount: true, - }, - }); - - const isSocialOptedIn = - analyticsControllerSelectors.selectOptedInForSocialAccount( - controller.state, - ); - - expect(isSocialOptedIn).toBe(true); - expect(isSocialOptedIn).toBe(controller.state.optedInForSocialAccount); - }); - - it('returns updated value when state changes', () => { - const { controller } = setupController({ - state: { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - }, - }); - - expect( - analyticsControllerSelectors.selectOptedInForSocialAccount( - controller.state, - ), - ).toBe(false); - - controller.optInForSocialAccount(); - - expect( - analyticsControllerSelectors.selectOptedInForSocialAccount( - controller.state, - ), - ).toBe(true); - - controller.optOutForSocialAccount(); - - expect( - analyticsControllerSelectors.selectOptedInForSocialAccount( - controller.state, - ), - ).toBe(false); - }); - }); }); From 3a98f9e96a4dd82647d9ca1a7a9c261131bdc4ba Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 24 Nov 2025 12:29:05 +0100 Subject: [PATCH 15/20] fix(analytics-controller): remove unnecessary identify check and document saveDataRecording - Remove unnecessary optional check for required identify method in platform adapter - Add documentation for saveDataRecording as legacy property that's ignored - saveDataRecording is handled by mobile app and will be removed from type in future --- packages/analytics-controller/src/AnalyticsController.ts | 6 ++---- .../src/AnalyticsPlatformAdapter.types.ts | 5 +++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index c309ba9b48b..e02eae9942b 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -268,10 +268,8 @@ export class AnalyticsController extends BaseController< return; } - // Delegate to platform adapter if supported, using the current analytics ID - if (this.#platformAdapter.identify) { - this.#platformAdapter.identify(this.state.analyticsId, traits); - } + // Delegate to platform adapter using the current analytics ID + this.#platformAdapter.identify(this.state.analyticsId, traits); } /** diff --git a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts index 2fca86a9793..39d22ff5928 100644 --- a/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts +++ b/packages/analytics-controller/src/AnalyticsPlatformAdapter.types.ts @@ -19,6 +19,11 @@ export type AnalyticsTrackingEvent = { readonly name: string; properties: AnalyticsEventProperties; sensitiveProperties: AnalyticsEventProperties; + /** + * Legacy property handled by the mobile app. + * This property is ignored by the analytics controller and will be removed from the type in the future. + * The mobile app will use the future analytics privacy controller to handle this functionality. + */ saveDataRecording: boolean; readonly hasProperties: boolean; }; From e1859487a296dac517b6cdfc4c112218afe7fc86 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 24 Nov 2025 12:31:01 +0100 Subject: [PATCH 16/20] chore(analytics-controller): remove duplicate validator files --- .../src/analyticsStateValidator.test.ts | 65 ------------------- .../src/analyticsStateValidator.ts | 21 ------ 2 files changed, 86 deletions(-) delete mode 100644 packages/analytics-controller/src/analyticsStateValidator.test.ts delete mode 100644 packages/analytics-controller/src/analyticsStateValidator.ts diff --git a/packages/analytics-controller/src/analyticsStateValidator.test.ts b/packages/analytics-controller/src/analyticsStateValidator.test.ts deleted file mode 100644 index 08c98a3dcd1..00000000000 --- a/packages/analytics-controller/src/analyticsStateValidator.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -import type { AnalyticsControllerState } from './AnalyticsController'; -import { validateAnalyticsControllerState } from './analyticsControllerStateValidator'; - -describe('analyticsControllerStateValidator', () => { - describe('validateAnalyticsControllerState', () => { - const validUUIDv4 = '550e8400-e29b-41d4-a716-446655440000'; - - it('does not throw when analyticsId is a valid UUIDv4', () => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: validUUIDv4, - }; - - expect(() => validateAnalyticsControllerState(state)).not.toThrow(); - }); - - it.each([ - [undefined], - [null], - [''], - ['not-a-uuid'], - ['12345'], - ['550e8400-e29b-41d4-a716'], - ['c232ab00-9414-11e8-8eb2-f2801f1b9fd1'], - ])( - 'throws with correct error message format for invalid input: %s', - (analyticsId) => { - let expectedMessage: string; - if (analyticsId === undefined) { - expectedMessage = 'undefined'; - } else if (analyticsId === null) { - expectedMessage = 'null'; - } else { - expectedMessage = JSON.stringify(analyticsId); - } - - const state = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId, - } as unknown as AnalyticsControllerState; - - expect(() => validateAnalyticsControllerState(state)).toThrow( - `Invalid analyticsId: expected a valid UUIDv4, but got ${expectedMessage}`, - ); - }, - ); - - it.each([ - ['550e8400-e29b-41d4-a716-446655440000'], - ['123e4567-e89b-42d3-a456-426614174000'], - ['00000000-0000-4000-8000-000000000000'], - ['ffffffff-ffff-4fff-8fff-ffffffffffff'], - ])('validates valid UUIDv4 format: %s', (uuid) => { - const state: AnalyticsControllerState = { - optedInForRegularAccount: false, - optedInForSocialAccount: false, - analyticsId: uuid, - }; - - expect(() => validateAnalyticsControllerState(state)).not.toThrow(); - }); - }); -}); diff --git a/packages/analytics-controller/src/analyticsStateValidator.ts b/packages/analytics-controller/src/analyticsStateValidator.ts deleted file mode 100644 index 0216f6127fb..00000000000 --- a/packages/analytics-controller/src/analyticsStateValidator.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { validate as validateUuid, version as getUuidVersion } from 'uuid'; - -import type { AnalyticsControllerState } from './AnalyticsController'; - -/** - * Validates that the analytics state has a valid UUIDv4 analyticsId. - * - * @param state - The analytics controller state to validate - * @throws {Error} If analyticsId is missing, invalid, or not a UUIDv4 - */ -export function validateAnalyticsControllerState(state: AnalyticsControllerState): void { - if ( - !state.analyticsId || - !validateUuid(state.analyticsId) || - getUuidVersion(state.analyticsId) !== 4 - ) { - throw new Error( - `Invalid analyticsId: expected a valid UUIDv4, but got ${JSON.stringify(state.analyticsId)}`, - ); - } -} From ac22cb18e40033dbe184cd9995129af2a806a200 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 24 Nov 2025 12:37:49 +0100 Subject: [PATCH 17/20] docs(analytics-controller): clarify dual-event behavior for sensitive-only properties --- .../src/AnalyticsController.test.ts | 27 ++++++++++++++++++- .../src/AnalyticsController.ts | 4 +++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index 6d931db1beb..7db0f5e05b8 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -272,7 +272,7 @@ describe('AnalyticsController', () => { expect(mockAdapter.track).toHaveBeenCalledWith('test_event'); }); - it('tracks sensitive event when sensitiveProperties are present', () => { + it('tracks sensitive event when both regular and sensitiveProperties are present', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -300,6 +300,31 @@ describe('AnalyticsController', () => { }); }); + it('tracks two events when only sensitiveProperties are present: one with empty props (user ID) and one with sensitive props (anonymous ID)', () => { + const mockAdapter = createMockAdapter(); + const { controller } = setupController({ + state: { + optedInForRegularAccount: true, + optedInForSocialAccount: false, + }, + platformAdapter: mockAdapter, + }); + + const event = createTestEvent( + 'test_event', + {}, + { sensitive_prop: 'sensitive value' }, + ); + controller.trackEvent(event); + + expect(mockAdapter.track).toHaveBeenCalledTimes(2); + expect(mockAdapter.track).toHaveBeenNthCalledWith(1, 'test_event', {}); + expect(mockAdapter.track).toHaveBeenNthCalledWith(2, 'test_event', { + isSensitive: true, + sensitive_prop: 'sensitive value', + }); + }); + it('does not call platform adapter when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index e02eae9942b..2c0211e70a7 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -244,6 +244,10 @@ export class AnalyticsController extends BaseController< } // Track regular properties (without isSensitive flag - it's the default) + // Note: Even if properties object is empty, we still send it to ensure + // an event with user ID is tracked. When only sensitiveProperties exist, + // this creates two events: one with empty props (user ID) and one with + // sensitive props (anonymous ID), which is the expected behavior. this.#platformAdapter.track(event.name, { ...event.properties, }); From 343017db99672664ddd7eab2423cd3582d25711d Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 24 Nov 2025 13:14:52 +0100 Subject: [PATCH 18/20] format: fix lint issues --- .../src/AnalyticsController.test.ts | 17 +++++++++-------- .../src/AnalyticsController.ts | 2 +- .../src/analyticsControllerStateValidator.ts | 4 +++- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/packages/analytics-controller/src/AnalyticsController.test.ts b/packages/analytics-controller/src/AnalyticsController.test.ts index 7db0f5e05b8..94f5aee5d8c 100644 --- a/packages/analytics-controller/src/AnalyticsController.test.ts +++ b/packages/analytics-controller/src/AnalyticsController.test.ts @@ -197,7 +197,8 @@ describe('AnalyticsController', () => { ); }); - it('ignores errors thrown by onSetupCompleted', () => { const mockAdapter = createMockAdapter(); + it('ignores errors thrown by onSetupCompleted', () => { + const mockAdapter = createMockAdapter(); const cause = new Error( 'MetaMetricsPrivacySegmentPlugin configure failed', ); @@ -220,7 +221,7 @@ describe('AnalyticsController', () => { }); describe('trackEvent', () => { - it('calls platform adapter to track event when enabled via regular opt-in', () => { + it('calls platform adapter to track event when enabled via regular opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -238,7 +239,7 @@ describe('AnalyticsController', () => { }); }); - it('tracks event via platform adapter when enabled via social opt-in', () => { + it('tracks event via platform adapter when enabled via social opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -325,7 +326,7 @@ describe('AnalyticsController', () => { }); }); - it('does not call platform adapter when disabled', () => { + it('does not call platform adapter when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -343,7 +344,7 @@ describe('AnalyticsController', () => { }); describe('identify', () => { - it('identifies user via platform adapter with traits using current analytics ID', () => { + it('identifies user via platform adapter with traits using current analytics ID', () => { const mockAdapter = createMockAdapter(); const customId = '550e8400-e29b-41d4-a716-446655440000'; const { controller } = setupController({ @@ -405,7 +406,7 @@ describe('AnalyticsController', () => { }); describe('trackView', () => { - it('calls platform adapter to track view when enabled via regular opt-in', () => { + it('calls platform adapter to track view when enabled via regular opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -422,7 +423,7 @@ describe('AnalyticsController', () => { }); }); - it('calls platform adapter to track view when enabled via social opt-in', () => { + it('calls platform adapter to track view when enabled via social opt-in', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { @@ -439,7 +440,7 @@ describe('AnalyticsController', () => { }); }); - it('does not call platform adapter when disabled', () => { + it('does not call platform adapter when disabled', () => { const mockAdapter = createMockAdapter(); const { controller } = setupController({ state: { diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index 2c0211e70a7..d7ecc3a9200 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -8,6 +8,7 @@ import type { Messenger } from '@metamask/messenger'; import { v4 as uuidv4 } from 'uuid'; import type { AnalyticsControllerMethodActions } from './AnalyticsController-method-action-types'; +import { validateAnalyticsControllerState } from './analyticsControllerStateValidator'; import { projectLogger } from './AnalyticsLogger'; import type { AnalyticsPlatformAdapter, @@ -16,7 +17,6 @@ import type { AnalyticsTrackingEvent, } from './AnalyticsPlatformAdapter.types'; import { analyticsControllerSelectors } from './selectors'; -import { validateAnalyticsControllerState } from './analyticsControllerStateValidator'; // === GENERAL === diff --git a/packages/analytics-controller/src/analyticsControllerStateValidator.ts b/packages/analytics-controller/src/analyticsControllerStateValidator.ts index 0216f6127fb..bd9acef26e8 100644 --- a/packages/analytics-controller/src/analyticsControllerStateValidator.ts +++ b/packages/analytics-controller/src/analyticsControllerStateValidator.ts @@ -8,7 +8,9 @@ import type { AnalyticsControllerState } from './AnalyticsController'; * @param state - The analytics controller state to validate * @throws {Error} If analyticsId is missing, invalid, or not a UUIDv4 */ -export function validateAnalyticsControllerState(state: AnalyticsControllerState): void { +export function validateAnalyticsControllerState( + state: AnalyticsControllerState, +): void { if ( !state.analyticsId || !validateUuid(state.analyticsId) || From 5b0ad0392ad55e49d2de79aa840f113007afb193 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 24 Nov 2025 13:50:18 +0100 Subject: [PATCH 19/20] fix: isSensitive prop override risk --- packages/analytics-controller/src/AnalyticsController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/analytics-controller/src/AnalyticsController.ts b/packages/analytics-controller/src/AnalyticsController.ts index d7ecc3a9200..83f6ac77ba5 100644 --- a/packages/analytics-controller/src/AnalyticsController.ts +++ b/packages/analytics-controller/src/AnalyticsController.ts @@ -255,9 +255,9 @@ export class AnalyticsController extends BaseController< // Track sensitive properties in a separate event with isSensitive flag if (hasSensitiveProperties) { this.#platformAdapter.track(event.name, { - isSensitive: true, ...event.properties, ...event.sensitiveProperties, + isSensitive: true, }); } } From c9fabe61c66446e9f0c585875fd7cdf0ef4979f5 Mon Sep 17 00:00:00 2001 From: Nicolas MASSART Date: Mon, 24 Nov 2025 17:51:14 +0100 Subject: [PATCH 20/20] format: fix readme format --- packages/analytics-controller/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/analytics-controller/README.md b/packages/analytics-controller/README.md index 644d9b75154..5db36d814ca 100644 --- a/packages/analytics-controller/README.md +++ b/packages/analytics-controller/README.md @@ -184,6 +184,7 @@ messenger.subscribe('AnalyticsController:stateChange', (state, prevState) => { ### Default State The controller initializes with default state values internally. The default state includes: + - `optedInForRegularAccount`: `false` - `optedInForSocialAccount`: `false` - `analyticsId`: Auto-generated UUIDv4