-
Notifications
You must be signed in to change notification settings - Fork 28
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
fix: Conversations and Profile Views are reported individually (WPB-14942) #3792
base: develop
Are you sure you want to change the base?
fix: Conversations and Profile Views are reported individually (WPB-14942) #3792
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3792 +/- ##
===========================================
- Coverage 45.79% 45.75% -0.05%
===========================================
Files 486 486
Lines 16632 16647 +15
Branches 2792 2799 +7
===========================================
Hits 7617 7617
- Misses 8234 8249 +15
Partials 781 781
Continue to review full report in Codecov by Sentry.
|
Built wire-android-staging-compat-pr-3792.apk is available for download |
Built wire-android-dev-debug-pr-3792.apk is available for download |
…are-reported-individually' into Conversations-and-Profile-Views-are-reported-individually
Built wire-android-staging-compat-pr-3792.apk is available for download |
app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt
Outdated
Show resolved
Hide resolved
Built wire-android-dev-debug-pr-3792.apk is available for download |
…are-reported-individually' into Conversations-and-Profile-Views-are-reported-individually
Built wire-android-staging-compat-pr-3792.apk is available for download |
Built wire-android-dev-debug-pr-3792.apk is available for download |
app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.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.
Looking good, left a question/concern and also to add, for this to work we need to remove this flag
Line 46 in a59ff2e
internal var VIEW_TRACKING_ENABLED: Boolean = false |
app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt
Outdated
Show resolved
Hide resolved
Yes, we still don't need it yet. |
Built wire-android-staging-compat-pr-3792.apk is available for download |
Built wire-android-dev-debug-pr-3792.apk is available for download |
app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt
Outdated
Show resolved
Hide resolved
Built wire-android-staging-compat-pr-3792.apk is available for download |
Built wire-android-dev-debug-pr-3792.apk is available for download |
app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt
Outdated
Show resolved
Hide resolved
Built wire-android-staging-compat-pr-3792.apk is available for download |
Built wire-android-dev-debug-pr-3792.apk is available for download |
@@ -182,7 +182,7 @@ object AnonymousAnalyticsManagerImpl : AnonymousAnalyticsManager { | |||
coroutineScope.launch { | |||
mutex.withLock { | |||
if (!isAnonymousUsageDataEnabled) return@withLock | |||
anonymousAnalyticsRecorder?.recordView(screen) | |||
anonymousAnalyticsRecorder?.recordView(screen.convertToCamelCase()) |
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.
is it really needed to convert this to camel case for countly ?
i mean is the screen names defined there or we can define them as we need on android ?
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.
it's for consistency, so we have clean data on the board. Otherwise some screen names are snake_case and some others are in camelCase.
Quality Gate passedIssues Measures |
Built wire-android-staging-compat-pr-3792.apk is available for download |
Built wire-android-dev-debug-pr-3792.apk is available for download |
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
Issues
Views for conversations are reported to Countly with a unique conversation ID attached to them: Conversation(eed94f9*)
For some screen we report to countly SomeOther with screen name as route: SomeOther(route=other_user_profile_screen)
Solutions
Check if current screen is SomeOther then we send the route otherwise we send class name
Needs releases with:
Testing
Test Coverage (Optional)
How to Test
Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.
Notes (Optional)
Specify here any other facts that you think are important for this issue.
Attachments (Optional)
Attachments like images, videos, etc. (drag and drop in the text box)
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764
.