PM-18410: Remove cipher type dropdown from add item screen (#4743)

This commit is contained in:
David Perez
2025-02-18 14:27:36 -06:00
committed by GitHub
parent f2eb524da4
commit 063003b4aa
5 changed files with 1 additions and 411 deletions

View File

@@ -42,8 +42,6 @@ import kotlinx.collections.immutable.toImmutableList
fun CoachMarkScope<AddEditItemCoachMark>.VaultAddEditContent(
state: VaultAddEditState.ViewState.Content,
isAddItemMode: Boolean,
typeOptions: List<VaultAddEditState.ItemTypeOption>,
onTypeOptionClicked: (VaultAddEditState.ItemTypeOption) -> Unit,
commonTypeHandlers: VaultAddEditCommonHandlers,
loginItemTypeHandlers: VaultAddEditLoginTypeHandlers,
identityItemTypeHandlers: VaultAddEditIdentityTypeHandlers,
@@ -116,18 +114,6 @@ fun CoachMarkScope<AddEditItemCoachMark>.VaultAddEditContent(
.standardHorizontalMargin()
.padding(horizontal = 16.dp),
)
Spacer(modifier = Modifier.height(8.dp))
}
item {
TypeOptionsItem(
entries = typeOptions,
itemType = state.type,
onTypeOptionClicked = onTypeOptionClicked,
modifier = Modifier
.testTag("ItemTypePicker")
.standardHorizontalMargin(),
)
}
}
@@ -282,31 +268,6 @@ fun CoachMarkScope<AddEditItemCoachMark>.VaultAddEditContent(
}
}
@Composable
private fun TypeOptionsItem(
entries: List<VaultAddEditState.ItemTypeOption>,
itemType: VaultAddEditState.ViewState.Content.ItemType,
onTypeOptionClicked: (VaultAddEditState.ItemTypeOption) -> Unit,
modifier: Modifier = Modifier,
) {
val optionsWithStrings = entries.associateWith { stringResource(id = it.labelRes) }
BitwardenMultiSelectButton(
label = stringResource(id = R.string.type),
options = optionsWithStrings.values.toImmutableList(),
selectedOption = stringResource(id = itemType.itemTypeOption.labelRes),
onOptionSelected = { selectedOption ->
val selectedOptionId = optionsWithStrings
.entries
.first { it.value == selectedOption }
.key
onTypeOptionClicked(selectedOptionId)
},
cardStyle = CardStyle.Full,
modifier = modifier,
)
}
/**
* Enumerated values representing the coach mark items to be shown.
*/

View File

