Conversation
* `EngageWorker` and `EngagePublisher` for background data publishing using WorkManager * `EngageBroadcastReceiver` for handling SDK publish intents * `ClusterRequestFactory` and `ItemToEntityConverter` for data mapping * `Constants` for holding common values like attempt counts, publish types etc. * Update `AndroidManifest.xml` with required permissions and receiver configuration * Add `engage-core` dependency and associated constants
|
Here is the summary of changes. You are about to add 10 region tags.
This comment is generated by snippet-bot.
|
There was a problem hiding this comment.
Code Review
This pull request integrates the Google Engage SDK, adding the required dependencies, manifest configurations, and a WorkManager-based publishing system. It includes a CoroutineWorker for handling data synchronization and a BroadcastReceiver for responding to Engage intents. Review feedback highlights several critical improvements: fixing an unsafe cast and potential compilation error in the exception handling logic, preventing worker crashes by handling unknown error codes gracefully, optimizing broadcast receiver registration by combining intent filters, and ensuring internationalization support by using the system locale instead of a hardcoded string.
* Update `EngageWorker` to use safe casts when handling `AppEngageException`. * Add a null check and default failure result in `handlePublishException`. * Fix a mismatched snippet tag in `ClusterRequestFactory`.
| androidx-core-telecom = { group = "androidx.core", name = "core-telecom", version.ref = "androidx-core-telecom" } | ||
| androidx-media3-session = { group = "androidx.media3", name = "media3-session", version.ref = "media3Session" } | ||
| play-services-fitness = { group = "com.google.android.gms", name = "play-services-fitness", version.ref = "playServicesFitness" } | ||
| engage-core = { group = "com.google.android.engage", name = "engage-core", version.ref = "engageCore" } |
There was a problem hiding this comment.
Can we also add for engage-tv ? Was it added to patterns.md ? @huewu
There was a problem hiding this comment.
Yes. We don't touch dependency part in pattern.md.
All the necessary dependecies (contents under ## Dependency Specifications (libs.versions.toml) section) will be there . This is only for making code snippet compilable.
* Update `EngageWorker` to return false instead of throwing an exception on unknown error codes. * Consolidate `EngageBroadcastReceiver` registration using a single `IntentFilter` for all actions. * Update `AccountProfile` to use the system default locale instead of a hardcoded value.
EngageWorkerandEngagePublisherfor background data publishing using WorkManagerEngageBroadcastReceiverfor handling SDK publish intentsClusterRequestFactoryandItemToEntityConverterfor data mappingConstantsfor holding common values like attempt counts, publish types etc.AndroidManifest.xmlwith required permissions and receiver configurationengage-coredependency and associated constants