Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-11303] Add button missing for folders #4250

Merged
merged 4 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,11 @@ fun NavGraphBuilder.vaultUnlockedGraph(
vaultItemListingDestinationAsRoot(
onNavigateBack = { navController.popBackStack() },
onNavigateToVaultItemScreen = { navController.navigateToVaultItem(vaultItemId = it) },
onNavigateToVaultAddItemScreen = {
navController.navigateToVaultAddEdit(VaultAddEditType.AddItem(it))
onNavigateToVaultAddItemScreen = { cipherType, selectedFolderId ->
navController.navigateToVaultAddEdit(
VaultAddEditType.AddItem(cipherType),
selectedFolderId,
)
},
onNavigateToSearchVault = { navController.navigateToSearch(searchType = it) },
onNavigateToVaultEditItemScreen = {
Expand All @@ -86,8 +89,11 @@ fun NavGraphBuilder.vaultUnlockedGraph(
vaultUnlockedNavBarDestination(
onNavigateToExportVault = { navController.navigateToExportVault() },
onNavigateToFolders = { navController.navigateToFolders() },
onNavigateToVaultAddItem = {
navController.navigateToVaultAddEdit(VaultAddEditType.AddItem(it))
onNavigateToVaultAddItem = { cipherType, selectedFolderId ->
navController.navigateToVaultAddEdit(
VaultAddEditType.AddItem(cipherType),
selectedFolderId,
)
},
onNavigateToVaultItem = { navController.navigateToVaultItem(it) },
onNavigateToVaultEditItem = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fun NavController.navigateToVaultUnlockedNavBar(navOptions: NavOptions? = null)
*/
@Suppress("LongParameterList")
fun NavGraphBuilder.vaultUnlockedNavBarDestination(
onNavigateToVaultAddItem: (VaultItemCipherType) -> Unit,
onNavigateToVaultAddItem: (VaultItemCipherType, String?) -> Unit,
onNavigateToVaultItem: (vaultItemId: String) -> Unit,
onNavigateToVaultEditItem: (vaultItemId: String) -> Unit,
onNavigateToSearchSend: (searchType: SearchType.Sends) -> Unit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import com.x8bit.bitwarden.ui.vault.model.VaultItemCipherType
fun VaultUnlockedNavBarScreen(
viewModel: VaultUnlockedNavBarViewModel = hiltViewModel(),
navController: NavHostController = rememberNavController(),
onNavigateToVaultAddItem: (VaultItemCipherType) -> Unit,
onNavigateToVaultAddItem: (VaultItemCipherType, String?) -> Unit,
onNavigateToVaultItem: (vaultItemId: String) -> Unit,
onNavigateToVaultEditItem: (vaultItemId: String) -> Unit,
onNavigateToSearchSend: (searchType: SearchType.Sends) -> Unit,
Expand Down Expand Up @@ -158,7 +158,7 @@ private fun VaultUnlockedNavBarScaffold(
sendTabClickedAction: () -> Unit,
generatorTabClickedAction: () -> Unit,
settingsTabClickedAction: () -> Unit,
navigateToVaultAddItem: (VaultItemCipherType) -> Unit,
navigateToVaultAddItem: (VaultItemCipherType, String?) -> Unit,
onNavigateToVaultItem: (vaultItemId: String) -> Unit,
onNavigateToVaultEditItem: (vaultItemId: String) -> Unit,
onNavigateToSearchSend: (searchType: SearchType.Sends) -> Unit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,22 @@ private const val ADD_ITEM_TYPE: String = "vault_add_item_type"

private const val ADD_EDIT_ITEM_PREFIX: String = "vault_add_edit_item"
private const val ADD_EDIT_ITEM_TYPE: String = "vault_add_edit_type"
private const val ADD_SELECTED_FOLDER_ID: String = "vault_add_selected_folder_id"

private const val ADD_EDIT_ITEM_ROUTE: String =
ADD_EDIT_ITEM_PREFIX +
"/{$ADD_EDIT_ITEM_TYPE}" +
"?$EDIT_ITEM_ID={$EDIT_ITEM_ID}" +
"?$ADD_ITEM_TYPE={$ADD_ITEM_TYPE}"
"?$ADD_ITEM_TYPE={$ADD_ITEM_TYPE}" +
"?$ADD_SELECTED_FOLDER_ID={$ADD_SELECTED_FOLDER_ID}"

/**
* Class to retrieve vault add & edit arguments from the [SavedStateHandle].
*/
@OmitFromCoverage
data class VaultAddEditArgs(
val vaultAddEditType: VaultAddEditType,
val selectedFolderId: String? = null,
) {
constructor(savedStateHandle: SavedStateHandle) : this(
vaultAddEditType = when (requireNotNull(savedStateHandle[ADD_EDIT_ITEM_TYPE])) {
Expand All @@ -53,6 +56,7 @@ data class VaultAddEditArgs(
CLONE_TYPE -> VaultAddEditType.CloneItem(requireNotNull(savedStateHandle[EDIT_ITEM_ID]))
else -> throw IllegalStateException("Unknown VaultAddEditType.")
},
selectedFolderId = savedStateHandle[ADD_SELECTED_FOLDER_ID],
)
}

Expand All @@ -72,6 +76,10 @@ fun NavGraphBuilder.vaultAddEditDestination(
route = ADD_EDIT_ITEM_ROUTE,
arguments = listOf(
navArgument(ADD_EDIT_ITEM_TYPE) { type = NavType.StringType },
navArgument(ADD_SELECTED_FOLDER_ID) {
type = NavType.StringType
nullable = true
},
),
) {
VaultAddEditScreen(
Expand All @@ -90,12 +98,14 @@ fun NavGraphBuilder.vaultAddEditDestination(
*/
fun NavController.navigateToVaultAddEdit(
vaultAddEditType: VaultAddEditType,
selectedFolderId: String? = null,
navOptions: NavOptions? = null,
) {
navigate(
route = "$ADD_EDIT_ITEM_PREFIX/${vaultAddEditType.toTypeString()}" +
"?$EDIT_ITEM_ID=${vaultAddEditType.toIdOrNull()}" +
"?$ADD_ITEM_TYPE=${vaultAddEditType.toVaultItemCipherTypeOrNull()}",
"?$ADD_ITEM_TYPE=${vaultAddEditType.toVaultItemCipherTypeOrNull()}" +
"?$ADD_SELECTED_FOLDER_ID=$selectedFolderId",
navOptions = navOptions,
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ class VaultAddEditViewModel @Inject constructor(
initialState = savedStateHandle[KEY_STATE]
?: run {
val vaultAddEditType = VaultAddEditArgs(savedStateHandle).vaultAddEditType
val selectedFolderId = VaultAddEditArgs(savedStateHandle).selectedFolderId
val isIndividualVaultDisabled = policyManager
.getActivePolicies(type = PolicyTypeJson.PERSONAL_OWNERSHIP)
.any()
Expand Down Expand Up @@ -151,7 +152,9 @@ class VaultAddEditViewModel @Inject constructor(
)
?: totpData?.toDefaultAddTypeContent(isIndividualVaultDisabled)
?: VaultAddEditState.ViewState.Content(
common = VaultAddEditState.ViewState.Content.Common(),
common = VaultAddEditState.ViewState.Content.Common(
selectedFolderId = selectedFolderId,
),
isIndividualVaultDisabled = isIndividualVaultDisabled,
type = vaultAddEditType.vaultItemCipherType.toItemType(),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ fun VaultAddEditState.ViewState.appendFolderAndOwnerData(
common = currentContentState.common.copy(
selectedFolderId = folderViewList.toSelectedFolderId(
cipherView = currentContentState.common.originalCipher,
),
)
?: currentContentState.common.selectedFolderId,
availableFolders = folderViewList.toAvailableFolders(
resourceManager = resourceManager,
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ fun NavGraphBuilder.vaultItemListingDestination(
onNavigateToVaultItemScreen: (id: String) -> Unit,
onNavigateToVaultEditItemScreen: (cipherId: String) -> Unit,
onNavigateToVaultItemListing: (vaultItemListingType: VaultItemListingType) -> Unit,
onNavigateToVaultAddItemScreen: (vaultItemCipherType: VaultItemCipherType) -> Unit,
onNavigateToVaultAddItemScreen: (
cipherType: VaultItemCipherType,
selectedFolderId: String?,
) -> Unit,
onNavigateToSearchVault: (searchType: SearchType.Vault) -> Unit,
) {
internalVaultItemListingDestination(
Expand All @@ -87,7 +90,10 @@ fun NavGraphBuilder.vaultItemListingDestinationAsRoot(
onNavigateBack: () -> Unit,
onNavigateToVaultItemScreen: (id: String) -> Unit,
onNavigateToVaultEditItemScreen: (cipherId: String) -> Unit,
onNavigateToVaultAddItemScreen: (VaultItemCipherType) -> Unit,
onNavigateToVaultAddItemScreen: (
cipherType: VaultItemCipherType,
selectedFolderId: String?,
) -> Unit,
onNavigateToSearchVault: (searchType: SearchType.Vault) -> Unit,
) {
composableWithStayTransitions(
Expand Down Expand Up @@ -128,7 +134,7 @@ fun NavGraphBuilder.sendItemListingDestination(
onNavigateBack = onNavigateBack,
onNavigateToAddSendItem = onNavigateToAddSendItem,
onNavigateToEditSendItem = onNavigateToEditSendItem,
onNavigateToVaultAddItemScreen = { },
onNavigateToVaultAddItemScreen = { _, _ -> },
onNavigateToVaultItemScreen = { },
onNavigateToVaultEditItemScreen = { },
onNavigateToVaultItemListing = { },
Expand All @@ -146,7 +152,10 @@ private fun NavGraphBuilder.internalVaultItemListingDestination(
onNavigateToVaultItemScreen: (id: String) -> Unit,
onNavigateToVaultEditItemScreen: (cipherId: String) -> Unit,
onNavigateToVaultItemListing: (vaultItemListingType: VaultItemListingType) -> Unit,
onNavigateToVaultAddItemScreen: (vaultItemCipherType: VaultItemCipherType) -> Unit,
onNavigateToVaultAddItemScreen: (
cipherType: VaultItemCipherType,
selectedFolderId: String?,
) -> Unit,
onNavigateToAddSendItem: () -> Unit,
onNavigateToEditSendItem: (sendId: String) -> Unit,
onNavigateToSearch: (searchType: SearchType) -> Unit,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ fun VaultItemListingScreen(
onNavigateToVaultItem: (id: String) -> Unit,
onNavigateToVaultEditItemScreen: (cipherVaultId: String) -> Unit,
onNavigateToVaultItemListing: (vaultItemListingType: VaultItemListingType) -> Unit,
onNavigateToVaultAddItemScreen: (vaultItemCipherType: VaultItemCipherType) -> Unit,
onNavigateToVaultAddItemScreen: (
vaultItemCipherType: VaultItemCipherType,
selectedFolderId: String?,
) -> Unit,
onNavigateToAddSendItem: () -> Unit,
onNavigateToEditSendItem: (sendId: String) -> Unit,
onNavigateToSearch: (searchType: SearchType) -> Unit,
Expand Down Expand Up @@ -113,7 +116,10 @@ fun VaultItemListingScreen(
}

is VaultItemListingEvent.NavigateToAddVaultItem -> {
onNavigateToVaultAddItemScreen(event.vaultItemCipherType)
onNavigateToVaultAddItemScreen(
event.vaultItemCipherType,
event.selectedFolderId,
)
}

is VaultItemListingEvent.NavigateToEditCipher -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,13 @@ class VaultItemListingViewModel @Inject constructor(

private fun handleAddVaultItemClick() {
val event = when (val itemListingType = state.itemListingType) {
is VaultItemListingState.ItemListingType.Vault.Folder -> {
VaultItemListingEvent.NavigateToAddVaultItem(
vaultItemCipherType = itemListingType.toVaultItemCipherType(),
selectedFolderId = itemListingType.folderId,
)
}

is VaultItemListingState.ItemListingType.Vault -> {
VaultItemListingEvent.NavigateToAddVaultItem(
vaultItemCipherType = itemListingType.toVaultItemCipherType(),
Expand Down Expand Up @@ -2089,7 +2096,7 @@ data class VaultItemListingState(
get() = folderId
?.let { folderName.asText() }
?: R.string.folder_none.asText()
override val hasFab: Boolean get() = false
override val hasFab: Boolean get() = true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

๐Ÿ‘

}

/**
Expand Down Expand Up @@ -2150,6 +2157,7 @@ sealed class VaultItemListingEvent {
*/
data class NavigateToAddVaultItem(
val vaultItemCipherType: VaultItemCipherType,
val selectedFolderId: String? = null,
) : VaultItemListingEvent()

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ fun VaultItemListingState.ItemListingType.Vault.toVaultItemCipherType(): VaultIt
is VaultItemListingState.ItemListingType.Vault.SshKey -> VaultItemCipherType.SSH_KEY
is VaultItemListingState.ItemListingType.Vault.Login -> VaultItemCipherType.LOGIN
is VaultItemListingState.ItemListingType.Vault.Collection -> VaultItemCipherType.LOGIN
is VaultItemListingState.ItemListingType.Vault.Folder -> VaultItemCipherType.LOGIN
is VaultItemListingState.ItemListingType.Vault.Trash,
is VaultItemListingState.ItemListingType.Vault.Folder,
-> {
throw IllegalStateException(
"Cannot create vault item from this VaultItemListingState!",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ const val VAULT_GRAPH_ROUTE: String = "vault_graph"
@Suppress("LongParameterList")
fun NavGraphBuilder.vaultGraph(
navController: NavController,
onNavigateToVaultAddItemScreen: (vaultItemCipherType: VaultItemCipherType) -> Unit,
onNavigateToVaultAddItemScreen: (
vaultItemCipherType: VaultItemCipherType,
selectedFolderId: String?,
) -> Unit,
onNavigateToVaultItemScreen: (vaultItemId: String) -> Unit,
onNavigateToVaultEditItemScreen: (vaultItemId: String) -> Unit,
onNavigateToSearchVault: (searchType: SearchType.Vault) -> Unit,
Expand All @@ -33,7 +36,7 @@ fun NavGraphBuilder.vaultGraph(
) {
vaultDestination(
onNavigateToVaultAddItemScreen = {
onNavigateToVaultAddItemScreen(VaultItemCipherType.LOGIN)
onNavigateToVaultAddItemScreen(VaultItemCipherType.LOGIN, null)
},
onNavigateToVaultItemScreen = onNavigateToVaultItemScreen,
onNavigateToVaultEditItemScreen = onNavigateToVaultEditItemScreen,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class VaultUnlockedNavBarScreenTest : BaseComposeTest() {
VaultUnlockedNavBarScreen(
viewModel = viewModel,
navController = fakeNavHostController,
onNavigateToVaultAddItem = {},
onNavigateToVaultAddItem = { _, _ -> },
onNavigateToVaultItem = {},
onNavigateToVaultEditItem = {},
onNavigateToAddSend = {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,9 @@ class VaultItemListingScreenTest : BaseComposeTest() {
biometricsManager = biometricsManager,
onNavigateBack = { onNavigateBackCalled = true },
onNavigateToVaultItem = { onNavigateToVaultItemId = it },
onNavigateToVaultAddItemScreen = { onNavigateToVaultAddItemScreenCalled = true },
onNavigateToVaultAddItemScreen = { _, _ ->
onNavigateToVaultAddItemScreenCalled = true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise it won't even compile.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I see the underscores now. ๐Ÿ˜„

},
onNavigateToAddSendItem = { onNavigateToAddSendScreenCalled = true },
onNavigateToEditSendItem = { onNavigateToEditSendItemId = it },
onNavigateToSearch = { onNavigateToSearchType = it },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,27 @@ class VaultItemListingViewModelTest : BaseViewModelTest() {
}
}

@Suppress("MaxLineLength")
@Test
fun `AddVaultItemClick inside a folder should emit NavigateToAddVaultItem with a selected folder id`() =
runTest {
val viewModel = createVaultItemListingViewModel(
savedStateHandle = createSavedStateHandleWithVaultItemListingType(
vaultItemListingType = VaultItemListingType.Folder(folderId = "id"),
),
)
viewModel.eventFlow.test {
viewModel.trySendAction(VaultItemListingsAction.AddVaultItemClick)
assertEquals(
VaultItemListingEvent.NavigateToAddVaultItem(
VaultItemCipherType.LOGIN,
selectedFolderId = "id",
),
awaitItem(),
)
}
}

@Test
fun `AddVaultItemClick for vault item should emit NavigateToAddVaultItem`() = runTest {
val viewModel = createVaultItemListingViewModel()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ class VaultItemListingStateExtensionsTest {
VaultItemListingState.ItemListingType.Vault.Login,
VaultItemListingState.ItemListingType.Vault.Collection(collectionId = "mockId"),
VaultItemListingState.ItemListingType.Vault.SshKey,
VaultItemListingState.ItemListingType.Vault.Folder(folderId = "mockId"),
)

val result = itemListingTypes.map { it.toVaultItemCipherType() }
Expand All @@ -142,22 +143,16 @@ class VaultItemListingStateExtensionsTest {
VaultItemCipherType.LOGIN,
VaultItemCipherType.LOGIN,
VaultItemCipherType.SSH_KEY,
VaultItemCipherType.LOGIN,
),
result,
)
}

@Test
fun `toVaultItemCipherType should throw an exception for unsupported ItemListingTypes`() {
val itemListingTypes = listOf(
VaultItemListingState.ItemListingType.Vault.Trash,
VaultItemListingState.ItemListingType.Vault.Folder(
folderId = "mockId",
),
)

itemListingTypes.forEach {
assertThrows<IllegalStateException> { it.toVaultItemCipherType() }
assertThrows<IllegalStateException> {
VaultItemListingState.ItemListingType.Vault.Trash.toVaultItemCipherType()
}
}
}