Skip to content

Commit df40d6d

Browse files
itrushitrush
authored
Fix typo and sent value for removing external user is value (#475)
* Fix typo and sent value for removing external user is value * Treating null, undefined and empty string as removing external user id
1 parent 122585d commit df40d6d

File tree

6 files changed

+55
-21
lines changed

6 files changed

+55
-21
lines changed

‎src/OneSignal.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ export default class OneSignal {
571571
// TODO: Throw an error here in future v2; for now it may break existing client implementations.
572572
Log.info(new InvalidArgumentError('tags', InvalidArgumentReason.Empty));
573573
}
574-
const tagsToSend = {};
574+
const tagsToSend = {} as {[key: string]: any};
575575
for (let tag of tags) {
576576
tagsToSend[tag] = '';
577577
}
@@ -584,7 +584,7 @@ export default class OneSignal {
584584
/**
585585
* @PublicApi
586586
*/
587-
public static async setExternalUserId(externalUserId: string | undefined): Promise<void> {
587+
public static async setExternalUserId(externalUserId: string | undefined | null): Promise<void> {
588588
await awaitOneSignalInitAndSupported();
589589
logMethodCall("setExternalUserId");
590590

@@ -601,7 +601,7 @@ export default class OneSignal {
601601
/**
602602
* @PublicApi
603603
*/
604-
public static async getExternalUserId(): Promise<string | undefined> {
604+
public static async getExternalUserId(): Promise<string | undefined | null> {
605605
await awaitOneSignalInitAndSupported();
606606
logMethodCall("getExternalUserId");
607607
return await OneSignal.database.getExternalUserId();

‎src/managers/UpdateManager.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import MainHelper from '../helpers/MainHelper';
66
import Database from "../services/Database";
77
import Log from "../libraries/Log";
88
import { ContextSWInterface } from '../models/ContextSW';
9+
import Utils from "../utils/Utils";
910

1011
export class UpdateManager {
1112
private context: ContextSWInterface;
@@ -107,10 +108,10 @@ export class UpdateManager {
107108
return this.onSessionSent;
108109
}
109110

110-
public async sendExternalUserIdUpdate(externalUserId: string | undefined): Promise<void> {
111+
public async sendExternalUserIdUpdate(externalUserId: string | undefined | null): Promise<void> {
111112
const deviceId: string = await this.getDeviceId();
112113
await OneSignalApiShared.updatePlayer(this.context.appConfig.appId, deviceId, {
113-
extenal_user_id: externalUserId
114+
external_user_id: Utils.getValueOrDefault(externalUserId, "")
114115
});
115116
}
116117
}

‎src/models/AppState.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
import { Notification } from "./Notification";
22
import { Timestamp } from "./Timestamp";
33

4+
export interface ClickedNotifications {
5+
[key: string]: [Notification, Timestamp]
6+
}
7+
48
class AppState {
5-
defaultNotificationUrl: string;
6-
defaultNotificationTitle: string;
9+
defaultNotificationUrl: string | undefined;
10+
defaultNotificationTitle: string | undefined;
711

812
/**
913
* Whether the user is currently completely subscribed, including not opted out. Database cached version of
1014
* isPushNotificationsEnabled().
1115
*/
12-
lastKnownPushEnabled: boolean;
16+
lastKnownPushEnabled: boolean | undefined;
1317

14-
clickedNotifications: Map<URL, [Notification, Timestamp]>
18+
clickedNotifications: ClickedNotifications | undefined;
1519
}
1620

1721
export { AppState };

‎src/services/Database.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,16 @@ import Emitter from "../libraries/Emitter";
22
import IndexedDb from "./IndexedDb";
33

44
import { AppConfig } from "../models/AppConfig";
5-
import { AppState } from "../models/AppState";
5+
import { AppState, ClickedNotifications } from "../models/AppState";
66
import { Notification } from "../models/Notification";
77
import { ServiceWorkerState } from "../models/ServiceWorkerState";
88
import { Subscription } from "../models/Subscription";
99
import { TestEnvironmentKind } from "../models/TestEnvironmentKind";
10-
import { Timestamp } from "../models/Timestamp";
1110
import { WindowEnvironmentKind } from "../models/WindowEnvironmentKind";
1211
import { EmailProfile } from "../models/EmailProfile";
1312
import SdkEnvironment from "../managers/SdkEnvironment";
1413
import OneSignalUtils from "../utils/OneSignalUtils";
14+
import Utils from "../utils/Utils";
1515

1616
enum DatabaseEventName {
1717
SET
@@ -182,12 +182,18 @@ export default class Database {
182182
return config;
183183
}
184184

185-
async getExternalUserId(): Promise<string | undefined> {
185+
async getExternalUserId(): Promise<string | undefined | null> {
186186
return await this.get<string>("Ids", "externalUserId");
187187
}
188188

189-
async setExternalUserId(externalUserId: string | undefined): Promise<void> {
190-
return await this.put("Ids", {type: "externalUserId", id: externalUserId})
189+
async setExternalUserId(externalUserId: string | undefined | null): Promise<void> {
190+
const emptyString: string = "";
191+
const externalIdToSave = Utils.getValueOrDefault(externalUserId, emptyString);
192+
if (externalIdToSave === emptyString) {
193+
await this.remove("Ids", "externalUserId");
194+
} else {
195+
await this.put("Ids", {type: "externalUserId", id: externalIdToSave});
196+
}
191197
}
192198

193199
async setAppConfig(appConfig: AppConfig): Promise<void> {
@@ -212,7 +218,7 @@ export default class Database {
212218
state.defaultNotificationUrl = await this.get<string>("Options", "defaultUrl");
213219
state.defaultNotificationTitle = await this.get<string>("Options", "defaultTitle");
214220
state.lastKnownPushEnabled = await this.get<boolean>("Options", "isPushEnabled");
215-
state.clickedNotifications = await this.get<Map<URL, [Notification, Timestamp]>>("NotificationOpened");
221+
state.clickedNotifications = await this.get<ClickedNotifications>("NotificationOpened");
216222
return state;
217223
}
218224

@@ -396,11 +402,11 @@ export default class Database {
396402
return await Database.singletonInstance.getAppConfig();
397403
}
398404

399-
static async getExternalUserId(): Promise<string | undefined> {
405+
static async getExternalUserId(): Promise<string | undefined | null> {
400406
return await Database.singletonInstance.getExternalUserId();
401407
}
402408

403-
static async setExternalUserId(externalUserId: string | undefined): Promise<void> {
409+
static async setExternalUserId(externalUserId: string | undefined | null): Promise<void> {
404410
await Database.singletonInstance.setExternalUserId(externalUserId);
405411
}
406412

‎test/unit/managers/UpdateManager.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,22 @@ test("sendExternalUserIdUpdate makes an api call with the provided external user
167167
t.is(updatePlayerSpy.getCalls().length, 1);
168168
t.is(updatePlayerSpy.getCall(0).args[0], OneSignal.context.appConfig.appId);
169169
t.is(updatePlayerSpy.getCall(0).args[1], deviceId);
170-
t.is(updatePlayerSpy.getCall(0).args[2].extenal_user_id, externalUserId);
170+
t.is(updatePlayerSpy.getCall(0).args[2].hasOwnProperty("external_user_id"), true);
171+
t.is(updatePlayerSpy.getCall(0).args[2].external_user_id, externalUserId);
171172

172173
await OneSignal.context.updateManager.sendExternalUserIdUpdate(undefined);
173174

174175
t.is(updatePlayerSpy.getCalls().length, 2);
175176
t.is(updatePlayerSpy.getCall(1).args[0], OneSignal.context.appConfig.appId);
176177
t.is(updatePlayerSpy.getCall(1).args[1], deviceId);
177-
t.is(updatePlayerSpy.getCall(1).args[2].extenal_user_id, undefined);
178-
t.is(updatePlayerSpy.getCall(1).args[2].hasOwnProperty("extenal_user_id"), true);
178+
t.is(updatePlayerSpy.getCall(1).args[2].hasOwnProperty("external_user_id"), true);
179+
t.is(updatePlayerSpy.getCall(1).args[2].external_user_id, "");
180+
181+
await OneSignal.context.updateManager.sendExternalUserIdUpdate(null);
182+
183+
t.is(updatePlayerSpy.getCalls().length, 3);
184+
t.is(updatePlayerSpy.getCall(2).args[0], OneSignal.context.appConfig.appId);
185+
t.is(updatePlayerSpy.getCall(2).args[1], deviceId);
186+
t.is(updatePlayerSpy.getCall(2).args[2].hasOwnProperty("external_user_id"), true);
187+
t.is(updatePlayerSpy.getCall(2).args[2].external_user_id, "");
179188
});

‎test/unit/meta/database.ts‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,23 @@ test('setExternalUserId saves value into database', async t => {
7575
await Database.setExternalUserId(externalUserId);
7676
t.is(await Database.getExternalUserId(), externalUserId);
7777

78-
// passing undefined as parameter clears out value from db
78+
// passing undefined, null or empty string as parameter clears out value from db
7979
await Database.setExternalUserId(undefined);
8080
t.is(isNullOrUndefined(await Database.getExternalUserId()), true);
81+
82+
//set it back so we can test removal
83+
await Database.setExternalUserId(externalUserId);
84+
t.is(await Database.getExternalUserId(), externalUserId);
85+
86+
await Database.setExternalUserId(null);
87+
t.is(isNullOrUndefined(await Database.getExternalUserId()), true);
88+
89+
//set it back so we can test removal
90+
await Database.setExternalUserId(externalUserId);
91+
t.is(await Database.getExternalUserId(), externalUserId);
92+
93+
await Database.setExternalUserId("");
94+
t.is(isNullOrUndefined(await Database.getExternalUserId()), true);
8195
});
8296

8397
test("getExternalUserId retrieves correct value from the database", async t => {

0 commit comments

Comments
 (0)