fix: 3rd party SDK compatibility when SDK forwards push events back to us #736
Conversation
This file contains 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
…round Part of: https://linear.app/customerio/issue/MBL-364/[bug]-push-notifications-not-showing-while-app-in-foreground-on-ios The CIO SDK forwards push events to 3rd party SDKs. Some 3rd party SDKs send the push event back to the CIO SDK, potentially causing infinite loops. The SDK is designed to handle this. Previously, the responsibility of calling the completionHandler was on the 3rd party SDKs. However, some 3rd party SDKs delegate this responsibility back to us (See: https://linear.app/customerio/issue/MBL-364/[bug]-push-notifications-not-showing-while-app-in-foreground-on-ios#comment-3ae0db4e). This commit addresses that scenario. Testing: * Modified integration tests to cover this scenario. * Added tests for both `onPushAction` and `shouldDisplayPushAppInForeground` delegate functions. * Ran regression QA tests on iOS, RN, and Flutter. Notes for reviewers: - Limited comments in `iOSPushEventListener` intentionally. Test functions and comments in AutomaticPushClickIntegrationTest explain the use cases and reproducing steps.
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.
|
levibostian
changed the title
Shahroz16
reviewed
Jun 11, 2024
Shahroz16
approved these changes
Jun 11, 2024
There was a problem hiding this comment.
Same as other PR, as discussed in comms we are good on this, only minor suggestion/question would be this.
Tests are currently failing but I think that's because device wasn't launched for some reason, lets please make sure tests are passing on CI before we merge it.
I cherry-picked the commit from the code targeting v2 branch. In v2, the logger instance is optional.
SDK binary size reports 📊SDK binary size of this PRSDK binary size diff report between this PR and the main branch |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #736 +/- ##
==========================================
+ Coverage 56.95% 57.30% +0.34%
==========================================
Files 139 139
Lines 3880 3900 +20
==========================================
+ Hits 2210 2235 +25
+ Misses 1670 1665 -5 ☔ View full report in Codecov by Sentry. |
github-actions bot
pushed a commit
that referenced
this pull request
Jun 13, 2024
## [3.2.3](3.2.2...3.2.3) (2024-06-13) ### Bug Fixes * 3rd party SDK compatibility when SDK forwards push events back to us ([#736](#736)) ([ccb0f47](ccb0f47))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Part of: https://linear.app/customerio/issue/MBL-364/[bug]-push-notifications-not-showing-while-app-in-foreground-on-ios
When a push event occurs in an iOS app (such as determining if a push should be shown while app in foreground), the CIO SDK forwards that event to all 3rd party SDKs in the app. Some 3rd party SDKs handle these push events by also forwarding those events back to the CIO SDK. This has resulted in push handling in an app to not behave as expected.
See the ticket and included tests in this PR to learn more about the implementations of these 3rd party SDKs and how we handle them.
Testing:
Note to reviewers: