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

Sender#sendUserNotification #107

Closed
hypesystem opened this issue Feb 27, 2015 · 4 comments · Fixed by #141
Closed

Sender#sendUserNotification #107

hypesystem opened this issue Feb 27, 2015 · 4 comments · Fixed by #141
Milestone

Comments

@hypesystem
Copy link
Collaborator

The only difference between a regular notification and a user notification is that the user notification uses the notification key to find its recipients (source):

// Regular notification data
{
  data: {
    some: "data"
  },
  registration_ids: [ "some-registration-id", "other-registration-id" ]
}

// User notification data
{
  data: {
    some: "data"
  },
  to: "some-notification-key"
}

It would be pretty straight forward to implement a Sender#sendUserNotification method. At the same time, this would be cause for generalizing a bunch of the code used in Sender#send.

We should probably add Sender#sendUserNotificationNoRetry as well.

The job of creating notification keys can be left out for now. It seems to me that it is immediately most relevant to handle on the client side (source), but I am open for discussion on this.

@hypesystem
Copy link
Collaborator Author

Okay, I did a bit of digging. According to an answer on StackOverflow and accompanying blog post, the correct field to contain the notification key is simply to. I have updated the original issue to reflect this.

The URL is also, according to the blog post, the same as used for regular messages. This makes implementation trivial.

@ToothlessGear
Copy link
Owner

Yes, seems like the sending part is easy.
Creating those keys also dont look that hard on the server-side, from catching a short glimpse at the docs.

@hypesystem
Copy link
Collaborator Author

I kind of want v0.10 out before we start considering user notifications (v0.11). But if it doesn't seem like too big a task, we could probably fit #108 (creating notification keys) in v0.11, too.

@ToothlessGear
Copy link
Owner

Sure, go ahead!

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 a pull request may close this issue.

2 participants