fix: retain FCM token when re-enabling notifications with all accounts disabled [GE-516]

packages/notification-services-controller/CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Bump `@metamask/profile-sync-controller` from `^32.1.1` to `^32.2.0` ([#10348](https://github.com/MetaMask/core/pull/10348))
 
+### Fixed
+
+- Retain a locally-created FCM token when re-enabling notifications with every wallet-activity account disabled ([#10401](https://github.com/MetaMask/core/pull/10401)).
+
 ## [29.0.0]
 
 ### Changed

packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts

@@ -1240,12 +1240,12 @@ describe('NotificationServicesController', () => {
         await controller.createOnChainTriggers();
 
         expect(mockTriggerUpdate.isDone()).toBe(false);
-        // No addresses to register, so the device is unregistered rather than
-        // sent an empty list the push API would reject.
+        // Register the device token without linking any wallet-activity
+        // addresses. A later account opt-in can then link this token.
         await waitFor(() => {
-          expect(mockDisablePushNotifications).toHaveBeenCalled();
+          expect(mockEnablePushNotifications).toHaveBeenCalledWith([]);
         });
-        expect(mockEnablePushNotifications).not.toHaveBeenCalled();
+        expect(mockDisablePushNotifications).not.toHaveBeenCalled();
       });
 
       it('ignores the legacy wallet-activity push toggle when registering push notifications', async () => {
@@ -1307,6 +1307,34 @@ describe('NotificationServicesController', () => {
         expect(mockUpdateNotifications).not.toHaveBeenCalled();
         expect(mockEnablePushNotifications).toHaveBeenCalled();
       });
+
+      it('creates a push registration when re-enabling with all wallet-activity accounts disabled', async () => {
+        const {
+          messenger,
+          mockEnablePushNotifications,
+          mockDisablePushNotifications,
+        } = arrangeMocks({
+          configurePrefs: (mock) =>
+            mock.mockResolvedValueOnce(mockPreferences()),
+        });
+        mockGetOnChainNotificationsConfig({
+          status: 200,
+          body: [{ address: ADDRESS_1.toLowerCase(), enabled: false }],
+        });
+
+        const controller = new NotificationServicesController({
+          messenger,
+          env: { featureAnnouncements: featureAnnouncementsEnv },
+        });
+        controller.init();
+
+        await controller.enableMetamaskNotifications();
+
+        await waitFor(() => {
+          expect(mockEnablePushNotifications).toHaveBeenCalledWith([]);
+        });
+        expect(mockDisablePushNotifications).not.toHaveBeenCalled();
+      });
     });
 
     it('throws if not given a valid auth & bearer token', async () => {
@@ -2213,7 +2241,7 @@ describe('NotificationServicesController', () => {
       expect(mockEnablePushNotifications).not.toHaveBeenCalled();
     });
 
-    it('unregisters the device when no account has notifications enabled', async () => {
+    it('creates a push registration without linking addresses when all accounts are disabled', async () => {
       const {
         messenger,
         mockEnablePushNotifications,
@@ -2234,10 +2262,8 @@ describe('NotificationServicesController', () => {
 
       await controller.enablePushNotifications();
 
-      // The push API rejects a registration with no addresses, which would
-      // leave the device's existing links in place.
-      expect(mockDisablePushNotifications).toHaveBeenCalled();
-      expect(mockEnablePushNotifications).not.toHaveBeenCalled();
+      expect(mockEnablePushNotifications).toHaveBeenCalledWith([]);
+      expect(mockDisablePushNotifications).not.toHaveBeenCalled();
     });
 
     it('leaves existing push links alone when the Trigger API is unreadable', async () => {

packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts

@@ -812,19 +812,12 @@ export class NotificationServicesController extends BaseController<
   /**
    * Registers this device for push notifications on the given addresses.
    *
-   * An empty list cannot be sent: the push API rejects a registration with no
-   * addresses, and because that request is what performs the delete-and-reinsert
-   * of the device's links, a rejection leaves the previous links in place and
-   * push keeps arriving. So "no addresses" has to mean unregistering the device.
+   * Always register the device token, including when no addresses are enabled.
+   * The links API rejects an empty list without removing the token.
    *
    * @param addresses - The addresses to receive push notifications for.
    */
   async #registerPushNotifications(addresses: string[]): Promise<void> {
-    if (addresses.length === 0) {
-      await this.#pushNotifications.disablePushNotifications();
-      return;
-    }
-
     await this.#pushNotifications.enablePushNotifications(addresses);
   }
 

packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts

@@ -97,6 +97,20 @@ describe('NotificationServicesPushController', () => {
       expect(controller.state.isUpdatingFCMToken).toBe(false);
     });
 
+    it('creates and retains an FCM token when no addresses are linked', async () => {
+      const mocks = arrangeServicesMocks();
+      const { controller, messenger } = arrangeMockMessenger();
+      mockAuthBearerTokenCall(messenger);
+
+      await controller.enablePushNotifications([]);
+
+      expect(mocks.activatePushNotificationsMock).toHaveBeenCalledWith(
+        expect.objectContaining({ addresses: [] }),
+      );
+      expect(controller.state.fcmToken).toBe(MOCK_FCM_TOKEN);
+      expect(controller.state.isPushEnabled).toBe(true);
+    });
+
     it('should call activatePushNotifications with correct parameters including oldToken', async () => {
       const mocks = arrangeServicesMocks();
       const { controller, messenger } = arrangeMockMessenger({

packages/notification-services-controller/src/NotificationServicesPushController/services/services.test.ts

@@ -170,6 +170,17 @@ describe('NotificationServicesPushController Services', () => {
       expect(result).toBeNull();
     });
 
+    it('creates an FCM token without calling the links API when there are no addresses', async () => {
+      const { params, apis } = arrangeMocks();
+      params.addresses = [];
+
+      const result = await activatePushNotifications(params);
+
+      expect(params.createRegToken).toHaveBeenCalled();
+      expect(apis.mockPut.isDone()).toBe(false);
+      expect(result).toBe(MOCK_NEW_REG_TOKEN);
+    });
+
     it('should handle oldToken parameter when provided', async () => {
       const { params, apis } = arrangeMocks();
       const paramsWithOldToken = {

packages/notification-services-controller/src/NotificationServicesPushController/services/services.ts

@@ -155,6 +155,12 @@ export async function activatePushNotifications(
     return null;
   }
 
+  // The links API rejects an empty address list. Skipping that request still
+  // returns the token created above.
+  if (params.addresses.length === 0) {
+    return regToken;
+  }
+
   await updateLinksAPI({
     bearerToken: params.bearerToken,
     addresses: params.addresses,