-
Notifications
You must be signed in to change notification settings - Fork 369
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
Add option to default to HMS over FCM #2163
Conversation
This is migrating over the PR from the player model pr #1984
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.
The logic looks good, just some nits on the code, feel free to ping me again once you add tests.
@@ -37,7 +39,17 @@ internal class DeviceService(private val _applicationService: IApplicationServic | |||
override val deviceType: IDeviceService.DeviceType | |||
get() { | |||
if (supportsADM()) return IDeviceService.DeviceType.Fire | |||
if (supportsGooglePush()) return IDeviceService.DeviceType.Android | |||
|
|||
val supportsHMS: Boolean = supportsHMS |
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.
nit, we don't need the type, remove : Boolean
from here.
val supportsFCM = supportsGooglePush() | ||
|
||
if (supportsFCM && supportsHMS) { | ||
val context: Context = _applicationService.appContext |
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.
nit, we don't need the type, remove : Context
from here.
This creates a new DeviceServiceTests file and moves supportsHMS and supportsGooglePush to the IDeviceService interface
68f2c44
to
12b5c44
Compare
Description
One Line Summary
Read a manifest metadata boolean to allow apps to prefer HMS over FCM for devices that can receive both.
Details
Apps can add the following to their
AndroidManifest.xml
<meta-data android:name="com.onesignal.preferHMS" android:value="true"/>
This setting will allow devices with both HMS and FCM capabilities to use HMS rather than the default of FCM.
I have left in the testing commits for review, but will rebase prior to merge. The only changed file is
DeviceService.kt
Motivation
Allows apps to message users who move in and out of zones reachable by FCM, but can always receive HMS.
Scope
subscription type
Testing
Unit testing
n/a
Manual testing
Tested on Huawei device. I have left in 3 testing commits if reviewers would like to test.
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is