Merge pull request #1077 from OneSignal/user-model/fix-update-existing-pushsub-on-token-changes-insead-of-creating-new

[User Model] [Fix] Update existing subscription on token changes instead of creating new

Author: jkasten2

__test__/support/helpers/core.ts

@@ -5,7 +5,8 @@ import { CoreDelta } from "../../../src/core/models/CoreDeltas";
 import { SupportedSubscription, SubscriptionType } from "../../../src/core/models/SubscriptionModels";
 import { ModelName } from "../../../src/core/models/SupportedModels";
 import { DUMMY_MODEL_ID, DUMMY_PUSH_TOKEN, DUMMY_SUBSCRIPTION_ID } from "../constants";
-
+import CoreModule from "../../../src/core/CoreModule";
+import { CoreModuleDirector } from "../../../src/core/CoreModuleDirector";
 
 export function generateNewSubscription(modelId = '0000000000') {
   return new OSModel<SupportedSubscription>(
@@ -40,6 +41,13 @@ export function getDummyPushSubscriptionOSModel(): OSModel<SupportedSubscription
   }, DUMMY_MODEL_ID);
 }
 
+// Requirement: Test must also call TestEnvironment.initialize();
+export async function getCoreModuleDirector(): Promise<CoreModuleDirector> {
+  const coreModule = new CoreModule();
+  await coreModule.init();
+  return new CoreModuleDirector(coreModule);
+}
+
 export const passIfBroadcastNTimes = (target: number, broadcastCount: number, pass: () => void) => {
   if (broadcastCount === target) {
     pass();

__test__/unit/core/coreModuleDirector.test.ts

@@ -0,0 +1,46 @@
+import { CoreModuleDirector } from "../../../src/core/CoreModuleDirector";
+import { OSModel } from "../../../src/core/modelRepo/OSModel";
+import { SupportedSubscription } from "../../../src/core/models/SubscriptionModels";
+import { TestEnvironment } from "../../support/environment/TestEnvironment";
+import { getCoreModuleDirector, getDummyPushSubscriptionOSModel } from "../../support/helpers/core";
+
+describe('CoreModuleDirector tests', () => {
+  beforeEach(() => {
+    TestEnvironment.initialize();
+  });
+  describe('getPushSubscriptionModel', () => {
+    beforeEach(() => {
+      jest.resetAllMocks();
+    });
+
+    async function getPushSubscriptionModel(): Promise<OSModel<SupportedSubscription> | undefined> {
+      return (await getCoreModuleDirector()).getPushSubscriptionModel();
+    }
+
+    test('returns undefined when it find no push records', async () => {
+      expect(await getPushSubscriptionModel()).toBe(undefined);
+    });
+
+    test('returns current subscription when available', async () => {
+      const pushModelCurrent = getDummyPushSubscriptionOSModel();
+      test.stub(CoreModuleDirector.prototype, "getPushSubscriptionModelByCurrentToken", Promise.resolve(pushModelCurrent));
+      expect(await getPushSubscriptionModel()).toBe(pushModelCurrent);
+    });
+
+    test('returns last known subscription when current is unavailable', async () => {
+      const pushModelLastKnown = getDummyPushSubscriptionOSModel();
+      test.stub(CoreModuleDirector.prototype, "getPushSubscriptionModelByLastKnownToken", Promise.resolve(pushModelLastKnown));
+      expect(await getPushSubscriptionModel()).toBe(pushModelLastKnown);
+    });
+
+    test('returns current subscription over last known', async () => {
+      const pushModelCurrent = getDummyPushSubscriptionOSModel();
+      test.stub(CoreModuleDirector.prototype, "getPushSubscriptionModelByCurrentToken", Promise.resolve(pushModelCurrent));
+
+      const pushModelLastKnown = getDummyPushSubscriptionOSModel();
+      test.stub(CoreModuleDirector.prototype, "getPushSubscriptionModelByLastKnownToken", Promise.resolve(pushModelLastKnown));
+
+      expect(await getPushSubscriptionModel()).toBe(pushModelCurrent);
+    });
+  });
+});

src/core/CoreModuleDirector.ts

@@ -160,25 +160,39 @@ export class CoreModuleDirector {
     return modelStores.pushSubscriptions.models as { [key: string]: OSModel<SupportedSubscription> };
   }
 
+  private async getPushSubscriptionModelByCurrentToken()
+    :Promise<OSModel<SupportedSubscription> | undefined> {
+      logMethodCall("CoreModuleDirector.getPushSubscriptionModelByCurrentToken");
+      const pushToken = await MainHelper.getCurrentPushToken();
+      if (pushToken) {
+        return this.getSubscriptionOfTypeWithToken(ModelName.PushSubscriptions, pushToken);
+      }
+      return undefined;
+  }
+
+  // Browser may return a different PushToken value, use the last-known value as a fallback.
+  //   - This happens if you disable notification permissions then re-enable them.
+  private async getPushSubscriptionModelByLastKnownToken()
+    :Promise<OSModel<SupportedSubscription> | undefined> {
+    logMethodCall("CoreModuleDirector.getPushSubscriptionModelByLastKnownToken");
+    const { lastKnownPushToken } = await Database.getAppState();
+    if (lastKnownPushToken) {
+      return this.getSubscriptionOfTypeWithToken(ModelName.PushSubscriptions, lastKnownPushToken);
+    }
+    return undefined;
+  }
+
   /**
    * Gets the current push subscription model for the current browser.
    * @returns The push subscription model for the current browser, or undefined if no push subscription exists.
    */
-  // TO DO: make synchronous by making getting the current push token synchronous
-  public async getCurrentPushSubscriptionModel(): Promise<OSModel<SupportedSubscription> | undefined> {
+  public async getPushSubscriptionModel()
+    :Promise<OSModel<SupportedSubscription> | undefined> {
     logMethodCall("CoreModuleDirector.getPushSubscriptionModel");
-    let pushToken = await MainHelper.getCurrentPushToken();
-
-    if (!pushToken) {
-      const appState = await Database.getAppState();
-      pushToken = appState.lastKnownPushToken;
-
-      if (!pushToken) {
-        Log.debug("No push token found, returning undefined");
-        return undefined;
-      }
-    }
-    return this.getSubscriptionOfTypeWithToken(ModelName.PushSubscriptions, pushToken);
+    return (
+      await this.getPushSubscriptionModelByCurrentToken() ||
+      await this.getPushSubscriptionModelByLastKnownToken()
+    );
   }
 
   public getIdentityModel(): OSModel<SupportedIdentity> | undefined {
@@ -199,7 +213,7 @@ export class CoreModuleDirector {
     logMethodCall("CoreModuleDirector.getAllSubscriptionsModels");
     const emailSubscriptions = this.getEmailSubscriptionModels();
     const smsSubscriptions = this.getSmsSubscriptionModels();
-    const pushSubscription = await this.getCurrentPushSubscriptionModel();
+    const pushSubscription = await this.getPushSubscriptionModel();
 
     const subscriptions = Object.values(emailSubscriptions)
       .concat(Object.values(smsSubscriptions))

src/onesignal/PushSubscriptionNamespace.ts

@@ -29,7 +29,7 @@ export default class PushSubscriptionNamespace extends EventListenerBase {
     this._permission = permission;
     this._token = subscription.subscriptionToken;
 
-    OneSignal.coreDirector.getCurrentPushSubscriptionModel()
+    OneSignal.coreDirector.getPushSubscriptionModel()
       .then((pushModel: OSModel<SupportedSubscription> | undefined) => {
         if (pushModel && isCompleteSubscriptionObject(pushModel.data)) {
           this._id = pushModel.data.id;

src/page/managers/LoginManager.ts

@@ -48,7 +48,7 @@ export default class LoginManager {
         return;
       }
 
-      const pushSubModel = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+      const pushSubModel = await OneSignal.coreDirector.getPushSubscriptionModel();
       let currentPushSubscriptionId;
 
       if (pushSubModel && isCompleteSubscriptionObject(pushSubModel.data)) {
@@ -73,7 +73,7 @@ export default class LoginManager {
           }
         };
 
-        const pushSubscription = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+        const pushSubscription = await OneSignal.coreDirector.getPushSubscriptionModel();
         if (pushSubscription) {
           userData.subscriptions = [pushSubscription.data];
         }
@@ -122,7 +122,7 @@ export default class LoginManager {
     // before, logging out, process anything waiting in the delta queue so it's not lost
     OneSignal.coreDirector.forceDeltaQueueProcessingOnAllExecutors();
     UserDirector.resetUserMetaProperties();
-    const pushSubModel = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const pushSubModel = await OneSignal.coreDirector.getPushSubscriptionModel();
     await OneSignal.coreDirector.resetModelRepoAndCache();
     // add the push subscription model back to the repo since we need at least 1 sub to create a new user
     OneSignal.coreDirector.add(ModelName.PushSubscriptions, pushSubModel as OSModel<SupportedModel>, false);

src/shared/helpers/EventHelper.ts

@@ -32,26 +32,25 @@ export default class EventHelper {
 
     const appState = await Database.getAppState();
     const { lastKnownPushEnabled, lastKnownPushId, lastKnownPushToken, lastKnownOptedIn } = appState;
+
+    const currentPushToken = await MainHelper.getCurrentPushToken();
+
+    const pushModel = await OneSignal.coreDirector.getPushSubscriptionModel();
+    const pushSubscriptionId = pushModel?.data?.id;
+
     const didStateChange = (
       lastKnownPushEnabled === null ||
-      isPushEnabled !== lastKnownPushEnabled
+      isPushEnabled !== lastKnownPushEnabled ||
+      currentPushToken !== lastKnownPushToken ||
+      pushSubscriptionId !== lastKnownPushId
     );
     if (!didStateChange) {
       return;
     }
-    Log.info(
-      `The user's subscription state changed from ` +
-        `${lastKnownPushEnabled === null ? '(not stored)' : lastKnownPushEnabled} ⟶ ${isPushEnabled}`
-    );
 
     // update notification_types via core module
-    await context.subscriptionManager.updateNotificationTypes();
-
-    const currentPushToken = await MainHelper.getCurrentPushToken();
-    const pushModel: OSModel<SubscriptionModel> = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
-    const pushSubscriptionId = pushModel.data?.id;
+    await context.subscriptionManager.updateNotificationTypes()
 
-    await Database.setIsPushEnabled(isPushEnabled);
     appState.lastKnownPushEnabled = isPushEnabled;
     appState.lastKnownPushToken = currentPushToken;
     appState.lastKnownPushId = pushSubscriptionId;
@@ -71,7 +70,7 @@ export default class EventHelper {
         optedIn: isOptedIn,
       }
     };
-
+    Log.info("Push Subscription state changed: ", change);
     EventHelper.triggerSubscriptionChanged(change);
   }
 

src/shared/helpers/SubscriptionHelper.ts

@@ -120,8 +120,8 @@ export default class SubscriptionHelper {
   static getRawPushSubscriptionForLegacySafari(safariWebId: string): RawPushSubscription {
     const subscription = new RawPushSubscription();
 
-    const { deviceToken: existingDeviceToken } = window.safari.pushNotification.permission(safariWebId);
-    subscription.existingSafariDeviceToken = existingDeviceToken;
+    const { deviceToken } = window.safari.pushNotification.permission(safariWebId);
+    subscription.setFromSafariSubscription(deviceToken);
 
     return subscription;
   }

src/shared/managers/SubscriptionManager.ts

@@ -163,7 +163,7 @@ export class SubscriptionManager {
   }
 
   private async _updatePushSubscriptionModelWithRawSubscription(rawPushSubscription: RawPushSubscription) {
-    const pushModel = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const pushModel = await OneSignal.coreDirector.getPushSubscriptionModel();
 
     if (!pushModel) {
       OneSignal.coreDirector.generatePushSubscriptionModel(rawPushSubscription);
@@ -206,11 +206,10 @@ export class SubscriptionManager {
   }
 
   async updatePushSubscriptionNotificationTypes(notificationTypes: SubscriptionStateKind): Promise<void> {
-    const pushModel = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const pushModel = await OneSignal.coreDirector.getPushSubscriptionModel();
     if (!pushModel) {
-      throw new OneSignalError(
-        `Cannot update notification_types for push subscription model because it does not exist.`
-        );
+      Log.info("No Push Subscription yet to update notification_types.");
+      return;
     }
 
     pushModel.set("notification_types", notificationTypes);
@@ -366,8 +365,6 @@ export class SubscriptionManager {
     }
 
     const { deviceToken: existingDeviceToken } = window.safari.pushNotification.permission(this.config.safariWebId);
-    pushSubscriptionDetails.existingSafariDeviceToken = existingDeviceToken;
-
     if (!existingDeviceToken) {
       /*
         We're about to show the Safari native permission request. It can fail for a number of
@@ -605,10 +602,6 @@ export class SubscriptionManager {
 
     // Create our own custom object from the browser's native PushSubscription object
     const pushSubscriptionDetails = RawPushSubscription.setFromW3cSubscription(newPushSubscription);
-    if (existingPushSubscription) {
-      pushSubscriptionDetails.existingW3cPushSubscription =
-        RawPushSubscription.setFromW3cSubscription(existingPushSubscription);
-    }
     return pushSubscriptionDetails;
   }
 
@@ -800,7 +793,7 @@ export class SubscriptionManager {
   private async getSubscriptionStateForSecure(): Promise<PushSubscriptionState> {
     const { optedOut, subscriptionToken } = await Database.getSubscription();
 
-    const pushSubscriptionOSModel: OSModel<SupportedSubscription> | undefined = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const pushSubscriptionOSModel: OSModel<SupportedSubscription> | undefined = await OneSignal.coreDirector.getPushSubscriptionModel();
     const isValidPushSubscription = isCompleteSubscriptionObject(pushSubscriptionOSModel?.data) && !!pushSubscriptionOSModel?.onesignalId;
 
     if (Environment.useSafariLegacyPush()) {

src/shared/managers/UpdateManager.ts

@@ -61,7 +61,7 @@ export class UpdateManager {
       return;
     }
 
-    const subscriptionModel = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const subscriptionModel = await OneSignal.coreDirector.getPushSubscriptionModel();
 
     if (subscriptionModel?.data.notification_types !== SubscriptionStateKind.Subscribed &&
       OneSignal.config?.enableOnSession !== true) {
@@ -151,7 +151,7 @@ export class UpdateManager {
 
   public async sendOutcomeDirect(appId: string, notificationIds: string[], outcomeName: string, value?: number) {
     logMethodCall("sendOutcomeDirect");
-    const pushSubscriptionModel = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const pushSubscriptionModel = await OneSignal.coreDirector.getPushSubscriptionModel();
 
     if (pushSubscriptionModel && isCompleteSubscriptionObject(pushSubscriptionModel?.data)) {
       const outcomeRequestData: OutcomeRequestData = {
@@ -175,7 +175,7 @@ export class UpdateManager {
 
   public async sendOutcomeInfluenced(appId: string, notificationIds: string[], outcomeName: string, value?: number) {
     logMethodCall("sendOutcomeInfluenced");
-    const pushSubscriptionModel = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const pushSubscriptionModel = await OneSignal.coreDirector.getPushSubscriptionModel();
 
     if (pushSubscriptionModel && isCompleteSubscriptionObject(pushSubscriptionModel?.data)) {
       const outcomeRequestData: OutcomeRequestData = {
@@ -199,7 +199,7 @@ export class UpdateManager {
 
   public async sendOutcomeUnattributed(appId: string, outcomeName: string, value?: number) {
     logMethodCall("sendOutcomeUnattributed");
-    const pushSubscriptionModel = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const pushSubscriptionModel = await OneSignal.coreDirector.getPushSubscriptionModel();
 
     if (pushSubscriptionModel && isCompleteSubscriptionObject(pushSubscriptionModel?.data)) {
       const outcomeRequestData: OutcomeRequestData = {

src/shared/managers/sessionManager/SessionManager.ts

@@ -92,7 +92,7 @@ export class SessionManager implements ISessionManager {
 
   async _getOneSignalAndSubscriptionIds(): Promise<{ onesignalId: string; subscriptionId: string }> {
     const identityModel = OneSignal.coreDirector.getIdentityModel();
-    const pushSubscriptionModel = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const pushSubscriptionModel = await OneSignal.coreDirector.getPushSubscriptionModel();
 
     if (!identityModel || !identityModel.onesignalId) {
       throw new OneSignalError("Abort _getOneSignalAndSubscriptionIds: no identity");
@@ -340,7 +340,7 @@ export class SessionManager implements ISessionManager {
       return;
     }
 
-    const pushSubscription = await OneSignal.coreDirector.getCurrentPushSubscriptionModel();
+    const pushSubscription = await OneSignal.coreDirector.getPushSubscriptionModel();
     if (pushSubscription?.data.notification_types !== SubscriptionStateKind.Subscribed &&
       OneSignal.config?.enableOnSession !== true) {
       return;