refactor: move push click handling to processor #268
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 Report
@@ Coverage Diff @@
## rehan/push-click-behavior #268 +/- ##
===============================================================
+ Coverage 50.84% 54.04% +3.19%
- Complexity 249 278 +29
===============================================================
Files 108 109 +1
Lines 2779 2781 +2
Branches 361 349 -12
===============================================================
+ Hits 1413 1503 +90
+ Misses 1249 1156 -93
- Partials 117 122 +5
|
|
Build available to test |
| } else { | ||
| processNotificationIntent(payload = payload) | ||
| sdkInstance.diGraph.pushMessageProcessor.processNotificationClick( |
There was a problem hiding this comment.
think this should be covered by test?
There was a problem hiding this comment.
We can. But I avoided because I don't think this adds a lot of value. It mostly relies on Android OS lifecycle and couple of other methods that are already being tested. Also, it requires setting up Activity lifecycle for testing which I plan to improve later to gain more confidence in testing. However, if you think we should still cover this, I can write a test for it.
There was a problem hiding this comment.
I think it still alots value because the whole logic revolves around what flags to expect and what behaviour it would display.
There was a problem hiding this comment.
I have pushed more tests, hope it helps.
PS: Github is having issues in PRs, so it might take some time to show new changes.
...ngpush/src/main/java/io/customer/messagingpush/activity/NotificationClickReceiverActivity.kt
Outdated
Show resolved
Hide resolved
messagingpush/src/main/java/io/customer/messagingpush/processor/PushMessageProcessorImpl.kt
Outdated
Show resolved
Hide resolved
...gpush/src/sharedTest/java/io/customer/messagingpush/CustomerIOPushNotificationHandlerTest.kt
Outdated
Show resolved
Hide resolved
|
@Shahroz16 I can see many warnings on |
|
@levibostian Most of the changes are actually copied from |
| } else { | ||
| processNotificationIntent(payload = payload) | ||
| sdkInstance.diGraph.pushMessageProcessor.processNotificationClick( |
There was a problem hiding this comment.
I think it still alots value because the whole logic revolves around what flags to expect and what behaviour it would display.
helps: https://github.com/customerio/issues/issues/10830
Changes
NotificationClickReceiverActivitytoPushMessageProcessorfor reusability and better testingCustomerIOPushNotificationHandler,ModuleMessagingConfigandPushMessageProcessor