Skip to content
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

Ensure that Presenters do not depend on other presenters. #3618

Merged
merged 16 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -44,7 +38,6 @@ class RootPresenterTest {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
skipItems(1)
val initialState = awaitItem()
assertThat(initialState.crashDetectionState.crashDetected).isFalse()
}
Expand All @@ -61,7 +54,7 @@ class RootPresenterTest {
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
skipItems(2)
skipItems(1)
lambda.assertions().isCalledOnce()
}
}
Expand All @@ -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
Expand All @@ -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,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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>
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
Expand All @@ -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()
)
Expand All @@ -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()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -47,9 +49,6 @@ class DefaultFtueServiceTest {
sessionVerificationService.givenVerifiedStatus(SessionVerifiedStatus.NotVerified)
assertThat(awaitItem()).isEqualTo(FtueState.Incomplete)
}

// Cleanup
coroutineScope.cancel()
}

@Test
Expand All @@ -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,
Expand All @@ -75,9 +71,6 @@ class DefaultFtueServiceTest {
service.updateState()

assertThat(service.state.value).isEqualTo(FtueState.Complete)

// Cleanup
coroutineScope.cancel()
}

@Test
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
)
Expand All @@ -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
Expand All @@ -199,7 +175,6 @@ class DefaultFtueServiceTest {
resetPermissionLambda = resetPermissionLambda
)
val service = createDefaultFtueService(
coroutineScope = coroutineScope,
sdkIntVersion = Build.VERSION_CODES.S,
permissionStateProvider = permissionStateProvider,
analyticsService = analyticsService,
Expand All @@ -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
Expand All @@ -221,7 +195,6 @@ class DefaultFtueServiceTest {
resetPermissionLambda = resetPermissionLambda
)
val service = createDefaultFtueService(
coroutineScope = coroutineScope,
sdkIntVersion = Build.VERSION_CODES.TIRAMISU,
permissionStateProvider = permissionStateProvider,
analyticsService = analyticsService,
Expand All @@ -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,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Loading
Loading