Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,10 @@ public void addOutcomeWithValue(String name, double value) {

@Override
public void login(String externalUserId) {
if (externalUserId == null || externalUserId.isEmpty()) {
Logging.error("login called with a null or empty externalUserId", null);
return;
}
OneSignal.login(externalUserId);
}

Expand Down
5 changes: 5 additions & 0 deletions ios/RCTOneSignal/RCTOneSignalEventEmitter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ + (void)sendEventWithName:(NSString *)name withBody:(NSDictionary *)body {
}

RCT_EXPORT_METHOD(login : (NSString *)externalId) {
if (externalId == nil || [externalId length] == 0) {
[OneSignalLog onesignalLog:ONE_S_LL_ERROR
message:@"login called with a nil or empty externalId"];
return;
}
[OneSignal login:externalId];
}

Expand Down
18 changes: 18 additions & 0 deletions src/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { IOS_NULL_SENTINEL } from './constants/internal';
import {
encodeNullsForIOS,
isNativeModuleLoaded,
isNonEmptyString,
isObjectSerializable,
isValidCallback,
} from './helpers';
Expand Down Expand Up @@ -66,6 +67,23 @@ describe('helpers', () => {
});
});

describe('isNonEmptyString', () => {
test.each([
{ description: 'a non-empty string', value: 'id', expected: true },
{ description: 'a whitespace string', value: ' ', expected: true },
{ description: 'an empty string', value: '', expected: false },
{ description: 'null', value: null, expected: false },
{ description: 'undefined', value: undefined, expected: false },
{ description: 'a number', value: 1, expected: false },
{ description: 'a boolean', value: true, expected: false },
])(
'should return $expected for $description',
({ value, expected }: { description: string; value: unknown; expected: boolean }) => {
expect(isNonEmptyString(value)).toBe(expected);
},
);
});

