diff --git a/__test__/unit/notifications/subscriptionmanager.test.ts b/__test__/unit/notifications/subscriptionmanager.test.ts new file mode 100644 index 000000000..e9e587948 --- /dev/null +++ b/__test__/unit/notifications/subscriptionmanager.test.ts @@ -0,0 +1,32 @@ +import MockNotification from '../../support/mocks/MockNotification'; +import { SubscriptionManager } from '../../../src/shared/managers/SubscriptionManager'; +import { NotificationPermission } from '../../../src/shared/models/NotificationPermission'; + +describe('SubscriptionManager', () => { + describe('requestNotificationPermission', () => { + beforeEach(() => { + window.Notification = MockNotification; + }); + + test('default', async () => { + MockNotification.permission = 'default'; + expect(await SubscriptionManager.requestNotificationPermission()).toBe( + NotificationPermission.Default, + ); + }); + + test('denied', async () => { + MockNotification.permission = 'denied'; + expect(await SubscriptionManager.requestNotificationPermission()).toBe( + NotificationPermission.Denied, + ); + }); + + test('granted', async () => { + MockNotification.permission = 'granted'; + expect(await SubscriptionManager.requestNotificationPermission()).toBe( + NotificationPermission.Granted, + ); + }); + }); +}); diff --git a/src/shared/helpers/ServiceWorkerHelper.ts b/src/shared/helpers/ServiceWorkerHelper.ts index fbc87b24c..ea06d38c6 100755 --- a/src/shared/helpers/ServiceWorkerHelper.ts +++ b/src/shared/helpers/ServiceWorkerHelper.ts @@ -279,12 +279,6 @@ export enum ServiceWorkerActiveState { * No service worker is installed. */ None = 'None', - /** - * Service workers are not supported in this environment. This status is used - * on HTTP pages where it isn't possible to know whether a service worker is - * installed or not or in any of the other states. - */ - Indeterminate = 'Indeterminate', } export interface ServiceWorkerManagerConfig { diff --git a/src/shared/libraries/WorkerMessenger.ts b/src/shared/libraries/WorkerMessenger.ts index a14a474bb..191d7fbf1 100644 --- a/src/shared/libraries/WorkerMessenger.ts +++ b/src/shared/libraries/WorkerMessenger.ts @@ -197,7 +197,7 @@ export class WorkerMessenger { ); const workerRegistration = - await this.context?.serviceWorkerManager.getRegistration(); + await this.context?.serviceWorkerManager.getOneSignalRegistration(); if (!workerRegistration) { Log.error( '`[Worker Messenger] [Page -> SW] Could not get ServiceWorkerRegistration to postMessage!', diff --git a/src/shared/managers/ServiceWorkerManager.ts b/src/shared/managers/ServiceWorkerManager.ts index 9299e4138..4582e5179 100644 --- a/src/shared/managers/ServiceWorkerManager.ts +++ b/src/shared/managers/ServiceWorkerManager.ts @@ -1,9 +1,7 @@ import Environment from '../helpers/Environment'; import { WorkerMessengerCommand } from '../libraries/WorkerMessenger'; import Path from '../models/Path'; -import SdkEnvironment from './SdkEnvironment'; import Database from '../services/Database'; -import { WindowEnvironmentKind } from '../models/WindowEnvironmentKind'; import Log from '../libraries/Log'; import OneSignalEvent from '../services/OneSignalEvent'; import ServiceWorkerRegistrationError from '../errors/ServiceWorkerRegistrationError'; @@ -35,18 +33,35 @@ export class ServiceWorkerManager { this.config = config; } - // Gets details on the OneSignal service-worker (if any) - public async getRegistration(): Promise< - ServiceWorkerRegistration | null | undefined + /** + * Gets the current ServiceWorkerRegistration in the scoped configured + * in OneSignal. + * WARNING: This might be a non-OneSignal service worker, use + * getOneSignalRegistration() instead if you need this guarantee. + */ + private async getRegistration(): Promise< + ServiceWorkerRegistration | undefined > { - return await ServiceWorkerUtilHelper.getRegistration( + return ServiceWorkerUtilHelper.getRegistration( this.config.registrationOptions.scope, ); } + /** + * Gets the OneSignal ServiceWorkerRegistration reference, if it was registered + */ + public async getOneSignalRegistration(): Promise< + ServiceWorkerRegistration | undefined + > { + const state = await this.getActiveState(); + if (state === ServiceWorkerActiveState.OneSignalWorker) { + return this.getRegistration(); + } + return undefined; + } + public async getActiveState(): Promise { - const workerRegistration = - await this.context.serviceWorkerManager.getRegistration(); + const workerRegistration = await this.getRegistration(); if (!workerRegistration) { return ServiceWorkerActiveState.None; } @@ -163,8 +178,7 @@ export class ServiceWorkerManager { private async haveParamsChanged(): Promise { // 1. No workerRegistration - const workerRegistration = - await this.context.serviceWorkerManager.getRegistration(); + const workerRegistration = await this.getRegistration(); if (!workerRegistration) { Log.info( '[changedServiceWorkerParams] workerRegistration not found at scope', @@ -382,7 +396,7 @@ export class ServiceWorkerManager { ServiceWorkerRegistration | undefined | null > { if (!(await this.shouldInstallWorker())) { - return this.getRegistration(); + return this.getOneSignalRegistration(); } Log.info('Installing worker...'); diff --git a/src/shared/managers/SubscriptionManager.ts b/src/shared/managers/SubscriptionManager.ts index 1c8abb549..7b37ce256 100644 --- a/src/shared/managers/SubscriptionManager.ts +++ b/src/shared/managers/SubscriptionManager.ts @@ -333,7 +333,7 @@ export class SubscriptionManager { const results = await window.Notification.requestPermission(); // TODO: Clean up our custom NotificationPermission enum // in favor of TS union type NotificationPermission instead of converting - return NotificationPermission[results]; + return results as NotificationPermission; } public async isAlreadyRegisteredWithOneSignal(): Promise { @@ -750,7 +750,7 @@ export class SubscriptionManager { } const serviceWorkerRegistration = - await this.context.serviceWorkerManager.getRegistration(); + await this.context.serviceWorkerManager.getOneSignalRegistration(); if (!serviceWorkerRegistration) return false; // It's possible to get here in Safari 11.1+ version @@ -834,17 +834,12 @@ export class SubscriptionManager { }; } - const workerState = - await this.context.serviceWorkerManager.getActiveState(); const workerRegistration = - await this.context.serviceWorkerManager.getRegistration(); + await this.context.serviceWorkerManager.getOneSignalRegistration(); const notificationPermission = await this.context.permissionManager.getNotificationPermission( this.context.appConfig.safariWebId, ); - const isWorkerActive = - workerState === ServiceWorkerActiveState.OneSignalWorker; - if (!workerRegistration) { /* You can't be subscribed without a service worker registration */ return { @@ -861,16 +856,14 @@ export class SubscriptionManager { * const isPushEnabled = !!( * pushSubscription && * deviceId && - * notificationPermission === NotificationPermission.Granted && - * isWorkerActive + * notificationPermission === NotificationPermission.Granted * ); */ const isPushEnabled = !!( isValidPushSubscription && subscriptionToken && - notificationPermission === NotificationPermission.Granted && - isWorkerActive + notificationPermission === NotificationPermission.Granted ); return { diff --git a/src/sw/helpers/ServiceWorkerUtilHelper.ts b/src/sw/helpers/ServiceWorkerUtilHelper.ts index 2222086ce..6df47ae46 100644 --- a/src/sw/helpers/ServiceWorkerUtilHelper.ts +++ b/src/sw/helpers/ServiceWorkerUtilHelper.ts @@ -5,7 +5,7 @@ export default class ServiceWorkerUtilHelper { // A relative scope is required as a domain can have none to many service workers installed. static async getRegistration( scope: string, - ): Promise { + ): Promise { try { // Adding location.origin to negate the effects of a possible html tag the page may have. const url = location.origin + scope; @@ -17,7 +17,7 @@ export default class ServiceWorkerUtilHelper { scope, e, ); - return null; + return undefined; } } diff --git a/test/unit/managers/ServiceWorkerManager.ts b/test/unit/managers/ServiceWorkerManager.ts index 91cc2a08c..cf65d0957 100644 --- a/test/unit/managers/ServiceWorkerManager.ts +++ b/test/unit/managers/ServiceWorkerManager.ts @@ -5,10 +5,7 @@ import sinon, { SinonSandbox, SinonStub } from 'sinon'; import nock from 'nock'; import { ServiceWorkerManager } from '../../../src/shared/managers/ServiceWorkerManager'; import { ServiceWorkerActiveState } from '../../../src/shared/helpers/ServiceWorkerHelper'; -import { - TestEnvironment, - TestEnvironmentConfig, -} from '../../support/sdk/TestEnvironment'; +import { TestEnvironment } from '../../support/sdk/TestEnvironment'; import Context from '../../../src/page/models/Context'; import SdkEnvironment from '../../../src/shared/managers/SdkEnvironment'; import { WindowEnvironmentKind } from '../../../src/shared/models/WindowEnvironmentKind'; @@ -25,7 +22,6 @@ import { ServiceWorkerRegistrationError } from '../../../src/shared/errors/Servi import OneSignalUtils from '../../../src/shared/utils/OneSignalUtils'; import { MockServiceWorkerRegistration } from '../../support/mocks/service-workers/models/MockServiceWorkerRegistration'; import { MockServiceWorker } from '../../support/mocks/service-workers/models/MockServiceWorker'; -import { ConfigIntegrationKind } from '../../../src/shared/models/AppConfig'; import Environment from '../../../src/shared/helpers/Environment'; import { MockServiceWorkerContainerWithAPIBan } from '../../support/mocks/service-workers/models/MockServiceWorkerContainerWithAPIBan'; import Path from '../../../src/shared/models/Path'; @@ -374,12 +370,14 @@ test('Service worker failed to install due to 404 on host page. Send notificatio test('ServiceWorkerManager.getRegistration() returns valid instance when sw is registered', async (t) => { await navigator.serviceWorker.register('/Worker.js'); - const result = await OneSignal.context.serviceWorkerManager.getRegistration(); + const result = + await OneSignal.context.serviceWorkerManager.getOneSignalRegistration(); t.truthy(result); }); test('ServiceWorkerManager.getRegistration() returns undefined when sw is not registered ', async (t) => { - const result = await OneSignal.context.serviceWorkerManager.getRegistration(); + const result = + await OneSignal.context.serviceWorkerManager.getOneSignalRegistration(); t.is(result, undefined); }); @@ -395,6 +393,7 @@ test('ServiceWorkerManager.getRegistration() handles throws by returning null', throw new Error('HTTP NOT SUPPORTED'); }), ); - const result = await OneSignal.context.serviceWorkerManager.getRegistration(); + const result = + await OneSignal.context.serviceWorkerManager.getOneSignalRegistration(); t.is(result, null); });