Skip to content

Push notification passphrase#52

Merged
frozon merged 3 commits into
frozon:masterfrom
avinoth:master
Sep 29, 2015
Merged

Push notification passphrase#52
frozon merged 3 commits into
frozon:masterfrom
avinoth:master

Conversation

@avinoth

@avinoth avinoth commented Sep 16, 2015

Copy link
Copy Markdown
Contributor

Support Push notification passphrase as config attribute.
Merge last-modified header feature from #26

Add an optional config field to set the passphrase for
push notification certificate. Grocer makes it optional.

Add spec for it, update readme with instructions. Increased patch
version.
@frozon

frozon commented Sep 16, 2015

Copy link
Copy Markdown
Owner

LGTM if you could also review @lgleasain

@rmosolgo

Copy link
Copy Markdown

👍 I was getting an error from apple for not including this header, but updating to this branch worked for me.

I also ran into the passphrase issue, but I worked around it by using Grocer's notification API directly:

    pusher = Grocer.pusher(
      certificate: Passbook.notification_cert,
      passphrase:  Passbook.p12_password,
      gateway: Rails.env.test? ? "localhost" : "gateway.push.apple.com",
    )

    PassbookDevice.find_each do |device|
      notification = Grocer::PassbookNotification.new(device_token: device.push_token)
      pusher.push(notification)
    end

Thanks for pushing this fix, I hope it can be merged soon!

@avinoth

avinoth commented Sep 20, 2015

Copy link
Copy Markdown
Contributor Author

@frozon Changed the way empty passphrase is handled, brought down the version bump and squashed the commits. 👍

frozon added a commit that referenced this pull request Sep 29, 2015
Push notification passphrase
@frozon frozon merged commit f27f9cc into frozon:master Sep 29, 2015
@frozon

frozon commented Sep 29, 2015

Copy link
Copy Markdown
Owner

Thanks for your work on it

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