describe('isObjectSerializable', () => {
test.each([
{ description: 'an empty object', value: {} },
Expand Down
4 changes: 4 additions & 0 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ export function isNativeModuleLoaded(module: object | null | undefined): boolean
return true;
}

export function isNonEmptyString(value: unknown): value is string {
return typeof value === 'string' && value.length > 0;
}

/**
* Returns true if the value is a JSON-serializable object.
*/
Expand Down
54 changes: 50 additions & 4 deletions src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,18 @@ describe('OneSignal', () => {
expect(mockRNOneSignal.initialize).not.toHaveBeenCalled();
});

test('should not initialize if appId is null', () => {
OneSignal.initialize(null as unknown as string);
expect(mockRNOneSignal.initialize).not.toHaveBeenCalled();
expect(errorSpy).toHaveBeenCalledWith('OneSignal: initialize: appId is required');
});

test('should not initialize if appId is empty', () => {
OneSignal.initialize('');
expect(mockRNOneSignal.initialize).not.toHaveBeenCalled();
expect(errorSpy).toHaveBeenCalledWith('OneSignal: initialize: appId is required');
});

test('should keep a permission event that arrives before the startup read resolves', async () => {
let resolveStartupRead: ((granted: boolean) => void) | undefined;
vi.mocked(mockRNOneSignal.hasNotificationPermission).mockReturnValueOnce(
Expand Down Expand Up @@ -180,6 +192,18 @@ describe('OneSignal', () => {
OneSignal.login('external-123');
expect(mockRNOneSignal.login).not.toHaveBeenCalled();
});

test('should not login if externalId is null', () => {
OneSignal.login(null as unknown as string);
expect(mockRNOneSignal.login).not.toHaveBeenCalled();
expect(errorSpy).toHaveBeenCalledWith('OneSignal: login: externalId is required');
});

test('should not login if externalId is empty', () => {
OneSignal.login('');
expect(mockRNOneSignal.login).not.toHaveBeenCalled();
expect(errorSpy).toHaveBeenCalledWith('OneSignal: login: externalId is required');
});
});

describe('logout', () => {
Expand Down Expand Up @@ -676,6 +700,16 @@ describe('OneSignal', () => {
expect(mockRNOneSignal.setLanguage).toHaveBeenCalledWith('en');
});

test('forwards an empty language so native can reset', () => {
OneSignal.User.setLanguage('');
expect(mockRNOneSignal.setLanguage).toHaveBeenCalledWith('');
});

test('does not set a null language', () => {
OneSignal.User.setLanguage(null as unknown as string);
expect(mockRNOneSignal.setLanguage).not.toHaveBeenCalled();
});

test('should not set language if native module is not loaded', () => {
isNativeLoadedSpy.mockReturnValue(false);
OneSignal.User.setLanguage('en');
Expand Down Expand Up @@ -748,6 +782,18 @@ describe('OneSignal', () => {
OneSignal.User.addEmail(EMAIL);
expect(mockRNOneSignal.addEmail).not.toHaveBeenCalled();
});

test('should not add email if email is null', () => {
OneSignal.User.addEmail(null as unknown as string);
expect(mockRNOneSignal.addEmail).not.toHaveBeenCalled();
expect(errorSpy).toHaveBeenCalledWith('OneSignal: addEmail: email is required');
});

test('should not add email if email is empty', () => {
OneSignal.User.addEmail('');
expect(mockRNOneSignal.addEmail).not.toHaveBeenCalled();
expect(errorSpy).toHaveBeenCalledWith('OneSignal: addEmail: email is required');
});
});

describe('removeEmail', () => {
Expand Down Expand Up @@ -1306,16 +1352,16 @@ describe('OneSignal', () => {
expect(mockRNOneSignal.addTrigger).toHaveBeenCalledWith('key', 'value');
});

test('should log error but still call native method if key is missing', () => {
test('should not add trigger if key is missing', () => {
OneSignal.InAppMessages.addTrigger('', 'value');
expect(errorSpy).toHaveBeenCalled();
expect(mockRNOneSignal.addTrigger).toHaveBeenCalledWith('', 'value');
expect(mockRNOneSignal.addTrigger).not.toHaveBeenCalled();
});

test('should log error but still call native method if value is null', () => {
test('should not add trigger if value is null', () => {
OneSignal.InAppMessages.addTrigger('key', null as unknown as string);
expect(errorSpy).toHaveBeenCalled();
expect(mockRNOneSignal.addTrigger).toHaveBeenCalledWith('key', null);
expect(mockRNOneSignal.addTrigger).not.toHaveBeenCalled();
});

test('should not add trigger if native module is not loaded', () => {
Expand Down
54 changes: 50 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import NotificationWillDisplayEvent from './events/NotificationWillDisplayEvent'
import {
encodeNullsForIOS,
isNativeModuleLoaded,
isNonEmptyString,
isObjectSerializable,
isValidCallback,
} from './helpers';
Expand Down Expand Up @@ -107,6 +108,10 @@ export namespace OneSignal {
/** Initializes the OneSignal SDK. This should be called during startup of the application. */
export function initialize(appId: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(appId)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can just be !appId , similar in other places

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept isNonEmptyString. !appId lets a non-string through, and it would reject "" for tag values, trigger values, and setLanguage.

console.error('OneSignal: initialize: appId is required');
return;
}

RNOneSignal.initialize(appId);

Expand All @@ -121,6 +126,10 @@ export namespace OneSignal {
*/
export function login(externalId: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(externalId)) {
console.error('OneSignal: login: externalId is required');
return;
}

RNOneSignal.login(externalId);
}
Expand Down Expand Up @@ -445,16 +454,24 @@ export namespace OneSignal {
return RNOneSignal.getExternalId();
}

/** Explicitly set a 2-character language code for the user. */
/** Explicitly set a 2-character language code for the user. Empty string resets to the device language. */
export function setLanguage(language: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (typeof language !== 'string') {
console.error('OneSignal: setLanguage: language is required');
return;
}

RNOneSignal.setLanguage(language);
}

/** Set an alias for the current user. If this alias label already exists on this user, it will be overwritten with the new alias id. */
export function addAlias(label: string, id: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(label) || !isNonEmptyString(id)) {
console.error('OneSignal: addAlias: must include a label and an id');
return;
}

RNOneSignal.addAlias(label, id);
}
Expand All @@ -469,6 +486,10 @@ export namespace OneSignal {
/** Remove an alias from the current user. */
export function removeAlias(label: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(label)) {
console.error('OneSignal: removeAlias: label is required');
return;
}

RNOneSignal.removeAlias(label);
}
Expand All @@ -483,6 +504,10 @@ export namespace OneSignal {
/** Add a new email subscription to the current user. */
export function addEmail(email: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(email)) {
console.error('OneSignal: addEmail: email is required');
return;
}

RNOneSignal.addEmail(email);
}
Expand All @@ -493,13 +518,21 @@ export namespace OneSignal {
*/
export function removeEmail(email: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(email)) {
console.error('OneSignal: removeEmail: email is required');
return;
}

RNOneSignal.removeEmail(email);
}

/** Add a new SMS subscription to the current user. */
export function addSms(smsNumber: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(smsNumber)) {
console.error('OneSignal: addSms: smsNumber is required');
return;
}

RNOneSignal.addSms(smsNumber);
}
Expand All @@ -510,6 +543,10 @@ export namespace OneSignal {
*/
export function removeSms(smsNumber: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(smsNumber)) {
console.error('OneSignal: removeSms: smsNumber is required');
return;
}

RNOneSignal.removeSms(smsNumber);
}
Expand All @@ -521,7 +558,7 @@ export namespace OneSignal {
export function addTag(key: string, value: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;

if (!key || value === undefined || value === null) {
if (!isNonEmptyString(key) || value == null) {
console.error('OneSignal: addTag: must include a key and a value');
return;
}
Expand All @@ -543,6 +580,10 @@ export namespace OneSignal {
/** Remove the data tag with the provided key from the current user. */
export function removeTag(key: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(key)) {
console.error('OneSignal: removeTag: key is required');
return;
}

RNOneSignal.removeTag(key);
}
Expand Down Expand Up @@ -787,9 +828,10 @@ export namespace OneSignal {
export function addTrigger(key: string, value: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;

// value can be assigned to `false` so we cannot just check `!value`
if (!key || value == null) {
// false is a valid trigger value, so reject only null/undefined for value.
if (!isNonEmptyString(key) || value == null) {
console.error('OneSignal: addTrigger: must include a key and a value');
return;
}

RNOneSignal.addTrigger(key, value);
Expand All @@ -808,6 +850,10 @@ export namespace OneSignal {
/** Remove the trigger with the provided key from the current user. */
export function removeTrigger(key: string) {
if (!isNativeModuleLoaded(RNOneSignal)) return;
if (!isNonEmptyString(key)) {
console.error('OneSignal: removeTrigger: key is required');
return;
}

RNOneSignal.removeTrigger(key);
}
Expand Down
Loading