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
Extracted method that 'detects push method by reg id' #149
Extracted method that 'detects push method by reg id' #149
Conversation
src/push-notifications.js
Outdated
@@ -42,17 +72,17 @@ class PN { | |||
|
|||
// Classify each pushId for corresponding device | |||
regIds.forEach((regId) => { | |||
if (typeof regId === 'object') { | |||
const pushMetdod = this.getPushMethodByRegId(regId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo pushMethod
@@ -30,6 +38,28 @@ class PN { | |||
}); | |||
} | |||
|
|||
getPushMethodByRegId(regId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add test in test/push-notifications/basic.js
@@ -30,6 +38,28 @@ class PN { | |||
}); | |||
} | |||
|
|||
getPushMethodByRegId(regId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add documentation about newly exported function in README.md
- added test and documentation
Thanks for your review and sorry for my late response. I tried to address your points and hope it matches your expectations. I am glad to hear from you. |
Extracted method that detects device type / used push method out of send method. I will use it for logging / persisting the device type that was used by send() method.