more pr feedback

Author: fadi-george

mocks/deepmerge.ts

@@ -8,6 +8,39 @@ export type DeepPartial<T> = T extends Function
         ? { [K in keyof T]?: DeepPartial<T[K]> }
         : T;
 
+/**
+ * Recursively merges a patch object into a target object.
+ *
+ * Behavior:
+ * - Primitive values, nulls, and functions from patch replace target values
+ * - Arrays from patch replace target arrays entirely
+ * - Plain objects are recursively merged
+ * - Undefined values in patch are skipped (target values are preserved)
+ *
+ * @template T - The type of the target object
+ * @param target - The base object to merge into
+ * @param patch - The partial object with values to merge; undefined/null values are ignored
+ * @returns A new merged object of type T
+ *
+ * @example
+ * const base = {
+ *   name: "John",
+ *   tags: ["a", "b"],
+ *   address: { city: "NYC", zip: "10001" }
+ * };
+ * const patch = {
+ *   tags: ["x", "y", "z"],  // array replaced entirely
+ *   address: { city: "LA" }, // merged (zip kept from base)
+ *   email: "[email protected]" // new property added
+ * };
+ * const result = deepMerge(base, patch);
+ * // Result: {
+ * //   name: "John",
+ * //   tags: ["x", "y", "z"],
+ * //   address: { city: "LA", zip: "10001" },
+ * //   email: "[email protected]"
+ * // }
+ */
 export function deepMerge<T>(target: T, patch: DeepPartial<T>): T {
   if (patch === undefined || patch === null) return target;
 
@@ -38,5 +71,3 @@ export function deepMerge<T>(target: T, patch: DeepPartial<T>): T {
 
   return out as T;
 }
-
-export const asDeepPartial = <T>(v: DeepPartial<T>) => v;

www/DebugNamespace.test.ts

@@ -14,42 +14,48 @@ describe('Debug', () => {
   });
 
   test.each([
-    [LogLevel.None],
-    [LogLevel.Fatal],
-    [LogLevel.Error],
-    [LogLevel.Warn],
-    [LogLevel.Info],
-    [LogLevel.Debug],
-    [LogLevel.Verbose],
-  ])('should call cordova.exec for setLogLevel with %s', (logLevel) => {
-    debug.setLogLevel(logLevel);
+    [LogLevel.None, 0],
+    [LogLevel.Fatal, 1],
+    [LogLevel.Error, 2],
+    [LogLevel.Warn, 3],
+    [LogLevel.Info, 4],
+    [LogLevel.Debug, 5],
+    [LogLevel.Verbose, 6],
+  ])(
+    'should call cordova.exec for setLogLevel with %s',
+    (logLevel, logLevelValue) => {
+      debug.setLogLevel(logLevel);
 
-    expect(window.cordova.exec).toHaveBeenCalledWith(
-      expect.any(Function),
-      expect.any(Function),
-      'OneSignalPush',
-      'setLogLevel',
-      [logLevel],
-    );
-  });
+      expect(window.cordova.exec).toHaveBeenCalledWith(
+        expect.any(Function),
+        expect.any(Function),
+        'OneSignalPush',
+        'setLogLevel',
+        [logLevelValue],
+      );
+    },
+  );
 
   test.each([
-    [LogLevel.None],
-    [LogLevel.Fatal],
-    [LogLevel.Error],
-    [LogLevel.Warn],
-    [LogLevel.Info],
-    [LogLevel.Debug],
-    [LogLevel.Verbose],
-  ])('should call cordova.exec for setAlertLevel with %s', (logLevel) => {
-    debug.setAlertLevel(logLevel);
+    [LogLevel.None, 0],
+    [LogLevel.Fatal, 1],
+    [LogLevel.Error, 2],
+    [LogLevel.Warn, 3],
+    [LogLevel.Info, 4],
+    [LogLevel.Debug, 5],
+    [LogLevel.Verbose, 6],
+  ])(
+    'should call cordova.exec for setAlertLevel with %s',
+    (logLevel, logLevelValue) => {
+      debug.setAlertLevel(logLevel);
 
-    expect(window.cordova.exec).toHaveBeenCalledWith(
-      expect.any(Function),
-      expect.any(Function),
-      'OneSignalPush',
-      'setAlertLevel',
-      [logLevel],
-    );
-  });
+      expect(window.cordova.exec).toHaveBeenCalledWith(
+        expect.any(Function),
+        expect.any(Function),
+        'OneSignalPush',
+        'setAlertLevel',
+        [logLevelValue],
+      );
+    },
+  );
 });

