From 624e60fd71fda1e33176abaa442d0507b13308bb Mon Sep 17 00:00:00 2001 From: Brian Yencho Date: Thu, 1 Feb 2024 02:04:06 -0600 Subject: [PATCH] Ensure more overflow action types get reprompts on Search (#940) --- .../platform/feature/search/SearchContent.kt | 52 ++++------- .../feature/search/SearchViewModel.kt | 84 ++++++----------- .../feature/search/SearchScreenTest.kt | 92 ++----------------- .../feature/search/SearchViewModelTest.kt | 50 ++-------- 4 files changed, 61 insertions(+), 217 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchContent.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchContent.kt index 05688f2fd..e2a4092a7 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchContent.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchContent.kt @@ -133,28 +133,13 @@ fun SearchContent( showConfirmationDialog = option } - is ListingItemOverflowAction.VaultAction.EditClick -> { - if (it.shouldDisplayMasterPasswordReprompt) { + is ListingItemOverflowAction.VaultAction -> { + if (option.requiresPasswordReprompt && + it.shouldDisplayMasterPasswordReprompt + ) { masterPasswordRepromptData = - MasterPasswordRepromptData( - cipherId = it.id, - type = MasterPasswordRepromptData.Type.Edit, - ) - } else { - searchHandlers.onOverflowItemClick(option) - } - } - - is ListingItemOverflowAction.VaultAction.CopyPasswordClick -> { - if (it.shouldDisplayMasterPasswordReprompt) { - masterPasswordRepromptData = - MasterPasswordRepromptData( - cipherId = it.id, - type = MasterPasswordRepromptData - .Type - .CopyPassword( - password = option.password, - ), + MasterPasswordRepromptData.OverflowItem( + action = option, ) } else { searchHandlers.onOverflowItemClick(option) @@ -195,24 +180,19 @@ private fun AutofillSelectionDialog( onMasterPasswordRepromptRequest: (MasterPasswordRepromptData) -> Unit, onDismissRequest: () -> Unit, ) { - val selectionCallback: (SearchState.DisplayItem, MasterPasswordRepromptData.Type) -> Unit = - { item, type -> + val selectionCallback: (SearchState.DisplayItem, MasterPasswordRepromptData) -> Unit = + { item, data -> onDismissRequest() if (item.shouldDisplayMasterPasswordReprompt) { - onMasterPasswordRepromptRequest( - MasterPasswordRepromptData( - cipherId = item.id, - type = type, - ), - ) + onMasterPasswordRepromptRequest(data) } else { - when (type) { - MasterPasswordRepromptData.Type.Autofill -> { - onAutofillItemClick(item.id) + when (data) { + is MasterPasswordRepromptData.Autofill -> { + onAutofillItemClick(data.cipherId) } - MasterPasswordRepromptData.Type.AutofillAndSave -> { - onAutofillAndSaveItemClick(item.id) + is MasterPasswordRepromptData.AutofillAndSave -> { + onAutofillAndSaveItemClick(data.cipherId) } else -> Unit @@ -229,7 +209,7 @@ private fun AutofillSelectionDialog( onClick = { selectionCallback( displayItem, - MasterPasswordRepromptData.Type.Autofill, + MasterPasswordRepromptData.Autofill(cipherId = displayItem.id), ) }, ) @@ -240,7 +220,7 @@ private fun AutofillSelectionDialog( onClick = { selectionCallback( displayItem, - MasterPasswordRepromptData.Type.AutofillAndSave, + MasterPasswordRepromptData.AutofillAndSave(cipherId = displayItem.id), ) }, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt index 2c0d03f7d..fee614e58 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModel.kt @@ -526,42 +526,27 @@ class SearchViewModel @Inject constructor( data: MasterPasswordRepromptData, ) { // Complete the deferred actions - val cipherId = data.cipherId - when (val type = data.type) { - MasterPasswordRepromptData.Type.Autofill -> { + when (data) { + is MasterPasswordRepromptData.Autofill -> { trySendAction( SearchAction.AutofillItemClick( - itemId = cipherId, + itemId = data.cipherId, ), ) } - MasterPasswordRepromptData.Type.AutofillAndSave -> { + is MasterPasswordRepromptData.AutofillAndSave -> { trySendAction( SearchAction.AutofillAndSaveItemClick( - itemId = cipherId, + itemId = data.cipherId, ), ) } - MasterPasswordRepromptData.Type.Edit -> { + is MasterPasswordRepromptData.OverflowItem -> { trySendAction( SearchAction.OverflowOptionClick( - overflowAction = ListingItemOverflowAction.VaultAction.EditClick( - cipherId = cipherId, - ), - ), - ) - } - - is MasterPasswordRepromptData.Type.CopyPassword -> { - trySendAction( - SearchAction.OverflowOptionClick( - overflowAction = ListingItemOverflowAction - .VaultAction - .CopyPasswordClick( - password = type.password, - ), + overflowAction = data.action, ), ) } @@ -1116,44 +1101,31 @@ sealed class SearchEvent { } /** - * Data tracking the type of request that triggered a master password reprompt during an autofill - * selection process. + * Data tracking the type of request that triggered a master password reprompt. */ -@Parcelize -data class MasterPasswordRepromptData( - val cipherId: String, - val type: Type, -) : Parcelable { +sealed class MasterPasswordRepromptData : Parcelable { /** - * The type of action that requires the prompt. + * Autofill was selected. */ - sealed class Type : Parcelable { + @Parcelize + data class Autofill( + val cipherId: String, + ) : MasterPasswordRepromptData() - /** - * Autofill was selected. - */ - @Parcelize - data object Autofill : Type() + /** + * Autofill-and-save was selected. + */ + @Parcelize + data class AutofillAndSave( + val cipherId: String, + ) : MasterPasswordRepromptData() - /** - * Autofill-and-save was selected. - */ - @Parcelize - data object AutofillAndSave : Type() - - /** - * Edit was selected. - */ - @Parcelize - data object Edit : Type() - - /** - * Copy password was selected. - */ - @Parcelize - data class CopyPassword( - val password: String, - ) : Type() - } + /** + * A cipher overflow menu item action was selected. + */ + @Parcelize + data class OverflowItem( + val action: ListingItemOverflowAction.VaultAction, + ) : MasterPasswordRepromptData() } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchScreenTest.kt index d88e7b962..1b16e1391 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchScreenTest.kt @@ -389,9 +389,8 @@ class SearchScreenTest : BaseComposeTest() { viewModel.trySendAction( SearchAction.MasterPasswordRepromptSubmit( password = "password", - masterPasswordRepromptData = MasterPasswordRepromptData( + masterPasswordRepromptData = MasterPasswordRepromptData.Autofill( cipherId = "mockId-1", - type = MasterPasswordRepromptData.Type.Autofill, ), ), ) @@ -425,9 +424,8 @@ class SearchScreenTest : BaseComposeTest() { viewModel.trySendAction( SearchAction.MasterPasswordRepromptSubmit( password = "password", - masterPasswordRepromptData = MasterPasswordRepromptData( + masterPasswordRepromptData = MasterPasswordRepromptData.AutofillAndSave( cipherId = "mockId-1", - type = MasterPasswordRepromptData.Type.AutofillAndSave, ), ), ) @@ -613,7 +611,7 @@ class SearchScreenTest : BaseComposeTest() { @Suppress("MaxLineLength") @Test - fun `on cipher item overflow edit click when reprompt required should show the master password dialog`() { + fun `on cipher item overflow item click when reprompt required should show the master password dialog`() { mutableStateFlow.update { it.copy( viewState = SearchState.ViewState.Content( @@ -642,7 +640,7 @@ class SearchScreenTest : BaseComposeTest() { @Suppress("MaxLineLength") @Test - fun `clicking submit on the master password dialog for edit should close the dialog and send MasterPasswordRepromptSubmit`() { + fun `clicking submit on the master password dialog for overflow item should close the dialog and send MasterPasswordRepromptSubmit`() { mutableStateFlow.update { it.copy( viewState = SearchState.ViewState.Content( @@ -676,85 +674,9 @@ class SearchScreenTest : BaseComposeTest() { viewModel.trySendAction( SearchAction.MasterPasswordRepromptSubmit( password = "password", - masterPasswordRepromptData = MasterPasswordRepromptData( - cipherId = "mockId-1", - type = MasterPasswordRepromptData.Type.Edit, - ), - ), - ) - } - composeTestRule.assertNoDialogExists() - } - - @Suppress("MaxLineLength") - @Test - fun `on cipher item overflow copy password click when reprompt required should show the master password dialog`() { - mutableStateFlow.update { - it.copy( - viewState = SearchState.ViewState.Content( - displayItems = listOf( - createMockDisplayItemForCipher(number = 1) - .copy(shouldDisplayMasterPasswordReprompt = true), - ), - ), - ) - } - composeTestRule - .onNodeWithContentDescription("Options") - .assertIsDisplayed() - .performClick() - - composeTestRule - .onNodeWithText("Copy password") - .assert(hasAnyAncestor(isDialog())) - .assertIsDisplayed() - .performClick() - - verify(exactly = 0) { viewModel.trySendAction(any()) } - - composeTestRule.assertMasterPasswordDialogDisplayed() - } - - @Suppress("MaxLineLength") - @Test - fun `clicking submit on the master password dialog for copy password should close the dialog and send MasterPasswordRepromptSubmit`() { - mutableStateFlow.update { - it.copy( - viewState = SearchState.ViewState.Content( - displayItems = listOf( - createMockDisplayItemForCipher(number = 1) - .copy(shouldDisplayMasterPasswordReprompt = true), - ), - ), - ) - } - composeTestRule - .onNodeWithContentDescription("Options") - .assertIsDisplayed() - .performClick() - composeTestRule - .onNodeWithText("Copy password") - .assert(hasAnyAncestor(isDialog())) - .assertIsDisplayed() - .performClick() - - composeTestRule - .onAllNodesWithText(text = "Master password") - .filterToOne(hasAnyAncestor(isDialog())) - .performTextInput("password") - composeTestRule - .onAllNodesWithText(text = "Submit") - .filterToOne(hasAnyAncestor(isDialog())) - .performClick() - - verify { - viewModel.trySendAction( - SearchAction.MasterPasswordRepromptSubmit( - password = "password", - masterPasswordRepromptData = MasterPasswordRepromptData( - cipherId = "mockId-1", - type = MasterPasswordRepromptData.Type.CopyPassword( - password = "mockPassword-1", + masterPasswordRepromptData = MasterPasswordRepromptData.OverflowItem( + action = ListingItemOverflowAction.VaultAction.EditClick( + cipherId = "mockId-1", ), ), ), diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt index 31b962d1d..6983a5c13 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/search/SearchViewModelTest.kt @@ -328,9 +328,8 @@ class SearchViewModelTest : BaseViewModelTest() { viewModel.trySendAction( SearchAction.MasterPasswordRepromptSubmit( password = password, - masterPasswordRepromptData = MasterPasswordRepromptData( + masterPasswordRepromptData = MasterPasswordRepromptData.Autofill( cipherId = cipherId, - type = MasterPasswordRepromptData.Type.Autofill, ), ), ) @@ -366,9 +365,8 @@ class SearchViewModelTest : BaseViewModelTest() { viewModel.trySendAction( SearchAction.MasterPasswordRepromptSubmit( password = password, - masterPasswordRepromptData = MasterPasswordRepromptData( + masterPasswordRepromptData = MasterPasswordRepromptData.Autofill( cipherId = cipherId, - type = MasterPasswordRepromptData.Type.Autofill, ), ), ) @@ -401,9 +399,8 @@ class SearchViewModelTest : BaseViewModelTest() { viewModel.trySendAction( SearchAction.MasterPasswordRepromptSubmit( password = password, - masterPasswordRepromptData = MasterPasswordRepromptData( + masterPasswordRepromptData = MasterPasswordRepromptData.Autofill( cipherId = cipherId, - type = MasterPasswordRepromptData.Type.Autofill, ), ), ) @@ -445,9 +442,8 @@ class SearchViewModelTest : BaseViewModelTest() { viewModel.trySendAction( SearchAction.MasterPasswordRepromptSubmit( password = password, - masterPasswordRepromptData = MasterPasswordRepromptData( + masterPasswordRepromptData = MasterPasswordRepromptData.AutofillAndSave( cipherId = cipherId, - type = MasterPasswordRepromptData.Type.AutofillAndSave, ), ), ) @@ -462,7 +458,7 @@ class SearchViewModelTest : BaseViewModelTest() { @Suppress("MaxLineLength") @Test - fun `MasterPasswordRepromptSubmit for a request Success with a valid password for edit should emit NavigateToEditCipher`() = + fun `MasterPasswordRepromptSubmit for a request Success with a valid password for an overflow action should perform the action`() = runTest { val cipherId = "cipherId-1234" val password = "password" @@ -475,44 +471,18 @@ class SearchViewModelTest : BaseViewModelTest() { viewModel.trySendAction( SearchAction.MasterPasswordRepromptSubmit( password = password, - masterPasswordRepromptData = MasterPasswordRepromptData( - cipherId = cipherId, - type = MasterPasswordRepromptData.Type.Edit, + masterPasswordRepromptData = MasterPasswordRepromptData.OverflowItem( + action = ListingItemOverflowAction.VaultAction.EditClick( + cipherId = cipherId, + ), ), ), ) + // Edit actions should navigate to the Edit screen assertEquals(SearchEvent.NavigateToEditCipher(cipherId), awaitItem()) } } - @Suppress("MaxLineLength") - @Test - fun `MasterPasswordRepromptSubmit for a request Success with a valid password for copy password should call setText on the ClipboardManager`() = - runTest { - val cipherId = "cipherId-1234" - val password = "password" - val viewModel = createViewModel() - coEvery { - authRepository.validatePassword(password = password) - } returns ValidatePasswordResult.Success(isValid = true) - - viewModel.trySendAction( - SearchAction.MasterPasswordRepromptSubmit( - password = password, - masterPasswordRepromptData = MasterPasswordRepromptData( - cipherId = cipherId, - type = MasterPasswordRepromptData.Type.CopyPassword( - password = password, - ), - ), - ), - ) - - verify(exactly = 1) { - clipboardManager.setText(password) - } - } - @Test fun `OverflowOptionClick Send EditClick should emit NavigateToEditSend`() = runTest { val sendId = "sendId"