Skip to content

Commit b069192

Browse files
fadi-georgefadi-george
committed
address pr comments
- remove duplicate code - add version check for prompts manager
1 parent b7667b0 commit b069192

13 files changed

Lines changed: 81 additions & 524 deletions

File tree

‎package-lock.json‎

Lines changed: 55 additions & 437 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353
"@types/uuid": "^10.0.0",
5454
"@typescript-eslint/eslint-plugin": "^5.36.1",
5555
"@typescript-eslint/parser": "^5.36.1",
56-
"@vitest/coverage-v8": "3.2.4",
56+
"@vitest/coverage-v8": "4.0.0-beta.6",
5757
"concurrently": "^9.2.0",
5858
"deepmerge": "^4.2.2",
5959
"eslint": "^8.23.0",
@@ -70,7 +70,7 @@
7070
"vite-bundle-analyzer": "^1.1.0",
7171
"vite-plugin-mkcert": "^1.17.8",
7272
"vite-tsconfig-paths": "^5.1.4",
73-
"vitest": "3.2.4"
73+
"vitest": "4.0.0-beta.6"
7474
},
7575
"size-limit": [
7676
{

‎src/onesignal/OneSignal.test.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ describe('OneSignal', () => {
9898
afterEach(async () => {
9999
window.OneSignal.coreDirector.operationRepo.queue = [];
100100
await Database.remove('operations');
101+
await waitForOperations();
101102
window.OneSignal.coreDirector.subscriptionModelStore.replaceAll(
102103
[],
103104
ModelChangeTags.HYDRATE,
@@ -270,9 +271,8 @@ describe('OneSignal', () => {
270271

271272
await waitForOperations(6);
272273
window.OneSignal.User.removeEmail(email);
273-
await waitForOperations(4);
274274

275-
expect(deleteSubscriptionFn).toHaveBeenCalled();
275+
await vi.waitUntil(() => deleteSubscriptionFn.mock.calls.length === 1);
276276
dbSubscriptions = await getEmailSubscriptionDbItems();
277277
expect(dbSubscriptions).toHaveLength(0);
278278
});
@@ -977,7 +977,7 @@ describe('OneSignal', () => {
977977
OneSignal.coreDirector
978978
.getIdentityModel()
979979
.setProperty('external_id', 'some-id', ModelChangeTags.NO_PROPOGATE);
980-
window.OneSignal.User.trackEvent(name, properties);
980+
window.OneSignal.User.trackEvent(name);
981981

982982
await vi.waitUntil(() => sendCustomEventFn.mock.calls.length === 1);
983983

@@ -989,7 +989,6 @@ describe('OneSignal', () => {
989989
onesignal_id: DUMMY_ONESIGNAL_ID,
990990
payload: {
991991
os_sdk: OS_SDK,
992-
test_property: 'test_value',
993992
},
994993
timestamp: expect.any(String),
995994
},

‎src/page/managers/PromptsManager.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
import {
1313
Browser,
1414
getBrowserName,
15+
getBrowserVersion,
1516
isMobileBrowser,
1617
isTabletBrowser,
1718
requiresUserInteraction,
@@ -45,8 +46,10 @@ export class PromptsManager {
4546
}
4647

4748
private shouldForceSlidedownOverNative(): boolean {
49+
const browserVersion = getBrowserVersion();
4850
return (
4951
(getBrowserName() === Browser.Chrome &&
52+
browserVersion >= 63 &&
5053
(isTabletBrowser() || isMobileBrowser())) ||
5154
requiresUserInteraction()
5255
);

‎src/page/userModel/FuturePushSubscriptionRecord.ts‎

Lines changed: 1 addition & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,14 @@ import type {
22
NotificationTypeValue,
33
SubscriptionTypeValue,
44
} from 'src/core/types/subscription';
5-
import {
6-
NotificationType,
7-
SubscriptionType,
8-
} from 'src/core/types/subscription';
5+
import { NotificationType } from 'src/core/types/subscription';
96
import {
107
getDeviceModel,
118
getDeviceOS,
129
getSubscriptionType,
13-
useSafariLegacyPush,
14-
useSafariVapidPush,
1510
} from 'src/shared/environment';
1611
import { RawPushSubscription } from 'src/shared/models/RawPushSubscription';
17-
import {
18-
Browser,
19-
getBrowserName,
20-
getBrowserVersion,
21-
} from 'src/shared/useragent';
2212
import { VERSION } from 'src/shared/utils/EnvVariables';
23-
import {
24-
DeliveryPlatformKind,
25-
type DeliveryPlatformKindValue,
26-
} from '../../shared/models/DeliveryPlatformKind';
2713
import type { Serializable } from '../models/Serializable';
2814

2915
export default class FuturePushSubscriptionRecord implements Serializable {
@@ -69,53 +55,4 @@ export default class FuturePushSubscriptionRecord implements Serializable {
6955
web_p256: this.webp256,
7056
};
7157
}
72-
73-
/* S T A T I C */
74-
75-
/**
76-
* Get the User Model Subscription type based on browser detection.
77-
*/
78-
public static getSubscriptionType(): SubscriptionTypeValue {
79-
const browserName = getBrowserName();
80-
if (browserName === Browser.Firefox) {
81-
return SubscriptionType.FirefoxPush;
82-
}
83-
if (useSafariVapidPush()) {
84-
return SubscriptionType.SafariPush;
85-
}
86-
if (useSafariLegacyPush) {
87-
return SubscriptionType.SafariLegacyPush;
88-
}
89-
// Other browsers, like Edge, are Chromium based so we consider them "Chrome".
90-
return SubscriptionType.ChromePush;
91-
}
92-
93-
/**
94-
* Get the legacy player.device_type
95-
* NOTE: Use getSubscriptionType() instead when possible.
96-
*/
97-
public static getDeviceType(): DeliveryPlatformKindValue {
98-
switch (this.getSubscriptionType()) {
99-
case SubscriptionType.FirefoxPush:
100-
return DeliveryPlatformKind.Firefox;
101-
case SubscriptionType.SafariLegacyPush:
102-
return DeliveryPlatformKind.SafariLegacy;
103-
case SubscriptionType.SafariPush:
104-
return DeliveryPlatformKind.SafariVapid;
105-
}
106-
return DeliveryPlatformKind.ChromeLike;
107-
}
108-
109-
public static getDeviceOS(): string | number {
110-
const browserVersion = getBrowserVersion();
111-
return isNaN(browserVersion) ? -1 : browserVersion;
112-
}
113-
114-
public static getDeviceModel(): string {
115-
return navigator.platform;
116-
}
117-
118-
public static getSdk(): string {
119-
return String(VERSION);
120-
}
12158
}

‎src/shared/environment/environment.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export const supportsServiceWorkers = () => {
1919

2020
export const windowEnvString = IS_SERVICE_WORKER ? 'Service Worker' : 'Browser';
2121

22-
export const useSafariLegacyPush =
22+
export const useSafariLegacyPush = () =>
2323
isBrowser && window.safari?.pushNotification != undefined;
2424

2525
export const supportsVapidPush =
@@ -30,7 +30,7 @@ export const supportsVapidPush =
3030
export const useSafariVapidPush = () =>
3131
getBrowserName() === Browser.Safari &&
3232
supportsVapidPush &&
33-
!useSafariLegacyPush;
33+
!useSafariLegacyPush();
3434

3535
// for determing the api url
3636
const API_URL_PORT = 3000;
@@ -80,7 +80,7 @@ export const getSubscriptionType = (): SubscriptionTypeValue => {
8080
if (useSafariVapidPush()) {
8181
return SubscriptionType.SafariPush;
8282
}
83-
if (useSafariLegacyPush) {
83+
if (useSafariLegacyPush()) {
8484
return SubscriptionType.SafariLegacyPush;
8585
}
8686
// Other browsers, like Edge, are Chromium based so we consider them "Chrome".

‎src/shared/helpers/MainHelper.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ export default class MainHelper {
239239

240240
// TO DO: unit test
241241
static async getCurrentPushToken(): Promise<string | undefined> {
242-
if (useSafariLegacyPush) {
242+
if (useSafariLegacyPush()) {
243243
const safariToken = window.safari?.pushNotification?.permission(
244244
OneSignal.config?.safariWebId,
245245
).deviceToken;

‎src/shared/helpers/dom.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import Log from 'src/sw/libraries/Log';
1+
import Log from 'src/shared/libraries/Log';
22

33
export function addDomElement(
44
targetSelectorOrElement: string | Element,

‎src/shared/listeners/listeners.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,11 +249,6 @@ export async function onInternalSubscriptionSet(optedOut: boolean) {
249249
LimitStore.put('subscription.optedOut', optedOut);
250250
}
251251

252-
/**
253-
* NOTE: This uses the OneSignal REST API POST /notifications with
254-
* include_player_ids. This field will be dropped by 2025 so a
255-
* replacement will needed by then.
256-
*/
257252
async function onSubscriptionChanged_showWelcomeNotification(
258253
isSubscribed: boolean | undefined,
259254
pushSubscriptionId: string | undefined | null,

‎src/shared/managers/PermissionManager.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export default class PermissionManager {
3939
public async getNotificationPermission(
4040
safariWebId?: string,
4141
): Promise<NotificationPermission> {
42-
if (useSafariLegacyPush) {
42+
if (useSafariLegacyPush()) {
4343
return PermissionManager.getLegacySafariNotificationPermission(
4444
safariWebId,
4545
);

0 commit comments

Comments
 (0)