From f1b0827a0b1d4e945d69a44830016cdceb79d2ab Mon Sep 17 00:00:00 2001 From: Grisha Kruglov Date: Wed, 23 Sep 2020 16:34:15 -0700 Subject: [PATCH] For #15296: Allow excluding bookmark subtrees when editing parent folder I'm really not a fan of how title overwriting and structure processing are mangled together, but will leave clearing that up for another day. --- .../mozilla/fenix/library/bookmarks/Utils.kt | 14 ++ .../SelectBookmarkFolderAdapter.kt | 24 +-- .../fenix/library/bookmarks/UtilsKtTest.kt | 147 ++++++++++++++++++ 3 files changed, 165 insertions(+), 20 deletions(-) create mode 100644 app/src/test/java/org/mozilla/fenix/library/bookmarks/UtilsKtTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/Utils.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/Utils.kt index 354b80bac..44a1c00e9 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/Utils.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/Utils.kt @@ -6,6 +6,7 @@ package org.mozilla.fenix.library.bookmarks import android.content.Context import mozilla.components.concept.storage.BookmarkNode +import mozilla.components.concept.storage.BookmarkNodeType import org.mozilla.fenix.R fun rootTitles(context: Context, withMobileRoot: Boolean): Map = if (withMobileRoot) { @@ -35,3 +36,16 @@ fun friendlyRootTitle( rootTitles.containsKey(node.title) -> rootTitles[node.title] else -> node.title } + +data class BookmarkNodeWithDepth(val depth: Int, val node: BookmarkNode, val parent: String?) + +fun BookmarkNode.flatNodeList(excludeSubtreeRoot: String?, depth: Int = 0): List { + if (this.type != BookmarkNodeType.FOLDER || this.guid == excludeSubtreeRoot) { + return emptyList() + } + val newList = listOf(BookmarkNodeWithDepth(depth, this, this.parentGuid)) + return newList + children + ?.filter { it.type == BookmarkNodeType.FOLDER } + ?.flatMap { it.flatNodeList(excludeSubtreeRoot = excludeSubtreeRoot, depth = depth + 1) } + .orEmpty() +} diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderAdapter.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderAdapter.kt index 7eb347bd3..938426bed 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/selectfolder/SelectBookmarkFolderAdapter.kt @@ -14,29 +14,23 @@ import androidx.recyclerview.widget.ListAdapter import androidx.recyclerview.widget.RecyclerView import kotlinx.android.extensions.LayoutContainer import mozilla.components.concept.storage.BookmarkNode -import mozilla.components.concept.storage.BookmarkNodeType import org.mozilla.fenix.R import org.mozilla.fenix.library.LibrarySiteItemView +import org.mozilla.fenix.library.bookmarks.BookmarkNodeWithDepth import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel +import org.mozilla.fenix.library.bookmarks.flatNodeList import org.mozilla.fenix.library.bookmarks.selectfolder.SelectBookmarkFolderAdapter.BookmarkFolderViewHolder -import org.mozilla.fenix.library.bookmarks.selectfolder.SelectBookmarkFolderAdapter.BookmarkNodeWithDepth class SelectBookmarkFolderAdapter(private val sharedViewModel: BookmarksSharedViewModel) : ListAdapter(DiffCallback) { fun updateData(tree: BookmarkNode?, hideFolderGuid: String?) { val updatedData = tree - ?.convertToFolderDepthTree() + ?.flatNodeList(hideFolderGuid) ?.drop(1) .orEmpty() - val filteredData = if (hideFolderGuid != null && updatedData.isNotEmpty()) { - updatedData.filter { it.node.guid != hideFolderGuid } - } else { - updatedData - } - - submitList(filteredData) + submitList(updatedData) } override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): BookmarkFolderViewHolder { @@ -101,16 +95,6 @@ class SelectBookmarkFolderAdapter(private val sharedViewModel: BookmarksSharedVi } } - data class BookmarkNodeWithDepth(val depth: Int, val node: BookmarkNode, val parent: String?) - - private fun BookmarkNode.convertToFolderDepthTree(depth: Int = 0): List { - val newList = listOf(BookmarkNodeWithDepth(depth, this, this.parentGuid)) - return newList + children - ?.filter { it.type == BookmarkNodeType.FOLDER } - ?.flatMap { it.convertToFolderDepthTree(depth = depth + 1) } - .orEmpty() - } - private fun getSelectedItemIndex(): Int? { val selectedNode = sharedViewModel.selectedFolder val selectedNodeIndex = currentList.indexOfFirst { it.node == selectedNode } diff --git a/app/src/test/java/org/mozilla/fenix/library/bookmarks/UtilsKtTest.kt b/app/src/test/java/org/mozilla/fenix/library/bookmarks/UtilsKtTest.kt new file mode 100644 index 000000000..42f185f32 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/library/bookmarks/UtilsKtTest.kt @@ -0,0 +1,147 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.library.bookmarks + +import mozilla.components.concept.storage.BookmarkNode +import mozilla.components.concept.storage.BookmarkNodeType +import mozilla.components.support.test.robolectric.testContext +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner + +@RunWith(FenixRobolectricTestRunner::class) +class UtilsKtTest { + @Test + fun `friendly root titles`() { + val url = BookmarkNode( + BookmarkNodeType.ITEM, + "456", + "folder", + 0, + "Mozilla", + "http://mozilla.org", + null + ) + assertEquals("Mozilla", friendlyRootTitle(testContext, url)) + + val folder = BookmarkNode( + BookmarkNodeType.FOLDER, + "456", + "folder", + 0, + "Folder", + null, + null + ) + assertEquals("Folder", friendlyRootTitle(testContext, folder)) + + val root = folder.copy(guid = "root________", title = "root") + assertEquals("Bookmarks", friendlyRootTitle(testContext, root, withMobileRoot = true)) + assertEquals("Desktop Bookmarks", friendlyRootTitle(testContext, root, withMobileRoot = false)) + + val mobileRoot = folder.copy(guid = "mobile______", title = "mobile") + assertEquals("Bookmarks", friendlyRootTitle(testContext, mobileRoot, withMobileRoot = true)) + assertEquals("mobile", friendlyRootTitle(testContext, mobileRoot, withMobileRoot = false)) + + val menuRoot = folder.copy(guid = "menu________", title = "menu") + assertEquals("Bookmarks Menu", friendlyRootTitle(testContext, menuRoot, withMobileRoot = true)) + assertEquals("Bookmarks Menu", friendlyRootTitle(testContext, menuRoot, withMobileRoot = false)) + + val toolbarRoot = folder.copy(guid = "toolbar_____", title = "toolbar") + assertEquals("Bookmarks Toolbar", friendlyRootTitle(testContext, toolbarRoot, withMobileRoot = true)) + assertEquals("Bookmarks Toolbar", friendlyRootTitle(testContext, toolbarRoot, withMobileRoot = false)) + + val unfiledRoot = folder.copy(guid = "unfiled_____", title = "unfiled") + assertEquals("Other Bookmarks", friendlyRootTitle(testContext, unfiledRoot, withMobileRoot = true)) + assertEquals("Other Bookmarks", friendlyRootTitle(testContext, unfiledRoot, withMobileRoot = false)) + + val almostRoot = folder.copy(guid = "notRoot________", title = "root") + assertEquals("root", friendlyRootTitle(testContext, almostRoot, withMobileRoot = true)) + assertEquals("root", friendlyRootTitle(testContext, almostRoot, withMobileRoot = false)) + } + + @Test + fun `flatNodeList various cases`() { + val url = BookmarkNode( + BookmarkNodeType.ITEM, + "456", + "folder", + 0, + "Mozilla", + "http://mozilla.org", + null + ) + val url2 = BookmarkNode( + BookmarkNodeType.ITEM, + "8674", + "folder2", + 0, + "Mozilla", + "http://mozilla.org", + null + ) + assertEquals(emptyList(), url.flatNodeList(null)) + + val root = BookmarkNode( + BookmarkNodeType.FOLDER, + "root", + null, + 0, + "root", + null, + null + ) + assertEquals(listOf(BookmarkNodeWithDepth(0, root, null)), root.flatNodeList(null)) + assertEquals(emptyList(), root.flatNodeList("root")) + + val folder = BookmarkNode( + BookmarkNodeType.FOLDER, + "folder", + root.guid, + 0, + "folder", + null, + listOf(url) + ) + + val folder3 = BookmarkNode( + BookmarkNodeType.FOLDER, + "folder3", + "folder2", + 0, + "folder3", + null, + null + ) + + val folder2 = BookmarkNode( + BookmarkNodeType.FOLDER, + "folder2", + root.guid, + 0, + "folder2", + null, + listOf(folder3, url2) + ) + + val rootWithChildren = root.copy(children = listOf(folder, folder2)) + assertEquals( + listOf( + BookmarkNodeWithDepth(0, rootWithChildren, null), + BookmarkNodeWithDepth(1, folder, "root"), + BookmarkNodeWithDepth(1, folder2, "root"), + BookmarkNodeWithDepth(2, folder3, "folder2") + ), rootWithChildren.flatNodeList(null) + ) + + assertEquals( + listOf( + BookmarkNodeWithDepth(0, rootWithChildren, null), + BookmarkNodeWithDepth(1, folder, "root") + ), rootWithChildren.flatNodeList(excludeSubtreeRoot = "folder2") + ) + } +}