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

fix: apns production option #3434

Merged
merged 2 commits into from
May 24, 2023

Conversation

michaldziuba03
Copy link
Contributor

What change does this PR introduce?

Add default value for apns production option. Undefined value leads to unexpected behaviour.

Why was this change needed?

Closes #3416 - everything is well described in this issue

https://github.com/node-apn/node-apn#connecting

By default, the provider will connect to the sandbox unless the environment variable NODE_ENV=production is set

Basically when user creates APNs integration, by default it creates credentials where secure is optional field so it's undefined when value not specified (secure field is used for production option for node-apn). So when secure is undefined, then node-apn package uses NODE_ENV to determine if production is true or false. It may lead to scenario where user has production set to undefined (in GUI it's switch which indicates that production should be disabled) but Novu servers are probably running with NODE_ENV=production and then we have bug or at least possible unexpected behaviour.

Important note - this PR is very closely related to PR#3430. 3430 solves other problem where secure used for production field is persisted as string, so if someone provides "false" then it's interpreted by node-apn as true value what makes whole provider just broken actually.

Other information (Screenshots)

Copy link
Contributor

@p-fernandez p-fernandez left a comment

Choose a reason for hiding this comment

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

🌟

@p-fernandez p-fernandez added this pull request to the merge queue May 24, 2023
Merged via the queue into novuhq:next with commit b097053 May 24, 2023
25 of 27 checks passed
@michaldziuba03 michaldziuba03 deleted the fix-apns-production-option branch May 24, 2023 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NV-2329] 🐛 Bug Report: Issues with integration secure option
2 participants