From 917aaac3a60daf9a409e4744d3c0e7b8d23459ab Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 3 Jul 2025 14:09:13 -0500 Subject: [PATCH] PM-23354: Replace Login Approval toasts with snackbar (#5478) --- .../loginapproval/LoginApprovalViewModel.kt | 36 +++++++++++++------ .../pendingrequests/PendingRequestsScreen.kt | 8 +++++ .../PendingRequestsViewModel.kt | 34 ++++++++++++++++++ .../manager/snackbar/SnackbarRelay.kt | 1 + .../LoginApprovalViewModelTest.kt | 30 +++++++++++----- .../PendingRequestsScreenTest.kt | 12 +++++++ .../PendingRequestsViewModelTest.kt | 20 +++++++++++ 7 files changed, 121 insertions(+), 20 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/loginapproval/LoginApprovalViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/loginapproval/LoginApprovalViewModel.kt index bbf504b9a4..fd90250fc0 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/loginapproval/LoginApprovalViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/loginapproval/LoginApprovalViewModel.kt @@ -15,6 +15,9 @@ import com.x8bit.bitwarden.data.auth.manager.model.AuthRequestUpdatesResult import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance +import com.x8bit.bitwarden.ui.platform.components.snackbar.BitwardenSnackbarData +import com.x8bit.bitwarden.ui.platform.manager.snackbar.SnackbarRelay +import com.x8bit.bitwarden.ui.platform.manager.snackbar.SnackbarRelayManager import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map @@ -36,6 +39,7 @@ class LoginApprovalViewModel @Inject constructor( private val clock: Clock, private val authRepository: AuthRepository, private val specialCircumstanceManager: SpecialCircumstanceManager, + private val snackbarRelayManager: SnackbarRelayManager, savedStateHandle: SavedStateHandle, ) : BaseViewModel( initialState = savedStateHandle[KEY_STATE] @@ -182,8 +186,7 @@ class LoginApprovalViewModel @Inject constructor( ) { when (val result = action.result) { is AuthRequestResult.Success -> { - sendEvent(LoginApprovalEvent.ShowToast(R.string.login_approved.asText())) - sendClosingEvent() + sendClosingEvent(message = R.string.login_approved.asText()) } is AuthRequestResult.Error -> { @@ -246,8 +249,7 @@ class LoginApprovalViewModel @Inject constructor( ) { when (val result = action.result) { is AuthRequestResult.Success -> { - sendEvent(LoginApprovalEvent.ShowToast(R.string.log_in_denied.asText())) - sendClosingEvent() + sendClosingEvent(message = R.string.log_in_denied.asText()) } is AuthRequestResult.Error -> { @@ -264,14 +266,26 @@ class LoginApprovalViewModel @Inject constructor( } } - private fun sendClosingEvent() { - val event = if (state.specialCircumstance?.shouldFinishWhenComplete == true) { - LoginApprovalEvent.ExitApp - } else { - LoginApprovalEvent.NavigateBack + private fun sendClosingEvent(message: Text? = null) { + val shouldFinishWhenComplete = state.specialCircumstance?.shouldFinishWhenComplete == true + message?.let { + if (shouldFinishWhenComplete) { + // We are about to exit the app, so we need to use a Toast here. + sendEvent(LoginApprovalEvent.ShowToast(it)) + } else { + snackbarRelayManager.sendSnackbarData( + data = BitwardenSnackbarData(message = it), + relay = SnackbarRelay.LOGIN_APPROVAL, + ) + } } - - sendEvent(event) + sendEvent( + event = if (shouldFinishWhenComplete) { + LoginApprovalEvent.ExitApp + } else { + LoginApprovalEvent.NavigateBack + }, + ) } } diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsScreen.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsScreen.kt index f7c1666019..7e65679067 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsScreen.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsScreen.kt @@ -61,6 +61,8 @@ import com.x8bit.bitwarden.ui.platform.components.content.BitwardenLoadingConten import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog import com.x8bit.bitwarden.ui.platform.components.model.rememberBitwardenPullToRefreshState import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold +import com.x8bit.bitwarden.ui.platform.components.snackbar.BitwardenSnackbarHost +import com.x8bit.bitwarden.ui.platform.components.snackbar.rememberBitwardenSnackbarHostState import com.x8bit.bitwarden.ui.platform.composition.LocalPermissionsManager import com.x8bit.bitwarden.ui.platform.manager.permissions.PermissionsManager @@ -84,12 +86,15 @@ fun PendingRequestsScreen( { viewModel.trySendAction(PendingRequestsAction.RefreshPull) } }, ) + val snackbarHostState = rememberBitwardenSnackbarHostState() EventsEffect(viewModel = viewModel) { event -> when (event) { PendingRequestsEvent.NavigateBack -> onNavigateBack() is PendingRequestsEvent.NavigateToLoginApproval -> { onNavigateToLoginApproval(event.fingerprint) } + + is PendingRequestsEvent.ShowSnackbar -> snackbarHostState.showSnackbar(event.data) } } @@ -142,6 +147,9 @@ fun PendingRequestsScreen( ) }, pullToRefreshState = pullToRefreshState, + snackbarHost = { + BitwardenSnackbarHost(bitwardenHostState = snackbarHostState) + }, ) { when (val viewState = state.viewState) { is PendingRequestsState.ViewState.Content -> { diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsViewModel.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsViewModel.kt index 4402813051..1a64b62b73 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsViewModel.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsViewModel.kt @@ -5,11 +5,15 @@ import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope import com.bitwarden.core.data.util.toFormattedDateTimeStyle import com.bitwarden.core.util.isOverFiveMinutesOld +import com.bitwarden.ui.platform.base.BackgroundEvent import com.bitwarden.ui.platform.base.BaseViewModel import com.x8bit.bitwarden.data.auth.manager.model.AuthRequest import com.x8bit.bitwarden.data.auth.manager.model.AuthRequestsUpdatesResult import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository +import com.x8bit.bitwarden.ui.platform.components.snackbar.BitwardenSnackbarData +import com.x8bit.bitwarden.ui.platform.manager.snackbar.SnackbarRelay +import com.x8bit.bitwarden.ui.platform.manager.snackbar.SnackbarRelayManager import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.Job import kotlinx.coroutines.flow.launchIn @@ -32,6 +36,7 @@ private const val KEY_STATE = "state" class PendingRequestsViewModel @Inject constructor( private val clock: Clock, private val authRepository: AuthRepository, + snackbarRelayManager: SnackbarRelayManager, settingsRepository: SettingsRepository, savedStateHandle: SavedStateHandle, ) : BaseViewModel( @@ -52,6 +57,11 @@ class PendingRequestsViewModel @Inject constructor( .map { PendingRequestsAction.Internal.PullToRefreshEnableReceive(it) } .onEach(::sendAction) .launchIn(viewModelScope) + snackbarRelayManager + .getSnackbarDataFlow(SnackbarRelay.LOGIN_APPROVAL) + .map { PendingRequestsAction.Internal.SnackbarDataReceive(it) } + .onEach(::sendAction) + .launchIn(viewModelScope) } override fun handleAction(action: PendingRequestsAction) { @@ -117,6 +127,10 @@ class PendingRequestsViewModel @Inject constructor( handlePullToRefreshEnableReceive(action) } + is PendingRequestsAction.Internal.SnackbarDataReceive -> { + handleSnackbarDataReceive(action) + } + is PendingRequestsAction.Internal.AuthRequestsResultReceive -> { handleAuthRequestsResultReceived(action) } @@ -131,6 +145,12 @@ class PendingRequestsViewModel @Inject constructor( } } + private fun handleSnackbarDataReceive( + action: PendingRequestsAction.Internal.SnackbarDataReceive, + ) { + sendEvent(PendingRequestsEvent.ShowSnackbar(action.data)) + } + private fun handleAuthRequestsResultReceived( action: PendingRequestsAction.Internal.AuthRequestsResultReceive, ) { @@ -282,6 +302,13 @@ sealed class PendingRequestsEvent { data class NavigateToLoginApproval( val fingerprint: String, ) : PendingRequestsEvent() + + /** + * Show a snackbar to the user. + */ + data class ShowSnackbar( + val data: BitwardenSnackbarData, + ) : PendingRequestsEvent(), BackgroundEvent } /** @@ -332,6 +359,13 @@ sealed class PendingRequestsAction { val isPullToRefreshEnabled: Boolean, ) : Internal() + /** + * Indicates that a snackbar data was received. + */ + data class SnackbarDataReceive( + val data: BitwardenSnackbarData, + ) : Internal() + /** * Indicates that a new auth requests result has been received. */ diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/manager/snackbar/SnackbarRelay.kt b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/manager/snackbar/SnackbarRelay.kt index 1124239417..db89dc33f6 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/manager/snackbar/SnackbarRelay.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/ui/platform/manager/snackbar/SnackbarRelay.kt @@ -11,6 +11,7 @@ import kotlinx.serialization.Serializable enum class SnackbarRelay { CIPHER_DELETED, CIPHER_RESTORED, + LOGIN_APPROVAL, LOGINS_IMPORTED, SEND_DELETED, SEND_UPDATED, diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/loginapproval/LoginApprovalViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/loginapproval/LoginApprovalViewModelTest.kt index f81a303aa7..da4714291a 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/loginapproval/LoginApprovalViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/loginapproval/LoginApprovalViewModelTest.kt @@ -18,11 +18,16 @@ import com.x8bit.bitwarden.data.platform.manager.SpecialCircumstanceManager import com.x8bit.bitwarden.data.platform.manager.model.FirstTimeState import com.x8bit.bitwarden.data.platform.manager.model.PasswordlessRequestData import com.x8bit.bitwarden.data.platform.manager.model.SpecialCircumstance +import com.x8bit.bitwarden.ui.platform.components.snackbar.BitwardenSnackbarData +import com.x8bit.bitwarden.ui.platform.manager.snackbar.SnackbarRelay +import com.x8bit.bitwarden.ui.platform.manager.snackbar.SnackbarRelayManager import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every +import io.mockk.just import io.mockk.mockk import io.mockk.mockkStatic +import io.mockk.runs import io.mockk.unmockkStatic import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow @@ -55,6 +60,9 @@ class LoginApprovalViewModelTest : BaseViewModelTest() { coEvery { getAuthRequestByIdFlow(REQUEST_ID) } returns mutableAuthRequestSharedFlow every { userStateFlow } returns mutableUserStateFlow } + private val snackbarRelayManager: SnackbarRelayManager = mockk { + every { sendSnackbarData(data = any(), relay = SnackbarRelay.LOGIN_APPROVAL) } just runs + } @BeforeEach fun setup() { @@ -270,13 +278,15 @@ class LoginApprovalViewModelTest : BaseViewModelTest() { viewModel.eventFlow.test { viewModel.trySendAction(LoginApprovalAction.ApproveRequestClick) - assertEquals( - LoginApprovalEvent.ShowToast(R.string.login_approved.asText()), - awaitItem(), - ) assertEquals(LoginApprovalEvent.NavigateBack, awaitItem()) } + verify { + snackbarRelayManager.sendSnackbarData( + data = BitwardenSnackbarData(message = R.string.login_approved.asText()), + relay = SnackbarRelay.LOGIN_APPROVAL, + ) + } coVerify { mockAuthRepository.updateAuthRequest( requestId = REQUEST_ID, @@ -339,13 +349,14 @@ class LoginApprovalViewModelTest : BaseViewModelTest() { viewModel.eventFlow.test { viewModel.trySendAction(LoginApprovalAction.DeclineRequestClick) - assertEquals( - LoginApprovalEvent.ShowToast(R.string.log_in_denied.asText()), - awaitItem(), - ) assertEquals(LoginApprovalEvent.NavigateBack, awaitItem()) } - + verify { + snackbarRelayManager.sendSnackbarData( + data = BitwardenSnackbarData(message = R.string.log_in_denied.asText()), + relay = SnackbarRelay.LOGIN_APPROVAL, + ) + } coVerify { mockAuthRepository.updateAuthRequest( requestId = REQUEST_ID, @@ -429,6 +440,7 @@ class LoginApprovalViewModelTest : BaseViewModelTest() { clock = fixedClock, authRepository = mockAuthRepository, specialCircumstanceManager = mockSpecialCircumstanceManager, + snackbarRelayManager = snackbarRelayManager, savedStateHandle = SavedStateHandle().apply { set("state", state) every { toLoginApprovalArgs() } returns LoginApprovalArgs(fingerprint = FINGERPRINT) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsScreenTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsScreenTest.kt index b6432f577c..8d618fe66a 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsScreenTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsScreenTest.kt @@ -2,6 +2,7 @@ package com.x8bit.bitwarden.ui.platform.feature.settings.accountsecurity.pending import androidx.compose.ui.semantics.SemanticsActions import androidx.compose.ui.test.assert +import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.filterToOne import androidx.compose.ui.test.hasAnyAncestor import androidx.compose.ui.test.hasClickAction @@ -13,10 +14,12 @@ import androidx.compose.ui.test.performScrollTo import androidx.compose.ui.test.performSemanticsAction import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow import com.bitwarden.core.util.isBuildVersionAtLeast +import com.bitwarden.ui.util.asText import com.bitwarden.ui.util.assertNoDialogExists import com.x8bit.bitwarden.data.platform.util.isFdroid import com.x8bit.bitwarden.data.util.advanceTimeByAndRunCurrent import com.x8bit.bitwarden.ui.platform.base.BitwardenComposeTest +import com.x8bit.bitwarden.ui.platform.components.snackbar.BitwardenSnackbarData import com.x8bit.bitwarden.ui.platform.manager.permissions.FakePermissionManager import io.mockk.every import io.mockk.just @@ -72,6 +75,15 @@ class PendingRequestsScreenTest : BitwardenComposeTest() { unmockkStatic(::isBuildVersionAtLeast) } + @Test + fun `on ShowSnackbar should display snackbar content`() { + val message = "message" + val data = BitwardenSnackbarData(message = message.asText()) + composeTestRule.onNodeWithText(text = message).assertDoesNotExist() + mutableEventFlow.tryEmit(PendingRequestsEvent.ShowSnackbar(data = data)) + composeTestRule.onNodeWithText(text = message).assertIsDisplayed() + } + @Test fun `on NavigateBack should call onNavigateBack`() { mutableEventFlow.tryEmit(PendingRequestsEvent.NavigateBack) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsViewModelTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsViewModelTest.kt index f16b749c87..734df0c6ac 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsViewModelTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/pendingrequests/PendingRequestsViewModelTest.kt @@ -4,12 +4,15 @@ import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow import com.bitwarden.ui.platform.base.BaseViewModelTest +import com.bitwarden.ui.util.asText import com.x8bit.bitwarden.data.auth.manager.model.AuthRequest import com.x8bit.bitwarden.data.auth.manager.model.AuthRequestResult import com.x8bit.bitwarden.data.auth.manager.model.AuthRequestsResult import com.x8bit.bitwarden.data.auth.manager.model.AuthRequestsUpdatesResult import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository +import com.x8bit.bitwarden.ui.platform.components.snackbar.BitwardenSnackbarData +import com.x8bit.bitwarden.ui.platform.manager.snackbar.SnackbarRelayManager import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every @@ -40,6 +43,12 @@ class PendingRequestsViewModelTest : BaseViewModelTest() { private val settingsRepository = mockk { every { getPullToRefreshEnabledFlow() } returns mutablePullToRefreshStateFlow } + private val mutableSnackbarDataFlow = bufferedMutableSharedFlow() + private val snackbarRelayManager: SnackbarRelayManager = mockk { + every { + getSnackbarDataFlow(relay = any(), relays = anyVararg()) + } returns mutableSnackbarDataFlow + } @Test fun `init should call getAuthRequestsWithUpdates`() { @@ -52,6 +61,16 @@ class PendingRequestsViewModelTest : BaseViewModelTest() { } } + @Test + fun `when SnackbarRelay flow updates, snackbar is shown`() = runTest { + val viewModel = createViewModel() + val expectedSnackbarData = BitwardenSnackbarData(message = "test message".asText()) + viewModel.eventFlow.test { + mutableSnackbarDataFlow.tryEmit(expectedSnackbarData) + assertEquals(PendingRequestsEvent.ShowSnackbar(expectedSnackbarData), awaitItem()) + } + } + @Suppress("LongMethod") @Test fun `getPendingResults success with content should update state with some requests filtered`() { @@ -373,6 +392,7 @@ class PendingRequestsViewModelTest : BaseViewModelTest() { clock = fixedClock, authRepository = authRepository, settingsRepository = settingsRepository, + snackbarRelayManager = snackbarRelayManager, savedStateHandle = SavedStateHandle().apply { set("state", state) }, ) }