Code review

wireapp · Dec 19, 2024 · 2565ffb · 2565ffb
1 parent 5530df1
commit 2565ffb
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 36 deletions.
diff --git a/app/src/main/kotlin/com/wire/android/WireApplication.kt b/app/src/main/kotlin/com/wire/android/WireApplication.kt
@@ -32,6 +32,7 @@ import com.wire.android.datastore.UserDataStoreProvider
 import com.wire.android.debug.DatabaseProfilingManager
 import com.wire.android.di.ApplicationScope
 import com.wire.android.di.KaliumCoreLogic
+import com.wire.android.feature.analytics.AnonymousAnalyticsManager
 import com.wire.android.feature.analytics.AnonymousAnalyticsManagerImpl
 import com.wire.android.feature.analytics.AnonymousAnalyticsRecorderImpl
 import com.wire.android.feature.analytics.globalAnalyticsManager
@@ -48,10 +49,12 @@ import com.wire.kalium.logger.KaliumLogLevel
 import com.wire.kalium.logger.KaliumLogger
 import com.wire.kalium.logic.CoreLogger
 import com.wire.kalium.logic.CoreLogic
+import com.wire.kalium.logic.feature.session.CurrentSessionResult
 import dagger.Lazy
 import dagger.hilt.android.HiltAndroidApp
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.filter
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.launch
@@ -93,6 +96,9 @@ class WireApplication : BaseApp() {
     @Inject
     lateinit var databaseProfilingManager: DatabaseProfilingManager
 
+    @Inject
+    lateinit var analyticsManager: Lazy<AnonymousAnalyticsManager>
+
     override val workManagerConfiguration: Configuration
         get() = Configuration.Builder()
             .setWorkerFactory(wireWorkerFactory.get())
@@ -121,6 +127,19 @@ class WireApplication : BaseApp() {
 
             appLogger.i("$TAG global observers")
             globalObserversManager.get().observe()
+
+            observeRecentlyEndedCall()
+        }
+    }
+
+    private suspend fun observeRecentlyEndedCall() {
+        coreLogic.get().getGlobalScope().session.currentSessionFlow().collectLatest { sessionResult ->
+            if (sessionResult is CurrentSessionResult.Success && sessionResult.accountInfo.isValid()) {
+                coreLogic.get().getSessionScope(sessionResult.accountInfo.userId).calls.observeRecentlyEndedCallMetadata()
+                    .collect { metadata ->
+                        analyticsManager.get().sendEvent(AnalyticsEvent.RecentlyEndedCallEvent(metadata))
+                    }
+            }
         }
     }
 

diff --git a/app/src/main/kotlin/com/wire/android/di/accountScoped/CallsModule.kt b/app/src/main/kotlin/com/wire/android/di/accountScoped/CallsModule.kt
@@ -19,8 +19,6 @@ package com.wire.android.di.accountScoped
 
 import com.wire.android.di.CurrentAccount
 import com.wire.android.di.KaliumCoreLogic
-import dagger.Module
-import dagger.Provides
 import com.wire.kalium.logic.CoreLogic
 import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.feature.call.CallsScope
@@ -40,6 +38,8 @@ import com.wire.kalium.logic.feature.call.usecase.TurnLoudSpeakerOnUseCase
 import com.wire.kalium.logic.feature.call.usecase.UnMuteCallUseCase
 import com.wire.kalium.logic.feature.call.usecase.video.SetVideoSendStateUseCase
 import com.wire.kalium.logic.feature.call.usecase.video.UpdateVideoStateUseCase
+import dagger.Module
+import dagger.Provides
 import dagger.hilt.InstallIn
 import dagger.hilt.android.components.ViewModelComponent
 import dagger.hilt.android.scopes.ViewModelScoped
@@ -202,9 +202,4 @@ class CallsModule {
     @Provides
     fun provideObserveConferenceCallingEnabledUseCase(callsScope: CallsScope) =
         callsScope.observeConferenceCallingEnabled
-
-    @ViewModelScoped
-    @Provides
-    fun provideObserveRecentlyEndedCallMetadataUseCase(callsScope: CallsScope) =
-        callsScope.observeRecentlyEndedCallMetadata
 }
diff --git a/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt b/app/src/main/kotlin/com/wire/android/ui/WireActivityViewModel.kt
@@ -37,8 +37,6 @@ import com.wire.android.feature.AccountSwitchUseCase
 import com.wire.android.feature.SwitchAccountActions
 import com.wire.android.feature.SwitchAccountParam
 import com.wire.android.feature.SwitchAccountResult
-import com.wire.android.feature.analytics.AnonymousAnalyticsManager
-import com.wire.android.feature.analytics.model.AnalyticsEvent
 import com.wire.android.migration.MigrationManager
 import com.wire.android.services.ServicesManager
 import com.wire.android.ui.authentication.devices.model.displayName
@@ -64,7 +62,6 @@ import com.wire.kalium.logic.data.logout.LogoutReason
 import com.wire.kalium.logic.data.sync.SyncState
 import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.feature.appVersioning.ObserveIfAppUpdateRequiredUseCase
-import com.wire.kalium.logic.feature.call.usecase.ObserveRecentlyEndedCallMetadataUseCase
 import com.wire.kalium.logic.feature.client.ClearNewClientsForUserUseCase
 import com.wire.kalium.logic.feature.client.NewClientResult
 import com.wire.kalium.logic.feature.client.ObserveNewClientsUseCase
