Skip to content

Commit d9b086a

Browse files
itrushfadi-george
andcommitted
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 d9b086a

16 files changed

Lines changed: 74 additions & 34 deletions

File tree

.github/workflows/cd.yml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ on:
55

66
jobs:
77
test_build_deploy:
8-
runs-on: ubuntu-20.04
8+
runs-on: ubuntu-latest
99
container:
1010
image: python:2.7.18-buster
1111

@@ -21,27 +21,27 @@ jobs:
2121
node-version: ${{ matrix.node-version }}
2222
- name: Install Yarn
2323
run: npm install -g yarn
24-
- name: "[Setup] Install dependencies"
24+
- name: '[Setup] Install dependencies'
2525
run: yarn
26-
- name: "[Test] Run all tests"
26+
- name: '[Test] Run all tests'
2727
run: yarn test:CI
28-
- name: "[Build] Staging"
28+
- name: '[Build] Staging'
2929
env:
3030
STAGING_DOMAIN: ${{ secrets.STAGING_DOMAIN }}
3131
run: BUILD_ORIGIN=${STAGING_DOMAIN} API=staging API_ORIGIN=${STAGING_DOMAIN} yarn build:staging
32-
- name: "[Build] Production"
32+
- name: '[Build] Production'
3333
run: yarn build:prod
3434
- name: 'Authenticate to Google Cloud'
3535
uses: 'google-github-actions/auth@v1'
3636
with:
3737
credentials_json: '${{ secrets.GOOGLE_CREDENTIALS }}'
38-
- name: "[Deploy] Upload SDK Files to GCS"
38+
- name: '[Deploy] Upload SDK Files to GCS'
3939
uses: 'google-github-actions/upload-cloud-storage@v1'
4040
with:
4141
path: 'build/releases'
4242
destination: 'sdk-builds-persistence-onesignal/web-sdk/${{ github.sha }}'
4343
parent: false
44-
- name: "[Deploy] Upload AMP SDK Files to GCS"
44+
- name: '[Deploy] Upload AMP SDK Files to GCS'
4545
uses: 'google-github-actions/upload-cloud-storage@v1'
4646
with:
4747
path: 'build/amp'

.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

0 commit comments

Comments
 (0)