fix tests

Author: fadi-george

__test__/support/environment/TestContext.ts

@@ -221,7 +221,7 @@ export default class TestContext {
                 right: 0,
                 bottom: 0,
               },
-              enabled: true,
+              enabled: false,
               message: {
                 subscribing: 'Thanks for subscribing!',
                 unsubscribing: "You won't receive notifications again",
@@ -239,7 +239,7 @@ export default class TestContext {
               prompts: [
                 {
                   type: DelayedPromptType.Push,
-                  autoPrompt: true,
+                  autoPrompt: false,
                   text: {
                     acceptButton: 'Allow',
                     cancelButton: 'No Thanks',
@@ -252,7 +252,7 @@ export default class TestContext {
             fullscreen: {
               title: 'example.com',
               caption: 'You can unsubscribe anytime',
-              enabled: true,
+              enabled: false,
               message: 'This is an example notification message.',
               acceptButton: 'Continue',
               cancelButton: 'No Thanks',
@@ -262,7 +262,7 @@ export default class TestContext {
               customizeTextEnabled: true,
             },
             customlink: {
-              enabled: true,
+              enabled: false,
               style: 'button',
               size: 'medium',
               color: {
@@ -350,7 +350,7 @@ export default class TestContext {
   static getFakeAppUserConfig(appId: string = APP_ID): AppUserConfig {
     return {
       appId,
-      autoRegister: true,
+      autoRegister: false,
       autoResubscribe: true,
       path: '/fake-page',
       serviceWorkerPath: 'fakeWorkerName.js',
@@ -365,7 +365,7 @@ export default class TestContext {
           prompts: [
             {
               type: DelayedPromptType.Push,
-              autoPrompt: true,
+              autoPrompt: false,
               text: {
                 acceptButton: 'Allow',
                 cancelButton: 'No Thanks',

src/core/operationRepo/OperationRepo.test.ts

@@ -1,6 +1,7 @@
 import { APP_ID, ONESIGNAL_ID, SUB_ID } from '__test__/constants';
 import { db } from 'src/shared/database/client';
 import type { IndexedDBSchema } from 'src/shared/database/types';
+import { setConsentRequired } from 'src/shared/helpers/localStorage';
 import Log from 'src/shared/libraries/Log';
 import { SubscriptionType } from 'src/shared/subscriptions/constants';
 import { describe, expect, type Mock, vi } from 'vitest';
@@ -25,6 +26,8 @@ vi.spyOn(Log, 'error').mockImplementation((msg) => {
     return '';
   return msg;
 });
+const debugSpy = vi.spyOn(Log, 'debug');
+
 vi.useFakeTimers();
 
 // for the sake of testing, we want to use a simple mock operation and execturo
@@ -34,6 +37,18 @@ vi.spyOn(OperationModelStore.prototype, 'create').mockImplementation(() => {
 
 let mockOperationModelStore: OperationModelStore;
 
+const executeOps = async (opRepo: OperationRepo) => {
+  await opRepo.start();
+  await vi.advanceTimersByTimeAsync(OP_REPO_EXECUTION_INTERVAL);
+};
+
+// need to mock this since it may timeout due to use of fake timers
+const isConsentRequired = vi.hoisted(() => vi.fn(() => false));
+
+vi.mock('src/shared/database/config', () => ({
+  isConsentRequired: isConsentRequired,
+}));
+
 describe('OperationRepo', () => {
   let opRepo: OperationRepo;
 
@@ -43,7 +58,7 @@ describe('OperationRepo', () => {
   ];
 
   beforeEach(async () => {
-    await db.clear('operations');
+    setConsentRequired(false);
 
     mockOperationModelStore = new OperationModelStore();
     opRepo = new OperationRepo(
@@ -56,7 +71,7 @@ describe('OperationRepo', () => {
   afterEach(async () => {
     // since the perist call in model store is not awaited, we need to flush the queue
     // for tests that call start on the op repo
-    if (!opRepo.isPaused) {
+    if (opRepo._timerID !== undefined) {
       await vi.waitUntil(async () => {
         const dbOps = await db.getAll('operations');
         return dbOps.length === 0;
@@ -268,11 +283,6 @@ describe('OperationRepo', () => {
   });
 
   describe('Executor Operations', () => {
-    const executeOps = async (opRepo: OperationRepo) => {
-      await opRepo.start();
-      await vi.advanceTimersByTimeAsync(OP_REPO_EXECUTION_INTERVAL);
-    };
-
     test('can handle success operation and process additional operations', async () => {
       const additionalOps = [
         new Operation('3', GroupComparisonType.NONE),
@@ -422,7 +432,7 @@ describe('OperationRepo', () => {
       ]);
 
       // op repo should be paused
-      expect(opRepo.isPaused).toBe(true);
+      expect(opRepo._timerID).toBe(undefined);
     });
 
     test('can process delay for translations', async () => {
@@ -491,6 +501,30 @@ describe('OperationRepo', () => {
       expect(executeOperationsSpy).toHaveBeenCalledTimes(2);
     });
   });
+
+  test('should not execute operations if consent is not given', async () => {
+    isConsentRequired.mockReturnValue(true);
+    setConsentRequired(true);
+
+    opRepo.enqueue(mockOperation);
+    await executeOps(opRepo);
+
+    await vi.waitUntil(() => opRepo._timerID === undefined);
+    expect(debugSpy).toHaveBeenCalledWith(
+      'Consent not given; not executing operations',
+    );
+
+    // operation did not execute
+    expect(opRepo.queue).toEqual([
+      {
+        operation: mockOperation,
+        bucket: 0,
+        retries: 0,
+      },
+    ]);
+
+    await db.clear('operations');
+  });
 });
 
 const translateIdsFn = vi.fn();

src/core/operationRepo/OperationRepo.ts

@@ -5,9 +5,8 @@ import {
   type IStartableService,
 } from 'src/core/types/operation';
 import { db } from 'src/shared/database/client';
-import { getConsentGiven } from 'src/shared/database/config';
+import { isConsentRequired } from 'src/shared/database/config';
 import { delay } from 'src/shared/helpers/general';
-import { getConsentRequired } from 'src/shared/helpers/localStorage';
 import Log from 'src/shared/libraries/Log';
 import { type OperationModelStore } from '../modelRepo/OperationModelStore';
 import { GroupComparisonType, type Operation } from '../operations/Operation';
@@ -51,7 +50,7 @@ export class OperationQueueItem {
 export class OperationRepo implements IOperationRepo, IStartableService {
   private executorsMap: Map<string, IOperationExecutor>;
   public queue: OperationQueueItem[] = [];
-  private paused = true;
+  public _timerID: NodeJS.Timeout | undefined = undefined;
   private enqueueIntoBucket = 0;
   private operationModelStore: OperationModelStore;
   private newRecordState: NewRecordsState;
@@ -76,10 +75,6 @@ export class OperationRepo implements IOperationRepo, IStartableService {
     this.queue = [];
   }
 
-  public get isPaused(): boolean {
-    return this.paused;
-  }
-
   public get records(): Map<string, number> {
     return this.newRecordState.records;
   }
@@ -95,11 +90,16 @@ export class OperationRepo implements IOperationRepo, IStartableService {
   }
 
   public async start(): Promise<void> {
-    this.paused = false;
     await this.loadSavedOperations();
     this.processQueueForever();
   }
 
+  public _pause(): void {
+    clearInterval(this._timerID);
+    this._timerID = undefined;
+    Log.debug('OperationRepo: Paused');
+  }
+
   public enqueue(operation: Operation): void {
     Log.debug(`OperationRepo.enqueue(operation: ${operation})`);
 
@@ -153,24 +153,18 @@ export class OperationRepo implements IOperationRepo, IStartableService {
     }
   }
 
-  private async _isAllowedToExecute(): Promise<boolean> {
-    const consentRequired = getConsentRequired();
-    const consentGiven = await getConsentGiven();
-
-    if (!consentRequired) return true;
-    return !!consentGiven;
-  }
-
   private async processQueueForever(): Promise<void> {
     this.enqueueIntoBucket++;
     let runningOps = false;
 
-    setInterval(async () => {
-      const isAllowedToExecute = await this._isAllowedToExecute();
+    if (isConsentRequired()) {
+      this._pause();
+      Log.debug('Consent not given; not executing operations');
+      return;
+    }
 
-      if (this.paused) return Log.debug('OpRepo is paused');
+    this._timerID = setInterval(async () => {
       if (runningOps) return Log.debug('Operations in progress');
-      if (!isAllowedToExecute) return Log.debug('Not allowed to execute');
 
       const ops = this.getNextOps(this.executeBucket);
 
@@ -261,7 +255,7 @@ export class OperationRepo implements IOperationRepo, IStartableService {
           Log.error(
             `Operation execution failed with eventual retry, pausing the operation repo: ${operations}`,
           );
-          this.paused = true;
+          this._pause();
           ops.toReversed().forEach((op) => {
             removeOpFromDB(op.operation);
             this.queue.unshift(op);

src/onesignal/OneSignal.test.ts

@@ -59,12 +59,13 @@ mockPageStylesCss();
 
 describe('OneSignal', () => {
   beforeAll(() => {
+    setConsentRequired(true);
+  });
+
+  beforeEach(() => {
     TestEnvironment.initialize({
       initUserAndPushSubscription: true,
     });
-  });
-
-  beforeEach(async () => {
     OneSignal.coreDirector.subscriptionModelStore.replaceAll(
       [],
       ModelChangeTags.NO_PROPAGATE,
@@ -368,15 +369,6 @@ describe('OneSignal', () => {
           await expect(OneSignal.login('', 1)).rejects.toThrowError(
             '"jwtToken" is the wrong type',
           );
-
-          // TODO: add consent required test
-          // if needing consent required
-          setConsentRequired(true);
-          await OneSignal.login(externalId);
-          await vi.waitUntil(() => errorSpy.mock.calls.length === 1);
-
-          const error = errorSpy.mock.calls[0][1] as Error;
-          expect(error.message).toBe('Consent required but not given');
         });
 
         test('can login with a new external id', async () => {
@@ -683,16 +675,6 @@ describe('OneSignal', () => {
 
         test('login then accept web push permissions - it should make two user calls', async () => {
           setGetUserResponse();
-          OneSignal.coreDirector.subscriptionModelStore.replaceAll(
-            [],
-            ModelChangeTags.NO_PROPAGATE,
-          );
-          setPushToken('');
-          subscribeFcmFromPageSpy.mockImplementation(
-            // @ts-expect-error - subscribeFcmFromPage is a private method of SubscriptionManagerPage
-            async () => rawPushSubscription,
-          );
-
           setCreateUserResponse({
             onesignalId: ONESIGNAL_ID,
             externalId,
@@ -703,6 +685,16 @@ describe('OneSignal', () => {
             ],
           });
 
+          OneSignal.coreDirector.subscriptionModelStore.replaceAll(
+            [],
+            ModelChangeTags.NO_PROPAGATE,
+          );
+          setPushToken('');
+          subscribeFcmFromPageSpy.mockImplementation(
+            // @ts-expect-error - subscribeFcmFromPage is a private method of SubscriptionManagerPage
+            async () => rawPushSubscription,
+          );
+
           // new/empty user
           setupIdentityModel(IDManager.createLocalId());
 
@@ -1105,6 +1097,48 @@ describe('OneSignal', () => {
     });
   });
 
+  describe('Consent', () => {
+    beforeEach(() => {
+      setConsentRequired(true);
+    });
+
+    test('should not execute operations if consent is not given', async () => {
+      setUpdateUserResponse();
+
+      OneSignal.User.addTag('some-tag', 'some-value');
+      await OneSignal.login('login-user');
+
+      // should not execute login op
+      await vi.waitUntil(() => {
+        const calls = debugSpy.mock.calls.map((call) => call[0]);
+        return calls.includes('Consent not given; not executing operations');
+      });
+
+      // should not execute any other ops
+      expect(addAliasFn).not.toHaveBeenCalled();
+      expect(OneSignal.coreDirector.operationRepo.queue).toMatchObject([
+        {
+          operation: {
+            name: 'set-property',
+          },
+        },
+      ]);
+
+      // can resume operations if consent is given
+      OneSignal.setConsentGiven(true);
+      expect(OneSignal._consentGiven).toBe(true);
+      await vi.waitUntil(() => updateUserFn.mock.calls.length === 1);
+      expect(updateUserFn).toHaveBeenCalledWith({
+        properties: {
+          tags: {
+            'some-tag': 'some-value',
+          },
+        },
+        refresh_device_metadata: false,
+      });
+    });
+  });
+
   test('should preserve operations order without needing await', async () => {
     await setupSubModelStore({
       id: SUB_ID,
@@ -1173,7 +1207,7 @@ Object.defineProperty(global.navigator, 'serviceWorker', {
   writable: true,
 });
 
-const errorSpy = vi.spyOn(Log, 'error').mockImplementation(() => '');
+vi.spyOn(Log, 'error').mockImplementation(() => '');
 const debugSpy = vi.spyOn(Log, 'debug');
 
 const rawPushSubscription = new RawPushSubscription();