-
Notifications
You must be signed in to change notification settings - Fork 374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add tag type fix #1696
Add tag type fix #1696
Conversation
Hi @dombartenope thank you so much for this PR! My observations of this bug and how behavior varied by platform:
Since all the SDKs require a string value parameter for tagging methods, I've reverted the Let me know if you have any questions or concerns! |
2499250
to
243d2d9
Compare
- ensure consistency of string conversion methods - check for null or undefined values instead of falsy checks
Description
One Line Summary
This PR fixes an issue where addTag will set an empty string, deleting existing keys, if a number is set as the value. Previously this value was of type 'string', and it will now booleans and numbers by casting the value to a string before sending the request to the OneSignal sever.
Details
Motivation
Reports of crashes due to sending unix timestamps as integers, when added as a value to the addTag method.
On the current major release / latest update, if passing in a unix timestamp as the value argument to the addTags method, you will see an empty string is sent as the value when making a change. This would delete a data tag per other SDK logic that determines an empty value to be the same as a "remove" request.
Scope
OPTIONAL - Other
The changes to the RTCOneSignalEventEmitter, are to determine whether the type is an NSNumber or not. The changes made to index.ts were for intellisense (allowing the boolean and number types to be used without an error thrown by the IDE) and to set the value to a string in case the value was a boolean (which would default to 1 or 0 otherwise).
Testing
Manual testing
I have tested this on both Android and iOS devices:
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is