Skip to content

Adds support for badging on iOS#740

Merged
flovilmart merged 1 commit intomasterfrom
flovilmart.badgeIncrement
Mar 3, 2016
Merged

Adds support for badging on iOS#740
flovilmart merged 1 commit intomasterfrom
flovilmart.badgeIncrement

Conversation

@flovilmart
Copy link
Contributor

Fix for #723

  • Stores the badge value on matching _Installation
  • Adds support for "Increment"
  • Throws when badge value is not right (not int or "Increment")

@anatolydwnld
Copy link

👍 FWIW

@flovilmart flovilmart force-pushed the flovilmart.badgeIncrement branch from 7a19ee5 to aea9c87 Compare March 2, 2016 20:36
@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@flovilmart flovilmart force-pushed the flovilmart.badgeIncrement branch from aea9c87 to ee3b37d Compare March 3, 2016 01:51
@facebook-github-bot
Copy link

@flovilmart updated the pull request.

return badgeUpdate.then(() => {
return rest.find(config, auth, '_Installation', where)
}).then((response) => {
if (body.badge && body.badge == "Increment") {
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks fine, what do you think if we would inplace mutate every installation instead before sending?
Which looks like you already sort of do with collection iteration aka results.reduce, but this would probably reduce complexity of reading this...

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, would actually remove the requirement to send multiple times, since the complexity of sending with a badge and without is the same.

@nlutsenko
Copy link
Contributor

Optimization suggestion, feel free to ignore though.

flovilmart added a commit that referenced this pull request Mar 3, 2016
@flovilmart flovilmart merged commit d8d3743 into master Mar 3, 2016
@flovilmart flovilmart deleted the flovilmart.badgeIncrement branch March 3, 2016 02:17
rest.find(config, auth, '_Installation', where).then(function(response) {
let badgeUpdate = Promise.resolve();

if (body.badge) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to reuse this code for Amazon SNS?

Copy link
Contributor

Choose a reason for hiding this comment

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

You actually don't need to use this piece, since this happens before PushAdapter, which is the extension point where SNS would be plugged in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even better...thanks!

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.

5 participants