-
Notifications
You must be signed in to change notification settings - Fork 15
feat: Location enrichment #570
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
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
ca41f6b
feat: add location tracking module support
Shahroz16 060ecad
chore: make location module an optional dependency
Shahroz16 9d6053d
refactor: use build flags instead of canImport and reflection for opt…
Shahroz16 9361227
fix: resolve simplify review findings for location module
Shahroz16 9bc1de6
style: fix pre-existing prettier formatting errors
Shahroz16 d3c40d3
fix: read gradle property value instead of just checking existence
Shahroz16 1df0a4b
fix: always register location module info to prevent import-time crash
Shahroz16 3f464a3
android version bump
Shahroz16 c93c4b6
fix: use TurboModuleRegistry.get for optional location module
Shahroz16 28f561b
fix: add R8 consumer rules for optional location compileOnly dependency
Shahroz16 c2c52a2
Fix wrapper layer for iOS
mahmoud-elmorabea b6ff4b2
Merge branch 'feat/location-module' of github.com:customerio/customer…
mahmoud-elmorabea b434f1d
Update public API baseline
mahmoud-elmorabea b7404d6
fix: log clear warning when location module is not enabled
Shahroz16 04b0570
Merge branch 'feat/location-module' of github.com:customerio/customer…
Shahroz16 d0531fd
style: fix prettier formatting in customerio-inapp.ts
Shahroz16 ca2bed7
Review comments
mahmoud-elmorabea 3bc2d88
Bump ios sdk version
mahmoud-elmorabea 2256101
chore: Build test screen for location module (#571)
mahmoud-elmorabea File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # Location module is an optional compileOnly dependency. | ||
| # When not enabled, R8 must not fail on missing location classes. | ||
| -dontwarn io.customer.location.** |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
66 changes: 66 additions & 0 deletions
66
android/src/main/java/io/customer/reactnative/sdk/location/NativeLocationModule.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| package io.customer.reactnative.sdk.location | ||
|
|
||
| import com.facebook.react.bridge.ReactApplicationContext | ||
| import com.facebook.react.module.annotations.ReactModule | ||
| import io.customer.location.LocationModuleConfig | ||
| import io.customer.location.LocationTrackingMode | ||
| import io.customer.location.ModuleLocation | ||
| import io.customer.reactnative.sdk.NativeCustomerIOLocationSpec | ||
| import io.customer.reactnative.sdk.extension.getTypedValue | ||
| import io.customer.sdk.CustomerIOBuilder | ||
| import io.customer.sdk.core.di.SDKComponent | ||
| import io.customer.sdk.core.util.Logger | ||
|
|
||
| /** | ||
| * React Native module implementation for Customer.io Location Native SDK | ||
| * using TurboModules with new architecture. | ||
| */ | ||
| @ReactModule(name = NativeLocationModule.NAME) | ||
| class NativeLocationModule( | ||
| private val reactContext: ReactApplicationContext, | ||
| ) : NativeCustomerIOLocationSpec(reactContext) { | ||
| private val logger: Logger | ||
| get() = SDKComponent.logger | ||
|
|
||
| private fun getLocationServices() = runCatching { | ||
| ModuleLocation.instance().locationServices | ||
| }.onFailure { | ||
| logger.error("Location module is not initialized. Ensure location config is provided during SDK initialization.") | ||
| }.getOrNull() | ||
|
|
||
| override fun setLastKnownLocation(latitude: Double, longitude: Double) { | ||
| getLocationServices()?.setLastKnownLocation(latitude, longitude) | ||
| } | ||
|
|
||
| override fun requestLocationUpdate() { | ||
| getLocationServices()?.requestLocationUpdate() | ||
| } | ||
|
|
||
| companion object { | ||
| const val NAME = "NativeCustomerIOLocation" | ||
|
|
||
| /** | ||
| * Adds location module to native Android SDK based on the configuration provided by | ||
| * customer app. | ||
| * | ||
| * @param builder instance of CustomerIOBuilder to add location module. | ||
| * @param config configuration provided by customer app for location module. | ||
| */ | ||
| internal fun addNativeModuleFromConfig( | ||
| builder: CustomerIOBuilder, | ||
| config: Map<String, Any> | ||
| ) { | ||
| val trackingModeValue = config.getTypedValue<String>("trackingMode") | ||
| val trackingMode = trackingModeValue?.let { value -> | ||
| runCatching { enumValueOf<LocationTrackingMode>(value) }.getOrNull() | ||
| } ?: LocationTrackingMode.MANUAL | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| val module = ModuleLocation( | ||
| LocationModuleConfig.Builder() | ||
| .setLocationTrackingMode(trackingMode) | ||
| .build() | ||
| ) | ||
| builder.addCustomerIOModule(module) | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,8 +34,18 @@ export type CioConfig = { | |
| pushClickBehavior?: PushClickBehaviorAndroid; | ||
| }; | ||
| }; | ||
| location?: { | ||
| trackingMode?: CioLocationTrackingMode; | ||
| }; | ||
| }; | ||
|
|
||
| // @public | ||
| export enum CioLocationTrackingMode { | ||
| Manual = "MANUAL", | ||
| Off = "OFF", | ||
| OnAppStart = "ON_APP_START" | ||
| } | ||
|
|
||
| // @public | ||
| export enum CioLogLevel { | ||
| Debug = "debug", | ||
|
|
@@ -74,20 +84,22 @@ export type CustomAttributes = Record<string, any>; | |
| export class CustomerIO { | ||
| static readonly clearIdentify: () => Promise<any>; | ||
| static readonly deleteDeviceToken: () => Promise<void>; | ||
| static readonly identify: ({ userId, traits, }?: IdentifyParams) => Promise<any>; | ||
| static readonly identify: (input?: IdentifyParams) => Promise<any>; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did we really change this? 🤔 |
||
| // (undocumented) | ||
| static readonly inAppMessaging: CustomerIOInAppMessaging; | ||
| static readonly initialize: (config: CioConfig) => Promise<void>; | ||
| // @deprecated | ||
| static readonly isInitialized: () => boolean; | ||
| // (undocumented) | ||
| static readonly location: CustomerIOLocation; | ||
| // (undocumented) | ||
| static readonly pushMessaging: CustomerIOPushMessaging; | ||
| static readonly registerDeviceToken: (token: string) => Promise<void>; | ||
| static readonly screen: (title: string, properties?: Record<string, any>) => Promise<any>; | ||
| static readonly setDeviceAttributes: (attributes: Record<string, any>) => Promise<any>; | ||
| static readonly setProfileAttributes: (attributes: Record<string, any>) => Promise<any>; | ||
| static readonly track: (name: string, properties?: Record<string, any>) => Promise<any>; | ||
| static readonly trackMetric: ({ deliveryID, deviceToken, event, }: { | ||
| static readonly trackMetric: (input: { | ||
| deliveryID: string; | ||
| deviceToken: string; | ||
| event: MetricEvent; | ||
|
|
@@ -104,6 +116,14 @@ export class CustomerIOInAppMessaging implements NativeInAppSpec { | |
| registerEventsListener(listener: (event: InAppMessageEvent) => void): EventSubscription; | ||
| } | ||
|
|
||
| // Warning: (ae-forgotten-export) The symbol "NativeLocationSpec" needs to be exported by the entry point index.d.ts | ||
| // | ||
| // @public (undocumented) | ||
| export class CustomerIOLocation implements NativeLocationSpec { | ||
| requestLocationUpdate(): void; | ||
| setLastKnownLocation(latitude: number, longitude: number): void; | ||
| } | ||
|
|
||
| // Warning: (ae-forgotten-export) The symbol "NativePushSpec" needs to be exported by the entry point index.d.ts | ||
| // | ||
| // @public (undocumented) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is a nice addition but it probably won't work properly since the code still references location classes. Ideally to solve this, we would need to completely remove location class references when the build flag is disabled to fully eliminate these challenges. So I think this is a bigger problem to solve. Unless we really need this for location right now, we should probably address it in a separate PR.
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.
BuildConfig.CIO_LOCATION_ENABLED = false, the compiler strips the entire if block from bytecode. I decompiled the AAR and confirmed:NativeCustomerIOModulehas zero references to anyio.customer.location.*class in its constant pool.The getModule() path compiles down to just aconst_null (return null).
merged manifest.
ACCESS_COARSE_LOCATIONis completely absent.compileOnlymeans the location AAR's manifest is never fed to the manifest merger.NoClassDefFoundError or ClassNotFoundException.
The compileOnly + BuildConfig approach serves our exact use case: customers who don't enable
location don't get the permission in their app.
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.
Thanks for the detailed response. Can you check why the sample app build is failing during
minifyReleaseWithR8then?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.
Seems like when
R8runs on a release build, it scans all classes and fails because thecompileOnlylocation classes aren't on theruntimeclasspath. Adding rule would solve that.