Skip to content

Commit 18cf90d

Browse files
itrushfadi-george
andauthored
fix: Ensure that notifications on legacy http integrations navigate to the origin (#1349)
Co-authored-by: Fadi George <fadii925@gmail.com>
1 parent 233eb9c commit 18cf90d

File tree

15 files changed

+67
-27
lines changed

15 files changed

+67
-27
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ on:
66

77
jobs:
88
test:
9-
runs-on: ubuntu-20.04
9+
runs-on: ubuntu-22.04
1010
container:
1111
image: python:2.7.18-buster
1212

Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
FROM node:14.21.1
1+
FROM node:22
22
WORKDIR /sdk
33
COPY package.json .
44
RUN yarn
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
importScripts("https://localhost:4001/sdks/Dev-OneSignalSDKWorker.js");
1+
importScripts("https://localhost:4000/sdks/Dev-OneSignalSDKWorker.js");
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
importScripts("https://localhost:4001/sdks/Dev-OneSignalSDKWorker.js");
1+
importScripts("https://localhost:4000/sdks/Dev-OneSignalSDKWorker.js");

package.json

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@
77
"bowser": "github:OneSignal/bowser#fix-android8-opr6-build-detection",
88
"jsdom": "^9.12.0",
99
"jsonp": "github:OneSignal/jsonp#onesignal",
10-
"sass": "^1.90.0",
1110
"npm-css": "https://registry.npmjs.org/npm-css/-/npm-css-0.2.3.tgz",
1211
"postcss-discard-comments": "https://registry.npmjs.org/postcss-discard-comments/-/postcss-discard-comments-2.0.4.tgz",
1312
"postcss-filter-plugins": "https://registry.npmjs.org/postcss-filter-plugins/-/postcss-filter-plugins-2.0.2.tgz",
13+
"sass": "^1.90.0",
1414
"tslib": "^1.9.0",
1515
"validator": "https://registry.npmjs.org/validator/-/validator-6.0.0.tgz"
1616
},
@@ -40,7 +40,7 @@
4040
"jest": "jest --coverage"
4141
},
4242
"config": {
43-
"sdkVersion": "151606"
43+
"sdkVersion": "151607"
4444
},
4545
"repository": {
4646
"type": "git",
@@ -150,5 +150,6 @@
150150
"maxSize": "9 kB",
151151
"compression": "gzip"
152152
}
153-
]
154-
}
153+
],
154+
"packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
155+
}

src/service-worker/ServiceWorker.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { NotificationReceived, NotificationClicked } from "../models/Notificatio
2828
import { cancelableTimeout } from "../helpers/sw/CancelableTimeout";
2929
import { DeviceRecord } from '../models/DeviceRecord';
3030
import { awaitableTimeout } from "../utils/AwaitableTimeout";
31+
import { AppConfig } from "src/models/AppConfig";
3132

3233
declare var self: ServiceWorkerGlobalScope & OSServiceWorkerFields;
3334

@@ -156,6 +157,11 @@ export class ServiceWorker {
156157
return appId;
157158
}
158159

160+
static async getAppConfig(): Promise<AppConfig> {
161+
const appId = await ServiceWorker.getAppId();
162+
return await ConfigHelper.getAppConfig({ appId }, OneSignalApiSW.downloadServerAppConfig);
163+
}
164+
159165
static setupMessageListeners() {
160166
ServiceWorker.workerMessenger.on(WorkerMessengerCommand.WorkerVersion, _ => {
161167
Log.debug('[Service Worker] Received worker version message.');
@@ -750,8 +756,30 @@ export class ServiceWorker {
750756

751757
// Use the user-provided default URL if one exists
752758
const { defaultNotificationUrl: dbDefaultNotificationUrl } = await Database.getAppState();
753-
if (dbDefaultNotificationUrl)
759+
if (dbDefaultNotificationUrl) {
754760
launchUrl = dbDefaultNotificationUrl;
761+
}
762+
else {
763+
// There was an issue with legacy HTTP integrations where the defaultNotificationUrl
764+
// was never saved to the DB. To account for this, we try to get the default URL
765+
// from the app config on notification click and save it to the DB for future use.
766+
// Choosing between notification received and notification clicked to do this logic,
767+
// we decided on notification clicked to avoid doing extra api call when the user
768+
// may never click the notification.
769+
try {
770+
const config = await ServiceWorker.getAppConfig();
771+
const defaultNotificationUrlFromConfig = config.origin;
772+
if (defaultNotificationUrlFromConfig) {
773+
launchUrl = defaultNotificationUrlFromConfig;
774+
775+
if (!dbDefaultNotificationUrl) {
776+
await Database.setAppState({ defaultNotificationUrl: defaultNotificationUrlFromConfig });
777+
}
778+
}
779+
} catch (e) {
780+
Log.error("Failed to update notification in the database", e);
781+
}
782+
}
755783

756784
// If the user clicked an action button, use the URL provided by the action button
757785
// Unless the action button URL is null

src/services/Database.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ export default class Database {
259259
return state;
260260
}
261261

262-
async setAppState(appState: AppState) {
262+
async setAppState(appState: Partial<AppState>) {
263263
if (appState.defaultNotificationUrl)
264264
await this.put("Options", { key: "defaultUrl", value: appState.defaultNotificationUrl });
265265
if (appState.defaultNotificationTitle || appState.defaultNotificationTitle === "")
@@ -525,7 +525,7 @@ export default class Database {
525525
return await Database.singletonInstance.getServiceWorkerState();
526526
}
527527

528-
static async setAppState(appState: AppState) {
528+
static async setAppState(appState: Partial<AppState>) {
529529
return await Database.singletonInstance.setAppState(appState);
530530
}
531531

test/support/mocks/service-workers/models/MockClients.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ export class MockClients implements Clients {
1010
return Promise.resolve(client || null);
1111
}
1212

13-
async matchAll(_options?: ClientQueryOptions): Promise<Client[]> {
14-
return this.clients;
13+
async matchAll<T extends ClientQueryOptions>(options?: T): Promise<ReadonlyArray<T["type"] extends "window" ? WindowClient : Client>> {
14+
return Object.freeze(this.clients) as ReadonlyArray<T["type"] extends "window" ? WindowClient : Client>;
1515
}
1616

1717
async openWindow(_url: string): Promise<WindowClient | null> {

test/support/mocks/service-workers/models/MockPushManager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export class MockPushManager implements PushManager {
1212
return this.subscription;
1313
}
1414

15-
public async permissionState(_options?: PushSubscriptionOptionsInit): Promise<PushPermissionState> {
15+
public async permissionState(_options?: PushSubscriptionOptionsInit): Promise<PermissionState> {
1616
return "granted";
1717
}
1818

test/support/mocks/service-workers/models/MockServiceWorker.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ export class MockServiceWorker implements ServiceWorker {
2222
throw new NotImplementedError();
2323
}
2424

25-
postMessage(message: any, transfer: Array<Transferable> | PostMessageOptions): void {
25+
postMessage(message: any, transfer: Array<Transferable> | any): void {
2626
}
2727

2828
removeEventListener<K extends keyof ServiceWorkerEventMap>(type: K, listener: (this: ServiceWorker, ev: ServiceWorkerEventMap[K]) => any, options?: boolean | EventListenerOptions): void;

0 commit comments

Comments
 (0)