From 7e0a14d3a0ebd1df9ad883fa17a0c41991642083 Mon Sep 17 00:00:00 2001 From: Brian Yencho Date: Thu, 11 Jan 2024 14:42:12 -0600 Subject: [PATCH] BIT-462: Add UI for custom vault timeout (#576) --- .../dialog/BitwardenTimePickerDialog.kt | 15 ++- .../accountsecurity/AccountSecurityScreen.kt | 65 +++++++++- .../AccountSecurityViewModel.kt | 44 +++++-- .../AccountSecurityScreenTest.kt | 114 +++++++++++++++++- .../AccountSecurityViewModelTest.kt | 32 ++++- 5 files changed, 252 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenTimePickerDialog.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenTimePickerDialog.kt index 006703085..cf1a1a59b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenTimePickerDialog.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/dialog/BitwardenTimePickerDialog.kt @@ -2,6 +2,7 @@ package com.x8bit.bitwarden.ui.platform.components.dialog import androidx.compose.foundation.background import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.ColumnScope import androidx.compose.foundation.layout.IntrinsicSize import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer @@ -45,6 +46,7 @@ import com.x8bit.bitwarden.R * with AM/PM. */ @OptIn(ExperimentalMaterial3Api::class) +@Suppress("LongMethod") @Composable fun BitwardenTimePickerDialog( initialHour: Int, @@ -99,10 +101,17 @@ fun BitwardenTimePickerDialog( } }, ) { + val modifier = Modifier.weight(1f) if (showTimeInput) { - TimeInput(state = timePickerState) + TimeInput( + state = timePickerState, + modifier = modifier, + ) } else { - TimePicker(state = timePickerState) + TimePicker( + state = timePickerState, + modifier = modifier, + ) } } } @@ -113,7 +122,7 @@ private fun TimePickerDialog( inputToggleButton: @Composable () -> Unit, dismissButton: @Composable () -> Unit, confirmButton: @Composable () -> Unit, - content: @Composable () -> Unit, + content: @Composable ColumnScope.() -> Unit, ) { Dialog( onDismissRequest = onDismissRequest, diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt index 8eb35689e..4a88e364b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt @@ -20,6 +20,7 @@ import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.input.nestedscroll.nestedScroll @@ -46,9 +47,14 @@ import com.x8bit.bitwarden.ui.platform.components.BitwardenTextRow import com.x8bit.bitwarden.ui.platform.components.BitwardenTopAppBar import com.x8bit.bitwarden.ui.platform.components.BitwardenTwoButtonDialog import com.x8bit.bitwarden.ui.platform.components.BitwardenWideSwitch +import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTimePickerDialog import com.x8bit.bitwarden.ui.platform.theme.LocalNonMaterialColors import com.x8bit.bitwarden.ui.platform.theme.LocalNonMaterialTypography import com.x8bit.bitwarden.ui.platform.util.displayLabel +import com.x8bit.bitwarden.ui.platform.util.toFormattedPattern +import java.time.LocalTime + +private const val MINUTES_PER_HOUR = 60 /** * Displays the account security screen. @@ -195,12 +201,25 @@ fun AccountSecurityScreen( .padding(horizontal = 16.dp), ) SessionTimeoutRow( - selectedVaultTimeoutType = state.vaultTimeoutType, + selectedVaultTimeoutType = state.vaultTimeout.type, onVaultTimeoutTypeSelect = remember(viewModel) { { viewModel.trySendAction(AccountSecurityAction.VaultTimeoutTypeSelect(it)) } }, modifier = Modifier.fillMaxWidth(), ) + (state.vaultTimeout as? VaultTimeout.Custom)?.let { customTimeout -> + SessionCustomTimeoutRow( + customVaultTimeout = customTimeout, + onCustomVaultTimeoutSelect = remember(viewModel) { + { + viewModel.trySendAction( + AccountSecurityAction.CustomVaultTimeoutSelect(it), + ) + } + }, + modifier = Modifier.fillMaxWidth(), + ) + } SessionTimeoutActionRow( selectedVaultTimeoutAction = state.vaultTimeoutAction, onVaultTimeoutActionSelect = remember(viewModel) { @@ -334,6 +353,50 @@ private fun SessionTimeoutRow( } } +@Suppress("LongMethod") +@Composable +private fun SessionCustomTimeoutRow( + customVaultTimeout: VaultTimeout.Custom, + onCustomVaultTimeoutSelect: (VaultTimeout.Custom) -> Unit, + modifier: Modifier = Modifier, +) { + var shouldShowTimePickerDialog by rememberSaveable { mutableStateOf(false) } + val vaultTimeoutInMinutes = customVaultTimeout.vaultTimeoutInMinutes + BitwardenTextRow( + text = stringResource(id = R.string.custom), + onClick = { shouldShowTimePickerDialog = true }, + modifier = modifier, + ) { + val formattedTime = LocalTime + .ofSecondOfDay( + vaultTimeoutInMinutes * MINUTES_PER_HOUR.toLong(), + ) + .toFormattedPattern("HH:mm") + Text( + text = formattedTime, + style = MaterialTheme.typography.labelSmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + + if (shouldShowTimePickerDialog) { + BitwardenTimePickerDialog( + initialHour = vaultTimeoutInMinutes / MINUTES_PER_HOUR, + initialMinute = vaultTimeoutInMinutes.mod(MINUTES_PER_HOUR), + onTimeSelect = { hour, minute -> + shouldShowTimePickerDialog = false + onCustomVaultTimeoutSelect( + VaultTimeout.Custom( + vaultTimeoutInMinutes = hour * MINUTES_PER_HOUR + minute, + ), + ) + }, + onDismissRequest = { shouldShowTimePickerDialog = false }, + is24Hour = true, + ) + } +} + @Suppress("LongMethod") @Composable private fun SessionTimeoutActionRow( diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModel.kt index c64302f4e..f4a9cf58a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModel.kt @@ -39,7 +39,7 @@ class AccountSecurityViewModel @Inject constructor( isApproveLoginRequestsEnabled = false, isUnlockWithBiometricsEnabled = false, isUnlockWithPinEnabled = false, - vaultTimeoutType = settingsRepository.vaultTimeout.type, + vaultTimeout = settingsRepository.vaultTimeout, vaultTimeoutAction = settingsRepository.vaultTimeoutAction, ), ) { @@ -63,6 +63,7 @@ class AccountSecurityViewModel @Inject constructor( AccountSecurityAction.LogoutClick -> handleLogoutClick() AccountSecurityAction.PendingLoginRequestsClick -> handlePendingLoginRequestsClick() is AccountSecurityAction.VaultTimeoutTypeSelect -> handleVaultTimeoutTypeSelect(action) + is AccountSecurityAction.CustomVaultTimeoutSelect -> handleCustomVaultTimeoutSelect(action) is AccountSecurityAction.VaultTimeoutActionSelect -> { handleVaultTimeoutActionSelect(action) } @@ -123,13 +124,8 @@ class AccountSecurityViewModel @Inject constructor( } private fun handleVaultTimeoutTypeSelect(action: AccountSecurityAction.VaultTimeoutTypeSelect) { - val vaultTimeoutType = action.vaultTimeoutType - mutableStateFlow.update { - it.copy( - vaultTimeoutType = action.vaultTimeoutType, - ) - } - val vaultTimeout = when (vaultTimeoutType) { + val previousTimeout = state.vaultTimeout + val vaultTimeout = when (action.vaultTimeoutType) { VaultTimeout.Type.IMMEDIATELY -> VaultTimeout.Immediately VaultTimeout.Type.ONE_MINUTE -> VaultTimeout.OneMinute VaultTimeout.Type.FIVE_MINUTES -> VaultTimeout.FiveMinutes @@ -139,7 +135,28 @@ class AccountSecurityViewModel @Inject constructor( VaultTimeout.Type.FOUR_HOURS -> VaultTimeout.FourHours VaultTimeout.Type.ON_APP_RESTART -> VaultTimeout.OnAppRestart VaultTimeout.Type.NEVER -> VaultTimeout.Never - VaultTimeout.Type.CUSTOM -> VaultTimeout.Custom(vaultTimeoutInMinutes = 0) + VaultTimeout.Type.CUSTOM -> { + if (previousTimeout is VaultTimeout.Custom) { + previousTimeout + } else { + VaultTimeout.Custom(vaultTimeoutInMinutes = 0) + } + } + } + handleVaultTimeoutSelect(vaultTimeout = vaultTimeout) + } + + private fun handleCustomVaultTimeoutSelect( + action: AccountSecurityAction.CustomVaultTimeoutSelect, + ) { + handleVaultTimeoutSelect(vaultTimeout = action.customVaultTimeout) + } + + private fun handleVaultTimeoutSelect(vaultTimeout: VaultTimeout) { + mutableStateFlow.update { + it.copy( + vaultTimeout = vaultTimeout, + ) } settingsRepository.vaultTimeout = vaultTimeout @@ -192,7 +209,7 @@ data class AccountSecurityState( val isApproveLoginRequestsEnabled: Boolean, val isUnlockWithBiometricsEnabled: Boolean, val isUnlockWithPinEnabled: Boolean, - val vaultTimeoutType: VaultTimeout.Type, + val vaultTimeout: VaultTimeout, val vaultTimeoutAction: VaultTimeoutAction, ) : Parcelable @@ -317,6 +334,13 @@ sealed class AccountSecurityAction { val vaultTimeoutType: VaultTimeout.Type, ) : AccountSecurityAction() + /** + * User selected an updated [VaultTimeout.Custom]. + */ + data class CustomVaultTimeoutSelect( + val customVaultTimeout: VaultTimeout.Custom, + ) : AccountSecurityAction() + /** * User selected a [VaultTimeoutAction]. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt index 202dca2f0..34769c811 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt @@ -8,6 +8,7 @@ import androidx.compose.ui.test.assertTextEquals import androidx.compose.ui.test.filterToOne import androidx.compose.ui.test.hasAnyAncestor import androidx.compose.ui.test.hasClickAction +import androidx.compose.ui.test.hasTextExactly import androidx.compose.ui.test.isDialog import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithContentDescription @@ -33,6 +34,7 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test +@Suppress("LargeClass") class AccountSecurityScreenTest : BaseComposeTest() { private var onNavigateBackCalled = false @@ -134,7 +136,7 @@ class AccountSecurityScreenTest : BaseComposeTest() { .filterToOne(hasClickAction()) .performScrollTo() .assertTextEquals("Session timeout", "30 minutes") - mutableStateFlow.update { it.copy(vaultTimeoutType = VaultTimeout.Type.FOUR_HOURS) } + mutableStateFlow.update { it.copy(vaultTimeout = VaultTimeout.FourHours) } composeTestRule .onAllNodesWithText("Session timeout") .filterToOne(hasClickAction()) @@ -345,6 +347,114 @@ class AccountSecurityScreenTest : BaseComposeTest() { composeTestRule.assertNoDialogExists() } + @Test + fun `custom session timeout should update according to state`() { + composeTestRule + .onNodeWithText("Custom") + .assertDoesNotExist() + + mutableStateFlow.update { + it.copy(vaultTimeout = VaultTimeout.Custom(vaultTimeoutInMinutes = 0)) + } + + composeTestRule + // Check for exact text to differentiate from the Custom label on the Vault Timeout + // item above. + .onNode(hasTextExactly("Custom", "00:00")) + .performScrollTo() + .assertIsDisplayed() + + mutableStateFlow.update { + it.copy(vaultTimeout = VaultTimeout.Custom(vaultTimeoutInMinutes = 123)) + } + + composeTestRule + .onNode(hasTextExactly("Custom", "02:03")) + .assertIsDisplayed() + + mutableStateFlow.update { + it.copy(vaultTimeout = VaultTimeout.Custom(vaultTimeoutInMinutes = 1234)) + } + + composeTestRule + .onNode(hasTextExactly("Custom", "20:34")) + .assertIsDisplayed() + } + + @Test + fun `custom session timeout click should show a time-picker dialog`() { + composeTestRule.assertNoDialogExists() + + mutableStateFlow.update { + it.copy(vaultTimeout = VaultTimeout.Custom(vaultTimeoutInMinutes = 123)) + } + composeTestRule + .onNode(hasTextExactly("Custom", "02:03")) + .performScrollTo() + .performClick() + + composeTestRule + .onAllNodesWithText("Time") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + composeTestRule + .onAllNodesWithText("Cancel") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + composeTestRule + .onAllNodesWithText("Ok") + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + } + + @Test + fun `custom session timeout dialog Cancel click should dismiss the dialog`() { + composeTestRule.assertNoDialogExists() + + mutableStateFlow.update { + it.copy(vaultTimeout = VaultTimeout.Custom(vaultTimeoutInMinutes = 123)) + } + composeTestRule + .onNode(hasTextExactly("Custom", "02:03")) + .performScrollTo() + .performClick() + + composeTestRule + .onAllNodesWithText("Cancel") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + composeTestRule.assertNoDialogExists() + } + + @Suppress("MaxLineLength") + @Test + fun `custom session timeout dialog Ok click should dismiss the dialog and send CustomVaultTimeoutSelect`() { + composeTestRule.assertNoDialogExists() + + mutableStateFlow.update { + it.copy(vaultTimeout = VaultTimeout.Custom(vaultTimeoutInMinutes = 123)) + } + composeTestRule + .onNode(hasTextExactly("Custom", "02:03")) + .performScrollTo() + .performClick() + + composeTestRule + .onAllNodesWithText("Ok") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + verify { + viewModel.trySendAction( + AccountSecurityAction.CustomVaultTimeoutSelect( + VaultTimeout.Custom(vaultTimeoutInMinutes = 123), + ), + ) + } + composeTestRule.assertNoDialogExists() + } + @Test fun `on session timeout action click should show a selection dialog`() { composeTestRule.assertNoDialogExists() @@ -598,7 +708,7 @@ class AccountSecurityScreenTest : BaseComposeTest() { isApproveLoginRequestsEnabled = false, isUnlockWithBiometricsEnabled = false, isUnlockWithPinEnabled = false, - vaultTimeoutType = VaultTimeout.Type.THIRTY_MINUTES, + vaultTimeout = VaultTimeout.ThirtyMinutes, vaultTimeoutAction = VaultTimeoutAction.LOCK, ) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModelTest.kt index 8e4cdf5d5..60ec61794 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModelTest.kt @@ -139,13 +139,41 @@ class AccountSecurityViewModelTest : BaseViewModelTest() { } assertEquals( DEFAULT_STATE.copy( - vaultTimeoutType = VaultTimeout.Type.FOUR_HOURS, + vaultTimeout = VaultTimeout.FourHours, ), viewModel.stateFlow.value, ) verify { settingsRepository.vaultTimeout = VaultTimeout.FourHours } } + @Test + fun `on CustomVaultTimeoutSelect should update the selection and emit ShowToast()`() = runTest { + val settingsRepository = mockk() { + every { vaultTimeout = any() } just runs + } + val viewModel = createViewModel(settingsRepository = settingsRepository) + viewModel.eventFlow.test { + viewModel.trySendAction( + AccountSecurityAction.CustomVaultTimeoutSelect( + customVaultTimeout = VaultTimeout.Custom(vaultTimeoutInMinutes = 360), + ), + ) + assertEquals( + AccountSecurityEvent.ShowToast("Not yet implemented.".asText()), + awaitItem(), + ) + } + assertEquals( + DEFAULT_STATE.copy( + vaultTimeout = VaultTimeout.Custom(vaultTimeoutInMinutes = 360), + ), + viewModel.stateFlow.value, + ) + verify { + settingsRepository.vaultTimeout = VaultTimeout.Custom(vaultTimeoutInMinutes = 360) + } + } + @Test fun `on VaultTimeoutActionSelect should update vault timeout action`() = runTest { val settingsRepository = mockk() { @@ -263,7 +291,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() { isApproveLoginRequestsEnabled = false, isUnlockWithBiometricsEnabled = false, isUnlockWithPinEnabled = false, - vaultTimeoutType = VaultTimeout.Type.THIRTY_MINUTES, + vaultTimeout = VaultTimeout.ThirtyMinutes, vaultTimeoutAction = VaultTimeoutAction.LOCK, ) }