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

Upgrade APN Library & Support Time-Sensitive Notifications #8047

Merged
merged 5 commits into from
Aug 5, 2023

Conversation

gestrich
Copy link
Contributor

@gestrich gestrich commented Jun 30, 2023

  • Update a deprecated APN package, used by Loop, to a more actively maintained fork from the parse community.
  • The library upgrade fixes several vulnerabilities: 3 low, 2 moderate, and 3 high
  • Remove 2 other libraries that were added when the old APN package was added: base64url, pem.
  • Activate "time sensitive" push notifications, a feature of iOS 15 exposed in the new APN package. This allows time sensitive notifications to be delivered more quickly. This should help caregivers that have flaky push notification problems.

Example of Loop push notifications being delivered as "time sensitive":
Screenshot 2023-07-01 at 6 54 11 AM

@gestrich
Copy link
Contributor Author

gestrich commented Jun 30, 2023

I don't normally work with NPM (I'm an iOS dev) so would appreciate any pointers on this PR and practices for working with node packages.

package.json Outdated Show resolved Hide resolved
@@ -71,9 +71,9 @@
"dependencies": {
"@babel/core": "^7.18.10",
"@babel/preset-env": "^7.18.10",
Copy link
Contributor Author

@gestrich gestrich Jun 30, 2023

Choose a reason for hiding this comment

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

I added this package using the command:

npm install @parse/node-apn

I originally tried this from node version 18* but it added a bunch of changes to the file unrelated to my update (adding the prefix "node_modules" to each depedency). So I downgraded my node to an earlier version, v14.21.3, per some advice from SO and then ran the command again.

@gestrich gestrich mentioned this pull request Jun 30, 2023
@gestrich gestrich force-pushed the feature/2023/06/bg/node-apn-upgrade branch from 92386e5 to d500fd8 Compare July 1, 2023 10:38
@gestrich gestrich changed the base branch from dev to master July 1, 2023 10:38
@gestrich gestrich changed the base branch from master to dev July 1, 2023 10:39
"acorn": "^8.0.5",
"acorn-jsx": "^5.3.1",
"apn": "^2.2.0",
"async": "^0.9.2",
"babel-loader": "^8.2.5",
Copy link
Contributor Author

@gestrich gestrich Jul 1, 2023

Choose a reason for hiding this comment

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

Looking at the original PR when the prior APN package was added, I see the "pem" and "base64url" libraries were added in the same PR. Those don't seem needed from what I can tell since upgrading the library. Below is the output of depcheck right after I removed apn, which shows those as unused. So I've removed from now but let me know if there is something I'm missing.

https://github.com/nightscout/cgm-remote-monitor/pull/5043/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

depcheck
Unused dependencies
* acorn
* acorn-jsx
* base64url
* buffer
* crypto-browserify
* moment-locales-webpack-plugin
* pem
* process
* stream-browserify
* swagger-ui-dist
* webpack-cli
Unused devDependencies
* @types/tough-cookie
....

@@ -2045,11 +2039,6 @@
"supports-color": "^5.3.0"
}
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Below here in the lock file, these updates seemed to implicitly happen from me removing the 3 libraries:

  • apn
  • base64url
  • pem

@gestrich gestrich changed the title Upgrade APN Library Upgrade APN Library & Support Time-Sensitive Notifications Jul 1, 2023
@gestrich gestrich marked this pull request as ready for review July 1, 2023 10:52
Copy link
Contributor

@ps2 ps2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@bewest
Copy link
Member

bewest commented Aug 3, 2023

Looks good, I will spend some time testing and integrating during my cycles, along with Nightscout Connect issues. Hopefully sometime this week.

@bewest bewest merged commit 613a699 into nightscout:dev Aug 5, 2023
8 checks passed
@gestrich
Copy link
Contributor Author

gestrich commented Aug 5, 2023

@bewest 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.

3 participants