www/InAppMessagesNamespace.test.ts

@@ -7,6 +7,7 @@ import type {
   InAppMessageDidDisplayEvent,
   InAppMessageWillDisplayEvent,
 } from './types/InAppMessage';
+
 describe('InAppMessages', () => {
   let inAppMessages: InAppMessages;
 

www/NotificationsNamespace.test.ts

@@ -5,6 +5,7 @@ import Notifications, {
   OSNotificationPermission,
 } from './NotificationsNamespace';
 import * as helpers from './helpers';
+
 describe('Notifications', () => {
   let notifications: Notifications;
 
@@ -176,6 +177,9 @@ describe('Notifications', () => {
       mockExec.mockImplementation((resolve, reject) => {
         reject(mockError);
       });
+      await expect(notifications.canRequestPermission()).rejects.toThrow(
+        mockError.message,
+      );
     });
   });
 
@@ -348,18 +352,6 @@ describe('Notifications', () => {
         [notificationId],
       );
     });
-
-    test('should call cordova.exec for removeNotification with different IDs', () => {
-      notifications.removeNotification(456);
-
-      expect(window.cordova.exec).toHaveBeenCalledWith(
-        expect.any(Function),
-        expect.any(Function),
-        'OneSignalPush',
-        'removeNotification',
-        [456],
-      );
-    });
   });
 
   describe('removeGroupedNotifications', () => {
@@ -391,7 +383,7 @@ describe('Notifications', () => {
       expect(result).toBe(true);
     });
 
-    test('should return test when permission is set via permission listener', () => {
+    test('should return true when permission is set via permission listener', () => {
       notifications._setPropertyAndObserver();
       const callback = mockExec.mock.calls.find(
         (call) => call[3] === 'addPermissionObserver',

www/OSNotification.test.ts

@@ -1,8 +1,7 @@
 import { mockCordova } from '../mocks/cordova';
-import { OSNotification } from './OSNotification';
+import { OSNotification, type ReceivedEvent } from './OSNotification';
 
-type RequireOptional<T> = Required<T>;
-type AllProperties = RequireOptional<Omit<OSNotification, 'display'>>;
+type AllProperties = Required<Omit<ReceivedEvent, 'display'>>;
 
 describe('OSNotification', () => {
   beforeEach(() => {

www/OSNotification.ts

@@ -1,11 +1,15 @@
 import { noop } from './helpers';
 
+export type ReceivedEvent = Omit<OSNotification, 'display' | 'rawPayload'> & {
+  rawPayload: string | object;
+};
+
 export class OSNotification {
   body: string;
   sound?: string;
   title?: string;
   launchURL?: string;
-  rawPayload: string | object;
+  rawPayload: object;
   actionButtons?: object[];
   additionalData: object;
   notificationId: string;
@@ -37,7 +41,7 @@ export class OSNotification {
   relevanceScore?: number;
   interruptionLevel?: string;
 
-  constructor(receivedEvent: Omit<OSNotification, 'display'>) {
+  constructor(receivedEvent: ReceivedEvent) {
     /// The OneSignal notification ID for this notification
     this.notificationId = receivedEvent.notificationId;
 

www/PushSubscriptionNamespace.test.ts

@@ -217,7 +217,7 @@ describe('PushSubscription', () => {
   });
 
   describe('addEventListener', () => {
-    test('should add listener to observer list and call cordova.exec', () => {
+    test('add listener should call cordova.exec', () => {
       const mockListener = vi.fn();
 
       pushSubscription.addEventListener('change', mockListener);