@@ -368,14 +368,6 @@ fun VaultAddEditScreen(
VaultAddEditContent(
state = viewState,
isAddItemMode = state.isAddItemMode,
typeOptions = state.supportedItemTypes,
onTypeOptionClicked = remember(viewModel) {
{
viewModel.trySendAction(
VaultAddEditAction.Common.TypeOptionSelect(it),
)
}
},
loginItemTypeHandlers = loginItemTypeHandlers,
commonTypeHandlers = commonTypeHandlers,
permissionsManager = permissionsManager,

View File

@@ -174,7 +174,6 @@ class VaultAddEditViewModel @Inject constructor(
// Set special conditions for autofill and fido2 save
shouldShowCloseButton = autofillSaveItem == null && fido2AttestationOptions == null,
shouldExitOnSave = shouldExitOnSave,
supportedItemTypes = getSupportedItemTypeOptions(),
shouldShowCoachMarkTour = false,
shouldClearSpecialCircumstance = autofillSelectionData == null,
)
@@ -268,7 +267,6 @@ class VaultAddEditViewModel @Inject constructor(
is VaultAddEditAction.Common.CloseClick -> handleCloseClick()
is VaultAddEditAction.Common.DismissDialog -> handleDismissDialog()
is VaultAddEditAction.Common.SaveClick -> handleSaveClick()
is VaultAddEditAction.Common.TypeOptionSelect -> handleTypeOptionSelect(action)
is VaultAddEditAction.Common.AddNewCustomFieldClick -> {
handleAddNewCustomFieldClick(action)
}
@@ -340,76 +338,6 @@ class VaultAddEditViewModel @Inject constructor(
}
}
private fun handleTypeOptionSelect(action: VaultAddEditAction.Common.TypeOptionSelect) {
when (action.typeOption) {
VaultAddEditState.ItemTypeOption.LOGIN -> handleSwitchToAddLoginItem()
VaultAddEditState.ItemTypeOption.CARD -> handleSwitchToAddCardItem()
VaultAddEditState.ItemTypeOption.IDENTITY -> handleSwitchToAddIdentityItem()
VaultAddEditState.ItemTypeOption.SECURE_NOTES -> handleSwitchToAddSecureNotesItem()
VaultAddEditState.ItemTypeOption.SSH_KEYS -> handleSwitchToSshKeyItem()
}
}
private fun handleSwitchToAddLoginItem() {
updateContent { currentContent ->
currentContent.copy(
common = currentContent.clearNonSharedData(),
type = currentContent.previousItemTypeOrDefault(
itemType = VaultAddEditState.ItemTypeOption.LOGIN,
),
previousItemTypes = currentContent.toUpdatedPreviousItemTypes(),
)
}
}
private fun handleSwitchToAddSecureNotesItem() {
updateContent { currentContent ->
currentContent.copy(
common = currentContent.clearNonSharedData(),
type = currentContent.previousItemTypeOrDefault(
itemType = VaultAddEditState.ItemTypeOption.SECURE_NOTES,
),
previousItemTypes = currentContent.toUpdatedPreviousItemTypes(),
)
}
}
private fun handleSwitchToAddCardItem() {
updateContent { currentContent ->
currentContent.copy(
common = currentContent.clearNonSharedData(),
type = currentContent.previousItemTypeOrDefault(
itemType = VaultAddEditState.ItemTypeOption.CARD,
),
previousItemTypes = currentContent.toUpdatedPreviousItemTypes(),
)
}
}
private fun handleSwitchToAddIdentityItem() {
updateContent { currentContent ->
currentContent.copy(
common = currentContent.clearNonSharedData(),
type = currentContent.previousItemTypeOrDefault(
itemType = VaultAddEditState.ItemTypeOption.IDENTITY,
),
previousItemTypes = currentContent.toUpdatedPreviousItemTypes(),
)
}
}
private fun handleSwitchToSshKeyItem() {
updateContent { currentContent ->
currentContent.copy(
common = currentContent.clearNonSharedData(),
type = currentContent.previousItemTypeOrDefault(
itemType = VaultAddEditState.ItemTypeOption.SSH_KEYS,
),
previousItemTypes = currentContent.toUpdatedPreviousItemTypes(),
)
}
}
@Suppress("LongMethod")
private fun handleSaveClick() = onContent { content ->
if (content.common.name.isBlank()) {
@@ -912,7 +840,6 @@ class VaultAddEditViewModel @Inject constructor(
sendEvent(VaultAddEditEvent.NavigateToTooltipUri)
}
@Suppress("MaxLineLength")
private fun handleCollectionSelect(
action: VaultAddEditAction.Common.CollectionSelect,
) {
@@ -1583,7 +1510,6 @@ class VaultAddEditViewModel @Inject constructor(
}
}
@Suppress("LongMethod")
private fun handleVaultDataReceive(action: VaultAddEditAction.Internal.VaultDataReceive) {
when (val vaultDataState = action.vaultData) {
is DataState.Error -> {
@@ -1962,47 +1888,6 @@ class VaultAddEditViewModel @Inject constructor(
}
}
private fun VaultAddEditState.ViewState.Content.clearNonSharedData():
VaultAddEditState.ViewState.Content.Common =
common.copy(
customFieldData = common.customFieldData
.filterNot { it is VaultAddEditState.Custom.LinkedField },
)
private fun VaultAddEditState.ViewState.Content.toUpdatedPreviousItemTypes():
Map<VaultAddEditState.ItemTypeOption, VaultAddEditState.ViewState.Content.ItemType> =
previousItemTypes
.toMutableMap()
.apply { set(type.itemTypeOption, type) }
private fun VaultAddEditState.ViewState.Content.previousItemTypeOrDefault(
itemType: VaultAddEditState.ItemTypeOption,
): VaultAddEditState.ViewState.Content.ItemType =
previousItemTypes.getOrDefault(
key = itemType,
defaultValue = when (itemType) {
VaultAddEditState.ItemTypeOption.LOGIN -> {
VaultAddEditState.ViewState.Content.ItemType.Login()
}
VaultAddEditState.ItemTypeOption.CARD -> {
VaultAddEditState.ViewState.Content.ItemType.Card()
}
VaultAddEditState.ItemTypeOption.IDENTITY -> {
VaultAddEditState.ViewState.Content.ItemType.Identity()
}
VaultAddEditState.ItemTypeOption.SECURE_NOTES -> {
VaultAddEditState.ViewState.Content.ItemType.SecureNotes
}
VaultAddEditState.ItemTypeOption.SSH_KEYS -> {
VaultAddEditState.ViewState.Content.ItemType.SshKey()
}
},
)
@Suppress("MaxLineLength")
private suspend fun VaultAddEditState.ViewState.Content.createCipherForAddAndCloneItemStates(): CreateCipherResult {
return common.selectedOwner?.collections
@@ -2059,7 +1944,6 @@ data class VaultAddEditState(
val viewState: ViewState,
val dialog: DialogState?,
val shouldShowCloseButton: Boolean = true,
val supportedItemTypes: List<ItemTypeOption>,
// Internal
val shouldExitOnSave: Boolean = false,
val shouldClearSpecialCircumstance: Boolean = true,
@@ -2727,15 +2611,6 @@ sealed class VaultAddEditAction {
*/
data object ConfirmOverwriteExistingPasskeyClick : Common()
/**
* Represents the action when a type option is selected.
*
* @property typeOption The selected type option.
*/
data class TypeOptionSelect(
val typeOption: VaultAddEditState.ItemTypeOption,
) : Common()
/**
* Fired when the name text input is changed.
*
@@ -3171,7 +3046,6 @@ sealed class VaultAddEditAction {
*
* @property expirationMonth The selected expiration month.
*/
@Suppress("MaxLineLength")
data class ExpirationMonthSelect(
val expirationMonth: VaultCardExpirationMonth,
) : CardType()
@@ -3292,11 +3166,3 @@ sealed class VaultAddEditAction {
) : Internal()
}
}
/**
* Returns a list of item type options that can be selected during item creation.
*
* TODO: [PM-10413] Allow SSH key creation when the SDK supports it.
*/
private fun getSupportedItemTypeOptions() = VaultAddEditState.ItemTypeOption.entries
.filter { it != VaultAddEditState.ItemTypeOption.SSH_KEYS }

View File

@@ -622,48 +622,6 @@ class VaultAddEditScreenTest : BaseComposeTest() {
composeTestRule.onAllNodes(isProgressBar).assertCountEquals(1)
}
@Test
fun `clicking a Type Option should send TypeOptionSelect action`() {
// Opens the menu
composeTestRule
.onNodeWithContentDescriptionAfterScroll(label = "Login. Type")
.performClick()
// Choose the option from the menu
composeTestRule
.onAllNodesWithText(text = "Login")
.onLast()
.performScrollTo()
.performClick()
verify {
viewModel.trySendAction(
VaultAddEditAction.Common.TypeOptionSelect(VaultAddEditState.ItemTypeOption.LOGIN),
)
}
}
@Test
fun `the Type Option field should display the text of the selected item type`() {
composeTestRule
.onNodeWithContentDescriptionAfterScroll(label = "Login. Type")
.assertIsDisplayed()
mutableStateFlow.update {
it.copy(
viewState = VaultAddEditState.ViewState.Content(
common = VaultAddEditState.ViewState.Content.Common(),
type = VaultAddEditState.ViewState.Content.ItemType.Card(),
isIndividualVaultDisabled = false,
),
)
}
composeTestRule
.onNodeWithContentDescriptionAfterScroll(label = "Card. Type")
.assertIsDisplayed()
}
@Test
fun `in ItemType_Login state the password should change according to state`() {
composeTestRule
@@ -671,7 +629,7 @@ class VaultAddEditScreenTest : BaseComposeTest() {
.assertTextEquals("Password", "")
.assertIsEnabled()
composeTestRule
.onNodeWithText("Check password for data breaches")
.onNodeWithTextAfterScroll("Check password for data breaches")
.assertIsDisplayed()
composeTestRule
.onNodeWithContentDescription("Generate password")
@@ -3926,7 +3884,6 @@ class VaultAddEditScreenTest : BaseComposeTest() {
),
dialog = VaultAddEditState.DialogState.Generic(message = "test".asText()),
vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.LOGIN),
supportedItemTypes = VaultAddEditState.ItemTypeOption.entries,
shouldShowCoachMarkTour = false,
)
@@ -3938,7 +3895,6 @@ class VaultAddEditScreenTest : BaseComposeTest() {
isIndividualVaultDisabled = false,
),
dialog = null,
supportedItemTypes = VaultAddEditState.ItemTypeOption.entries,
shouldShowCoachMarkTour = false,
)
@@ -3950,7 +3906,6 @@ class VaultAddEditScreenTest : BaseComposeTest() {
isIndividualVaultDisabled = false,
),
dialog = null,
supportedItemTypes = VaultAddEditState.ItemTypeOption.entries,
shouldShowCoachMarkTour = false,
)
@@ -3962,11 +3917,9 @@ class VaultAddEditScreenTest : BaseComposeTest() {
isIndividualVaultDisabled = false,
),
dialog = null,
supportedItemTypes = VaultAddEditState.ItemTypeOption.entries,
shouldShowCoachMarkTour = false,
)
@Suppress("MaxLineLength")
private val DEFAULT_STATE_SECURE_NOTES_CUSTOM_FIELDS = VaultAddEditState(
viewState = VaultAddEditState.ViewState.Content(
common = VaultAddEditState.ViewState.Content.Common(
@@ -3985,7 +3938,6 @@ class VaultAddEditScreenTest : BaseComposeTest() {
),
dialog = null,
vaultAddEditType = VaultAddEditType.AddItem(VaultItemCipherType.SECURE_NOTE),
supportedItemTypes = VaultAddEditState.ItemTypeOption.entries,
shouldShowCoachMarkTour = false,
)
@@ -3997,7 +3949,6 @@ class VaultAddEditScreenTest : BaseComposeTest() {
isIndividualVaultDisabled = false,
),
dialog = null,
supportedItemTypes = VaultAddEditState.ItemTypeOption.entries,
shouldShowCoachMarkTour = false,
)
@@ -4009,7 +3960,6 @@ class VaultAddEditScreenTest : BaseComposeTest() {
isIndividualVaultDisabled = false,
),
dialog = null,
supportedItemTypes = VaultAddEditState.ItemTypeOption.entries,
shouldShowCoachMarkTour = false,
)

View File

@@ -203,8 +203,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
totpData = null,
shouldShowCloseButton = true,
shouldExitOnSave = false,
supportedItemTypes = VaultAddEditState.ItemTypeOption.entries
.filter { it != VaultAddEditState.ItemTypeOption.SSH_KEYS },
shouldShowCoachMarkTour = false,
)
val viewModel = createAddVaultItemViewModel(
@@ -286,8 +284,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
type = VaultAddEditState.ViewState.Content.ItemType.Login(),
),
dialog = null,
supportedItemTypes = VaultAddEditState.ItemTypeOption.entries
.filter { it != VaultAddEditState.ItemTypeOption.SSH_KEYS },
shouldShowCoachMarkTour = false,
),
viewModel.stateFlow.value,
@@ -2121,156 +2117,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `TypeOptionSelect LOGIN should switch to LoginItem`() = runTest {
mutableVaultDataFlow.value = DataState.Loaded(
createVaultData(cipherView = createMockCipherView(1)),
)
val viewModel = createAddVaultItemViewModel()
val action = VaultAddEditAction.Common.TypeOptionSelect(
VaultAddEditState.ItemTypeOption.LOGIN,
)
viewModel.trySendAction(action)
val expectedState = loginInitialState.copy(
viewState = VaultAddEditState.ViewState.Content(
common = createCommonContentViewState(),
isIndividualVaultDisabled = false,
type = createLoginTypeContentViewState(),
previousItemTypes = mapOf(
VaultAddEditState.ItemTypeOption.LOGIN
to VaultAddEditState.ViewState.Content.ItemType.Login(),
),
),
)
assertEquals(
expectedState,
viewModel.stateFlow.value,
)
}
@Test
fun `TypeOptionSelect CARD should switch to CardItem`() = runTest {
mutableVaultDataFlow.value = DataState.Loaded(
createVaultData(cipherView = createMockCipherView(1)),
)
val viewModel = createAddVaultItemViewModel()
val action = VaultAddEditAction.Common.TypeOptionSelect(
VaultAddEditState.ItemTypeOption.CARD,
)
viewModel.trySendAction(action)
val expectedState = loginInitialState.copy(
viewState = VaultAddEditState.ViewState.Content(
common = createCommonContentViewState(),
isIndividualVaultDisabled = false,
type = VaultAddEditState.ViewState.Content.ItemType.Card(),
previousItemTypes = mapOf(
VaultAddEditState.ItemTypeOption.LOGIN
to VaultAddEditState.ViewState.Content.ItemType.Login(),
),
),
)
assertEquals(
expectedState,
viewModel.stateFlow.value,
)
}
@Test
fun `TypeOptionSelect IDENTITY should switch to IdentityItem`() = runTest {
mutableVaultDataFlow.value = DataState.Loaded(
createVaultData(cipherView = createMockCipherView(1)),
)
val viewModel = createAddVaultItemViewModel()
val action = VaultAddEditAction.Common.TypeOptionSelect(
VaultAddEditState.ItemTypeOption.IDENTITY,
)
viewModel.trySendAction(action)
val expectedState = loginInitialState.copy(
viewState = VaultAddEditState.ViewState.Content(
common = createCommonContentViewState(),
isIndividualVaultDisabled = false,
type = VaultAddEditState.ViewState.Content.ItemType.Identity(),
previousItemTypes = mapOf(
VaultAddEditState.ItemTypeOption.LOGIN
to VaultAddEditState.ViewState.Content.ItemType.Login(),
),
),
)
assertEquals(
expectedState,
viewModel.stateFlow.value,
)
}
@Test
fun `TypeOptionSelect SECURE_NOTES should switch to SecureNotesItem`() = runTest {
mutableVaultDataFlow.value = DataState.Loaded(
createVaultData(cipherView = createMockCipherView(1)),
)
val viewModel = createAddVaultItemViewModel()
val action = VaultAddEditAction.Common.TypeOptionSelect(
VaultAddEditState.ItemTypeOption.SECURE_NOTES,
)
viewModel.trySendAction(action)
val expectedState = loginInitialState.copy(
viewState = VaultAddEditState.ViewState.Content(
common = createCommonContentViewState(),
isIndividualVaultDisabled = false,
type = VaultAddEditState.ViewState.Content.ItemType.SecureNotes,
previousItemTypes = mapOf(
VaultAddEditState.ItemTypeOption.LOGIN
to VaultAddEditState.ViewState.Content.ItemType.Login(),
),
),
)
assertEquals(
expectedState,
viewModel.stateFlow.value,
)
}
@Test
fun `TypeOptionSelect SSH_KEYS should switch to SshKeysItem`() = runTest {
mutableVaultDataFlow.value = DataState.Loaded(
createVaultData(cipherView = createMockCipherView(1)),
)
val viewModel = createAddVaultItemViewModel()
val action = VaultAddEditAction.Common.TypeOptionSelect(
VaultAddEditState.ItemTypeOption.SSH_KEYS,
)
viewModel.trySendAction(action)
val expectedState = loginInitialState.copy(
viewState = VaultAddEditState.ViewState.Content(
common = createCommonContentViewState(),
isIndividualVaultDisabled = false,
type = VaultAddEditState.ViewState.Content.ItemType.SshKey(),
previousItemTypes = mapOf(
VaultAddEditState.ItemTypeOption.LOGIN
to VaultAddEditState.ViewState.Content.ItemType.Login(),
),
),
)
assertEquals(
expectedState,
viewModel.stateFlow.value,
)
}
@Nested
inner class VaultAddEditLoginTypeItemActions {
private lateinit var viewModel: VaultAddEditViewModel
@@ -2712,29 +2558,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
assertFalse(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins)
}
@Suppress("MaxLineLength")
@Test
fun `when first time action manager value is true, but type content is not login shouldShowLearnAboutNewLogins should be false`() {
mutableShouldShowAddLoginCoachMarkFlow.update { true }
val viewModel = createAddVaultItemViewModel(
savedStateHandle = createSavedStateHandleWithState(
state = createVaultAddItemState(
typeContentViewState = createLoginTypeContentViewState(),
),
vaultAddEditType = VaultAddEditType.AddItem(
vaultItemCipherType = VaultItemCipherType.LOGIN,
),
),
)
assertTrue(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins)
viewModel.trySendAction(
VaultAddEditAction.Common.TypeOptionSelect(
VaultAddEditState.ItemTypeOption.SSH_KEYS,
),
)
assertFalse(viewModel.stateFlow.value.shouldShowLearnAboutNewLogins)
}
@Suppress("MaxLineLength")
@Test
fun `when first time action manager value is false, but edit type is EditItem shouldShowLearnAboutNewLogins should be false`() {
@@ -4440,7 +4263,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
typeContentViewState: VaultAddEditState.ViewState.Content.ItemType = createLoginTypeContentViewState(),
dialogState: VaultAddEditState.DialogState? = null,
totpData: TotpData? = null,
supportedItemTypes: List<VaultAddEditState.ItemTypeOption> = VaultAddEditState.ItemTypeOption.entries,
shouldClearSpecialCircumstance: Boolean = true,
): VaultAddEditState =
VaultAddEditState(
@@ -4453,7 +4275,6 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
dialog = dialogState,
shouldExitOnSave = shouldExitOnSave,
totpData = totpData,
supportedItemTypes = supportedItemTypes,
shouldShowCoachMarkTour = false,
shouldClearSpecialCircumstance = shouldClearSpecialCircumstance,
)