@@ -124,9 +121,7 @@ class WireActivityViewModel @Inject constructor(
     private val observeScreenshotCensoringConfigUseCaseProviderFactory: ObserveScreenshotCensoringConfigUseCaseProvider.Factory,
     private val globalDataStore: Lazy<GlobalDataStore>,
     private val observeIfE2EIRequiredDuringLoginUseCaseProviderFactory: ObserveIfE2EIRequiredDuringLoginUseCaseProvider.Factory,
-    private val workManager: Lazy<WorkManager>,
-    private val observeRecentlyEndedCallMetadata: ObserveRecentlyEndedCallMetadataUseCase,
-    private val analyticsManager: AnonymousAnalyticsManager
+    private val workManager: Lazy<WorkManager>
 ) : ViewModel() {
 
     var globalAppState: GlobalAppState by mutableStateOf(GlobalAppState())
@@ -156,16 +151,6 @@ class WireActivityViewModel @Inject constructor(
         observeScreenshotCensoringConfigState()
         observeAppThemeState()
         observeLogoutState()
-        trackRecentlyEndedCall()
-    }
-
-    private fun trackRecentlyEndedCall() {
-        viewModelScope.launch {
-            observeRecentlyEndedCallMetadata()
-                .collect { metadata ->
-                    analyticsManager.sendEvent(AnalyticsEvent.RecentlyEndedCallEvent(metadata))
-                }
-        }
     }
 
     private suspend fun shouldEnrollToE2ei(): Boolean = observeCurrentValidUserId.first()?.let {

diff --git a/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt b/app/src/test/kotlin/com/wire/android/ui/WireActivityViewModelTest.kt
@@ -33,7 +33,6 @@ import com.wire.android.di.ObserveIfE2EIRequiredDuringLoginUseCaseProvider
 import com.wire.android.di.ObserveScreenshotCensoringConfigUseCaseProvider
 import com.wire.android.di.ObserveSyncStateUseCaseProvider
 import com.wire.android.feature.AccountSwitchUseCase
-import com.wire.android.feature.analytics.AnonymousAnalyticsManager
 import com.wire.android.framework.TestClient
 import com.wire.android.framework.TestUser
 import com.wire.android.migration.MigrationManager
@@ -63,7 +62,6 @@ import com.wire.kalium.logic.data.sync.SyncState
 import com.wire.kalium.logic.data.user.UserId
 import com.wire.kalium.logic.feature.appVersioning.ObserveIfAppUpdateRequiredUseCase
 import com.wire.kalium.logic.feature.call.usecase.ObserveEstablishedCallsUseCase
-import com.wire.kalium.logic.feature.call.usecase.ObserveRecentlyEndedCallMetadataUseCase
 import com.wire.kalium.logic.feature.client.ClearNewClientsForUserUseCase
 import com.wire.kalium.logic.feature.client.NewClientResult
 import com.wire.kalium.logic.feature.client.ObserveNewClientsUseCase
@@ -732,7 +730,6 @@ class WireActivityViewModelTest {
                     flowOf(false)
             every { workManager.cancelAllWorkByTag(any()) } returns OperationImpl()
             every { workManager.enqueueUniquePeriodicWork(any(), any(), any()) } returns OperationImpl()
-            coEvery { observeRecentlyEndedCallMetadata() } returns flowOf(recentlyEndedCallMetadata)
         }
 
         @MockK
@@ -797,12 +794,6 @@ class WireActivityViewModelTest {
         @MockK
         lateinit var workManager: WorkManager
 
-        @MockK
-        lateinit var observeRecentlyEndedCallMetadata: ObserveRecentlyEndedCallMetadataUseCase
-
-        @MockK
-        lateinit var analyticsManager: AnonymousAnalyticsManager
-
         @MockK
         lateinit var observeEstablishedCalls: ObserveEstablishedCallsUseCase
 
@@ -836,9 +827,7 @@ class WireActivityViewModelTest {
                 observeScreenshotCensoringConfigUseCaseProviderFactory = observeScreenshotCensoringConfigUseCaseProviderFactory,
                 globalDataStore = { globalDataStore },
                 observeIfE2EIRequiredDuringLoginUseCaseProviderFactory = observeIfE2EIRequiredDuringLoginUseCaseProviderFactory,
-                workManager = { workManager },
-                observeRecentlyEndedCallMetadata = observeRecentlyEndedCallMetadata,
-                analyticsManager = analyticsManager
+                workManager = { workManager }
             )
         }
 

diff --git a/core/analytics/src/main/kotlin/com/wire/android/feature/analytics/model/AnalyticsEvent.kt b/core/analytics/src/main/kotlin/com/wire/android/feature/analytics/model/AnalyticsEvent.kt
@@ -180,7 +180,7 @@ interface AnalyticsEvent {
             return when (conversationDetails.conversationType) {
                 Conversation.Type.ONE_ON_ONE -> "one_to_one"
                 Conversation.Type.GROUP -> "group"
-                else -> "unknown"
+                else -> throw IllegalStateException("Call should not happen for ${conversationDetails.conversationType}")
             }
         }
     }