Skip to content

Commit

Permalink
Merge pull request #1079 from OneSignal/fix/sw-click-and-confirm-deli…
Browse files Browse the repository at this point in the history
…very-calls

[User Model] [Fix] Notification Open & Confirm Delivery REST API calls failing
  • Loading branch information
jkasten2 authored Aug 10, 2023
2 parents 92608b8 + e1547e3 commit d00db6d
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
20 changes: 20 additions & 0 deletions src/sw/helpers/ModelCacheDirectAccess.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import EncodedModel from "../../core/caching/EncodedModel";
import { ModelName } from "../../core/models/SupportedModels";
import Database from "../../shared/services/Database";

/**
* WARNING: This is a temp workaround for the ServiceWorker context only!
* PURPOSE: CoreModuleDirector doesn't work in the SW context.
* TODO: This is duplicated logic tech debt to address later
*/
export class ModelCacheDirectAccess {
static async getPushSubscriptionIdByToken(token: string): Promise<string | undefined> {
const pushSubscriptions = await Database.getAll<EncodedModel>(ModelName.PushSubscriptions);
for(const pushSubscription of pushSubscriptions) {
if (pushSubscription["token"] === token) {
return pushSubscription["id"] as string;
}
}
return undefined;
}
}
22 changes: 16 additions & 6 deletions src/sw/serviceWorker/ServiceWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { OSMinifiedNotificationPayload, OSMinifiedNotificationPayloadHelper } fr
import { bowserCastle } from "../../shared/utils/bowserCastle";
import { OSWebhookNotificationEventSender } from "../webhooks/notifications/OSWebhookNotificationEventSender";
import { OSNotificationButtonsConverter } from "../models/OSNotificationButtonsConverter";
import { ModelCacheDirectAccess } from "../helpers/ModelCacheDirectAccess";

declare const self: ServiceWorkerGlobalScope & OSServiceWorkerFields;

Expand Down Expand Up @@ -80,6 +81,15 @@ export class ServiceWorker {
return new OSWebhookNotificationEventSender();
}

static async getPushSubscriptionId(): Promise<string | undefined> {
const pushSubscription = await self.registration.pushManager.getSubscription();
const pushToken = pushSubscription?.endpoint;
if (!pushToken) {
return undefined;
}
return ModelCacheDirectAccess.getPushSubscriptionIdByToken(pushToken);
}

/**
* Allows message passing between this service worker and pages on the same domain.
* Clients include any HTTPS site page, or the nested iFrame pointing to OneSignal on any HTTP site. This allows
Expand Down Expand Up @@ -302,7 +312,7 @@ export class ServiceWorker {
return;

const appId = await ServiceWorker.getAppId();
const { deviceId } = await Database.getSubscription();
const pushSubscriptionId = await this.getPushSubscriptionId();

// app and notification ids are required, decided to exclude deviceId from required params
// In rare case we don't have it we can still report as confirmed to backend to increment count
Expand All @@ -314,7 +324,7 @@ export class ServiceWorker {
// JSON.stringify() does not include undefined values
// Our response will not contain those fields here which have undefined values
const postData = {
player_id : deviceId,
player_id : pushSubscriptionId,
app_id : appId,
device_type: DeviceRecord.prototype.getDeliveryPlatform()
};
Expand Down Expand Up @@ -728,8 +738,8 @@ export class ServiceWorker {

// Start making REST API requests BEFORE self.clients.openWindow is called.
// It will cause the service worker to stop on Chrome for Android when site is added to the home screen.
const { deviceId } = await Database.getSubscription();
const convertedAPIRequests = ServiceWorker.sendConvertedAPIRequests(appId, deviceId, notificationClickEvent, deviceType);
const pushSubscriptionId = await this.getPushSubscriptionId();
const convertedAPIRequests = ServiceWorker.sendConvertedAPIRequests(appId, pushSubscriptionId, notificationClickEvent, deviceType);

/*
Check if we can focus on an existing tab instead of opening a new url.
Expand Down Expand Up @@ -846,7 +856,7 @@ export class ServiceWorker {
*/
static async sendConvertedAPIRequests(
appId: string | undefined | null,
deviceId: string | undefined,
pushSubscriptionId: string | undefined,
notificationClickEvent: NotificationClickEventInternal,
deviceType: DeliveryPlatformKind,
): Promise<void> {
Expand All @@ -862,7 +872,7 @@ export class ServiceWorker {
if (appId) {
onesignalRestPromise = OneSignalApiBase.put(`notifications/${notificationData.notificationId}`, {
app_id: appId,
player_id: deviceId,
player_id: pushSubscriptionId,
opened: true,
device_type: deviceType
});
Expand Down

0 comments on commit d00db6d

Please sign in to comment.