-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: add option to disable screen view usage #474
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. |
Build available to test |
|
📏 SDK Binary Size Comparison ReportNo changes detected in SDK binary size ✅ |
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #474 +/- ##
=============================================
+ Coverage 41.98% 52.28% +10.29%
- Complexity 259 302 +43
=============================================
Files 99 98 -1
Lines 2320 2607 +287
Branches 344 360 +16
=============================================
+ Hits 974 1363 +389
+ Misses 1247 1135 -112
- Partials 99 109 +10 ☔ View full report in Codecov by Sentry. |
datapipelines/src/main/kotlin/io/customer/datapipelines/config/ScreenView.kt
Outdated
Show resolved
Hide resolved
|
|
|
|
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.
I think we should minimize public API changes and limit it to work around the issue we have at the moment and leave larger API changes to a later point where we re-think having parts of our SDK enabled using a comprehensive mechanism.
/** | ||
* Enum class to define how CustomerIO SDK should handle screen view events. | ||
*/ | ||
enum class ScreenView { |
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.
Does this mean it's a mutually exclusive choice? I can either choose analytics or in-app? But what if a customer needs both?
datapipelines/src/main/kotlin/io/customer/datapipelines/plugins/ScreenFilterPlugin.kt
Outdated
Show resolved
Hide resolved
/** | ||
* Enum class to define how CustomerIO SDK should handle screen view events. | ||
*/ | ||
enum class ScreenView { |
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.
Curious why we are opting for making this an enum instead of:
- Boolean to allow screen analytics
- If in app is enabled, we always use screen views for in-app
|
|
|
|
|
|
## [4.5.0](4.4.1...4.5.0) (2024-12-17) ### Features * add option to disable screen view usage ([#474](#474)) ([b5ea8e9](b5ea8e9))
part of: MBL-755
Changes
screenViewUse
enum option to allow disable sending screen view events to server and use them locally onlyScreenFilterPlugin
to filter events based on thescreenViewUse
configurationScreenView
OptionsAnalytics
-> Sends all screen events to the server (same as before) - Default behaviorInApp
-> Retains screen events locally for in-app use onlySample Usage