Add apns-priority support

[ilijapuaca]

As far as I can see there is no way to specify apns-priority, and after inspecting code in node-apn, it uses a default value of 10 unless otherwise specified (https://github.com/node-apn/node-apn/blob/master/lib/notification/index.js#L13).

My silent notifications are not being delivered at all, and I currently suspect that priority might be the cause. Quote from this article:

In a 2019 WWDC session, Apple said that a value of 5 is now required for all “content-available” notifications. These are notifications that may not contain user-visible data, but are used to invoke the app in the background to download updated content, such as recently received messages. Omitting this may cause the app to not launch.

That being said, it would be great to be able to specify the priority through UI, and simply have it set as the notification property when it's being passed to node-apn.

[Ariandr]

Hi @ilijapuaca Good point. But I believe it can be done without any additional UI. We can simply set it to 5 when it's a silent/background notification, and to 10 for alert notifications.

[ilijapuaca]

It would likely address the scenario I mentioned above, but I'm sure there are cases where the extra flexibility would be appreciated as the value can be set independently from the notification type, and can have values other than 5 and 10.

[Ariandr]

@ilijapuaca I'm not sure other values provide different/unique behavior. Anyway, that's what node-apn documentation says, but I haven't looked into Apple docs.

https://github.com/node-apn/node-apn/blob/master/doc/notification.markdown

For now, I will put priority 5 to background notifications to solve the issue.

[ilijapuaca]

Sounds good. I'll keep an eye out for when a build is out with this included and will update whether it made a difference in my case specifically.

[Ariandr]

@ilijapuaca I pushed a change in this commit 79f0c20c6 Should be available soon in v.1.7.1

Ping @onmyway133 Can you create a notarized app with this change for mac (new release 1.7.1), please? Also, now I cannot build artifacts on my machine because of afterSignHook.js. Do I need to install any additional dev dependencies? We need to fix it, so users can manually build it on their machines in case of any issues. I tried to build on Windows.

stackTrace=
ReferenceError: regeneratorRuntime is not defined at C:\Users\***\PushNotifications\afterSignHook.js:10:48

[Ariandr]

Hi @onmyway133 Have you had a chance to look into it?

[onmyway133]

@Ariandr Hi, I still have problem with notarization, it says OK but people have problem opening it. About regeneratorRuntime, it is just error in that afterSignHook and does not seem to affect the archiving.

[Ariandr]

@onmyway133 No, it does affect the building. I don't get any distributable files generated now.

[Ariandr]

@onmyway133 For example, on Windows I didn't get .msi file, because the error was thrown immediately after the building process started. On macOS the same, I didn't get neither .zip not .dmg, same error thrown (ReferenceError: regeneratorRuntime is not defined)

Maybe you have some npm dependency globally installed on your machine?

[onmyway133]

@Ariandr regeneratorRuntime is not defined seems to be related to Babel, whose version 6 we are using now. People suggest to use Babel 7 🙄

[Ariandr]

@onmyway133 If I'm not mistaken, Babel isn't added as a Dev dependency?

[Ariandr]

Hi @onmyway133 Any updates? I think the ability to be able to build the app is more important than notarization. As I understand you can build the app, because you installed some npm dependencies, but didn't add them to dev dependencies in package.json. As a result, nobody can build the app including me, because we don't know which additional packages are required.

[onmyway133]

@Ariandr I think we can remove afterSignHook for now, and you can build like before?

[Ariandr]

@onmyway133 I have an idea of how we can manage it in a better way. We will have a second branch, which will have afterSignHook, but apart from that will be the copy of master (we will merge any updates there). In this way if users download the source code, they will get master without any notarization logic and I will be able to build the app too.

[onmyway133]

@Ariandr ja, good idea, let's do that. Can you move the stuff you don't want from master to another branch, maybe call it sign

[Ariandr]

@onmyway133 I've already done that. The new branch is notarization_branch.

[Ariandr]

@onmyway133 I will create a short instruction in Readme of how to install our not-notarized app on Catalina

[onmyway133]

@Ariandr awesome 👍

[Ariandr]

Hi @ilijapuaca Please, check out this release https://github.com/onmyway133/PushNotifications/releases/tag/1.7.1

[ilijapuaca]

Got it. I was able to download the .dmg and run it using the instructions you provided. I didn't test whether the priority is being set appropriately now (I don't see why it wouldn't be, plus it's a bit hard to test as I could only sniff the network traffic in order to confirm?), but I'll get around to testing whether the notifications are coming through properly within the next day or two.

One note -- the app name has changed from PushNotifications to Push Notifications. Not sure whether that was done intentionally, so thought I'd mention it.

Thanks @Ariandr!

[Ariandr]

@ilijapuaca Great, thanks for the fast response. When you are able to test it, please let us know if the fix is working)

[ilijapuaca]

@Ariandr initial results seem pretty good, I'm getting the notification through while the app is in the foreground (yet to test background), which wasn't the case before.

I think that solves the original issue, thanks!

[Ariandr]

Hi @ilijapuaca Great. iOS 13 has new behavior regarding receiving the notifications, that might be the case.