From 3d5f27584f31f4b095c42e8fe74a9c9144b448bc Mon Sep 17 00:00:00 2001 From: Ramsey Smith <142836716+ramsey-livefront@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:07:45 -0700 Subject: [PATCH] BIT-506: Hard delete (#910) --- .../vault/datasource/disk/dao/CiphersDao.kt | 2 +- .../ui/vault/feature/item/VaultItemScreen.kt | 3 +- .../vault/feature/item/VaultItemViewModel.kt | 41 +++++++++++++--- .../vault/feature/item/VaultItemScreenTest.kt | 38 +++++++++++++++ .../feature/item/VaultItemViewModelTest.kt | 47 +++++++++++++++++++ 5 files changed, 122 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CiphersDao.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CiphersDao.kt index b724f1d623..698267e087 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CiphersDao.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CiphersDao.kt @@ -40,7 +40,7 @@ interface CiphersDao { * Deletes the specified cipher associated with the given [userId] and [cipherId]. This will * return the number of rows deleted by this query. */ - @Query("DELETE FROM sends WHERE user_id = :userId AND id = :cipherId") + @Query("DELETE FROM ciphers WHERE user_id = :userId AND id = :cipherId") suspend fun deleteCipher(userId: String, cipherId: String): Int /** diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt index bac5ee7030..7019507453 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreen.kt @@ -127,7 +127,7 @@ fun VaultItemScreen( if (pendingDeleteCipher) { BitwardenTwoButtonDialog( title = stringResource(id = R.string.delete), - message = stringResource(id = R.string.do_you_really_want_to_soft_delete_cipher), + message = state.deletionConfirmationText(), confirmButtonText = stringResource(id = R.string.ok), dismissButtonText = stringResource(id = R.string.cancel), onConfirmClick = { @@ -182,7 +182,6 @@ fun VaultItemScreen( onClick = { pendingRestoreCipher = true }, ) } - // TODO make action list dependent on item being in an organization BIT-1446 BitwardenOverflowActionItem( menuItemDataList = persistentListOfNotNull( OverflowMenuItemData( diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt index 13d5fc773e..d533b7a3b0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModel.kt @@ -297,7 +297,11 @@ class VaultItemViewModel @Inject constructor( mutableStateFlow.update { it.copy( dialog = VaultItemState.DialogState.Loading( - R.string.soft_deleting.asText(), + if (state.isCipherDeleted) { + R.string.deleting.asText() + } else { + R.string.soft_deleting.asText() + }, ), ) } @@ -305,10 +309,16 @@ class VaultItemViewModel @Inject constructor( viewModelScope.launch { trySendAction( VaultItemAction.Internal.DeleteCipherReceive( - result = vaultRepository.softDeleteCipher( - cipherId = state.vaultItemId, - cipherView = cipher, - ), + if (state.isCipherDeleted) { + vaultRepository.hardDeleteCipher( + cipherId = state.vaultItemId, + ) + } else { + vaultRepository.softDeleteCipher( + cipherId = state.vaultItemId, + cipherView = cipher, + ) + }, ), ) } @@ -693,7 +703,15 @@ class VaultItemViewModel @Inject constructor( DeleteCipherResult.Success -> { mutableStateFlow.update { it.copy(dialog = null) } - sendEvent(VaultItemEvent.ShowToast(message = R.string.item_soft_deleted.asText())) + sendEvent( + VaultItemEvent.ShowToast( + message = if (state.isCipherDeleted) { + R.string.item_deleted.asText() + } else { + R.string.item_soft_deleted.asText() + }, + ), + ) sendEvent(VaultItemEvent.NavigateBack) } } @@ -794,6 +812,17 @@ data class VaultItemState( ?.isNotEmpty() ?: false + /** + * The text to display on the deletion confirmation dialog. + */ + val deletionConfirmationText: Text + get() = if (isCipherDeleted) { + R.string.do_you_really_want_to_permanently_delete_cipher + } else { + R.string.do_you_really_want_to_soft_delete_cipher + } + .asText() + /** * Represents the specific view states for the [VaultItemScreen]. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt index b864ed4c9e..f747107135 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemScreenTest.kt @@ -660,6 +660,44 @@ 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() + + mutableStateFlow.update { + it.copy( + viewState = DEFAULT_LOGIN_VIEW_STATE.copy( + common = DEFAULT_COMMON.copy( + currentCipher = createMockCipherView( + number = 1, + isDeleted = true, + ), + ), + ), + ) + } + + composeTestRule + .onAllNodesWithText( + text = "Do you really want to permanently delete? This cannot be undone.", + ) + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + } + @Test fun `Restore click should send show restore confirmation dialog`() { mutableStateFlow.update { diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt index dba400be44..395a2f2536 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/vault/feature/item/VaultItemViewModelTest.kt @@ -42,6 +42,7 @@ import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test +import java.time.Instant @Suppress("LargeClass") class VaultItemViewModelTest : BaseViewModelTest() { @@ -240,6 +241,52 @@ class VaultItemViewModelTest : BaseViewModelTest() { ) } + @Test + fun `ConfirmDeleteClick with deleted cipher should should invoke hardDeleteCipher`() = + runTest { + val loginViewState = DEFAULT_VIEW_STATE.copy( + common = DEFAULT_COMMON + .copy( + requiresReprompt = false, + currentCipher = DEFAULT_COMMON + .currentCipher + ?.copy(deletedDate = Instant.MIN), + ), + ) + val mockCipherView = mockk { + every { + toViewState( + isPremiumUser = true, + totpCodeItemData = createTotpCodeData(), + ) + } returns loginViewState + } + mutableVaultItemFlow.value = DataState.Loaded(data = mockCipherView) + mutableAuthCodeItemFlow.value = + DataState.Loaded(data = createVerificationCodeItem()) + + val viewModel = createViewModel(state = DEFAULT_STATE) + coEvery { + vaultRepo.hardDeleteCipher( + cipherId = VAULT_ITEM_ID, + ) + } returns DeleteCipherResult.Success + + viewModel.trySendAction(VaultItemAction.Common.ConfirmDeleteClick) + + viewModel.eventFlow.test { + assertEquals( + VaultItemEvent.ShowToast(R.string.item_deleted.asText()), + awaitItem(), + ) + assertEquals( + VaultItemEvent.NavigateBack, + awaitItem(), + ) + } + coVerify { vaultRepo.hardDeleteCipher(cipherId = VAULT_ITEM_ID) } + } + @Test @Suppress("MaxLineLength") fun `ConfirmRestoreClick with RestoreCipherResult Success should should ShowToast and NavigateBack`() =