feat(analytics-controller): resolve geolocation only after opt-in

packages/analytics-controller/CHANGELOG.md

@@ -9,9 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
-- **BREAKING:** Optionally enrich non-anonymous track, identify, and view payloads with the user's `country_code`, `region`, and `timezone` under `context.location`, gated behind the new `isGeolocationEnabled` constructor option (default `false`) ([#9691](https://github.com/MetaMask/core/pull/9691))
-  - `AnalyticsController.init` is now asynchronous and returns a `Promise<void>`, so await it before tracking events
-  - When `isGeolocationEnabled` is `true`, the geolocation is resolved during `init` via `GeolocationController:getGeolocationData`, which compositions must register and initialize before `AnalyticsController` (otherwise enrichment is skipped for the session)
+- **BREAKING:** Optionally enrich non-anonymous track, identify, and view payloads with the user's `country_code`, `region`, and `timezone` under `context.location`, gated behind the new `isGeolocationEnabled` constructor option (default `false`) ([#9691](https://github.com/MetaMask/core/pull/9691), [#9728](https://github.com/MetaMask/core/pull/9728))
+  - `AnalyticsController.init` and `AnalyticsController.optIn` are now asynchronous and return a `Promise<void>`, so await them before tracking events
+  - When enabled, geolocation is resolved via `GeolocationController:getGeolocationData` (which compositions must register) only after the user opts in, so location is never requested before they consent to analytics; queued pre-consent events are then enriched on replay (anonymous payloads excluded)
   - Adds `@metamask/geolocation-controller` `^0.1.3` as a dependency
 - Bump `@metamask/messenger` from `^1.2.0` to `^2.0.0` ([#9392](https://github.com/MetaMask/core/pull/9392))
 

packages/analytics-controller/src/AnalyticsController-method-action-types.ts

@@ -46,6 +46,12 @@ export type AnalyticsControllerTrackViewAction = {
  *
  * Records that a consent decision has been made and replays any events that
  * were queued while the user was undecided.
+ *
+ * When geolocation enrichment is enabled, geolocation is resolved here (once
+ * the user has consented) and awaited before the queued events are replayed,
+ * so those events are enriched with the resolved location as they are sent.
+ *
+ * @returns A promise that resolves once opt-in processing has completed.
  */
 export type AnalyticsControllerOptInAction = {
   type: `AnalyticsController:optIn`;

packages/analytics-controller/src/AnalyticsController.test.ts

@@ -1726,7 +1726,10 @@ describe('AnalyticsController', () => {
       });
     });
 
-    it('replays pre-consent events with the location context captured at track time', async () => {
+    it('defers geolocation to opt-in, then enriches replayed pre-consent events and later events', async () => {
+      const geolocationHandler = jest.fn(() =>
+        Promise.resolve(buildGeolocationData(fullGeolocation)),
+      );
       const mockAdapter = createMockAdapter();
       const { controller } = await setupController({
         state: {
@@ -1736,20 +1739,144 @@ describe('AnalyticsController', () => {
         },
         platformAdapter: mockAdapter,
         isPreConsentQueueEnabled: true,
-        geolocation: fullGeolocation,
+        isGeolocationEnabled: true,
+        geolocationHandler,
       });
 
-      controller.trackEvent(createTestEvent('test_event'));
+      // While undecided, the event is queued and geolocation is not requested.
+      controller.trackEvent(createTestEvent('preconsent_event'));
       expect(mockAdapter.track).not.toHaveBeenCalled();
+      expect(geolocationHandler).not.toHaveBeenCalled();
 
-      controller.optIn();
+      // optIn resolves geolocation (awaited) before replaying the queue.
+      await controller.optIn();
+      expect(geolocationHandler).toHaveBeenCalledTimes(1);
 
+      // The replayed pre-consent event is enriched with the location resolved
+      // on opt-in.
       expect(mockAdapter.track).toHaveBeenCalledWith(
-        'test_event',
+        'preconsent_event',
         undefined,
         { location: fullLocationContext },
         expect.any(Object),
       );
+
+      // Events tracked after opt-in are enriched too.
+      controller.trackEvent(createTestEvent('postconsent_event'));
+      expect(mockAdapter.track).toHaveBeenLastCalledWith(
+        'postconsent_event',
+        undefined,
+        { location: fullLocationContext },
+      );
+    });
+
+    it('does not enrich an anonymous pre-consent payload on replay', async () => {
+      const mockAdapter = createMockAdapter();
+      const { controller } = await setupController({
+        state: {
+          optedIn: false,
+          consentDecisionMade: false,
+          analyticsId,
+        },
+        platformAdapter: mockAdapter,
+        isPreConsentQueueEnabled: true,
+        isAnonymousEventsFeatureEnabled: true,
+        isGeolocationEnabled: true,
+        geolocation: fullGeolocation,
+      });
+
+      controller.trackEvent(
+        createTestEvent(
+          'test_event',
+          { prop: 'value' },
+          { sensitive_prop: 'sensitive value' },
+        ),
+      );
+
+      await controller.optIn();
+
+      // The identified payload is enriched...
+      expect(mockAdapter.track).toHaveBeenCalledWith(
+        'test_event',
+        { prop: 'value' },
+        { location: fullLocationContext },
+        expect.any(Object),
+      );
+      // ...but the anonymous payload carries no location.
+      expect(mockAdapter.track).toHaveBeenCalledWith(
+        'test_event',
+        {
+          prop: 'value',
+          sensitive_prop: 'sensitive value',
+          anonymous: true,
+        },
+        undefined,
+        expect.any(Object),
+      );
+    });
+
+    it('does not resolve geolocation at init when the user is not opted in', async () => {
+      const geolocationHandler = jest.fn(() =>
+        Promise.resolve(buildGeolocationData(fullGeolocation)),
+      );
+      const mockAdapter = createMockAdapter();
+      await setupController({
+        state: {
+          optedIn: false,
+          consentDecisionMade: true,
+          analyticsId,
+        },
+        platformAdapter: mockAdapter,
+        isGeolocationEnabled: true,
+        geolocationHandler,
+      });
+
+      // init awaits geolocation resolution, so if it were going to request
+      // location it would have by now.
+      expect(geolocationHandler).not.toHaveBeenCalled();
+    });
+
+    it('does not replay pre-consent events if consent is reset while geolocation resolves', async () => {
+      let resolveGeolocation: (data: GeolocationData) => void = () => undefined;
+      const geolocationHandler = jest.fn(
+        () =>
+          new Promise<GeolocationData>((resolve) => {
+            resolveGeolocation = resolve;
+          }),
+      );
+      const mockAdapter = createMockAdapter();
+      const { controller } = await setupController({
+        state: {
+          optedIn: false,
+          consentDecisionMade: false,
+          analyticsId,
+        },
+        platformAdapter: mockAdapter,
+        isPreConsentQueueEnabled: true,
+        isGeolocationEnabled: true,
+        geolocationHandler,
+      });
+
+      controller.trackEvent(createTestEvent('preconsent_event'));
+      expect(mockAdapter.track).not.toHaveBeenCalled();
+
+      // Opt in, but leave geolocation resolving (do not await yet).
+      const optInPromise = controller.optIn();
+      expect(geolocationHandler).toHaveBeenCalledTimes(1);
+
+      // The user resets their consent decision before geolocation resolves.
+      controller.resetConsentDecision();
+
+      // Geolocation resolves and optIn finishes reconciling.
+      resolveGeolocation(buildGeolocationData(fullGeolocation));
+      await optInPromise;
+
+      // The pre-consent event is not delivered — the user is undecided again —
+      // and it remains queued for a later decision.
+      expect(mockAdapter.track).not.toHaveBeenCalled();
+      expect(
+        Object.values(controller.state.preConsentEventQueue ?? {}),
+      ).toHaveLength(1);
     });
   });
 
@@ -2393,7 +2520,7 @@ describe('AnalyticsController', () => {
         },
       });
 
-      controller.optIn();
+      await controller.optIn();
 
       expect(controller.state.optedIn).toBe(true);
       expect(controller.state.consentDecisionMade).toBe(true);
@@ -2499,7 +2626,7 @@ describe('AnalyticsController', () => {
 
       expect(controller.state.preConsentEventQueue).toStrictEqual({});
 
-      controller.optIn();
+      await controller.optIn();
 
       expect(controller.state.optedIn).toBe(true);
       expect(mockAdapter.track).not.toHaveBeenCalled();
@@ -2516,7 +2643,7 @@ describe('AnalyticsController', () => {
         isPreConsentQueueEnabled: true,
       });
 
-      controller.optIn();
+      await controller.optIn();
 
       expect(controller.state.optedIn).toBe(true);
       expect(controller.state.consentDecisionMade).toBe(true);
@@ -2545,7 +2672,7 @@ describe('AnalyticsController', () => {
       const { controller, mockAdapter } =
         await setupControllerWithQueuedEvent();
 
-      controller.optIn();
+      await controller.optIn();
 
       expect(controller.state.optedIn).toBe(true);
       expect(controller.state.consentDecisionMade).toBe(true);
@@ -2575,7 +2702,7 @@ describe('AnalyticsController', () => {
       // Held in the pre-consent queue, not yet in the delivery queue.
       expect(controller.state.eventQueue ?? {}).toStrictEqual({});
 
-      controller.optIn();
+      await controller.optIn();
 
       // The pre-consent queue is drained and the event is now tracked for
       // delivery (the mock adapter never acks, so it remains in eventQueue).
@@ -2707,7 +2834,7 @@ describe('AnalyticsController', () => {
         Object.values(controller.state.preConsentEventQueue ?? {}),
       ).toHaveLength(2);
 
-      controller.optIn();
+      await controller.optIn();
 
       expect(mockAdapter.track).toHaveBeenCalledTimes(2);
       expect(mockAdapter.track).toHaveBeenCalledWith(
@@ -2751,7 +2878,7 @@ describe('AnalyticsController', () => {
         isPreConsentQueueEnabled: true,
       });
 
-      controller.optIn();
+      await controller.optIn();
 
       // Only the valid entry is replayed; every malformed entry is dropped.
       expect(mockAdapter.track).toHaveBeenCalledTimes(1);