Bug 1949591 - Prevent the folder selection screen from presenting any children of a folder that is being moved. r=android-reviewers,matt-tighe

Differential Revision: https://phabricator.services.mozilla.com/D242431
This commit is contained in:
Jeff Boek
2025-03-21 02:44:22 +00:00
parent 7df2c7072f
commit 7c789f0312
3 changed files with 28 additions and 8 deletions

View File

@@ -340,12 +340,12 @@ internal class BookmarksMiddleware(
rootNode.children?.find { it.guid == BookmarkRoot.Mobile.id }?.let { rootNode.children?.find { it.guid == BookmarkRoot.Mobile.id }?.let {
val newChildren = listOf(desktopRoot) + it.children.orEmpty() val newChildren = listOf(desktopRoot) + it.children.orEmpty()
it.copy(children = newChildren) it.copy(children = newChildren)
}?.let { collectFolders(it) } }?.let { collectFolders(it, shouldCollect = { node -> !state.isGuidBeingMoved(node.guid) }) }
} }
} else { } else {
bookmarksStorage.getTree(BookmarkRoot.Mobile.id, recursive = true) bookmarksStorage.getTree(BookmarkRoot.Mobile.id, recursive = true)
?.let { rootNode -> ?.let { rootNode ->
collectFolders(rootNode) collectFolders(rootNode, shouldCollect = { node -> !state.isGuidBeingMoved(node.guid) })
} }
} }
@@ -426,9 +426,10 @@ internal class BookmarksMiddleware(
private fun collectFolders( private fun collectFolders(
node: BookmarkNode, node: BookmarkNode,
indentation: Int = 0, indentation: Int = 0,
shouldCollect: (BookmarkNode) -> Boolean = { _ -> true },
folders: MutableList<SelectFolderItem> = mutableListOf(), folders: MutableList<SelectFolderItem> = mutableListOf(),
): List<SelectFolderItem> { ): List<SelectFolderItem> {
if (node.type == BookmarkNodeType.FOLDER) { if (node.type == BookmarkNodeType.FOLDER && shouldCollect(node)) {
folders.add( folders.add(
SelectFolderItem( SelectFolderItem(
indentation = indentation, indentation = indentation,
@@ -440,7 +441,7 @@ internal class BookmarksMiddleware(
) )
node.children?.forEach { child -> node.children?.forEach { child ->
folders.addAll(collectFolders(child, indentation + 1)) folders.addAll(collectFolders(child, indentation + 1, shouldCollect))
} }
} }

View File

@@ -632,10 +632,6 @@ private fun SelectFolderScreen(
.padding(vertical = 16.dp), .padding(vertical = 16.dp),
) { ) {
items(state?.folders ?: listOf()) { folder -> items(state?.folders ?: listOf()) { folder ->
if (store.state.isGuidBeingMoved(folder.guid)) {
return@items
}
if (folder.isDesktopRoot) { if (folder.isDesktopRoot) {
Row(modifier = Modifier.padding(start = (40 * folder.indentation).dp)) { Row(modifier = Modifier.padding(start = (40 * folder.indentation).dp)) {
// We need to account for not having an icon // We need to account for not having an icon

View File

@@ -630,6 +630,29 @@ class BookmarksMiddlewareTest {
assertEquals(6, store.state.bookmarksSelectFolderState?.folders?.count()) assertEquals(6, store.state.bookmarksSelectFolderState?.folders?.count())
} }
@Test
fun `GIVEN a folder with subfolders WHEN select folder sub screen view is loaded THEN load folders into sub screen state without the selected folder`() = runTestOnMain {
`when`(bookmarksStorage.countBookmarksInTrees(listOf(BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id))).thenReturn(0u)
val rootNode = generateBookmarkFolder("parent", "first", BookmarkRoot.Mobile.id).copy(
children = generateBookmarkFolders("parent"),
)
`when`(bookmarksStorage.getTree(BookmarkRoot.Mobile.id, recursive = true)).thenReturn(rootNode)
val middleware = buildMiddleware()
val store = middleware.makeStore(
initialState = BookmarksState.default.copy(
bookmarksSelectFolderState = BookmarksSelectFolderState(outerSelectionGuid = "selection guid"),
bookmarksEditFolderState = BookmarksEditFolderState(
parent = BookmarkItem.Folder("Bookmarks", "guid0"),
folder = BookmarkItem.Folder("first", "parent"),
),
),
)
store.dispatch(SelectFolderAction.ViewAppeared)
assertEquals(0, store.state.bookmarksSelectFolderState?.folders?.count())
}
@Test @Test
fun `GIVEN bookmarks in storage and not signed into sync but have pre-existing desktop bookmarks saved WHEN select folder sub screen view is loaded THEN load folders, including desktop folders into sub screen state`() = runTestOnMain { fun `GIVEN bookmarks in storage and not signed into sync but have pre-existing desktop bookmarks saved WHEN select folder sub screen view is loaded THEN load folders, including desktop folders into sub screen state`() = runTestOnMain {
`when`(bookmarksStorage.countBookmarksInTrees(listOf(BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id))).thenReturn(1u) `when`(bookmarksStorage.countBookmarksInTrees(listOf(BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id))).thenReturn(1u)