Fix requestPermission to reject the promise when permission is denied
Description
One Line Summary
Fix the requestPermission method when the permission is denied, and resolve ONLY if the permission is granted. Also fixing NullPointerException when attempting to access the data and/or result.getThrowable().getMessage().
Details
Motivation
I faced this issue while implementing the notification permissions, when I deny the permission request the promise is never rejected.
Testing
Manual testing
Tested on Samsung A33
- Tested the promise response when the permission is accepted and denied.
- Tested changes to the permissions in the app settings and called requestPermission.
- Tested changing the permissions within the app by toggling requestPermission and optOut.
Affected code checklist
- [x] Notifications
- [ ] Display
- [ ] Open
- [ ] Push Processing
- [ ] Confirm Deliveries
- [ ] Outcomes
- [ ] Sessions
- [ ] In-App Messaging
- [ ] REST API requests
- [ ] Public API changes
Checklist
Overview
- [x] I have filled out all REQUIRED sections above
- [x] PR does one thing
- [x] Any Public API changes are explained in the PR details and conform to existing APIs
Testing
- [x] I have included test coverage for these changes, or explained why they are not needed
- [x] All automated tests pass, or I explained why that is not possible
- [x] I have personally tested this on my device, or explained why that is not possible
Final pass
- [x] Code is as readable as possible.
- [x] I have reviewed this PR myself, ensuring it meets each checklist item
Hi @Basil-Code,
Thanks for submitting this PR.
Fix the requestPermission method when the permission is denied, and resolve ONLY if the permission is granted.
I think this description in the PR changed after your second commit? It looks like these changes are primarily making null checks on result.getData() and result.getThrowable().
Did you encounter these as issues?
Hi @Basil-Code,
Thanks for submitting this PR.
Fix the requestPermission method when the permission is denied, and resolve ONLY if the permission is granted.
I think this description in the PR changed after your second commit? It looks like these changes are primarily making null checks on
result.getData()andresult.getThrowable().Did you encounter these as issues?
Hi @nan-li, I'm facing NullPointerException crashes on Sentry related to OneSignal, but I'm not sure if it's related to this method yet. But do you think the null checks can cause other issues?
Hi @Basil-Code, sorry for the delay.
The null checks you added are good for safety and don't have any side effects, so they look good to me.
Can you tell me more about the NPE crashes you experienced?
I'm facing NullPointerException crashes on Sentry related to OneSignal
Feel free to open an issue with your crashes.
Hi @Basil-Code, sorry for the delay.
The null checks you added are good for safety and don't have any side effects, so they look good to me.
Can you tell me more about the NPE crashes you experienced?
I'm facing NullPointerException crashes on Sentry related to OneSignal
Feel free to open an issue with your crashes.
Sure, I'll open an issue if the crash still happens.