Skip to content
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

Set External User ID #679

Merged
merged 4 commits into from
Dec 6, 2018
Merged

Set External User ID #679

merged 4 commits into from
Dec 6, 2018

Conversation

Nightsd01
Copy link
Contributor

@Nightsd01 Nightsd01 commented Dec 5, 2018

• Adds SDK support to allow developers to map their own system's user identifiers to OneSignal users, this way they don't have to save the user's OneSignal user ID
• This PR adds two new methods:

setExternalUserId(String): Sets the external ID for the current user. OneSignal maintains separate account records internally for both Email and Push accounts, so if a device has a Push subscription and has also set an Email subscription, this method will generate two API requests.

removeExternalUserId: This method is actually just a wrapper on top of setExternalUserId except that it sends an empty string as the external user ID. This is because our API requires an empty string instead of null to remove the external ID

• Adds tests to verify that the external ID methods generate correctly formatted API requests for both Push and Email players


This change is Reviewable

• Adds public methods setExternalUserId() and removeExternalUserId() that let developers map their own userID's in our system
• TODO: Add tests to ensure the methods generate correct API requests and that it uses the on_session request if it hasn't already occurred
• Adds tests to make sure that setExternalUserId requests are correctly formatted and will wait for registration request if needed
• Fixes the setExternalUserId() request to use a PUT HTTP request if registration has already occurred
• It turns out the Android SDK already has a state synchronizer that can persist previously set player values and will not send duplicate requests if the value is unchanged.
• This commit switches to utilizing then state synchronizer
• Adds a new test to make sure that the external ID is not sent if setExternalUserId() is called but the ID is the same as the previous session/s
• Adds a test to verify that the externalUserId is set on both Email and Push player accounts
Copy link
Member

@jkasten2 jkasten2 left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@Nightsd01 Nightsd01 merged commit 3af1cba into master Dec 6, 2018
@jkasten2 jkasten2 deleted the set_external_id branch June 13, 2019 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants