Skip to content

Fixes header assignment #11

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

Closed
wants to merge 2 commits into from
Closed

Conversation

drdaz
Copy link
Member

@drdaz drdaz commented Oct 18, 2019

It looks like iOS notification headers aren't actually being set. This became most obvious recently with the required headers for iOS 13, as described in parse-community/parse-server#6106

This PR alters header generation so that the supported apns headers are set.

@dplewis
Copy link
Member

dplewis commented Oct 19, 2019

@drdaz Thanks for the PR. I left a few comments.

@dplewis
Copy link
Member

dplewis commented Oct 19, 2019

Closing as headers are sent properly and throughly tested.

Please check the push notification guide

@dplewis dplewis closed this Oct 19, 2019
@drdaz
Copy link
Member Author

drdaz commented Oct 19, 2019

I'm testing them, and they really don't work without these modifications. Have you personally verified this end-to-end?

@dplewis dplewis reopened this Oct 19, 2019
@drdaz
Copy link
Member Author

drdaz commented Oct 19, 2019

@dplewis if you want to see the issue, set priority to anything other than 10 when sending the notification. You'll see in the device console when the notification is received that the priority is always 10. Which breaks silent notifications in iOS 13.

EDIT: Or set any of the other apns header fields actually.

@dplewis
Copy link
Member

dplewis commented Oct 19, 2019

@drdaz Sorry for jumping to conclusions 😅.

I'm trying to determine if the issue is with this module or the parse-server-push-adapter that uses this module.

I don't think there is an issue with this module, but to be sure I'll open a PR to make testing easier.

@drdaz drdaz closed this Oct 21, 2019
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