From 4bc977d8dcc68fbb804bdcdceb2809bdd7e26a10 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 20 Feb 2024 10:23:48 +0100 Subject: [PATCH] Introduce SecurityBannerState to replace Boolean set. Also get the sessionVerificationService from the matrixClient, instead of injecting it separately. --- .../roomlist/impl/RoomListPresenter.kt | 32 +++++++++--------- .../features/roomlist/impl/RoomListState.kt | 9 +++-- .../roomlist/impl/RoomListStateProvider.kt | 7 ++-- .../features/roomlist/impl/RoomListView.kt | 32 +++++++++--------- .../roomlist/impl/RoomListPresenterTests.kt | 33 +++++++------------ .../android/samples/minimal/RoomListScreen.kt | 1 - 6 files changed, 55 insertions(+), 59 deletions(-) diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt index 3349eacc55..d5a09355cb 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt @@ -73,7 +73,6 @@ private const val EXTENDED_RANGE_SIZE = 40 class RoomListPresenter @Inject constructor( private val client: MatrixClient, - private val sessionVerificationService: SessionVerificationService, private val networkMonitor: NetworkMonitor, private val snackbarDispatcher: SnackbarDispatcher, private val inviteStateDataSource: InviteStateDataSource, @@ -87,6 +86,7 @@ class RoomListPresenter @Inject constructor( private val analyticsService: AnalyticsService, ) : Presenter { private val encryptionService: EncryptionService = client.encryptionService() + private val sessionVerificationService: SessionVerificationService = client.sessionVerificationService() @Composable override fun present(): RoomListState { @@ -108,22 +108,25 @@ class RoomListPresenter @Inject constructor( val isMigrating = migrationScreenPresenter.present().isMigrating - // Session verification status (unknown, not verified, verified) - val canVerifySession by sessionVerificationService.canVerifySessionFlow.collectAsState(initial = false) - var verificationPromptDismissed by rememberSaveable { mutableStateOf(false) } - // We combine both values to only display the prompt if the session is not verified and it wasn't dismissed - val displayVerificationPrompt by remember { - derivedStateOf { canVerifySession && !verificationPromptDismissed } - } + var securityBannerDismissed by rememberSaveable { mutableStateOf(false) } + val displayVerificationPrompt by sessionVerificationService.canVerifySessionFlow.collectAsState(initial = false) val recoveryState by encryptionService.recoveryStateStateFlow.collectAsState() val secureStorageFlag by featureFlagService.isFeatureEnabledFlow(FeatureFlags.SecureStorage) .collectAsState(initial = null) - var recoveryKeyPromptDismissed by rememberSaveable { mutableStateOf(false) } val displayRecoveryKeyPrompt by remember { derivedStateOf { secureStorageFlag == true && - recoveryState == RecoveryState.INCOMPLETE && - !recoveryKeyPromptDismissed + recoveryState == RecoveryState.INCOMPLETE + } + } + val securityBannerState by remember { + derivedStateOf { + when { + securityBannerDismissed -> SecurityBannerState.None + displayVerificationPrompt -> SecurityBannerState.SessionVerification + displayRecoveryKeyPrompt -> SecurityBannerState.RecoveryKeyConfirmation + else -> SecurityBannerState.None + } } } @@ -135,8 +138,8 @@ class RoomListPresenter @Inject constructor( fun handleEvents(event: RoomListEvents) { when (event) { is RoomListEvents.UpdateVisibleRange -> updateVisibleRange(event.range) - RoomListEvents.DismissRequestVerificationPrompt -> verificationPromptDismissed = true - RoomListEvents.DismissRecoveryKeyPrompt -> recoveryKeyPromptDismissed = true + RoomListEvents.DismissRequestVerificationPrompt -> securityBannerDismissed = true + RoomListEvents.DismissRecoveryKeyPrompt -> securityBannerDismissed = true RoomListEvents.ToggleSearchResults -> searchState.eventSink(RoomListSearchEvents.ToggleSearchVisibility) is RoomListEvents.ShowContextMenu -> { coroutineScope.showContextMenu(event, contextMenu) @@ -157,8 +160,7 @@ class RoomListPresenter @Inject constructor( matrixUser = matrixUser.value, showAvatarIndicator = showAvatarIndicator, roomList = roomList, - displayVerificationPrompt = displayVerificationPrompt, - displayRecoveryKeyPrompt = displayRecoveryKeyPrompt, + securityBannerState = securityBannerState, snackbarMessage = snackbarMessage, hasNetworkConnection = networkConnectionStatus == NetworkStatus.Online, invitesState = inviteStateDataSource.inviteState(), diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListState.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListState.kt index b89cb2a25c..076f6bb10e 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListState.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListState.kt @@ -31,8 +31,7 @@ data class RoomListState( val matrixUser: MatrixUser?, val showAvatarIndicator: Boolean, val roomList: AsyncData>, - val displayVerificationPrompt: Boolean, - val displayRecoveryKeyPrompt: Boolean, + val securityBannerState: SecurityBannerState, val hasNetworkConnection: Boolean, val snackbarMessage: SnackbarMessage?, val invitesState: InvitesState, @@ -62,3 +61,9 @@ enum class InvitesState { SeenInvites, NewInvites, } + +enum class SecurityBannerState { + None, + SessionVerification, + RecoveryKeyConfirmation, +} diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListStateProvider.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListStateProvider.kt index ed1a948400..e35e785e70 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListStateProvider.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListStateProvider.kt @@ -37,14 +37,14 @@ open class RoomListStateProvider : PreviewParameterProvider { override val values: Sequence get() = sequenceOf( aRoomListState(), - aRoomListState().copy(displayVerificationPrompt = true), + aRoomListState().copy(securityBannerState = SecurityBannerState.SessionVerification), aRoomListState().copy(snackbarMessage = SnackbarMessage(CommonStrings.common_verification_complete)), aRoomListState().copy(hasNetworkConnection = false), aRoomListState().copy(invitesState = InvitesState.SeenInvites), aRoomListState().copy(invitesState = InvitesState.NewInvites), aRoomListState().copy(contextMenu = aContextMenuShown(roomName = "A nice room name")), aRoomListState().copy(contextMenu = aContextMenuShown(isFavorite = true)), - aRoomListState().copy(displayRecoveryKeyPrompt = true), + aRoomListState().copy(securityBannerState = SecurityBannerState.RecoveryKeyConfirmation), aRoomListState().copy(roomList = AsyncData.Success(persistentListOf())), aRoomListState().copy(roomList = AsyncData.Loading(prevData = RoomListRoomSummaryFactory.createFakeList())), aRoomListState().copy(matrixUser = null, displayMigrationStatus = true), @@ -58,8 +58,7 @@ internal fun aRoomListState() = RoomListState( roomList = AsyncData.Success(aRoomListRoomSummaryList()), hasNetworkConnection = true, snackbarMessage = null, - displayVerificationPrompt = false, - displayRecoveryKeyPrompt = false, + securityBannerState = SecurityBannerState.None, invitesState = InvitesState.NoInvites, contextMenu = RoomListState.ContextMenu.Hidden, leaveRoomState = aLeaveRoomState(), diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt index ff956b0ec3..575ace8cf7 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt @@ -225,23 +225,25 @@ private fun RoomListContent( // FAB height is 56dp, bottom padding is 16dp, we add 8dp as extra margin -> 56+16+8 = 80 contentPadding = PaddingValues(bottom = 80.dp) ) { - when { - state.displayEmptyState -> Unit - state.displayVerificationPrompt -> { - item { - RequestVerificationHeader( - onVerifyClicked = onVerifyClicked, - onDismissClicked = { state.eventSink(RoomListEvents.DismissRequestVerificationPrompt) } - ) + if (state.displayEmptyState.not()) { + when (state.securityBannerState) { + SecurityBannerState.SessionVerification -> { + item { + RequestVerificationHeader( + onVerifyClicked = onVerifyClicked, + onDismissClicked = { state.eventSink(RoomListEvents.DismissRequestVerificationPrompt) } + ) + } } - } - state.displayRecoveryKeyPrompt -> { - item { - ConfirmRecoveryKeyBanner( - onContinueClicked = onOpenSettings, - onDismissClicked = { state.eventSink(RoomListEvents.DismissRecoveryKeyPrompt) } - ) + SecurityBannerState.RecoveryKeyConfirmation -> { + item { + ConfirmRecoveryKeyBanner( + onContinueClicked = onOpenSettings, + onDismissClicked = { state.eventSink(RoomListEvents.DismissRecoveryKeyPrompt) } + ) + } } + SecurityBannerState.None -> Unit } } diff --git a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt index 57afadcc64..c62d7a3adb 100644 --- a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt +++ b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/RoomListPresenterTests.kt @@ -55,8 +55,6 @@ import io.element.android.libraries.matrix.api.encryption.RecoveryState import io.element.android.libraries.matrix.api.room.RoomNotificationMode import io.element.android.libraries.matrix.api.roomlist.RoomListService import io.element.android.libraries.matrix.api.timeline.ReceiptType -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.AN_AVATAR_URL import io.element.android.libraries.matrix.test.AN_EXCEPTION import io.element.android.libraries.matrix.test.A_ROOM_ID @@ -114,13 +112,13 @@ class RoomListPresenterTests { fun `present - show avatar indicator`() = runTest { val scope = CoroutineScope(coroutineContext + SupervisorJob()) val encryptionService = FakeEncryptionService() + val sessionVerificationService = FakeSessionVerificationService() val matrixClient = FakeMatrixClient( encryptionService = encryptionService, + sessionVerificationService = sessionVerificationService, ) - val sessionVerificationService = FakeSessionVerificationService() val presenter = createRoomListPresenter( client = matrixClient, - sessionVerificationService = sessionVerificationService, coroutineScope = scope ) moleculeFlow(RecompositionMode.Immediate) { @@ -239,27 +237,17 @@ class RoomListPresenterTests { @Test fun `present - handle DismissRequestVerificationPrompt`() = runTest { - val roomListService = FakeRoomListService() - val matrixClient = FakeMatrixClient( - roomListService = roomListService, - ) val scope = CoroutineScope(context = coroutineContext + SupervisorJob()) val presenter = createRoomListPresenter( - client = matrixClient, - sessionVerificationService = FakeSessionVerificationService().apply { - givenIsReady(true) - givenVerifiedStatus(SessionVerifiedStatus.NotVerified) - }, coroutineScope = scope, ) moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { val eventSink = awaitItem().eventSink - assertThat(awaitItem().displayVerificationPrompt).isTrue() - + assertThat(awaitItem().securityBannerState).isEqualTo(SecurityBannerState.SessionVerification) eventSink(RoomListEvents.DismissRequestVerificationPrompt) - assertThat(awaitItem().displayVerificationPrompt).isFalse() + assertThat(awaitItem().securityBannerState).isEqualTo(SecurityBannerState.None) scope.cancel() } } @@ -269,6 +257,9 @@ class RoomListPresenterTests { val encryptionService = FakeEncryptionService() val matrixClient = FakeMatrixClient( encryptionService = encryptionService, + sessionVerificationService = FakeSessionVerificationService().apply { + givenCanVerifySession(false) + }, ) val scope = CoroutineScope(context = coroutineContext + SupervisorJob()) val presenter = createRoomListPresenter( @@ -280,13 +271,13 @@ class RoomListPresenterTests { }.test { skipItems(1) val initialState = awaitItem() - assertThat(initialState.displayRecoveryKeyPrompt).isFalse() + assertThat(initialState.securityBannerState).isEqualTo(SecurityBannerState.None) encryptionService.emitRecoveryState(RecoveryState.INCOMPLETE) val nextState = awaitItem() - assertThat(nextState.displayRecoveryKeyPrompt).isTrue() + assertThat(nextState.securityBannerState).isEqualTo(SecurityBannerState.RecoveryKeyConfirmation) nextState.eventSink(RoomListEvents.DismissRecoveryKeyPrompt) val finalState = awaitItem() - assertThat(finalState.displayRecoveryKeyPrompt).isFalse() + assertThat(finalState.securityBannerState).isEqualTo(SecurityBannerState.None) scope.cancel() } } @@ -579,7 +570,6 @@ class RoomListPresenterTests { private fun TestScope.createRoomListPresenter( client: MatrixClient = FakeMatrixClient(), - sessionVerificationService: SessionVerificationService = FakeSessionVerificationService(), networkMonitor: NetworkMonitor = FakeNetworkMonitor(), snackbarDispatcher: SnackbarDispatcher = SnackbarDispatcher(), inviteStateDataSource: InviteStateDataSource = FakeInviteDataSource(), @@ -599,7 +589,6 @@ class RoomListPresenterTests { searchPresenter: Presenter = Presenter { aRoomListSearchState() }, ) = RoomListPresenter( client = client, - sessionVerificationService = sessionVerificationService, networkMonitor = networkMonitor, snackbarDispatcher = snackbarDispatcher, inviteStateDataSource = inviteStateDataSource, @@ -616,7 +605,7 @@ class RoomListPresenterTests { ), featureFlagService = featureFlagService, indicatorService = DefaultIndicatorService( - sessionVerificationService = sessionVerificationService, + sessionVerificationService = client.sessionVerificationService(), encryptionService = client.encryptionService(), featureFlagService = featureFlagService, ), diff --git a/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt b/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt index 17e948a1a8..1d5b27f731 100644 --- a/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt +++ b/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt @@ -90,7 +90,6 @@ class RoomListScreen( ) private val presenter = RoomListPresenter( client = matrixClient, - sessionVerificationService = sessionVerificationService, networkMonitor = NetworkMonitorImpl(context, Singleton.appScope), snackbarDispatcher = SnackbarDispatcher(), inviteStateDataSource = DefaultInviteStateDataSource(matrixClient, DefaultSeenInvitesStore(context), coroutineDispatchers),