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

Target external user IDs #86

Merged
merged 3 commits into from
Jan 3, 2019
Merged

Target external user IDs #86

merged 3 commits into from
Jan 3, 2019

Conversation

JeffreyHyer
Copy link
Contributor

A small change that allows you to send notifications to users by specifying an external user ID.

See the OneSignal docs under 'include_external_user_ids' for all the details.

@LKaemmerling
Copy link
Collaborator

Can you add a test for this please?

@JeffreyHyer
Copy link
Contributor Author

@LKaemmerling I've added the test, thanks for the feedback.

@LKaemmerling
Copy link
Collaborator

LGTM @Lloople what do you think?

@Lloople
Copy link
Collaborator

Lloople commented Jan 2, 2019

Looks good. Is there a way to test the content of the $payload? It’s the only thing I’m missing, being sure the external_ids are there.

@LKaemmerling
Copy link
Collaborator

@Lloople
Copy link
Collaborator

Lloople commented Jan 2, 2019

You’re right! I didn’t saw it from the phone 😅

@LKaemmerling LKaemmerling merged commit 065321b into laravel-notification-channels:master Jan 3, 2019
@JeffreyHyer JeffreyHyer deleted the target-external-ids branch February 5, 2019 19:14
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.

3 participants