chore: event bus integration with push #358
Conversation
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/cdp #358 +/- ##
==================================================
+ Coverage 54.45% 90.90% +36.44%
==================================================
Files 109 1 -108
Lines 2534 11 -2523
Branches 355 2 -353
==================================================
- Hits 1380 10 -1370
+ Misses 1032 1 -1031
+ Partials 122 0 -122 ☔ View full report in Codecov by Sentry. |
|
Build available to test |
📏 SDK Binary Size Comparison ReportNo changes detected in SDK binary size ✅ |
| // https://github.com/mockk/mockk/issues/681 | ||
| jvmArgs("--add-opens", "java.base/java.time=ALL-UNNAMED", | ||
| "--add-opens", "java.base/java.util=ALL-UNNAMED" | ||
| ) |
There was a problem hiding this comment.
For future, once we have all modules using JUnit5, we can pull these changes up to common gradle file.
There was a problem hiding this comment.
Yup, that's the plan 👍
| @@ -91,6 +93,10 @@ open class CustomerIOFirebaseMessagingService : FirebaseMessagingService() { | |||
| } | |||
|
|
|||
| private fun handleNewToken(context: Context, token: String) { | |||
| eventBus.publish( | |||
There was a problem hiding this comment.
I would suggest to either create class variable or use SDKComponent.eventBus for improved readability.
There was a problem hiding this comment.
hmm i thought import or event clicking on it would be sufficient, I can add the extended name just felt like a overhead.
| event = MetricEvent.delivered | ||
| eventBus.publish( | ||
| Event.TrackPushMetricEvent( | ||
| event = MetricEvent.delivered.name, |
There was a problem hiding this comment.
Might be outside the scope of this PR, but curious if there is a reason for using String instead of MetricEvent enum? Is this for wrappers flexibility?
There was a problem hiding this comment.
MetricEvent has a Moshi dependency to it, which we will be getting rid of. So pulling it in core didn't make sense, on top of that wrappers would also not be using it.
End goal would be to create another enum and replace the string and remove the tracking one.
messagingpush/src/main/java/io/customer/messagingpush/util/PushTrackingUtil.kt
Outdated
Show resolved
Hide resolved
closes: https://linear.app/customerio/issue/MBL-346/event-bus-integration-with-feature-modules