From e3f9399667aab5c0642b9126306a7a0e5c133955 Mon Sep 17 00:00:00 2001 From: ohassine Date: Thu, 9 Jan 2025 15:24:29 +0100 Subject: [PATCH 1/9] fix: Conversations and Profile Views are reported individually --- .../wire/android/util/CurrentScreenManager.kt | 17 +++++-- .../AnonymousAnalyticsManagerImpl.kt | 6 +-- .../android/feature/analytics/StringExt.kt | 27 ++++++++++ .../feature/analytics/StringExtTest.kt | 49 +++++++++++++++++++ 4 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/StringExt.kt create mode 100644 core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/StringExtTest.kt diff --git a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt index b254197dd1a..62aee4fb86e 100644 --- a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt +++ b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt @@ -123,8 +123,10 @@ class CurrentScreenManager @Inject constructor( } override fun onDestinationChanged(controller: NavController, destination: NavDestination, arguments: Bundle?) { - val currentView = currentScreenState.value.toString() - AnonymousAnalyticsManagerImpl.stopView(currentView) + val currentView = currentScreenState.value + handleViewAction(currentView) { screenName -> + AnonymousAnalyticsManagerImpl.stopView(screenName) + } val currentItem = destination.toDestination() currentScreenState.value = CurrentScreen.fromDestination( currentItem, @@ -132,8 +134,15 @@ class CurrentScreenManager @Inject constructor( isApplicationVisibleFlow.value ) - val newView = currentScreenState.value.toString() - AnonymousAnalyticsManagerImpl.recordView(newView) + val newView = currentScreenState.value + handleViewAction(newView) { screenName -> + AnonymousAnalyticsManagerImpl.recordView(screenName) + } + } + + private fun handleViewAction(screen: CurrentScreen, action: (String) -> Unit) { + val screenName = (screen as? CurrentScreen.SomeOther)?.route ?: screen.javaClass.simpleName + action(screenName) } override fun onCreate(owner: LifecycleOwner) { diff --git a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt index 0eba82a61ae..ae5764354d9 100644 --- a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt +++ b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt @@ -43,7 +43,7 @@ object AnonymousAnalyticsManagerImpl : AnonymousAnalyticsManager { private lateinit var coroutineScope: CoroutineScope // TODO: Sync with product, when we want to enable view tracking, var for testing purposes - internal var VIEW_TRACKING_ENABLED: Boolean = false + internal var VIEW_TRACKING_ENABLED: Boolean = true override fun init( context: Context, @@ -182,7 +182,7 @@ object AnonymousAnalyticsManagerImpl : AnonymousAnalyticsManager { coroutineScope.launch { mutex.withLock { if (!isAnonymousUsageDataEnabled) return@withLock - anonymousAnalyticsRecorder?.recordView(screen) + anonymousAnalyticsRecorder?.recordView(screen.convertToCamelCase()) } } } @@ -195,7 +195,7 @@ object AnonymousAnalyticsManagerImpl : AnonymousAnalyticsManager { coroutineScope.launch { mutex.withLock { if (!isAnonymousUsageDataEnabled) return@withLock - anonymousAnalyticsRecorder?.stopView(screen) + anonymousAnalyticsRecorder?.stopView(screen.convertToCamelCase()) } } } diff --git a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/StringExt.kt b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/StringExt.kt new file mode 100644 index 00000000000..98752d4ff57 --- /dev/null +++ b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/StringExt.kt @@ -0,0 +1,27 @@ +/* + * Wire + * Copyright (C) 2025 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.feature.analytics + +/** + * Converts a snake_case string to camelCase. + */ +fun String.convertToCamelCase(): String { + return this + .split('_') + .joinToString("") { it.replaceFirstChar { char -> char.uppercase() } } +} diff --git a/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/StringExtTest.kt b/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/StringExtTest.kt new file mode 100644 index 00000000000..8f64ddedc49 --- /dev/null +++ b/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/StringExtTest.kt @@ -0,0 +1,49 @@ +/* + * Wire + * Copyright (C) 2025 Wire Swiss GmbH + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see http://www.gnu.org/licenses/. + */ +package com.wire.android.feature.analytics + +import org.junit.Assert.assertEquals +import org.junit.Test + +class StringExtTest { + + @Test + fun `given single word string when converted then same string`() { + assertEquals("Username", "username".convertToCamelCase()) + } + + @Test + fun `given string with multiple underscores when converted then camel case string`() { + assertEquals("ThisIsATestCase", ("this_is_a_test_case").convertToCamelCase()) + } + + @Test + fun `given empty string when converted then empty string`() { + assertEquals("", ("").convertToCamelCase()) + } + + @Test + fun `given string with leading and trailing underscores when converted then camel case string`() { + assertEquals("LeadingAndTrailing", ("_leading_and_trailing_").convertToCamelCase()) + } + + @Test + fun `given string with numbers when converted then camel case string`() { + assertEquals("User123Name", ("user_123_name").convertToCamelCase()) + } +} From b7025181393ee3507dc436606087e798779bc554 Mon Sep 17 00:00:00 2001 From: ohassine Date: Thu, 9 Jan 2025 15:32:48 +0100 Subject: [PATCH 2/9] fix: cleanup --- .../wire/android/feature/analytics/StringExtTest.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/StringExtTest.kt b/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/StringExtTest.kt index 8f64ddedc49..7a275dc1dc0 100644 --- a/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/StringExtTest.kt +++ b/core/analytics-enabled/src/test/kotlin/com/wire/android/feature/analytics/StringExtTest.kt @@ -23,27 +23,27 @@ import org.junit.Test class StringExtTest { @Test - fun `given single word string when converted then same string`() { + fun `given single word string when converted then return same string`() { assertEquals("Username", "username".convertToCamelCase()) } @Test - fun `given string with multiple underscores when converted then camel case string`() { + fun `given string with multiple underscores when converted then return camel case string`() { assertEquals("ThisIsATestCase", ("this_is_a_test_case").convertToCamelCase()) } @Test - fun `given empty string when converted then empty string`() { + fun `given empty string when converted then return empty string`() { assertEquals("", ("").convertToCamelCase()) } @Test - fun `given string with leading and trailing underscores when converted then camel case string`() { + fun `given string with leading and trailing underscores when converted then return camel case string`() { assertEquals("LeadingAndTrailing", ("_leading_and_trailing_").convertToCamelCase()) } @Test - fun `given string with numbers when converted then camel case string`() { + fun `given string with numbers when converted then return camel case string`() { assertEquals("User123Name", ("user_123_name").convertToCamelCase()) } } From 860fd15b6a7f6b829aa6f20cc04016f620c3bd17 Mon Sep 17 00:00:00 2001 From: ohassine Date: Thu, 9 Jan 2025 16:41:35 +0100 Subject: [PATCH 3/9] fix: disable view tracking --- .../android/feature/analytics/AnonymousAnalyticsManagerImpl.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt index ae5764354d9..60c1100911f 100644 --- a/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt +++ b/core/analytics-enabled/src/main/kotlin/com/wire/android/feature/analytics/AnonymousAnalyticsManagerImpl.kt @@ -43,7 +43,7 @@ object AnonymousAnalyticsManagerImpl : AnonymousAnalyticsManager { private lateinit var coroutineScope: CoroutineScope // TODO: Sync with product, when we want to enable view tracking, var for testing purposes - internal var VIEW_TRACKING_ENABLED: Boolean = true + internal var VIEW_TRACKING_ENABLED: Boolean = false override fun init( context: Context, From d4dfca83f8d3960930614f958143bbbda34c80fd Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 10 Jan 2025 09:22:14 +0100 Subject: [PATCH 4/9] fix: address comment --- .../wire/android/util/CurrentScreenManager.kt | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt index 62aee4fb86e..d6c41e52a99 100644 --- a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt +++ b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt @@ -123,10 +123,9 @@ class CurrentScreenManager @Inject constructor( } override fun onDestinationChanged(controller: NavController, destination: NavDestination, arguments: Bundle?) { - val currentView = currentScreenState.value - handleViewAction(currentView) { screenName -> - AnonymousAnalyticsManagerImpl.stopView(screenName) - } + val currentScreenName = currentScreenName() + AnonymousAnalyticsManagerImpl.stopView(currentScreenName) + val currentItem = destination.toDestination() currentScreenState.value = CurrentScreen.fromDestination( currentItem, @@ -134,15 +133,12 @@ class CurrentScreenManager @Inject constructor( isApplicationVisibleFlow.value ) - val newView = currentScreenState.value - handleViewAction(newView) { screenName -> - AnonymousAnalyticsManagerImpl.recordView(screenName) - } + val newScreenName = currentScreenName() + AnonymousAnalyticsManagerImpl.recordView(newScreenName) } - private fun handleViewAction(screen: CurrentScreen, action: (String) -> Unit) { - val screenName = (screen as? CurrentScreen.SomeOther)?.route ?: screen.javaClass.simpleName - action(screenName) + private fun currentScreenName() = currentScreenState.value.let { currentScreen -> + (currentScreen as? CurrentScreen.SomeOther)?.route ?: currentScreen.javaClass.simpleName } override fun onCreate(owner: LifecycleOwner) { From 262e37fb4a8af55446a783943c5335eefd4fd5ee Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 10 Jan 2025 14:22:01 +0100 Subject: [PATCH 5/9] fix: address comment --- .../wire/android/util/CurrentScreenManager.kt | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt index d6c41e52a99..da6cfd51d83 100644 --- a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt +++ b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt @@ -124,7 +124,9 @@ class CurrentScreenManager @Inject constructor( override fun onDestinationChanged(controller: NavController, destination: NavDestination, arguments: Bundle?) { val currentScreenName = currentScreenName() - AnonymousAnalyticsManagerImpl.stopView(currentScreenName) + currentScreenName?.let { + AnonymousAnalyticsManagerImpl.stopView(it) + } val currentItem = destination.toDestination() currentScreenState.value = CurrentScreen.fromDestination( @@ -134,11 +136,21 @@ class CurrentScreenManager @Inject constructor( ) val newScreenName = currentScreenName() - AnonymousAnalyticsManagerImpl.recordView(newScreenName) + newScreenName?.let { + AnonymousAnalyticsManagerImpl.recordView(it) + } } private fun currentScreenName() = currentScreenState.value.let { currentScreen -> - (currentScreen as? CurrentScreen.SomeOther)?.route ?: currentScreen.javaClass.simpleName + when (currentScreen) { + is CurrentScreen.Home, + is CurrentScreen.Conversation, + is CurrentScreen.OtherUserProfile, + is CurrentScreen.ImportMedia, + is CurrentScreen.DeviceManager -> return@let currentScreen.toScreenName() + + else -> return@let (currentScreen as? CurrentScreen.SomeOther)?.route + } } override fun onCreate(owner: LifecycleOwner) { @@ -169,23 +181,31 @@ class CurrentScreenManager @Inject constructor( sealed class CurrentScreen { // Home Screen is being displayed - data object Home : CurrentScreen() + data object Home : CurrentScreen() { + override fun toScreenName() = "HomeScreen" + } // Some Conversation is opened data class Conversation(val id: ConversationId) : CurrentScreen() { override fun toString(): String = "Conversation(${id.toString().obfuscateId()})" + override fun toScreenName() = "ConversationScreen" } // Another User Profile Screen is opened data class OtherUserProfile(val id: ConversationId) : CurrentScreen() { override fun toString(): String = "OtherUserProfile(${id.toString().obfuscateId()})" + override fun toScreenName() = "OtherUserProfileScreen" } // Import media screen is opened - data object ImportMedia : CurrentScreen() + data object ImportMedia : CurrentScreen() { + override fun toScreenName() = "ImportMediaScreen" + } // SelfDevices screen is opened - data object DeviceManager : CurrentScreen() + data object DeviceManager : CurrentScreen() { + override fun toScreenName() = "DeviceManagerScreen" + } // Auth related screen is opened data class AuthRelated(val route: String?) : CurrentScreen() @@ -196,6 +216,8 @@ sealed class CurrentScreen { // App is in background (screen is turned off, or covered by another app), non of the screens is visible data object InBackground : CurrentScreen() + open fun toScreenName(): String = "UnknownScreen" + companion object { @SuppressLint("RestrictedApi") @Suppress("ComplexMethod") From 10694aef08b2f56cfa80f9cd3480d86740fd7eb9 Mon Sep 17 00:00:00 2001 From: ohassine Date: Fri, 10 Jan 2025 18:55:17 +0100 Subject: [PATCH 6/9] fix: use base route --- .../main/kotlin/com/wire/android/util/CurrentScreenManager.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt index da6cfd51d83..ef5453b0e0c 100644 --- a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt +++ b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt @@ -29,6 +29,7 @@ import androidx.navigation.NavDestination import com.ramcosta.composedestinations.spec.DestinationSpec import com.wire.android.appLogger import com.wire.android.feature.analytics.AnonymousAnalyticsManagerImpl +import com.wire.android.navigation.getBaseRoute import com.wire.android.navigation.toDestination import com.wire.android.ui.destinations.ConversationScreenDestination import com.wire.android.ui.destinations.CreateAccountDetailsScreenDestination @@ -149,7 +150,7 @@ class CurrentScreenManager @Inject constructor( is CurrentScreen.ImportMedia, is CurrentScreen.DeviceManager -> return@let currentScreen.toScreenName() - else -> return@let (currentScreen as? CurrentScreen.SomeOther)?.route + else -> return@let (currentScreen as? CurrentScreen.SomeOther)?.route?.getBaseRoute() } } From f66bdb0dcc476dd1938f58c2c0d974b5a6d0eae7 Mon Sep 17 00:00:00 2001 From: ohassine Date: Mon, 20 Jan 2025 12:46:30 +0100 Subject: [PATCH 7/9] fix: address comment --- .../main/kotlin/com/wire/android/util/CurrentScreenManager.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt index ef5453b0e0c..6d47fe39acd 100644 --- a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt +++ b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt @@ -150,7 +150,8 @@ class CurrentScreenManager @Inject constructor( is CurrentScreen.ImportMedia, is CurrentScreen.DeviceManager -> return@let currentScreen.toScreenName() - else -> return@let (currentScreen as? CurrentScreen.SomeOther)?.route?.getBaseRoute() + is CurrentScreen.AuthRelated -> return@let currentScreen.route?.getBaseRoute() ?: currentScreen.toString() + else -> return@let (currentScreen as? CurrentScreen.SomeOther)?.route?.getBaseRoute() ?: currentScreen.toString() } } From b979835aeb4112ee836cf9dab82e04e855c52a36 Mon Sep 17 00:00:00 2001 From: ohassine Date: Mon, 20 Jan 2025 12:47:35 +0100 Subject: [PATCH 8/9] chore: cleanup --- .../kotlin/com/wire/android/util/CurrentScreenManager.kt | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt index 6d47fe39acd..8fb90aaa2a1 100644 --- a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt +++ b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt @@ -125,9 +125,8 @@ class CurrentScreenManager @Inject constructor( override fun onDestinationChanged(controller: NavController, destination: NavDestination, arguments: Bundle?) { val currentScreenName = currentScreenName() - currentScreenName?.let { - AnonymousAnalyticsManagerImpl.stopView(it) - } + AnonymousAnalyticsManagerImpl.stopView(currentScreenName) + val currentItem = destination.toDestination() currentScreenState.value = CurrentScreen.fromDestination( @@ -137,9 +136,7 @@ class CurrentScreenManager @Inject constructor( ) val newScreenName = currentScreenName() - newScreenName?.let { - AnonymousAnalyticsManagerImpl.recordView(it) - } + AnonymousAnalyticsManagerImpl.recordView(newScreenName) } private fun currentScreenName() = currentScreenState.value.let { currentScreen -> From d2b9c439017112164c0c66be679d3859f03482b5 Mon Sep 17 00:00:00 2001 From: ohassine Date: Mon, 20 Jan 2025 12:57:10 +0100 Subject: [PATCH 9/9] chore: cleanup NoConsecutiveBlankLines --- .../main/kotlin/com/wire/android/util/CurrentScreenManager.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt index 8fb90aaa2a1..7bee954ab37 100644 --- a/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt +++ b/app/src/main/kotlin/com/wire/android/util/CurrentScreenManager.kt @@ -127,7 +127,6 @@ class CurrentScreenManager @Inject constructor( val currentScreenName = currentScreenName() AnonymousAnalyticsManagerImpl.stopView(currentScreenName) - val currentItem = destination.toDestination() currentScreenState.value = CurrentScreen.fromDestination( currentItem,