From f7f6d3dd52b272417f1ca7a524d8a9cd667a777b Mon Sep 17 00:00:00 2001 From: Fadi George Date: Tue, 3 Mar 2026 14:37:03 -0800 Subject: [PATCH 1/4] fix(auth): improve logout flow and subscription handling --- src/core/CoreModuleDirector.ts | 15 ++++---- src/core/operationRepo/OperationRepo.ts | 1 + src/onesignal/userDirector.ts | 27 +++++--------- src/page/managers/LoginManager.ts | 47 +++++++++++++++++++------ src/shared/helpers/dom.ts | 27 -------------- 5 files changed, 55 insertions(+), 62 deletions(-) diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index 20fb5b125..dbcf91edd 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -116,6 +116,7 @@ export class CoreModuleDirector { > { logMethodCall('CoreModuleDirector.getPushSubscriptionModelByCurrentToken'); const pushToken = await getCurrentPushToken(); + console.log({ pushToken }); if (pushToken) { return this._getSubscriptionOfTypeWithToken( SubscriptionChannel._Push, @@ -133,8 +134,11 @@ export class CoreModuleDirector { logMethodCall( 'CoreModuleDirector.getPushSubscriptionModelByLastKnownToken', ); - const lastKnownPushToken = await getPushToken(); - if (lastKnownPushToken) { + + // Checking '' in case we create a temp/fake subscription on logi + const lastKnownPushToken = (await getPushToken()) ?? ''; + console.log({ lastKnownPushToken }); + if (lastKnownPushToken !== null) { return this._getSubscriptionOfTypeWithToken( SubscriptionChannel._Push, lastKnownPushToken, @@ -151,10 +155,9 @@ export class CoreModuleDirector { SubscriptionModel | undefined > { logMethodCall('CoreModuleDirector.getPushSubscriptionModel'); - return ( - (await this._getPushSubscriptionModelByCurrentToken()) || - (await this._getPushSubscriptionModelByLastKnownToken()) - ); + const sub = await this._getPushSubscriptionModelByLastKnownToken(); + console.log({ sub }); + return (await this._getPushSubscriptionModelByCurrentToken()) || sub; } public _getIdentityModel(): IdentityModel { diff --git a/src/core/operationRepo/OperationRepo.ts b/src/core/operationRepo/OperationRepo.ts index ce5a5517d..040e1be1c 100644 --- a/src/core/operationRepo/OperationRepo.ts +++ b/src/core/operationRepo/OperationRepo.ts @@ -146,6 +146,7 @@ export class OperationRepo implements IOperationRepo, IStartableService { if (runningOps) return Log._debug('Operations in progress'); const ops = this._getNextOps(this._executeBucket); + console.log('ops', ops); if (ops) { runningOps = true; diff --git a/src/onesignal/userDirector.ts b/src/onesignal/userDirector.ts index 48366ba17..af4d8a9be 100644 --- a/src/onesignal/userDirector.ts +++ b/src/onesignal/userDirector.ts @@ -21,32 +21,23 @@ export async function createUserOnServer(): Promise { } const pushOp = await OneSignal._coreDirector._getPushSubscriptionModel(); - if (pushOp) { - const subData = pushOp.toJSON(); - OneSignal._coreDirector._operationRepo._enqueue( - new LoginUserOperation( - appId, - identityModel._onesignalId, - identityModel._externalId, - ), - ); + OneSignal._coreDirector._operationRepo._enqueue( + new LoginUserOperation( + appId, + identityModel._onesignalId, + identityModel._externalId, + ), + ); + if (pushOp) { await OneSignal._coreDirector._operationRepo._enqueueAndWait( new CreateSubscriptionOperation({ - ...subData, + ...pushOp.toJSON(), appId, onesignalId: identityModel._onesignalId, subscriptionId: pushOp.id!, }), ); - } else { - OneSignal._coreDirector._operationRepo._enqueue( - new LoginUserOperation( - appId, - identityModel._onesignalId, - identityModel._externalId, - ), - ); } } diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index d42514c5f..7f5d6d351 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -1,14 +1,13 @@ import { IdentityConstants } from 'src/core/constants'; +import { SubscriptionModel } from 'src/core/models/SubscriptionModel'; import { LoginUserOperation } from 'src/core/operations/LoginUserOperation'; import { TransferSubscriptionOperation } from 'src/core/operations/TransferSubscriptionOperation'; import { ModelChangeTags } from 'src/core/types/models'; import { db } from 'src/shared/database/client'; +import { getSubscriptionType } from 'src/shared/environment/detect'; import { getAppId } from 'src/shared/helpers/main'; import { IDManager } from 'src/shared/managers/IDManager'; -import { - createUserOnServer, - resetUserModels, -} from '../../onesignal/userDirector'; +import { resetUserModels } from '../../onesignal/userDirector'; import Log from '../../shared/libraries/Log'; export default class LoginManager { @@ -66,6 +65,16 @@ export default class LoginManager { pushOp.id, ), ); + } else { + // unlike mobile which creates a "fake" sub right away, for Web we just want to create a new sub when we login + const newSub = new SubscriptionModel(); + newSub._mergeData({ + id: IDManager._createLocalId(), + onesignalId: newIdentityOneSignalId, + type: getSubscriptionType(), + token: '', + }); + OneSignal._coreDirector._subscriptionModelStore._add(newSub); } }), OneSignal._coreDirector._operationRepo._enqueueAndWait( @@ -88,20 +97,36 @@ export default class LoginManager { private static async _logout(): Promise { // check if user is already logged out - const identityModel = OneSignal._coreDirector._getIdentityModel(); + let identityModel = OneSignal._coreDirector._getIdentityModel(); if (!identityModel._externalId) return Log._debug('Logout: User is not logged in, skipping logout'); - const hasAnySubscription = - OneSignal._coreDirector._subscriptionModelStore._list().length > 0; - resetUserModels(); + identityModel = OneSignal._coreDirector._getIdentityModel(); // Only create an anonymous user if there is a subscription to associate with it. // Without a subscription, there is nothing meaningful to register on the server. - if (hasAnySubscription) { - return createUserOnServer(); - } + const appId = getAppId(); + let newIdentityOneSignalId = identityModel._onesignalId; + + const promises: Promise[] = [ + OneSignal._coreDirector._getPushSubscriptionModel().then((pushOp) => { + if (pushOp) { + OneSignal._coreDirector._operationRepo._enqueue( + new TransferSubscriptionOperation( + appId, + newIdentityOneSignalId, + pushOp.id, + ), + ); + } + }), + OneSignal._coreDirector._operationRepo._enqueueAndWait( + new LoginUserOperation(appId, newIdentityOneSignalId), + ), + ]; + + await Promise.all(promises); } } diff --git a/src/shared/helpers/dom.ts b/src/shared/helpers/dom.ts index 38d4b3133..a20cecdf6 100644 --- a/src/shared/helpers/dom.ts +++ b/src/shared/helpers/dom.ts @@ -107,33 +107,6 @@ export function removeCssClass( } } -export function hasCssClass( - targetSelectorOrElement: Element | string, - cssClass: string, -) { - if (typeof targetSelectorOrElement === 'string') { - const element = document.querySelector(targetSelectorOrElement); - if (element === null) { - throw new Error( - `Cannot find element with selector "${targetSelectorOrElement}"`, - ); - } - return element.classList.contains(cssClass); - } else if (typeof targetSelectorOrElement === 'object') { - return targetSelectorOrElement.classList.contains(cssClass); - } else { - throw new Error( - `${targetSelectorOrElement} must be a CSS selector string or DOM Element object.`, - ); - } -} - -export async function waitForAnimations(el: HTMLElement | null) { - if (!el) return; - const anims = el.getAnimations(); - if (anims.length) await Promise.allSettled(anims.map((a) => a.finished)); -} - export function decodeHtmlEntities(text: string): string { if (typeof DOMParser === 'undefined') { return text; From 2ede466d4325fbb6f863f7158174a3cec3541188 Mon Sep 17 00:00:00 2001 From: Fadi George Date: Wed, 4 Mar 2026 14:26:08 -0800 Subject: [PATCH 2/4] refactor(auth): extract shared user switching logic --- src/page/managers/LoginManager.ts | 96 ++++++++++++------------------- 1 file changed, 37 insertions(+), 59 deletions(-) diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index 7f5d6d351..e2fc48609 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -30,64 +30,31 @@ export default class LoginManager { db.put('Ids', { id: token, type: 'jwtToken' }); } - let identityModel = OneSignal._coreDirector._getIdentityModel(); + const identityModel = OneSignal._coreDirector._getIdentityModel(); const currentOneSignalId = !IDManager._isLocalId(identityModel._onesignalId) ? identityModel._onesignalId : undefined; const currentExternalId = identityModel._externalId; - // if the current externalId is the same as the one we're trying to set, do nothing if (currentExternalId === externalId) { Log._debug('Login: External ID already set, skipping login'); return; } - resetUserModels(); - identityModel = OneSignal._coreDirector._getIdentityModel(); - - // avoid duplicate identity requests, this is needed if dev calls init and login in quick succession e.g. - // e.g. OneSignalDeferred.push(OneSignal) => OneSignal.init({...})); OneSignalDeferred.push(OneSignal) => OneSignal.login('some-external-id')); - identityModel._setProperty( + // avoid duplicate identity requests when dev calls init and login in quick succession + const newIdentityModel = LoginManager._resetAndGetIdentityModel(); + newIdentityModel._setProperty( IdentityConstants._ExternalID, externalId, ModelChangeTags._Hydrate, ); - const newIdentityOneSignalId = identityModel._onesignalId; - const appId = getAppId(); - const promises: Promise[] = [ - OneSignal._coreDirector._getPushSubscriptionModel().then((pushOp) => { - if (pushOp) { - OneSignal._coreDirector._operationRepo._enqueue( - new TransferSubscriptionOperation( - appId, - newIdentityOneSignalId, - pushOp.id, - ), - ); - } else { - // unlike mobile which creates a "fake" sub right away, for Web we just want to create a new sub when we login - const newSub = new SubscriptionModel(); - newSub._mergeData({ - id: IDManager._createLocalId(), - onesignalId: newIdentityOneSignalId, - type: getSubscriptionType(), - token: '', - }); - OneSignal._coreDirector._subscriptionModelStore._add(newSub); - } - }), - OneSignal._coreDirector._operationRepo._enqueueAndWait( - new LoginUserOperation( - appId, - newIdentityOneSignalId, - externalId, - !currentExternalId ? currentOneSignalId : undefined, - ), - ), - ]; - - await Promise.all(promises); + await LoginManager._switchUser( + newIdentityModel._onesignalId, + externalId, + !currentExternalId ? currentOneSignalId : undefined, + true, + ); } // public api @@ -96,37 +63,48 @@ export default class LoginManager { } private static async _logout(): Promise { - // check if user is already logged out - let identityModel = OneSignal._coreDirector._getIdentityModel(); + const identityModel = OneSignal._coreDirector._getIdentityModel(); if (!identityModel._externalId) return Log._debug('Logout: User is not logged in, skipping logout'); + const newIdentityModel = LoginManager._resetAndGetIdentityModel(); + await LoginManager._switchUser(newIdentityModel._onesignalId); + } + + private static _resetAndGetIdentityModel() { resetUserModels(); - identityModel = OneSignal._coreDirector._getIdentityModel(); + return OneSignal._coreDirector._getIdentityModel(); + } - // Only create an anonymous user if there is a subscription to associate with it. - // Without a subscription, there is nothing meaningful to register on the server. + private static async _switchUser( + newOneSignalId: string, + externalId?: string, + existingOneSignalId?: string, + createSubIfMissing = false, + ): Promise { const appId = getAppId(); - let newIdentityOneSignalId = identityModel._onesignalId; - const promises: Promise[] = [ + await Promise.all([ OneSignal._coreDirector._getPushSubscriptionModel().then((pushOp) => { if (pushOp) { OneSignal._coreDirector._operationRepo._enqueue( - new TransferSubscriptionOperation( - appId, - newIdentityOneSignalId, - pushOp.id, - ), + new TransferSubscriptionOperation(appId, newOneSignalId, pushOp.id), ); + } else if (createSubIfMissing) { + const newSub = new SubscriptionModel(); + newSub._mergeData({ + id: IDManager._createLocalId(), + onesignalId: newOneSignalId, + type: getSubscriptionType(), + token: '', + }); + OneSignal._coreDirector._subscriptionModelStore._add(newSub); } }), OneSignal._coreDirector._operationRepo._enqueueAndWait( - new LoginUserOperation(appId, newIdentityOneSignalId), + new LoginUserOperation(appId, newOneSignalId, externalId, existingOneSignalId), ), - ]; - - await Promise.all(promises); + ]); } } From 45467c860ca91a2f3d3e8726db54bab49a433d03 Mon Sep 17 00:00:00 2001 From: Fadi George Date: Wed, 4 Mar 2026 14:48:39 -0800 Subject: [PATCH 3/4] test(auth): update LoginManager tests for operation-based flow --- index.html | 1 - package.json | 2 +- src/core/CoreModuleDirector.ts | 3 - src/core/operationRepo/OperationRepo.ts | 1 - src/onesignal/OneSignal.test.ts | 47 +++++++++------- src/page/bell/Message.ts | 1 - src/page/managers/LoginManager.test.ts | 75 ++++++++++++++++++++----- src/page/managers/LoginManager.ts | 8 ++- 8 files changed, 95 insertions(+), 43 deletions(-) diff --git a/index.html b/index.html index 09b819538..ffe986662 100644 --- a/index.html +++ b/index.html @@ -95,7 +95,6 @@ // for testing, normally this is set on (temporary) in the database client OneSignal._isNewVisitor = true; - console.log('_isNewVisitor', OneSignal._isNewVisitor); OneSignal.init({ appId, diff --git a/package.json b/package.json index 42ed7878e..ea4cebd63 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ }, { "path": "./build/releases/OneSignalSDK.page.es6.js", - "limit": "43.77 kB", + "limit": "43.793 kB", "gzip": true }, { diff --git a/src/core/CoreModuleDirector.ts b/src/core/CoreModuleDirector.ts index dbcf91edd..7a1213f80 100644 --- a/src/core/CoreModuleDirector.ts +++ b/src/core/CoreModuleDirector.ts @@ -116,7 +116,6 @@ export class CoreModuleDirector { > { logMethodCall('CoreModuleDirector.getPushSubscriptionModelByCurrentToken'); const pushToken = await getCurrentPushToken(); - console.log({ pushToken }); if (pushToken) { return this._getSubscriptionOfTypeWithToken( SubscriptionChannel._Push, @@ -137,7 +136,6 @@ export class CoreModuleDirector { // Checking '' in case we create a temp/fake subscription on logi const lastKnownPushToken = (await getPushToken()) ?? ''; - console.log({ lastKnownPushToken }); if (lastKnownPushToken !== null) { return this._getSubscriptionOfTypeWithToken( SubscriptionChannel._Push, @@ -156,7 +154,6 @@ export class CoreModuleDirector { > { logMethodCall('CoreModuleDirector.getPushSubscriptionModel'); const sub = await this._getPushSubscriptionModelByLastKnownToken(); - console.log({ sub }); return (await this._getPushSubscriptionModelByCurrentToken()) || sub; } diff --git a/src/core/operationRepo/OperationRepo.ts b/src/core/operationRepo/OperationRepo.ts index 040e1be1c..ce5a5517d 100644 --- a/src/core/operationRepo/OperationRepo.ts +++ b/src/core/operationRepo/OperationRepo.ts @@ -146,7 +146,6 @@ export class OperationRepo implements IOperationRepo, IStartableService { if (runningOps) return Log._debug('Operations in progress'); const ops = this._getNextOps(this._executeBucket); - console.log('ops', ops); if (ops) { runningOps = true; diff --git a/src/onesignal/OneSignal.test.ts b/src/onesignal/OneSignal.test.ts index 59919485e..92c14c5a3 100644 --- a/src/onesignal/OneSignal.test.ts +++ b/src/onesignal/OneSignal.test.ts @@ -29,8 +29,10 @@ import { setGetUserResponse, setSendCustomEventResponse, setTransferSubscriptionResponse, + setUpdateSubscriptionResponse, setUpdateUserResponse, transferSubscriptionFn, + updateSubscriptionFn, updateUserFn, } from '__test__/support/helpers/requests'; import { @@ -713,7 +715,16 @@ describe('OneSignal - No Consent Required', () => { external_id: externalId, }, ...BASE_IDENTITY, - subscriptions: [], + subscriptions: [ + { + device_model: '', + device_os: DEVICE_OS, + enabled: false, + sdk: expect.any(String), + token: '', + type: 'ChromePush', + }, + ], }); }); @@ -747,7 +758,7 @@ describe('OneSignal - No Consent Required', () => { }); }); - test('login then accept web push permissions - it should make two user calls', async () => { + test('login then accept web push permissions - it should create user then update subscription', async () => { const { promise, resolve } = Promise.withResolvers(); OneSignal._emitter.on(OneSignal.EVENTS.SUBSCRIPTION_CHANGED, resolve); setGetUserResponse(); @@ -760,6 +771,7 @@ describe('OneSignal - No Consent Required', () => { }, ], }); + setUpdateSubscriptionResponse({ subscriptionId: SUB_ID }); OneSignal._coreDirector._subscriptionModelStore._replaceAll( [], @@ -783,23 +795,11 @@ describe('OneSignal - No Consent Required', () => { }; registerForPushNotifications(); - // first call just sets the external id + // create user call includes the empty subscription await vi.waitUntil(() => createUserFn.mock.calls.length === 1, { interval: 0, }); - expect(createUserFn).toHaveBeenCalledWith({ - identity: { - external_id: externalId, - }, - ...BASE_IDENTITY, - subscriptions: [], - }); - await promise; - - // second call creates the subscription - await vi.waitUntil(() => createUserFn.mock.calls.length === 2, { - interval: 1, - }); + expect(createUserFn).toHaveBeenCalledTimes(1); expect(createUserFn).toHaveBeenCalledWith({ identity: { external_id: externalId, @@ -807,14 +807,21 @@ describe('OneSignal - No Consent Required', () => { ...BASE_IDENTITY, subscriptions: [ { - ...BASE_SUB, - token: PUSH_TOKEN, + device_model: '', + device_os: DEVICE_OS, + enabled: false, + sdk: expect.any(String), + token: '', type: 'ChromePush', - web_auth: 'w3cAuth', - web_p256: 'w3cP256dh', }, ], }); + await promise; + + // accepting permissions patches the existing subscription with the token + await vi.waitUntil(() => updateSubscriptionFn.mock.calls.length >= 1, { + interval: 1, + }); let pushSub: SubscriptionSchema | undefined; await vi.waitUntil( diff --git a/src/page/bell/Message.ts b/src/page/bell/Message.ts index 8ce1772e3..a3d8940ab 100755 --- a/src/page/bell/Message.ts +++ b/src/page/bell/Message.ts @@ -59,7 +59,6 @@ export default class Message { } async _display(type: string, content: string, duration = 0) { - console.log('zzzzzdisplay', type, content, duration); Log._debug(`Calling display(${type}, ${content}, ${duration}).`); if (this._shown) await this._hide(); this._content = decodeHtmlEntities(content); diff --git a/src/page/managers/LoginManager.test.ts b/src/page/managers/LoginManager.test.ts index 9490f250b..c666b47fd 100644 --- a/src/page/managers/LoginManager.test.ts +++ b/src/page/managers/LoginManager.test.ts @@ -1,13 +1,11 @@ import { TestEnvironment } from '__test__/support/environment/TestEnvironment'; import { updateIdentityModel } from '__test__/support/helpers/setup'; import { SubscriptionModel } from 'src/core/models/SubscriptionModel'; +import { BaseSubscriptionOperation } from 'src/core/operations/BaseSubscriptionOperation'; import { db } from 'src/shared/database/client'; import Log from 'src/shared/libraries/Log'; -import * as userDirector from '../../onesignal/userDirector'; import LoginManager from './LoginManager'; -const createUserOnServerSpy = vi.spyOn(userDirector, 'createUserOnServer'); - describe('LoginManager', () => { beforeEach(() => { TestEnvironment.initialize(); @@ -52,6 +50,48 @@ describe('LoginManager', () => { expect(enqueueAndWaitSpy).toHaveBeenCalled(); }); + test('login: with existing push sub enqueues transfer operation', async () => { + const mockPushSub = { id: 'push-sub-id' } as SubscriptionModel; + vi.spyOn( + OneSignal._coreDirector, + '_getPushSubscriptionModel', + ).mockResolvedValue(mockPushSub); + const enqueueSpy = vi.spyOn( + OneSignal._coreDirector._operationRepo, + '_enqueue', + ); + vi.spyOn( + OneSignal._coreDirector._operationRepo, + '_enqueueAndWait', + ).mockResolvedValue(undefined); + + await LoginManager.login('new-id'); + + expect(enqueueSpy).toHaveBeenCalled(); + const transferOp = enqueueSpy.mock.calls[0][0] as BaseSubscriptionOperation; + expect(transferOp._subscriptionId).toBe('push-sub-id'); + }); + + test('login: without push sub creates new subscription model', async () => { + vi.spyOn( + OneSignal._coreDirector, + '_getPushSubscriptionModel', + ).mockResolvedValue(undefined); + vi.spyOn( + OneSignal._coreDirector._operationRepo, + '_enqueueAndWait', + ).mockResolvedValue(undefined); + const addSpy = vi.spyOn( + OneSignal._coreDirector._subscriptionModelStore, + '_add', + ); + + await LoginManager.login('new-id'); + + expect(addSpy).toHaveBeenCalled(); + expect(addSpy.mock.calls[0][0].token).toBe(''); + }); + test('logout: no external id logs debug and returns', async () => { const debugSpy = vi .spyOn(Log, '_debug') @@ -63,21 +103,26 @@ describe('LoginManager', () => { ); }); - test('logout: with external id and no subscriptions resets models and skips user creation', async () => { + test('logout: with external id and push sub enqueues transfer and login operations', async () => { await updateIdentityModel('external_id', 'abc'); - await LoginManager.logout(); - expect(createUserOnServerSpy).not.toHaveBeenCalled(); - }); - - test('logout: with external id and a subscription resets models and creates anonymous user', async () => { - await updateIdentityModel('external_id', 'abc'); - const mockSub = { id: 'sub-id' } as SubscriptionModel; + const mockPushSub = { id: 'sub-id' } as SubscriptionModel; vi.spyOn( - OneSignal._coreDirector._subscriptionModelStore, - '_list', - ).mockReturnValue([mockSub]); + OneSignal._coreDirector, + '_getPushSubscriptionModel', + ).mockResolvedValue(mockPushSub); + const enqueueSpy = vi.spyOn( + OneSignal._coreDirector._operationRepo, + '_enqueue', + ); + vi.spyOn( + OneSignal._coreDirector._operationRepo, + '_enqueueAndWait', + ).mockResolvedValue(undefined); await LoginManager.logout(); - expect(createUserOnServerSpy).toHaveBeenCalled(); + + expect(enqueueSpy).toHaveBeenCalled(); + const transferOp = enqueueSpy.mock.calls[0][0] as BaseSubscriptionOperation; + expect(transferOp._subscriptionId).toBe('sub-id'); }); }); diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index e2fc48609..45eb7f433 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -94,6 +94,7 @@ export default class LoginManager { } else if (createSubIfMissing) { const newSub = new SubscriptionModel(); newSub._mergeData({ + enabled: true, id: IDManager._createLocalId(), onesignalId: newOneSignalId, type: getSubscriptionType(), @@ -103,7 +104,12 @@ export default class LoginManager { } }), OneSignal._coreDirector._operationRepo._enqueueAndWait( - new LoginUserOperation(appId, newOneSignalId, externalId, existingOneSignalId), + new LoginUserOperation( + appId, + newOneSignalId, + externalId, + existingOneSignalId, + ), ), ]); } From 3a66d9916d6f61aae91dd5642c6507cd24b25afe Mon Sep 17 00:00:00 2001 From: Fadi George Date: Wed, 4 Mar 2026 15:20:50 -0800 Subject: [PATCH 4/4] refactor(auth): inline userDirector functions --- package.json | 2 +- src/onesignal/OneSignal.test.ts | 9 ++-- src/onesignal/userDirector.ts | 57 ------------------------ src/page/managers/LoginManager.ts | 16 +++++-- src/shared/managers/subscription/page.ts | 34 ++++++++++++-- 5 files changed, 50 insertions(+), 68 deletions(-) delete mode 100644 src/onesignal/userDirector.ts diff --git a/package.json b/package.json index ea4cebd63..672991787 100644 --- a/package.json +++ b/package.json @@ -81,7 +81,7 @@ }, { "path": "./build/releases/OneSignalSDK.page.es6.js", - "limit": "43.793 kB", + "limit": "43.75 kB", "gzip": true }, { diff --git a/src/onesignal/OneSignal.test.ts b/src/onesignal/OneSignal.test.ts index 92c14c5a3..c2de72bcb 100644 --- a/src/onesignal/OneSignal.test.ts +++ b/src/onesignal/OneSignal.test.ts @@ -819,9 +819,12 @@ describe('OneSignal - No Consent Required', () => { await promise; // accepting permissions patches the existing subscription with the token - await vi.waitUntil(() => updateSubscriptionFn.mock.calls.length >= 1, { - interval: 1, - }); + await vi.waitUntil( + () => updateSubscriptionFn.mock.calls.length >= 1, + { + interval: 1, + }, + ); let pushSub: SubscriptionSchema | undefined; await vi.waitUntil( diff --git a/src/onesignal/userDirector.ts b/src/onesignal/userDirector.ts deleted file mode 100644 index af4d8a9be..000000000 --- a/src/onesignal/userDirector.ts +++ /dev/null @@ -1,57 +0,0 @@ -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 Log from 'src/shared/libraries/Log'; -import { IDManager } from 'src/shared/managers/IDManager'; -import { getAppId } from '../shared/helpers/main'; - -export async function createUserOnServer(): Promise { - const identityModel = OneSignal._coreDirector._getIdentityModel(); - const appId = getAppId(); - - const hasAnySubscription = - OneSignal._coreDirector._subscriptionModelStore._list().length > 0; - - const hasExternalId = !!identityModel._externalId; - - if (!hasAnySubscription && !hasExternalId) { - Log._error('No subscriptions or external ID found, skipping user creation'); - return; - } - - const pushOp = await OneSignal._coreDirector._getPushSubscriptionModel(); - - OneSignal._coreDirector._operationRepo._enqueue( - new LoginUserOperation( - appId, - identityModel._onesignalId, - identityModel._externalId, - ), - ); - if (pushOp) { - await OneSignal._coreDirector._operationRepo._enqueueAndWait( - new CreateSubscriptionOperation({ - ...pushOp.toJSON(), - appId, - onesignalId: identityModel._onesignalId, - subscriptionId: pushOp.id!, - }), - ); - } -} - -// 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 -export function resetUserModels() { - // replace models - const newIdentityModel = new IdentityModel(); - const newPropertiesModel = new PropertiesModel(); - - const sdkId = IDManager._createLocalId(); - newIdentityModel._onesignalId = sdkId; - newPropertiesModel._onesignalId = sdkId; - - OneSignal._coreDirector._identityModelStore._replace(newIdentityModel); - OneSignal._coreDirector._propertiesModelStore._replace(newPropertiesModel); -} diff --git a/src/page/managers/LoginManager.ts b/src/page/managers/LoginManager.ts index 45eb7f433..092976438 100644 --- a/src/page/managers/LoginManager.ts +++ b/src/page/managers/LoginManager.ts @@ -1,4 +1,6 @@ import { IdentityConstants } from 'src/core/constants'; +import { IdentityModel } from 'src/core/models/IdentityModel'; +import { PropertiesModel } from 'src/core/models/PropertiesModel'; import { SubscriptionModel } from 'src/core/models/SubscriptionModel'; import { LoginUserOperation } from 'src/core/operations/LoginUserOperation'; import { TransferSubscriptionOperation } from 'src/core/operations/TransferSubscriptionOperation'; @@ -6,9 +8,8 @@ import { ModelChangeTags } from 'src/core/types/models'; import { db } from 'src/shared/database/client'; import { getSubscriptionType } from 'src/shared/environment/detect'; import { getAppId } from 'src/shared/helpers/main'; +import Log from 'src/shared/libraries/Log'; import { IDManager } from 'src/shared/managers/IDManager'; -import { resetUserModels } from '../../onesignal/userDirector'; -import Log from '../../shared/libraries/Log'; export default class LoginManager { // Other internal classes should await on this if they access users @@ -73,7 +74,16 @@ export default class LoginManager { } private static _resetAndGetIdentityModel() { - resetUserModels(); + const newIdentityModel = new IdentityModel(); + const newPropertiesModel = new PropertiesModel(); + + const sdkId = IDManager._createLocalId(); + newIdentityModel._onesignalId = sdkId; + newPropertiesModel._onesignalId = sdkId; + + OneSignal._coreDirector._identityModelStore._replace(newIdentityModel); + OneSignal._coreDirector._propertiesModelStore._replace(newPropertiesModel); + return OneSignal._coreDirector._getIdentityModel(); } diff --git a/src/shared/managers/subscription/page.ts b/src/shared/managers/subscription/page.ts index 3b6b1c698..ed05a3c6e 100644 --- a/src/shared/managers/subscription/page.ts +++ b/src/shared/managers/subscription/page.ts @@ -1,4 +1,6 @@ -import { createUserOnServer } from 'src/onesignal/userDirector'; +import type { SubscriptionModel } from 'src/core/models/SubscriptionModel'; +import { CreateSubscriptionOperation } from 'src/core/operations/CreateSubscriptionOperation'; +import { LoginUserOperation } from 'src/core/operations/LoginUserOperation'; import LoginManager from 'src/page/managers/LoginManager'; import FuturePushSubscriptionRecord from 'src/page/userModel/FuturePushSubscriptionRecord'; import type { ContextInterface } from 'src/shared/context/types'; @@ -12,6 +14,7 @@ import { PermissionBlockedError, SWRegistrationError, } from 'src/shared/errors/common'; +import { getAppId } from 'src/shared/helpers/main'; import { incrementPageViewCount, isFirstPageView, @@ -48,6 +51,29 @@ function executeCallback(callback?: (...args: any[]) => T, ...args: any[]) { } } +async function createSubscribedUser( + pushModel: SubscriptionModel, +): Promise { + const identityModel = OneSignal._coreDirector._getIdentityModel(); + const appId = getAppId(); + + OneSignal._coreDirector._operationRepo._enqueue( + new LoginUserOperation( + appId, + identityModel._onesignalId, + identityModel._externalId, + ), + ); + await OneSignal._coreDirector._operationRepo._enqueueAndWait( + new CreateSubscriptionOperation({ + ...pushModel.toJSON(), + appId, + onesignalId: identityModel._onesignalId, + subscriptionId: pushModel.id!, + }), + ); +} + export const updatePushSubscriptionModelWithRawSubscription = async ( rawPushSubscription: RawPushSubscription, ) => { @@ -55,18 +81,18 @@ export const updatePushSubscriptionModelWithRawSubscription = async ( // otherwise there would be two login ops in the same bucket for LoginOperationExecutor which would error await LoginManager._switchingUsersPromise; + // for new Anonymous/not-logged-in users, we need to create a new push subscription model and also save its push id to IndexedDB let pushModel = await OneSignal._coreDirector._getPushSubscriptionModel(); - // for new users, we need to create a new push subscription model and also save its push id to IndexedDB if (!pushModel) { pushModel = OneSignal._coreDirector._generatePushSubscriptionModel( rawPushSubscription, ); - return createUserOnServer(); + return createSubscribedUser(pushModel); } // for users with data failed to create a user or user + subscription on the server if (IDManager._isLocalId(pushModel.id)) { - return createUserOnServer(); + return createSubscribedUser(pushModel); } // in case of notification state changes, we need to update its web_auth, web_p256, and other keys