Convert scrollToTop to an Event

This avoids the stale information of this field to be kept in the uiState and
be consumed by the UI incorrectly multiple times. An event makes more sense here
since we want the action to happen once at a very specific moment, only that
event is triggered.

WordPress/src/main/java/org/wordpress/android/ui/reader/discover/ReaderDiscoverFragment.kt

@@ -131,9 +131,6 @@ class ReaderDiscoverFragment : ViewPagerFragment(R.layout.reader_discover_fragme
             when (it) {
                 is DiscoverUiState.ContentUiState -> {
                     (recyclerView.adapter as ReaderDiscoverAdapter).update(it.cards)
-                    if (it.scrollToTop) {
-                        recyclerView.scrollToPosition(0)
-                    }
                 }
                 is DiscoverUiState.EmptyUiState -> {
                     uiHelpers.setTextOrHide(actionableEmptyView.title, it.titleResId)
@@ -153,9 +150,10 @@ class ReaderDiscoverFragment : ViewPagerFragment(R.layout.reader_discover_fragme
             ptrLayout.isEnabled = it.swipeToRefreshEnabled
             ptrLayout.isRefreshing = it.reloadProgressVisibility
         }
+        viewModel.scrollToTopEvent.observeEvent(viewLifecycleOwner) { recyclerView.scrollToPosition(0) }
         viewModel.navigationEvents.observeEvent(viewLifecycleOwner) { handleNavigation(it) }
-        viewModel.snackbarEvents.observeEvent(viewLifecycleOwner, { it.showSnackbar() })
-        viewModel.preloadPostEvents.observeEvent(viewLifecycleOwner, { it.addWebViewCachingFragment() })
+        viewModel.snackbarEvents.observeEvent(viewLifecycleOwner) { it.showSnackbar() }
+        viewModel.preloadPostEvents.observeEvent(viewLifecycleOwner) { it.addWebViewCachingFragment() }
         viewModel.start(parentViewModel)
     }
 

WordPress/src/main/java/org/wordpress/android/ui/reader/discover/ReaderDiscoverViewModel.kt

@@ -2,6 +2,7 @@ package org.wordpress.android.ui.reader.discover
 
 import androidx.lifecycle.LiveData
 import androidx.lifecycle.MediatorLiveData
+import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.Observer
 import androidx.lifecycle.viewModelScope
 import kotlinx.coroutines.CoroutineDispatcher
@@ -79,6 +80,9 @@ class ReaderDiscoverViewModel @Inject constructor(
     private val _preloadPostEvents = MediatorLiveData<Event<PreLoadPostContent>>()
     val preloadPostEvents: LiveData<Event<PreLoadPostContent>> = _preloadPostEvents
 
+    private val _scrollToTopEvent = MutableLiveData<Event<Unit>>()
+    val scrollToTopEvent: LiveData<Event<Unit>> = _scrollToTopEvent
+
     /**
      * Post which is about to be reblogged after the user selects a target site.
      */
@@ -155,9 +159,11 @@ class ReaderDiscoverViewModel @Inject constructor(
                             convertCardsToUiStates(posts),
                             reloadProgressVisibility = false,
                             loadMoreProgressVisibility = false,
-                            scrollToTop = swipeToRefreshTriggered
                         )
-                        swipeToRefreshTriggered = false
+                        if (swipeToRefreshTriggered) {
+                            _scrollToTopEvent.postValue(Event(Unit))
+                            swipeToRefreshTriggered = false
+                        }
                     } else {
                         _uiState.value = DiscoverUiState.EmptyUiState.ShowNoPostsUiState {
                             _navigationEvents.value = Event(ShowReaderSubs)
@@ -523,7 +529,6 @@ class ReaderDiscoverViewModel @Inject constructor(
         val fullscreenProgressVisibility: Boolean = false,
         val swipeToRefreshEnabled: Boolean = false,
         open val fullscreenEmptyVisibility: Boolean = false,
-        open val scrollToTop: Boolean = false
     ) {
         open val reloadProgressVisibility: Boolean = false
         open val loadMoreProgressVisibility: Boolean = false
@@ -532,7 +537,6 @@ class ReaderDiscoverViewModel @Inject constructor(
             val cards: List<ReaderCardUiState>,
             override val reloadProgressVisibility: Boolean,
             override val loadMoreProgressVisibility: Boolean,
-            override val scrollToTop: Boolean
         ) : DiscoverUiState(contentVisiblity = true, swipeToRefreshEnabled = true)
 
         object LoadingUiState : DiscoverUiState(fullscreenProgressVisibility = true)

WordPress/src/test/java/org/wordpress/android/ui/reader/discover/ReaderDiscoverViewModelTest.kt

@@ -608,18 +608,19 @@ class ReaderDiscoverViewModelTest : BaseUnitTest() {
     @Test
     fun `Scroll to top is triggered when discover feed is updated after swipe to refresh`() = test {
         // Arrange
-        val uiStates = init().uiStates
+        val scrollToTopCounter = init().scrollToTopCounter
+
         // Act
         viewModel.swipeToRefresh()
         fakeDiscoverFeed.value = createDummyReaderCardsList()
         // Assert
-        assertThat(uiStates.last().scrollToTop).isTrue
+        assertThat(scrollToTopCounter.count).isEqualTo(1)
     }
 
     @Test
     fun `Scroll to top is not triggered when discover feed is updated after load more action`() = test {
         // Arrange
-        val uiStates = init().uiStates
+        val scrollToTopCounter = init().scrollToTopCounter
         val closeToEndIndex = NUMBER_OF_ITEMS.toInt() - INITIATE_LOAD_MORE_OFFSET
         init()
         // Act
@@ -628,7 +629,34 @@ class ReaderDiscoverViewModelTest : BaseUnitTest() {
         }
         fakeDiscoverFeed.value = createDummyReaderCardsList()
         // Assert
-        assertThat(uiStates.last().scrollToTop).isFalse
+        assertThat(scrollToTopCounter.count).isZero()
+    }
+
+    @Test
+    fun `Scroll to top is triggered only once when discover feed is updated with load more after swipe to refresh`() =
+        test {
+            // Arrange
+            val scrollToTopCounter = init().scrollToTopCounter
+            val closeToEndIndex = NUMBER_OF_ITEMS.toInt() - INITIATE_LOAD_MORE_OFFSET
+
+            // Act for swipe to refresh
+            viewModel.swipeToRefresh()
+            fakeDiscoverFeed.value = createDummyReaderCardsList()
+
+            // Assert for swipe to refresh
+            assertThat(scrollToTopCounter.count).isEqualTo(1)
+
+            // Arrange for load more
+            scrollToTopCounter.reset()
+
+            // Act for load more
+            ((viewModel.uiState.value as ContentUiState).cards[closeToEndIndex] as ReaderPostUiState).let {
+                it.onItemRendered.invoke(it)
+            }
+            fakeDiscoverFeed.value = createDummyReaderCardsList()
+
+            // Assert for load more
+            assertThat(scrollToTopCounter.count).isZero()
     }
 
     @Test
@@ -679,11 +707,15 @@ class ReaderDiscoverViewModelTest : BaseUnitTest() {
         viewModel.snackbarEvents.observeForever {
             msgs.add(it)
         }
+        val scrollToTop = SimpleCounter()
+        viewModel.scrollToTopEvent.observeForever {
+            scrollToTop.increment()
+        }
         viewModel.start(parentViewModel)
         if (autoUpdateFeed) {
             fakeDiscoverFeed.value = createDummyReaderCardsList()
         }
-        return Observers(uiStates, navigation, msgs)
+        return Observers(uiStates, navigation, msgs, scrollToTop)
     }
 
     // since we are adding an InterestsYouMayLikeCard we remove one item from the numberOfItems since it counts as 1.
@@ -881,6 +913,18 @@ class ReaderDiscoverViewModelTest : BaseUnitTest() {
     private data class Observers(
         val uiStates: List<DiscoverUiState>,
         val navigation: List<Event<ReaderNavigationEvents>>,
-        val snackbarMsgs: List<Event<SnackbarMessageHolder>>
+        val snackbarMsgs: List<Event<SnackbarMessageHolder>>,
+        val scrollToTopCounter: SimpleCounter, // number of calls to this event
     )
+
+    private class SimpleCounter {
+        var count: Int = 0
+            private set
+
+        fun increment() = count++
+
+        fun reset() {
+            count = 0
+        }
+    }
 }