warn user must be logged in first for track event

Author: fadi-george

src/onesignal/OneSignal.test.ts

@@ -972,6 +972,9 @@ describe('OneSignal', () => {
     test('can send a custom event', async () => {
       setSendCustomEventResponse();
 
+      OneSignal.coreDirector
+        .getIdentityModel()
+        .setProperty('external_id', 'some-id', ModelChangeTags.NO_PROPOGATE);
       window.OneSignal.User.trackEvent(name, properties);
 
       await vi.waitUntil(() => sendCustomEventFn.mock.calls.length === 1);
@@ -980,6 +983,7 @@ describe('OneSignal', () => {
         events: [
           {
             name,
+            external_id: 'some-id',
             onesignal_id: DUMMY_ONESIGNAL_ID,
             payload: {
               os_sdk: OS_SDK,
@@ -991,7 +995,7 @@ describe('OneSignal', () => {
       });
     });
 
-    test('custom event should wait for login to complete for a fresh user', async () => {
+    test('can send a custom event after login', async () => {
       setCreateUserResponse({});
       setGetUserResponse({
         onesignalId: DUMMY_ONESIGNAL_ID,
@@ -1000,21 +1004,19 @@ describe('OneSignal', () => {
       setSendCustomEventResponse();
 
       const identityModel = window.OneSignal.coreDirector.getIdentityModel();
-      const startingOnesignalId = IDManager.createLocalId();
       identityModel.setProperty(
-        'onesignal_id',
-        startingOnesignalId,
+        'external_id',
+        'some-id',
         ModelChangeTags.NO_PROPOGATE,
       );
 
+      window.OneSignal.login('some-id-2');
       window.OneSignal.User.trackEvent(name, properties);
-      window.OneSignal.login('some-id');
 
       const queue = await getQueue(2);
 
       // login and custom event should have matching id (via UserDirector reset user models)
       const newLocalId = queue[0].operation.onesignalId;
-      expect(newLocalId).not.toBe(startingOnesignalId);
       expect(queue[1].operation.onesignalId).toBe(newLocalId);
 
       // should translate ids for the custom event
@@ -1027,6 +1029,7 @@ describe('OneSignal', () => {
       expect(sendCustomEventFn).toHaveBeenCalledWith({
         events: [
           {
+            external_id: 'some-id-2',
             name,
             onesignal_id: DUMMY_ONESIGNAL_ID,
             payload: {

src/onesignal/User.ts

@@ -161,7 +161,7 @@ export default class User {
     const hasOneSignalId =
       !!OneSignal.coreDirector.getIdentityModel().onesignalId;
     if (!hasOneSignalId) {
-      Log.error('Call login before adding an email/sms subscription');
+      Log.error('User must be logged in first.');
     }
     return hasOneSignalId;
   }
@@ -283,6 +283,7 @@ export default class User {
   }
 
   public trackEvent(name: string, properties: Record<string, unknown> = {}) {
+    if (!this.validateUserExists()) return;
     if (!isObjectSerializable(properties)) {
       return Log.error(
         'Custom event properties must be a JSON-serializable object',

src/onesignal/UserDirector.ts

@@ -2,7 +2,6 @@ import { IdentityModel } from 'src/core/models/IdentityModel';
 import { PropertiesModel } from 'src/core/models/PropertiesModel';
 import { CreateSubscriptionOperation } from 'src/core/operations/CreateSubscriptionOperation';
 import { LoginUserOperation } from 'src/core/operations/LoginUserOperation';
-import { ModelChangeTags } from 'src/core/types/models';
 import Log from 'src/shared/libraries/Log';
 import { IDManager } from 'src/shared/managers/IDManager';
 import MainHelper from '../shared/helpers/MainHelper';
@@ -56,9 +55,6 @@ export default class UserDirector {
   // Resets models similar to Android SDK
   // https://github.com/OneSignal/OneSignal-Android-SDK/blob/ed2e87618ea3af81b75f97b0a4cbb8f658c7fc80/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/internal/OneSignalImp.kt#L448
   static resetUserModels() {
-    const prevOnesignalId =
-      OneSignal.coreDirector.getIdentityModel().onesignalId;
-
     // replace models
     const newIdentityModel = new IdentityModel();
     const newPropertiesModel = new PropertiesModel();
@@ -67,17 +63,6 @@ export default class UserDirector {
     newIdentityModel.onesignalId = sdkId;
     newPropertiesModel.onesignalId = sdkId;
 
-    // update onesignalId for all operations in the queue
-    const opList = OneSignal.coreDirector.operationRepo.queue;
-    opList.forEach((op) => {
-      if (
-        IDManager.isLocalId(op.operation.onesignalId) &&
-        op.operation.onesignalId === prevOnesignalId
-      ) {
-        op.operation.setProperty('onesignalId', sdkId, ModelChangeTags.HYDRATE);
-      }
-    });
-
     OneSignal.coreDirector.identityModelStore.replace(newIdentityModel);
     OneSignal.coreDirector.propertiesModelStore.replace(newPropertiesModel);
   }

src/onesignal/UserNamespace.test.ts

@@ -2,6 +2,7 @@ import {
   DUMMY_ONESIGNAL_ID,
   DUMMY_PUSH_TOKEN,
 } from '__test__/support/constants';
+import { ModelChangeTags } from 'src/core/types/models';
 import Log from 'src/shared/libraries/Log';
 import { IDManager } from 'src/shared/managers/IDManager';
 import { TestEnvironment } from '../../__test__/support/environment/TestEnvironment';
@@ -438,6 +439,18 @@ describe('UserNamespace', () => {
         test_property: 'test_value',
       };
 
+      userNamespace.trackEvent(name, {});
+      expect(errorSpy).toHaveBeenCalledWith('User must be logged in first.');
+      errorSpy.mockClear();
+
+      const identityModel = OneSignal.coreDirector.getIdentityModel();
+      identityModel.setProperty(
+        'onesignal_id',
+        DUMMY_ONESIGNAL_ID,
+        ModelChangeTags.NO_PROPOGATE,
+      );
+
+      console.log('zzz');
       // should validate properties
       // @ts-expect-error - mock invalid argument
       userNamespace.trackEvent(name, 123);

src/onesignal/UserNamespace.ts

@@ -1,5 +1,3 @@
-import { ModelChangeTags } from 'src/core/types/models';
-import { IDManager } from 'src/shared/managers/IDManager';
 import type { UserChangeEvent } from '../page/models/UserChangeEvent';
 import { EventListenerBase } from '../page/userModel/EventListenerBase';
 import Emitter from '../shared/libraries/Emitter';
@@ -21,16 +19,6 @@ export default class UserNamespace extends EventListenerBase {
   ) {
     super();
     if (initialize) {
-      // operation model store expect operations to have a onesignalId even if it's a local id
-      const identityModel = OneSignal.coreDirector.getIdentityModel();
-      if (!identityModel.onesignalId) {
-        identityModel.setProperty(
-          'onesignal_id',
-          IDManager.createLocalId(),
-          ModelChangeTags.HYDRATE,
-        );
-      }
-
       this._currentUser = User.createOrGetInstance();
       this.PushSubscription = new PushSubscriptionNamespace(
         true,

src/page/managers/LoginManager.ts

@@ -2,7 +2,6 @@ import { LoginUserOperation } from 'src/core/operations/LoginUserOperation';
 import { TransferSubscriptionOperation } from 'src/core/operations/TransferSubscriptionOperation';
 import { ModelChangeTags } from 'src/core/types/models';
 import MainHelper from 'src/shared/helpers/MainHelper';
-import { IDManager } from 'src/shared/managers/IDManager';
 import OneSignal from '../../onesignal/OneSignal';
 import UserDirector from '../../onesignal/UserDirector';
 import Log from '../../shared/libraries/Log';
@@ -25,9 +24,7 @@ export default class LoginManager {
     }
 
     let identityModel = OneSignal.coreDirector.getIdentityModel();
-    const currentOneSignalId = IDManager.isLocalId(identityModel.onesignalId)
-      ? undefined
-      : identityModel.onesignalId;
+    const currentOneSignalId = identityModel.onesignalId;
     const currentExternalId = identityModel.externalId;
 
     // if the current externalId is the same as the one we're trying to set, do nothing