Merge pull request #3618 from element-hq/feature/bma/injectPresenter

Ensure that `Presenter`s do not depend on other presenters.

Author: bmarty

appnav/src/main/kotlin/io/element/android/appnav/root/RootPresenter.kt

@@ -12,8 +12,8 @@ import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import im.vector.app.features.analytics.plan.SuperProperties
-import io.element.android.features.rageshake.api.crash.CrashDetectionPresenter
-import io.element.android.features.rageshake.api.detection.RageshakeDetectionPresenter
+import io.element.android.features.rageshake.api.crash.CrashDetectionState
+import io.element.android.features.rageshake.api.detection.RageshakeDetectionState
 import io.element.android.features.share.api.ShareService
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.matrix.api.SdkMetadata
@@ -22,8 +22,8 @@ import io.element.android.services.apperror.api.AppErrorStateService
 import javax.inject.Inject
 
 class RootPresenter @Inject constructor(
-    private val crashDetectionPresenter: CrashDetectionPresenter,
-    private val rageshakeDetectionPresenter: RageshakeDetectionPresenter,
+    private val crashDetectionPresenter: Presenter<CrashDetectionState>,
+    private val rageshakeDetectionPresenter: Presenter<RageshakeDetectionState>,
     private val appErrorStateService: AppErrorStateService,
     private val analyticsService: AnalyticsService,
     private val shareService: ShareService,

appnav/src/test/kotlin/io/element/android/appnav/RootPresenterTest.kt

@@ -12,17 +12,11 @@ import app.cash.molecule.moleculeFlow
 import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
 import io.element.android.appnav.root.RootPresenter
-import io.element.android.features.rageshake.impl.crash.DefaultCrashDetectionPresenter
-import io.element.android.features.rageshake.impl.detection.DefaultRageshakeDetectionPresenter
-import io.element.android.features.rageshake.impl.preferences.DefaultRageshakePreferencesPresenter
-import io.element.android.features.rageshake.test.crash.FakeCrashDataStore
-import io.element.android.features.rageshake.test.rageshake.FakeRageShake
-import io.element.android.features.rageshake.test.rageshake.FakeRageshakeDataStore
-import io.element.android.features.rageshake.test.screenshot.FakeScreenshotHolder
+import io.element.android.features.rageshake.api.crash.aCrashDetectionState
+import io.element.android.features.rageshake.api.detection.aRageshakeDetectionState
 import io.element.android.features.share.api.ShareService
 import io.element.android.features.share.test.FakeShareService
 import io.element.android.libraries.matrix.test.FakeSdkMetadata
-import io.element.android.libraries.matrix.test.core.aBuildMeta
 import io.element.android.services.analytics.test.FakeAnalyticsService
 import io.element.android.services.apperror.api.AppErrorState
 import io.element.android.services.apperror.api.AppErrorStateService
@@ -44,7 +38,6 @@ class RootPresenterTest {
         moleculeFlow(RecompositionMode.Immediate) {
             presenter.present()
         }.test {
-            skipItems(1)
             val initialState = awaitItem()
             assertThat(initialState.crashDetectionState.crashDetected).isFalse()
         }
@@ -61,7 +54,7 @@ class RootPresenterTest {
         moleculeFlow(RecompositionMode.Immediate) {
             presenter.present()
         }.test {
-            skipItems(2)
+            skipItems(1)
             lambda.assertions().isCalledOnce()
         }
     }
@@ -76,8 +69,6 @@ class RootPresenterTest {
         moleculeFlow(RecompositionMode.Immediate) {
             presenter.present()
         }.test {
-            skipItems(1)
-
             val initialState = awaitItem()
             assertThat(initialState.errorState).isInstanceOf(AppErrorState.Error::class.java)
             val initialErrorState = initialState.errorState as AppErrorState.Error
@@ -93,25 +84,9 @@ class RootPresenterTest {
         appErrorService: AppErrorStateService = DefaultAppErrorStateService(),
         shareService: ShareService = FakeShareService {},
     ): RootPresenter {
-        val crashDataStore = FakeCrashDataStore()
-        val rageshakeDataStore = FakeRageshakeDataStore()
-        val rageshake = FakeRageShake()
-        val screenshotHolder = FakeScreenshotHolder()
-        val crashDetectionPresenter = DefaultCrashDetectionPresenter(
-            buildMeta = aBuildMeta(),
-            crashDataStore = crashDataStore
-        )
-        val rageshakeDetectionPresenter = DefaultRageshakeDetectionPresenter(
-            screenshotHolder = screenshotHolder,
-            rageShake = rageshake,
-            preferencesPresenter = DefaultRageshakePreferencesPresenter(
-                rageshake = rageshake,
-                rageshakeDataStore = rageshakeDataStore,
-            )
-        )
         return RootPresenter(
-            crashDetectionPresenter = crashDetectionPresenter,
-            rageshakeDetectionPresenter = rageshakeDetectionPresenter,
+            crashDetectionPresenter = { aCrashDetectionState() },
+            rageshakeDetectionPresenter = { aRageshakeDetectionState() },
             appErrorStateService = appErrorService,
             analyticsService = FakeAnalyticsService(),
             shareService = shareService,

features/analytics/api/src/main/kotlin/io/element/android/features/analytics/api/preferences/AnalyticsPreferencesPresenter.kt

@@ -0,0 +1,23 @@
+/*
+ * Copyright 2024 New Vector Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ * Please see LICENSE in the repository root for full details.
+ */
+
+package io.element.android.features.analytics.impl.di
+
+import com.squareup.anvil.annotations.ContributesTo
+import dagger.Binds
+import dagger.Module
+import io.element.android.features.analytics.api.preferences.AnalyticsPreferencesState
+import io.element.android.features.analytics.impl.preferences.AnalyticsPreferencesPresenter
+import io.element.android.libraries.architecture.Presenter
+import io.element.android.libraries.di.AppScope
+
+@ContributesTo(AppScope::class)
+@Module
+interface AnalyticsModule {
+    @Binds
+    fun bindAnalyticsPreferencesPresenter(presenter: AnalyticsPreferencesPresenter): Presenter<AnalyticsPreferencesState>
+}

features/analytics/impl/src/main/kotlin/io/element/android/features/analytics/impl/di/AnalyticsModule.kt

@@ -10,23 +10,20 @@ package io.element.android.features.analytics.impl.preferences
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.rememberCoroutineScope
-import com.squareup.anvil.annotations.ContributesBinding
 import io.element.android.appconfig.AnalyticsConfig
 import io.element.android.features.analytics.api.AnalyticsOptInEvents
-import io.element.android.features.analytics.api.preferences.AnalyticsPreferencesPresenter
 import io.element.android.features.analytics.api.preferences.AnalyticsPreferencesState
+import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.core.meta.BuildMeta
-import io.element.android.libraries.di.AppScope
 import io.element.android.services.analytics.api.AnalyticsService
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import javax.inject.Inject
 
-@ContributesBinding(AppScope::class)
-class DefaultAnalyticsPreferencesPresenter @Inject constructor(
+class AnalyticsPreferencesPresenter @Inject constructor(
     private val analyticsService: AnalyticsService,
     private val buildMeta: BuildMeta,
-) : AnalyticsPreferencesPresenter {
+) : Presenter<AnalyticsPreferencesState> {
     @Composable
     override fun present(): AnalyticsPreferencesState {
         val localCoroutineScope = rememberCoroutineScope()

…/DefaultAnalyticsPreferencesPresenter.kt …erences/AnalyticsPreferencesPresenter.ktfeatures/analytics/impl/src/main/kotlin/io/element/android/features/analytics/impl/preferences/DefaultAnalyticsPreferencesPresenter.kt renamed to features/analytics/impl/src/main/kotlin/io/element/android/features/analytics/impl/preferences/AnalyticsPreferencesPresenter.kt features/analytics/impl/src/main/kotlin/io/element/android/features/analytics/impl/preferences/DefaultAnalyticsPreferencesPresenter.kt renamed to features/analytics/impl/src/main/kotlin/io/element/android/features/analytics/impl/preferences/AnalyticsPreferencesPresenter.kt

@@ -25,7 +25,7 @@ class AnalyticsPreferencesPresenterTest {
 
     @Test
     fun `present - initial state available`() = runTest {
-        val presenter = DefaultAnalyticsPreferencesPresenter(
+        val presenter = AnalyticsPreferencesPresenter(
             FakeAnalyticsService(isEnabled = true, didAskUserConsent = true),
             aBuildMeta()
         )
@@ -41,7 +41,7 @@ class AnalyticsPreferencesPresenterTest {
 
     @Test
     fun `present - initial state not available`() = runTest {
-        val presenter = DefaultAnalyticsPreferencesPresenter(
+        val presenter = AnalyticsPreferencesPresenter(
             FakeAnalyticsService(isEnabled = false, didAskUserConsent = false),
             aBuildMeta()
         )
@@ -55,7 +55,7 @@ class AnalyticsPreferencesPresenterTest {
 
     @Test
     fun `present - enable and disable`() = runTest {
-        val presenter = DefaultAnalyticsPreferencesPresenter(
+        val presenter = AnalyticsPreferencesPresenter(
             FakeAnalyticsService(isEnabled = true, didAskUserConsent = true),
             aBuildMeta()
         )

features/analytics/impl/src/test/kotlin/io/element/android/features/analytics/impl/preferences/AnalyticsPreferencesPresenterTest.kt

@@ -15,18 +15,19 @@ import io.element.android.features.ftue.impl.state.DefaultFtueService
 import io.element.android.features.ftue.impl.state.FtueStep
 import io.element.android.features.lockscreen.api.LockScreenService
 import io.element.android.features.lockscreen.test.FakeLockScreenService
+import io.element.android.libraries.matrix.api.verification.SessionVerificationService
 import io.element.android.libraries.matrix.api.verification.SessionVerifiedStatus
 import io.element.android.libraries.matrix.test.verification.FakeSessionVerificationService
+import io.element.android.libraries.permissions.api.PermissionStateProvider
 import io.element.android.libraries.permissions.impl.FakePermissionStateProvider
+import io.element.android.libraries.preferences.api.store.SessionPreferencesStore
 import io.element.android.libraries.preferences.test.InMemorySessionPreferencesStore
 import io.element.android.services.analytics.api.AnalyticsService
 import io.element.android.services.analytics.test.FakeAnalyticsService
 import io.element.android.services.toolbox.test.sdk.FakeBuildVersionSdkIntProvider
 import io.element.android.tests.testutils.lambda.lambdaRecorder
 import io.element.android.tests.testutils.lambda.value
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.SupervisorJob
-import kotlinx.coroutines.cancel
+import kotlinx.coroutines.test.TestScope
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 
@@ -36,8 +37,9 @@ class DefaultFtueServiceTest {
         val sessionVerificationService = FakeSessionVerificationService().apply {
             givenVerifiedStatus(SessionVerifiedStatus.Unknown)
         }
-        val coroutineScope = CoroutineScope(coroutineContext + SupervisorJob())
-        val service = createDefaultFtueService(coroutineScope, sessionVerificationService)
+        val service = createDefaultFtueService(
+            sessionVerificationService = sessionVerificationService,
+        )
 
         service.state.test {
             // Verification state is unknown, we don't display the flow yet
@@ -47,9 +49,6 @@ class DefaultFtueServiceTest {
             sessionVerificationService.givenVerifiedStatus(SessionVerifiedStatus.NotVerified)
             assertThat(awaitItem()).isEqualTo(FtueState.Incomplete)
         }
-
-        // Cleanup
-        coroutineScope.cancel()
     }
 
     @Test
@@ -58,10 +57,7 @@ class DefaultFtueServiceTest {
         val sessionVerificationService = FakeSessionVerificationService()
         val permissionStateProvider = FakePermissionStateProvider(permissionGranted = true)
         val lockScreenService = FakeLockScreenService()
-        val coroutineScope = CoroutineScope(coroutineContext + SupervisorJob())
-
         val service = createDefaultFtueService(
-            coroutineScope = coroutineScope,
             sessionVerificationService = sessionVerificationService,
             analyticsService = analyticsService,
             permissionStateProvider = permissionStateProvider,
@@ -75,9 +71,6 @@ class DefaultFtueServiceTest {
         service.updateState()
 
         assertThat(service.state.value).isEqualTo(FtueState.Complete)
-
-        // Cleanup
-        coroutineScope.cancel()
     }
 
     @Test
@@ -88,10 +81,7 @@ class DefaultFtueServiceTest {
         val analyticsService = FakeAnalyticsService()
         val permissionStateProvider = FakePermissionStateProvider(permissionGranted = false)
         val lockScreenService = FakeLockScreenService()
-        val coroutineScope = CoroutineScope(coroutineContext + SupervisorJob())
-
         val service = createDefaultFtueService(
-            coroutineScope = coroutineScope,
             sessionVerificationService = sessionVerificationService,
             analyticsService = analyticsService,
             permissionStateProvider = permissionStateProvider,
@@ -126,20 +116,15 @@ class DefaultFtueServiceTest {
             // Final state
             null,
         )
-
-        // Cleanup
-        coroutineScope.cancel()
     }
 
     @Test
     fun `if a check for a step is true, start from the next one`() = runTest {
-        val coroutineScope = CoroutineScope(coroutineContext + SupervisorJob())
         val sessionVerificationService = FakeSessionVerificationService()
         val analyticsService = FakeAnalyticsService()
         val permissionStateProvider = FakePermissionStateProvider(permissionGranted = false)
         val lockScreenService = FakeLockScreenService()
         val service = createDefaultFtueService(
-            coroutineScope = coroutineScope,
             sessionVerificationService = sessionVerificationService,
             analyticsService = analyticsService,
             permissionStateProvider = permissionStateProvider,
@@ -155,22 +140,17 @@ class DefaultFtueServiceTest {
 
         analyticsService.setDidAskUserConsent()
         assertThat(service.getNextStep(null)).isNull()
-
-        // Cleanup
-        coroutineScope.cancel()
     }
 
     @Test
     fun `if version is older than 13 we don't display the notification opt in screen`() = runTest {
-        val coroutineScope = CoroutineScope(coroutineContext + SupervisorJob())
         val sessionVerificationService = FakeSessionVerificationService()
         val analyticsService = FakeAnalyticsService()
         val lockScreenService = FakeLockScreenService()
 
         val service = createDefaultFtueService(
             sdkIntVersion = Build.VERSION_CODES.M,
             sessionVerificationService = sessionVerificationService,
-            coroutineScope = coroutineScope,
             analyticsService = analyticsService,
             lockScreenService = lockScreenService,
         )
@@ -182,14 +162,10 @@ class DefaultFtueServiceTest {
 
         analyticsService.setDidAskUserConsent()
         assertThat(service.getNextStep(null)).isNull()
-
-        // Cleanup
-        coroutineScope.cancel()
     }
 
     @Test
     fun `reset do the expected actions S`() = runTest {
-        val coroutineScope = CoroutineScope(coroutineContext + SupervisorJob())
         val resetAnalyticsLambda = lambdaRecorder<Unit> { }
         val analyticsService = FakeAnalyticsService(
             resetLambda = resetAnalyticsLambda
@@ -199,7 +175,6 @@ class DefaultFtueServiceTest {
             resetPermissionLambda = resetPermissionLambda
         )
         val service = createDefaultFtueService(
-            coroutineScope = coroutineScope,
             sdkIntVersion = Build.VERSION_CODES.S,
             permissionStateProvider = permissionStateProvider,
             analyticsService = analyticsService,
@@ -211,7 +186,6 @@ class DefaultFtueServiceTest {
 
     @Test
     fun `reset do the expected actions TIRAMISU`() = runTest {
-        val coroutineScope = CoroutineScope(coroutineContext + SupervisorJob())
         val resetLambda = lambdaRecorder<Unit> { }
         val analyticsService = FakeAnalyticsService(
             resetLambda = resetLambda
@@ -221,7 +195,6 @@ class DefaultFtueServiceTest {
             resetPermissionLambda = resetPermissionLambda
         )
         val service = createDefaultFtueService(
-            coroutineScope = coroutineScope,
             sdkIntVersion = Build.VERSION_CODES.TIRAMISU,
             permissionStateProvider = permissionStateProvider,
             analyticsService = analyticsService,
@@ -232,17 +205,16 @@ class DefaultFtueServiceTest {
             .with(value("android.permission.POST_NOTIFICATIONS"))
     }
 
-    private fun createDefaultFtueService(
-        coroutineScope: CoroutineScope,
-        sessionVerificationService: FakeSessionVerificationService = FakeSessionVerificationService(),
+    private fun TestScope.createDefaultFtueService(
+        sessionVerificationService: SessionVerificationService = FakeSessionVerificationService(),
         analyticsService: AnalyticsService = FakeAnalyticsService(),
-        permissionStateProvider: FakePermissionStateProvider = FakePermissionStateProvider(permissionGranted = false),
+        permissionStateProvider: PermissionStateProvider = FakePermissionStateProvider(permissionGranted = false),
         lockScreenService: LockScreenService = FakeLockScreenService(),
-        sessionPreferencesStore: InMemorySessionPreferencesStore = InMemorySessionPreferencesStore(),
+        sessionPreferencesStore: SessionPreferencesStore = InMemorySessionPreferencesStore(),
         // First version where notification permission is required
         sdkIntVersion: Int = Build.VERSION_CODES.TIRAMISU,
     ) = DefaultFtueService(
-        sessionCoroutineScope = coroutineScope,
+        sessionCoroutineScope = backgroundScope,
         sessionVerificationService = sessionVerificationService,
         sdkVersionProvider = FakeBuildVersionSdkIntProvider(sdkIntVersion),
         analyticsService = analyticsService,

features/ftue/impl/src/test/kotlin/io/element/android/features/ftue/impl/DefaultFtueServiceTest.kt

@@ -57,9 +57,10 @@ fun aLeaveRoomState(
     confirmation: LeaveRoomState.Confirmation = LeaveRoomState.Confirmation.Hidden,
     progress: LeaveRoomState.Progress = LeaveRoomState.Progress.Hidden,
     error: LeaveRoomState.Error = LeaveRoomState.Error.Hidden,
+    eventSink: (LeaveRoomEvent) -> Unit = {},
 ) = LeaveRoomState(
     confirmation = confirmation,
     progress = progress,
     error = error,
-    eventSink = {},
+    eventSink = eventSink,
 )