For mozilla-mobile#2165 - Add swipe to refresh gesture to bookmarks v…

…iew.

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt

@@ -12,7 +12,6 @@ import androidx.core.content.getSystemService
 import androidx.navigation.NavController
 import androidx.navigation.NavDirections
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers.IO
 import kotlinx.coroutines.Dispatchers.Main
 import kotlinx.coroutines.launch
 import mozilla.components.concept.engine.prompt.ShareData
@@ -44,6 +43,7 @@ interface BookmarkController {
     fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
     fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event)
     fun handleBookmarkFolderDeletion(node: BookmarkNode)
+    fun handleRequestSync()
     fun handleBackPressed()
 }
 
@@ -55,6 +55,7 @@ class DefaultBookmarkController(
     private val store: BookmarkFragmentStore,
     private val sharedViewModel: BookmarksSharedViewModel,
     private val loadBookmarkNode: suspend (String) -> BookmarkNode?,
+    private val syncBookmarks: suspend () -> Unit,
     private val showSnackbar: (String) -> Unit,
     private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit,
     private val deleteBookmarkFolder: (BookmarkNode) -> Unit,
@@ -92,6 +93,10 @@ class DefaultBookmarkController(
     }
 
     override fun handleBookmarkSelected(node: BookmarkNode) {
+        if (store.state.mode is BookmarkFragmentState.Mode.Syncing) {
+            return
+        }
+
         when (node.inRoots()) {
             true -> showSnackbar(resources.getString(R.string.bookmark_cannot_edit_root))
             false -> store.dispatch(BookmarkFragmentAction.Select(node))
@@ -132,16 +137,25 @@ class DefaultBookmarkController(
         deleteBookmarkFolder(node)
     }
 
+    override fun handleRequestSync() {
+        scope.launch {
+            store.dispatch(BookmarkFragmentAction.StartSync)
+            syncBookmarks.invoke()
+            store.dispatch(BookmarkFragmentAction.FinishSync)
+        }
+    }
+
     override fun handleBackPressed() {
         invokePendingDeletion.invoke()
-        val parentGuid = store.state.tree?.parentGuid
-        if (parentGuid == null) {
-            navController.popBackStack()
-        } else {
-            scope.launch(IO) {
-                context.bookmarkStorage.getBookmark(parentGuid)?.let {
-                    handleBookmarkExpand(it)
-                }
+        scope.launch(Main) {
+            val parentGuid = store.state.guidBackstack.findLast { guid ->
+                store.state.tree?.guid != guid && context.bookmarkStorage.getBookmark(guid) != null
+            }
+            if (parentGuid == null) {
+                navController.popBackStack()
+            } else {
+                val parent = context.bookmarkStorage.getBookmark(parentGuid)!!
+                handleBookmarkExpand(parent)
             }
         }
     }

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt

@@ -34,6 +34,7 @@ import mozilla.components.concept.engine.prompt.ShareData
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
 import mozilla.components.lib.state.ext.consumeFrom
+import mozilla.components.service.fxa.sync.SyncReason
 import mozilla.components.support.base.feature.UserInteractionHandler
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
@@ -89,6 +90,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
                 store = bookmarkStore,
                 sharedViewModel = sharedViewModel,
                 loadBookmarkNode = ::loadBookmarkNode,
+                syncBookmarks = ::syncBookmarks,
                 showSnackbar = ::showSnackBarWithText,
                 deleteBookmarkNodes = ::deleteMulti,
                 deleteBookmarkFolder = ::showRemoveFolderDialog,
@@ -393,4 +395,17 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
             }
         }
     }
+
+    private suspend fun syncBookmarks() {
+        invokePendingDeletion()
+        requireComponents.backgroundServices.accountManager.syncNowAsync(SyncReason.User).await()
+        // The current bookmark node we are viewing may be made invalid after syncing so we
+        // check if the current node is valid and if it isn't we find the nearest valid ancestor
+        // and open it
+        val validAncestorGuid = bookmarkStore.state.guidBackstack.findLast { guid ->
+            requireContext().bookmarkStorage.getBookmark(guid) != null
+        } ?: BookmarkRoot.Mobile.id
+        val node = requireContext().bookmarkStorage.getBookmark(validAncestorGuid)!!
+        bookmarkInteractor.open(node)
+    }
 }

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractor.kt

@@ -116,4 +116,8 @@ class BookmarkFragmentInteractor(
     override fun deselect(item: BookmarkNode) {
         bookmarksController.handleBookmarkDeselected(item)
     }
+
+    override fun onRequestSync() {
+        bookmarksController.handleRequestSync()
+    }
 }

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentStore.kt

@@ -20,17 +20,22 @@ class BookmarkFragmentStore(
  * The complete state of the bookmarks tree and multi-selection mode
  * @property tree The current tree of bookmarks, if one is loaded
  * @property mode The current bookmark multi-selection mode
+ * @property guidBackstack A set of guids for bookmark nodes we have visited. Used to traverse back
+ *                  up the tree after a sync.
+ * @property isLoading true if bookmarks are still being loaded from disk
  */
 data class BookmarkFragmentState(
     val tree: BookmarkNode?,
     val mode: Mode = Mode.Normal(),
+    val guidBackstack: Set<String> = linkedSetOf(),
     val isLoading: Boolean = true
 ) : State {
     sealed class Mode {
         open val selectedItems = emptySet<BookmarkNode>()
 
         data class Normal(val showMenu: Boolean = true) : Mode()
         data class Selecting(override val selectedItems: Set<BookmarkNode>) : Mode()
+        object Syncing : Mode()
     }
 }
 
@@ -42,6 +47,8 @@ sealed class BookmarkFragmentAction : Action {
     data class Select(val item: BookmarkNode) : BookmarkFragmentAction()
     data class Deselect(val item: BookmarkNode) : BookmarkFragmentAction()
     object DeselectAll : BookmarkFragmentAction()
+    object StartSync : BookmarkFragmentAction()
+    object FinishSync : BookmarkFragmentAction()
 }
 
 /**
@@ -56,16 +63,26 @@ private fun bookmarkFragmentStateReducer(
 ): BookmarkFragmentState {
     return when (action) {
         is BookmarkFragmentAction.Change -> {
+            // If we change to a node we have already visited, we pop the backstack until the node
+            // is the last item. If we haven't visited the node yet, we just add it to the end of the
+            // backstack
+            val backstack = state.guidBackstack.takeWhile { guid ->
+                guid != action.tree.guid
+            } + action.tree.guid
+
             val items = state.mode.selectedItems.filter { it in action.tree }
             state.copy(
                 tree = action.tree,
-                mode = if (BookmarkRoot.Root.id == action.tree.guid) {
-                    BookmarkFragmentState.Mode.Normal(false)
-                } else if (items.isEmpty()) {
-                    BookmarkFragmentState.Mode.Normal()
-                } else {
-                    BookmarkFragmentState.Mode.Selecting(items.toSet())
+                mode = when {
+                    state.mode is BookmarkFragmentState.Mode.Syncing -> {
+                        BookmarkFragmentState.Mode.Syncing
+                    }
+                    items.isEmpty() -> {
+                        BookmarkFragmentState.Mode.Normal(shouldShowMenu(action.tree.guid))
+                    }
+                    else -> BookmarkFragmentState.Mode.Selecting(items.toSet())
                 },
+                guidBackstack = backstack.toSet(),
                 isLoading = false
             )
         }
@@ -81,11 +98,30 @@ private fun bookmarkFragmentStateReducer(
                 }
             )
         }
-        BookmarkFragmentAction.DeselectAll ->
-            state.copy(mode = BookmarkFragmentState.Mode.Normal())
+        is BookmarkFragmentAction.DeselectAll ->
+            state.copy(
+                mode = if (state.mode is BookmarkFragmentState.Mode.Syncing) {
+                    BookmarkFragmentState.Mode.Syncing
+                } else {
+                    BookmarkFragmentState.Mode.Normal()
+                }
+            )
+        is BookmarkFragmentAction.StartSync ->
+            state.copy(
+                mode = BookmarkFragmentState.Mode.Syncing
+            )
+        is BookmarkFragmentAction.FinishSync ->
+            state.copy(
+                mode = BookmarkFragmentState.Mode.Normal(
+                    showMenu = shouldShowMenu(state.tree?.guid)
+                )
+            )
     }
 }
 
+private fun shouldShowMenu(currentGuid: String?): Boolean =
+    BookmarkRoot.Root.id != currentGuid
+
 operator fun BookmarkNode.contains(item: BookmarkNode): Boolean {
     return children?.contains(item) ?: false
 }

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkView.kt

@@ -93,6 +93,12 @@ interface BookmarkViewInteractor : SelectionInteractor<BookmarkNode> {
      *
      */
     fun onBackPressed()
+
+    /**
+     * Handles user requested sync of bookmarks.
+     *
+     */
+    fun onRequestSync()
 }
 
 class BookmarkView(
@@ -117,13 +123,18 @@ class BookmarkView(
         view.bookmark_folders_sign_in.setOnClickListener {
             navController.navigate(NavGraphDirections.actionGlobalTurnOnSync())
         }
+        view.swipe_refresh.setOnRefreshListener {
+            interactor.onRequestSync()
+        }
     }
 
     fun update(state: BookmarkFragmentState) {
         tree = state.tree
         if (state.mode != mode) {
             mode = state.mode
-            interactor.onSelectionModeSwitch(mode)
+            if (mode is BookmarkFragmentState.Mode.Normal || mode is BookmarkFragmentState.Mode.Selecting) {
+                interactor.onSelectionModeSwitch(mode)
+            }
         }
 
         bookmarkAdapter.updateData(state.tree, mode)
@@ -149,6 +160,9 @@ class BookmarkView(
             }
         }
         view.bookmarks_progress_bar.isVisible = state.isLoading
+        view.swipe_refresh.isEnabled =
+            state.mode is BookmarkFragmentState.Mode.Normal || state.mode is BookmarkFragmentState.Mode.Syncing
+        view.swipe_refresh.isRefreshing = state.mode is BookmarkFragmentState.Mode.Syncing
     }
 
     override fun onBackPressed(): Boolean {

app/src/main/java/org/mozilla/fenix/library/bookmarks/DesktopFolders.kt

@@ -50,7 +50,7 @@ class DesktopFolders(
                 val childrenWithVirtualFolder =
                     listOfNotNull(virtualDesktopFolder()) + node.children.orEmpty()
 
-                node.copy(children = childrenWithVirtualFolder, parentGuid = null)
+                node.copy(children = childrenWithVirtualFolder)
             }
             BookmarkRoot.Root.id ->
                 node.copy(
@@ -59,8 +59,7 @@ class DesktopFolders(
                         restructureMobileRoots(node.children)
                     } else {
                         restructureDesktopRoots(node.children)
-                    },
-                    parentGuid = BookmarkRoot.Mobile.id
+                    }
                 )
             BookmarkRoot.Menu.id, BookmarkRoot.Toolbar.id, BookmarkRoot.Unfiled.id ->
                 // If we're looking at one of the desktop roots, change their titles to friendly names.

app/src/main/res/layout/component_bookmark.xml

@@ -9,15 +9,21 @@
     android:layout_width="match_parent"
     android:layout_height="match_parent">
 
-    <androidx.recyclerview.widget.RecyclerView
-        android:id="@+id/bookmark_list"
+    <androidx.swiperefreshlayout.widget.SwipeRefreshLayout
+        android:id="@+id/swipe_refresh"
         android:layout_width="match_parent"
-        android:layout_height="wrap_content"
-        app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager"
-        app:layout_constraintEnd_toEndOf="parent"
-        app:layout_constraintStart_toStartOf="parent"
-        app:layout_constraintTop_toTopOf="parent"
-        tools:listitem="@layout/library_site_item" />
+        android:layout_height="wrap_content" >
+
+        <androidx.recyclerview.widget.RecyclerView
+            android:id="@+id/bookmark_list"
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager"
+            app:layout_constraintEnd_toEndOf="parent"
+            app:layout_constraintStart_toStartOf="parent"
+            app:layout_constraintTop_toTopOf="parent"
+            tools:listitem="@layout/library_site_item" />
+    </androidx.swiperefreshlayout.widget.SwipeRefreshLayout>
 
     <TextView
         android:id="@+id/bookmarks_empty_view"

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt

@@ -15,6 +15,7 @@ import io.mockk.Runs
 import io.mockk.called
 import io.mockk.coEvery
 import io.mockk.coVerify
+import io.mockk.coVerifyOrder
 import io.mockk.every
 import io.mockk.just
 import io.mockk.mockk
@@ -59,6 +60,7 @@ class BookmarkControllerTest {
     private val navController: NavController = mockk(relaxed = true)
     private val sharedViewModel: BookmarksSharedViewModel = mockk()
     private val loadBookmarkNode: suspend (String) -> BookmarkNode? = mockk(relaxed = true)
+    private val syncBookmarks: suspend () -> Unit = mockk(relaxed = true)
     private val showSnackbar: (String) -> Unit = mockk(relaxed = true)
     private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = mockk(relaxed = true)
     private val deleteBookmarkFolder: (BookmarkNode) -> Unit = mockk(relaxed = true)
@@ -118,6 +120,7 @@ class BookmarkControllerTest {
             store = bookmarkStore,
             sharedViewModel = sharedViewModel,
             loadBookmarkNode = loadBookmarkNode,
+            syncBookmarks = syncBookmarks,
             showSnackbar = showSnackbar,
             deleteBookmarkNodes = deleteBookmarkNodes,
             deleteBookmarkFolder = deleteBookmarkFolder,
@@ -237,10 +240,12 @@ class BookmarkControllerTest {
     }
 
     @Test
-    fun `handleBookmarkSelected does not dispatch Select action for bookmark roots`() {
-        controller.handleBookmarkSelected(root)
+    fun `handleBookmarkSelected does not select in Syncing mode`() {
+        every { bookmarkStore.state.mode } returns BookmarkFragmentState.Mode.Syncing
+
+        controller.handleBookmarkSelected(item)
 
-        verify { bookmarkStore wasNot called }
+        verify { bookmarkStore.dispatch(BookmarkFragmentAction.Select(item)) wasNot called }
     }
 
     @Test
@@ -329,7 +334,19 @@ class BookmarkControllerTest {
     }
 
     @Test
-    fun `handleBackPressed from root bookmark node should trigger handleBackPressed in NavController`() {
+    fun `handleRequestSync dispatches actions in the correct order`() {
+        controller.handleRequestSync()
+
+        coVerifyOrder {
+            bookmarkStore.dispatch(BookmarkFragmentAction.StartSync)
+            syncBookmarks.invoke()
+            bookmarkStore.dispatch(BookmarkFragmentAction.FinishSync)
+        }
+    }
+
+    @Test
+    fun `handleBackPressed with one item in backstack should trigger handleBackPressed in NavController`() {
+        every { bookmarkStore.state.guidBackstack } returns linkedSetOf(tree.guid)
         every { bookmarkStore.state.tree } returns tree
 
         controller.handleBackPressed()

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkFragmentInteractorTest.kt

@@ -201,4 +201,13 @@ class BookmarkFragmentInteractorTest {
             bookmarkController.handleBackPressed()
         }
     }
+
+    @Test
+    fun `request a sync`() {
+        interactor.onRequestSync()
+
+        verify {
+            bookmarkController.handleRequestSync()
+        }
+    }
 }