BIT-1646 Flip the order for the delete menu password prompt (#937)

This commit is contained in:
Oleg Semenenko 2024-02-01 01:55:08 -06:00 committed by Álison Fernandes
parent 267fd8d077
commit b800b77194
4 changed files with 163 additions and 103 deletions

View file

@ -67,13 +67,10 @@ fun VaultItemScreen(
val state by viewModel.stateFlow.collectAsStateWithLifecycle()
val context = LocalContext.current
val resources = context.resources
val confirmDeleteClickAction = remember(viewModel) {
{ viewModel.trySendAction(VaultItemAction.Common.ConfirmDeleteClick) }
}
val confirmRestoreAction = remember(viewModel) {
{ viewModel.trySendAction(VaultItemAction.Common.ConfirmRestoreClick) }
}
var pendingDeleteCipher by rememberSaveable { mutableStateOf(false) }
var pendingRestoreCipher by rememberSaveable { mutableStateOf(false) }
val fileChooserLauncher = intentManager.getActivityResultLauncher { activityResult ->
@ -138,26 +135,15 @@ fun VaultItemScreen(
)
}
},
onConfirmDeleteClick = remember {
{
viewModel.trySendAction(
VaultItemAction.Common.ConfirmDeleteClick,
)
}
},
)
if (pendingDeleteCipher) {
BitwardenTwoButtonDialog(
title = stringResource(id = R.string.delete),
message = state.deletionConfirmationText(),
confirmButtonText = stringResource(id = R.string.ok),
dismissButtonText = stringResource(id = R.string.cancel),
onConfirmClick = {
pendingDeleteCipher = false
confirmDeleteClickAction()
},
onDismissClick = {
pendingDeleteCipher = false
},
onDismissRequest = {
pendingDeleteCipher = false
},
)
}
if (pendingRestoreCipher) {
BitwardenTwoButtonDialog(
title = stringResource(id = R.string.restore),
@ -241,7 +227,13 @@ fun VaultItemScreen(
.takeIf { state.isCipherInCollection },
OverflowMenuItemData(
text = stringResource(id = R.string.delete),
onClick = { pendingDeleteCipher = true },
onClick = remember(viewModel) {
{
viewModel.trySendAction(
VaultItemAction.Common.DeleteClick,
)
}
},
),
),
)
@ -292,6 +284,7 @@ fun VaultItemScreen(
private fun VaultItemDialogs(
dialog: VaultItemState.DialogState?,
onDismissRequest: () -> Unit,
onConfirmDeleteClick: () -> Unit,
onSubmitMasterPassword: (masterPassword: String, action: PasswordRepromptAction) -> Unit,
) {
when (dialog) {
@ -314,6 +307,18 @@ private fun VaultItemDialogs(
)
}
is VaultItemState.DialogState.DeleteConfirmationPrompt -> {
BitwardenTwoButtonDialog(
title = stringResource(id = R.string.delete),
message = dialog.message.invoke(),
confirmButtonText = stringResource(id = R.string.ok),
dismissButtonText = stringResource(id = R.string.cancel),
onConfirmClick = onConfirmDeleteClick,
onDismissClick = onDismissRequest,
onDismissRequest = onDismissRequest,
)
}
null -> Unit
}
}

View file

@ -150,6 +150,7 @@ class VaultItemViewModel @Inject constructor(
is VaultItemAction.Common.CollectionsClick -> handleCollectionsClick()
is VaultItemAction.Common.ConfirmDeleteClick -> handleConfirmDeleteClick()
is VaultItemAction.Common.ConfirmRestoreClick -> handleConfirmRestoreClick()
is VaultItemAction.Common.DeleteClick -> handleDeleteClick()
}
}
@ -161,6 +162,29 @@ class VaultItemViewModel @Inject constructor(
mutableStateFlow.update { it.copy(dialog = null) }
}
private fun handleDeleteClick() {
onContent { content ->
if (content.common.requiresReprompt) {
mutableStateFlow.update {
it.copy(
dialog = VaultItemState.DialogState.MasterPasswordDialog(
action = PasswordRepromptAction.DeleteClick,
),
)
}
return@onContent
} else {
mutableStateFlow.update {
it.copy(
dialog = VaultItemState
.DialogState
.DeleteConfirmationPrompt(state.deletionConfirmationText),
)
}
}
}
}
private fun handleEditClick() {
onContent { content ->
if (content.common.requiresReprompt) {
@ -387,16 +411,6 @@ class VaultItemViewModel @Inject constructor(
private fun handleConfirmDeleteClick() {
onContent { content ->
if (content.common.requiresReprompt) {
mutableStateFlow.update {
it.copy(
dialog = VaultItemState.DialogState.MasterPasswordDialog(
action = PasswordRepromptAction.DeleteClick,
),
)
}
return@onContent
}
mutableStateFlow.update {
it.copy(
dialog = VaultItemState.DialogState.Loading(
@ -1224,6 +1238,14 @@ data class VaultItemState(
data class MasterPasswordDialog(
val action: PasswordRepromptAction,
) : DialogState()
/**
* Displays the dialog for deleting the item to the user.
*/
@Parcelize
data class DeleteConfirmationPrompt(
val message: Text,
) : DialogState()
}
}
@ -1310,6 +1332,11 @@ sealed class VaultItemAction {
*/
data object CloseClick : Common()
/**
* The user has clicked the delete button.
*/
data object DeleteClick : Common()
/**
* The user has confirmed to deleted the cipher.
*/
@ -1482,6 +1509,7 @@ sealed class VaultItemAction {
* Models actions that the [VaultItemViewModel] itself might send.
*/
sealed class Internal : VaultItemAction() {
/**
* Copies the given [value] to the clipboard.
*/
@ -1622,7 +1650,7 @@ sealed class PasswordRepromptAction : Parcelable {
@Parcelize
data object DeleteClick : PasswordRepromptAction() {
override val vaultItemAction: VaultItemAction
get() = VaultItemAction.Common.ConfirmDeleteClick
get() = VaultItemAction.Common.DeleteClick
}
/**

View file

@ -771,7 +771,8 @@ class VaultItemScreenTest : BaseComposeTest() {
}
@Test
fun `menu Delete option click should send show deletion confirmation dialog`() {
fun `menu Delete option click should be displayed`() {
// Confirm dropdown version of item is absent
composeTestRule
.onAllNodesWithText("Delete")
@ -785,52 +786,22 @@ class VaultItemScreenTest : BaseComposeTest() {
composeTestRule
.onAllNodesWithText("Delete")
.filterToOne(hasAnyAncestor(isPopup()))
.performClick()
composeTestRule
.onNodeWithText("Do you really want to send to the trash?")
.assertIsDisplayed()
}
@Test
fun `Delete dialog cancel click should hide deletion confirmation menu`() {
// Open the overflow menu
composeTestRule
.onNodeWithContentDescription("More")
.performClick()
// Click on the delete item in the dropdown
composeTestRule
.onAllNodesWithText("Delete")
.filterToOne(hasAnyAncestor(isPopup()))
.performClick()
composeTestRule
.onNodeWithText("Do you really want to send to the trash?")
.assertIsDisplayed()
composeTestRule
.onNodeWithText("Cancel")
.performClick()
composeTestRule
.onNodeWithText("Do you really want to send to the trash?")
.assertIsNotDisplayed()
}
@Test
fun `Delete dialog ok click should send ConfirmDeleteClick`() {
// Open the overflow menu
composeTestRule
.onNodeWithContentDescription("More")
.performClick()
// Click on the delete item in the dropdown
composeTestRule
.onAllNodesWithText("Delete")
.filterToOne(hasAnyAncestor(isPopup()))
.performClick()
mutableStateFlow.update {
it.copy(
dialog = VaultItemState
.DialogState
.DeleteConfirmationPrompt("TestText".asText()),
)
}
composeTestRule
.onNodeWithText("Do you really want to send to the trash?")
.onAllNodesWithText("Delete")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
@ -843,41 +814,25 @@ class VaultItemScreenTest : BaseComposeTest() {
}
@Test
fun `Delete dialog text should display according to state`() {
// Open the overflow menu
composeTestRule
.onNodeWithContentDescription("More")
.performClick()
// Click on the delete item in the dropdown
composeTestRule
.onAllNodesWithText("Delete")
.filterToOne(hasAnyAncestor(isPopup()))
.performClick()
composeTestRule
.onAllNodesWithText("Do you really want to send to the trash?")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
fun `Delete Confirmation dialog text should display according to state`() {
mutableStateFlow.update {
it.copy(
viewState = DEFAULT_LOGIN_VIEW_STATE.copy(
common = DEFAULT_COMMON.copy(
currentCipher = createMockCipherView(
number = 1,
isDeleted = true,
),
),
),
dialog = VaultItemState
.DialogState
.DeleteConfirmationPrompt("TestText".asText()),
)
}
composeTestRule
.onAllNodesWithText(
text = "Do you really want to permanently delete? This cannot be undone.",
)
.onAllNodesWithText("Delete")
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(dialog = null)
}
composeTestRule.assertNoDialogExists()
}
@Test

View file

@ -134,7 +134,7 @@ class VaultItemViewModelTest : BaseViewModelTest() {
}
@Test
fun `ConfirmDeleteClick should show password dialog when re-prompt is required`() =
fun `DeleteClick should show password dialog when re-prompt is required`() =
runTest {
val loginState = DEFAULT_STATE.copy(viewState = DEFAULT_VIEW_STATE)
val mockCipherView = mockk<CipherView> {
@ -149,7 +149,7 @@ class VaultItemViewModelTest : BaseViewModelTest() {
mutableAuthCodeItemFlow.value = DataState.Loaded(data = null)
assertEquals(loginState, viewModel.stateFlow.value)
viewModel.trySendAction(VaultItemAction.Common.ConfirmDeleteClick)
viewModel.trySendAction(VaultItemAction.Common.DeleteClick)
assertEquals(
loginState.copy(
dialog = VaultItemState.DialogState.MasterPasswordDialog(
@ -167,6 +167,78 @@ class VaultItemViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `DeleteClick should update state when re-prompt is not required`() =
runTest {
val loginState = DEFAULT_VIEW_STATE.copy(
common = DEFAULT_COMMON
.copy(requiresReprompt = false),
)
val mockCipherView = mockk<CipherView> {
every {
toViewState(
isPremiumUser = true,
totpCodeItemData = null,
)
} returns loginState
}
val expected = DEFAULT_STATE.copy(
viewState = DEFAULT_VIEW_STATE.copy(
common = DEFAULT_COMMON.copy(
requiresReprompt = false,
),
),
dialog = VaultItemState.DialogState.DeleteConfirmationPrompt(
R.string.do_you_really_want_to_soft_delete_cipher.asText(),
),
)
mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView)
mutableAuthCodeItemFlow.value = DataState.Loaded(data = null)
viewModel.trySendAction(VaultItemAction.Common.DeleteClick)
assertEquals(expected, viewModel.stateFlow.value)
}
@Suppress("MaxLineLength")
@Test
fun `DeleteClick should update state when re-prompt is not required and it is a hard delete`() =
runTest {
val loginState = DEFAULT_VIEW_STATE.copy(
common = DEFAULT_COMMON
.copy(
requiresReprompt = false,
currentCipher = DEFAULT_COMMON
.currentCipher
?.copy(deletedDate = Instant.MIN),
),
)
val mockCipherView = mockk<CipherView> {
every {
toViewState(
isPremiumUser = true,
totpCodeItemData = null,
)
} returns loginState
}
val expected = DEFAULT_STATE.copy(
viewState = loginState,
dialog = VaultItemState.DialogState.DeleteConfirmationPrompt(
R.string.do_you_really_want_to_permanently_delete_cipher.asText(),
),
)
mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView)
mutableAuthCodeItemFlow.value = DataState.Loaded(data = null)
viewModel.trySendAction(VaultItemAction.Common.DeleteClick)
assertEquals(expected, viewModel.stateFlow.value)
}
@Test
@Suppress("MaxLineLength")
fun `ConfirmDeleteClick with DeleteCipherResult Success should should ShowToast and NavigateBack`() =