Merge pull request #20216 from wordpress-mobile/issue/20010-reader-ia-subscription-infinite-loading

Thomas Horta · web-flow · commit a3b6de717a4c · 2024-02-21T17:10:15.000-03:00
[Reader][0 blogs/0 tags] Fix Subscription feed infinite loading and fetch when tapping filters
diff --git a/WordPress/src/main/java/org/wordpress/android/datasets/ReaderTagTable.java b/WordPress/src/main/java/org/wordpress/android/datasets/ReaderTagTable.java
@@ -232,6 +232,10 @@ public static ReaderTagList getBookmarkTags() {
         return getTagsOfType(ReaderTagType.BOOKMARKED);
     }
 
+    public static ReaderTagList getDiscoverPostCardsTags() {
+        return getTagsOfType(ReaderTagType.DISCOVER_POST_CARDS);
+    }
+
     private static ReaderTagList getTagsOfType(ReaderTagType tagType) {
         String[] args = {Integer.toString(tagType.toInt())};
         Cursor c = ReaderDatabase.getReadableDb()
diff --git a/WordPress/src/main/java/org/wordpress/android/models/ReaderTagList.java b/WordPress/src/main/java/org/wordpress/android/models/ReaderTagList.java
@@ -44,6 +44,8 @@ public boolean isSameList(ReaderTagList otherList) {
                 return false;
             } else if (!otherTag.getTagTitle().equals(this.get(i).getTagTitle())) {
                 return false;
+            } else if (!otherTag.getTagDisplayName().equals(this.get(i).getTagDisplayName())) {
+                return false;
             }
         }
 
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderEvents.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderEvents.java
@@ -26,16 +26,27 @@ private ReaderEvents() {
         throw new AssertionError();
     }
 
-    public static class FollowedTagsChanged {
+    public static class FollowedTagsFetched {
         private final boolean mDidSucceed;
+        private final boolean mDidChange;
 
-        public FollowedTagsChanged(boolean didSucceed) {
+        public FollowedTagsFetched(boolean didSucceed) {
             mDidSucceed = didSucceed;
+            mDidChange = true;
+        }
+
+        public FollowedTagsFetched(boolean didSucceed, boolean didChange) {
+            mDidSucceed = didSucceed;
+            mDidChange = didChange;
         }
 
         public boolean didSucceed() {
             return mDidSucceed;
         }
+
+        public boolean didChange() {
+            return mDidChange;
+        }
     }
 
     public static class TagAdded {
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostListFragment.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostListFragment.java
@@ -84,6 +84,7 @@
 import org.wordpress.android.ui.mysite.jetpackbadge.JetpackPoweredBottomSheetFragment;
 import org.wordpress.android.ui.pages.SnackbarMessageHolder;
 import org.wordpress.android.ui.prefs.AppPrefs;
+import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsFetched;
 import org.wordpress.android.ui.reader.ReaderEvents.TagAdded;
 import org.wordpress.android.ui.reader.ReaderTypes.ReaderPostListType;
 import org.wordpress.android.ui.reader.actions.ReaderActions;
@@ -934,14 +935,16 @@ private void resetPostAdapter(ReaderPostListType postListType) {
 
     @SuppressWarnings("unused")
     @Subscribe(threadMode = ThreadMode.MAIN)
-    public void onEventMainThread(ReaderEvents.FollowedTagsChanged event) {
+    public void onEventMainThread(FollowedTagsFetched event) {
         if (getPostListType() == ReaderPostListType.TAG_FOLLOWED) {
-            // reload the tag filter since tags have changed
-            reloadTags();
+            if (event.didChange()) {
+                // reload the tag filter since tags have changed or we just opened the fragment
+                reloadTags();
+            }
 
             // update the current tag if the list fragment is empty - this will happen if
             // the tag table was previously empty (ie: first run)
-            if (isPostAdapterEmpty()) {
+            if (isPostAdapterEmpty() && (ReaderBlogTable.hasFollowedBlogs() || !mHasUpdatedPosts)) {
                 updateCurrentTag();
             }
         }
@@ -1724,9 +1727,6 @@ private void setEmptyTitleDescriptionAndButton(boolean requestFailed) {
         // Ensure the default image is reset for empty views before applying logic
         mActionableEmptyView.image.setImageResource(R.drawable.illustration_reader_empty);
 
-        // TODO thomashortadev
-        //  try to quickly hack some way of making the button black
-
         if (shouldShowEmptyViewForSelfHostedCta()) {
             setEmptyTitleAndDescriptionForSelfHostedCta();
             return;
@@ -1940,7 +1940,9 @@ private void showBookmarksSavedLocallyDialog(ShowBookmarkedSavedOnlyLocallyDialo
                 .setCancelable(false)
                 .create();
         mBookmarksSavedLocallyDialog.show();
-    }    /*
+    }
+
+    /*
      * called by post adapter when data has been loaded
      */
     private final ReaderInterfaces.DataLoadedListener mDataLoadedListener = new ReaderInterfaces.DataLoadedListener() {
@@ -2333,7 +2335,7 @@ public void run() {
                             mActionableEmptyView.button.setVisibility(View.GONE);
                             mActionableEmptyView.subtitle.setVisibility(View.GONE);
                             showEmptyView();
-                        } else {
+                        } else if (!isPostAdapterEmpty()) {
                             hideEmptyView();
                         }
                     });
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderSubsActivity.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderSubsActivity.java
@@ -41,6 +41,7 @@
 import org.wordpress.android.ui.LocaleAwareActivity;
 import org.wordpress.android.ui.RequestCodes;
 import org.wordpress.android.ui.prefs.AppPrefs;
+import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsFetched;
 import org.wordpress.android.ui.reader.actions.ReaderActions;
 import org.wordpress.android.ui.reader.actions.ReaderBlogActions;
 import org.wordpress.android.ui.reader.actions.ReaderTagActions;
@@ -203,7 +204,7 @@ protected void onResume() {
 
     @SuppressWarnings("unused")
     @Subscribe(threadMode = ThreadMode.MAIN)
-    public void onEventMainThread(ReaderEvents.FollowedTagsChanged event) {
+    public void onEventMainThread(FollowedTagsFetched event) {
         AppLog.d(AppLog.T.READER, "reader subs > followed tags changed");
         getPageAdapter().refreshFollowedTagFragment();
     }
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/actions/ReaderTagActions.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/actions/ReaderTagActions.java
@@ -13,7 +13,7 @@
 import org.wordpress.android.models.ReaderTagList;
 import org.wordpress.android.models.ReaderTagType;
 import org.wordpress.android.ui.reader.ReaderConstants;
-import org.wordpress.android.ui.reader.ReaderEvents;
+import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsFetched;
 import org.wordpress.android.ui.reader.actions.ReaderActions.ActionListener;
 import org.wordpress.android.ui.reader.utils.ReaderUtils;
 import org.wordpress.android.util.AppLog;
@@ -51,7 +51,7 @@ public static boolean deleteTag(final ReaderTag tag,
     private static boolean deleteTagsLocallyOnly(ActionListener actionListener, ReaderTag tag) {
         ReaderTagTable.deleteTag(tag);
         ReaderActions.callActionListener(actionListener, true);
-        EventBus.getDefault().post(new ReaderEvents.FollowedTagsChanged(true));
+        EventBus.getDefault().post(new FollowedTagsFetched(true));
 
         return true;
     }
@@ -130,7 +130,7 @@ public static boolean addTags(@NonNull final List<ReaderTag> tags,
     private static boolean saveTagsLocallyOnly(ActionListener actionListener, ReaderTagList newTags) {
         ReaderTagTable.addOrUpdateTags(newTags);
         ReaderActions.callActionListener(actionListener, true);
-        EventBus.getDefault().post(new ReaderEvents.FollowedTagsChanged(true));
+        EventBus.getDefault().post(new FollowedTagsFetched(true));
 
         return true;
     }
@@ -147,7 +147,7 @@ private static boolean saveTagsLocallyAndRemotely(ActionListener actionListener,
             if (actionListener != null) {
                 ReaderActions.callActionListener(actionListener, true);
             }
-            EventBus.getDefault().post(new ReaderEvents.FollowedTagsChanged(true));
+            EventBus.getDefault().post(new FollowedTagsFetched(true));
         };
 
         RestRequest.ErrorListener errorListener = volleyError -> {
@@ -159,7 +159,7 @@ private static boolean saveTagsLocallyAndRemotely(ActionListener actionListener,
                 if (actionListener != null) {
                     ReaderActions.callActionListener(actionListener, true);
                 }
-                EventBus.getDefault().post(new ReaderEvents.FollowedTagsChanged(true));
+                EventBus.getDefault().post(new FollowedTagsFetched(true));
                 return;
             }
 
@@ -171,7 +171,7 @@ private static boolean saveTagsLocallyAndRemotely(ActionListener actionListener,
             if (actionListener != null) {
                 ReaderActions.callActionListener(actionListener, false);
             }
-            EventBus.getDefault().post(new ReaderEvents.FollowedTagsChanged(false));
+            EventBus.getDefault().post(new FollowedTagsFetched(false));
         };
 
         ReaderTagTable.addOrUpdateTags(newTags);
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/ReaderDiscoverDataProvider.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/ReaderDiscoverDataProvider.kt
@@ -14,7 +14,7 @@ import org.wordpress.android.models.discover.ReaderDiscoverCards
 import org.wordpress.android.modules.IO_THREAD
 import org.wordpress.android.modules.UI_THREAD
 import org.wordpress.android.ui.reader.ReaderEvents.FetchDiscoverCardsEnded
-import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsChanged
+import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsFetched
 import org.wordpress.android.ui.reader.actions.ReaderActions.UpdateResult.CHANGED
 import org.wordpress.android.ui.reader.actions.ReaderActions.UpdateResult.FAILED
 import org.wordpress.android.ui.reader.actions.ReaderActions.UpdateResult.HAS_NEW
@@ -211,7 +211,7 @@ class ReaderDiscoverDataProvider @Inject constructor(
 
     @Suppress("unused", "UNUSED_PARAMETER")
     @Subscribe(threadMode = BACKGROUND)
-    fun onFollowedTagsChanged(event: FollowedTagsChanged) {
+    fun onFollowedTagsFetched(event: FollowedTagsFetched) {
         launch {
             refreshCards()
         }
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/usecases/tags/FetchFollowedTagsUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/usecases/tags/FetchFollowedTagsUseCase.kt
@@ -2,7 +2,7 @@ package org.wordpress.android.ui.reader.repository.usecases.tags
 
 import org.greenrobot.eventbus.Subscribe
 import org.greenrobot.eventbus.ThreadMode
-import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsChanged
+import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsFetched
 import org.wordpress.android.ui.reader.repository.ReaderRepositoryCommunication
 import org.wordpress.android.ui.reader.repository.ReaderRepositoryCommunication.Error.NetworkUnavailable
 import org.wordpress.android.ui.reader.repository.ReaderRepositoryCommunication.Error.RemoteRequestFailure
@@ -48,7 +48,7 @@ class FetchFollowedTagsUseCase @Inject constructor(
 
     @Suppress("unused")
     @Subscribe(threadMode = ThreadMode.BACKGROUND)
-    fun onFollowedTagsChanged(event: FollowedTagsChanged) {
+    fun onFollowedTagsFetched(event: FollowedTagsFetched) {
         val result = if (event.didSucceed()) {
             Success
         } else {
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/usecases/tags/FollowInterestTagsUseCase.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/usecases/tags/FollowInterestTagsUseCase.kt
@@ -4,7 +4,7 @@ import org.greenrobot.eventbus.Subscribe
 import org.greenrobot.eventbus.ThreadMode
 import org.wordpress.android.fluxc.store.AccountStore
 import org.wordpress.android.models.ReaderTag
-import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsChanged
+import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsFetched
 import org.wordpress.android.ui.reader.actions.ReaderTagActions
 import org.wordpress.android.ui.reader.repository.ReaderRepositoryCommunication
 import org.wordpress.android.ui.reader.repository.ReaderRepositoryCommunication.Error.NetworkUnavailable
@@ -43,7 +43,7 @@ class FollowInterestTagsUseCase @Inject constructor(
 
     @Suppress("unused")
     @Subscribe(threadMode = ThreadMode.BACKGROUND)
-    fun onFollowedTagsChanged(event: FollowedTagsChanged) {
+    fun onFollowedTagsFetched(event: FollowedTagsFetched) {
         val result = if (event.didSucceed()) {
             Success
         } else {
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/services/update/ReaderUpdateLogic.java b/WordPress/src/main/java/org/wordpress/android/ui/reader/services/update/ReaderUpdateLogic.java
@@ -24,6 +24,7 @@
 import org.wordpress.android.ui.prefs.AppPrefs;
 import org.wordpress.android.ui.reader.ReaderConstants;
 import org.wordpress.android.ui.reader.ReaderEvents;
+import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsFetched;
 import org.wordpress.android.ui.reader.ReaderEvents.InterestTagsFetchEnded;
 import org.wordpress.android.ui.reader.services.ServiceCompletionListener;
 import org.wordpress.android.util.AppLog;
@@ -122,24 +123,21 @@ public void onErrorResponse(VolleyError volleyError) {
                             .get("read/menu", params, null, listener, errorListener);
     }
 
-    private boolean displayNameUpdateWasNeeded(ReaderTagList serverTopics) {
-        boolean updateDone = false;
-
+    /**
+     * Update the display names of the default tags (such as Subscribed and Discover) in the serverTopics list.
+     *
+     * @param serverTopics The list of default tags.
+     */
+    private void updateDisplayNamesIfNeeded(@NonNull ReaderTagList serverTopics) {
         for (ReaderTag tag : serverTopics) {
-            String tagNameBefore = tag.getTagDisplayName();
             if (tag.isFollowedSites()) {
                 tag.setTagDisplayName(mContext.getString(R.string.reader_subscribed_display_name));
-                if (!tagNameBefore.equals(tag.getTagDisplayName())) updateDone = true;
             } else if (tag.isDiscover()) {
                 tag.setTagDisplayName(mContext.getString(R.string.reader_discover_display_name));
-                if (!tagNameBefore.equals(tag.getTagDisplayName())) updateDone = true;
             } else if (tag.isPostsILike()) {
                 tag.setTagDisplayName(mContext.getString(R.string.reader_my_likes_display_name));
-                if (!tagNameBefore.equals(tag.getTagDisplayName())) updateDone = true;
             }
         }
-
-        return updateDone;
     }
 
     private void handleUpdateTagsResponse(final JSONObject jsonObject) {
@@ -151,7 +149,7 @@ public void run() {
                 ReaderTagList serverTopics = new ReaderTagList();
                 serverTopics.addAll(parseTags(jsonObject, "default", ReaderTagType.DEFAULT));
 
-                boolean displayNameUpdateWasNeeded = displayNameUpdateWasNeeded(serverTopics);
+                updateDisplayNamesIfNeeded(serverTopics);
 
                 serverTopics.addAll(parseTags(jsonObject, "subscribed", ReaderTagType.FOLLOWED));
 
@@ -176,13 +174,11 @@ public void run() {
                 localTopics.addAll(ReaderTagTable.getFollowedTags());
                 localTopics.addAll(ReaderTagTable.getBookmarkTags());
                 localTopics.addAll(ReaderTagTable.getCustomListTags());
+                localTopics.addAll(ReaderTagTable.getDiscoverPostCardsTags());
 
-                if (
-                        !localTopics.isSameList(serverTopics)
-                        || displayNameUpdateWasNeeded
-                ) {
-                    AppLog.d(AppLog.T.READER, "reader service > followed topics changed "
-                                              + "updatedDisplayNames [" + displayNameUpdateWasNeeded + "]");
+                boolean didChangeFollowedTags = false;
+                if (!localTopics.isSameList(serverTopics)) {
+                    AppLog.d(AppLog.T.READER, "reader service > followed topics changed");
 
                     if (!mAccountStore.hasAccessToken()) {
                         // Do not delete locally saved tags for logged out user
@@ -195,8 +191,9 @@ public void run() {
                         ReaderTagTable.replaceTags(serverTopics);
                     }
                     // broadcast the fact that there are changes
-                    EventBus.getDefault().post(new ReaderEvents.FollowedTagsChanged(true));
+                    didChangeFollowedTags = true;
                 }
+                EventBus.getDefault().post(new FollowedTagsFetched(true, didChangeFollowedTags));
                 AppPrefs.setReaderTagsUpdatedTimestamp(new Date().getTime());
 
                 taskCompleted(UpdateTask.TAGS);
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/subfilter/SubFilterViewModel.kt
@@ -393,7 +393,7 @@ class SubFilterViewModel @Inject constructor(
 
     @Suppress("unused", "UNUSED_PARAMETER")
     @Subscribe(threadMode = ThreadMode.MAIN)
-    fun onEventMainThread(event: ReaderEvents.FollowedTagsChanged) {
+    fun onEventMainThread(event: ReaderEvents.FollowedTagsFetched) {
         AppLog.d(T.READER, "Subfilter bottom sheet > followed tags changed")
         loadSubFilters()
     }
diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModel.kt
@@ -135,7 +135,8 @@ class ReaderViewModel @Inject constructor(
 //        _showJetpackPoweredBottomSheet.value = Event(true)
     }
 
-    private fun loadTabs(savedInstanceState: Bundle? = null) {
+    @JvmOverloads
+    fun loadTabs(savedInstanceState: Bundle? = null) {
         launch {
             val tagList = loadReaderTabsUseCase.loadTabs()
             if (tagList.isNotEmpty() && readerTagsList != tagList) {
@@ -215,7 +216,7 @@ class ReaderViewModel @Inject constructor(
 
     @Suppress("unused", "UNUSED_PARAMETER")
     @Subscribe(threadMode = MAIN)
-    fun onTagsUpdated(event: ReaderEvents.FollowedTagsChanged) {
+    fun onTagsUpdated(event: ReaderEvents.FollowedTagsFetched) {
         loadTabs()
     }
 
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/repository/FetchFollowedTagsUseCaseTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/repository/FetchFollowedTagsUseCaseTest.kt
@@ -9,7 +9,7 @@ import org.mockito.Mock
 import org.mockito.junit.MockitoJUnitRunner
 import org.mockito.kotlin.whenever
 import org.wordpress.android.BaseUnitTest
-import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsChanged
+import org.wordpress.android.ui.reader.ReaderEvents.FollowedTagsFetched
 import org.wordpress.android.ui.reader.repository.ReaderRepositoryCommunication.Error.NetworkUnavailable
 import org.wordpress.android.ui.reader.repository.ReaderRepositoryCommunication.Error.RemoteRequestFailure
 import org.wordpress.android.ui.reader.repository.ReaderRepositoryCommunication.Success
@@ -61,12 +61,12 @@ class FetchFollowedTagsUseCaseTest : BaseUnitTest() {
     }
 
     @Test
-    fun `Success returned when FollowedTagsChanged event is posted with true`() = test {
+    fun `Success returned when FollowedTagsFetched event is posted with success`() = test {
         // Given
         whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(true)
-        val event = FollowedTagsChanged(true)
+        val event = FollowedTagsFetched(true)
         whenever(readerUpdateServiceStarterWrapper.startService(contextProvider.getContext(), EnumSet.of(TAGS)))
-            .then { useCase.onFollowedTagsChanged(event) }
+            .then { useCase.onFollowedTagsFetched(event) }
 
         // When
         val result = useCase.fetch()
@@ -76,12 +76,12 @@ class FetchFollowedTagsUseCaseTest : BaseUnitTest() {
     }
 
     @Test
-    fun `RemoteRequestFailure returned when FollowedTagsChanged event is posted with false`() = test {
+    fun `RemoteRequestFailure returned when FollowedTagsFetched event is posted with failure`() = test {
         // Given
         whenever(networkUtilsWrapper.isNetworkAvailable()).thenReturn(true)
-        val event = FollowedTagsChanged(false)
+        val event = FollowedTagsFetched(false)
         whenever(readerUpdateServiceStarterWrapper.startService(contextProvider.getContext(), EnumSet.of(TAGS)))
-            .then { useCase.onFollowedTagsChanged(event) }
+            .then { useCase.onFollowedTagsFetched(event) }
 
         // When
         val result = useCase.fetch()
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/repository/ReaderDiscoverDataProviderTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/repository/ReaderDiscoverDataProviderTest.kt
diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderViewModelTest.kt

Original file line number	Diff line number	Diff line change
`@@ -44,6 +44,8 @@ public boolean isSameList(ReaderTagList otherList) {`
`44`	`44`	`return false;`
`45`	`45`	`} else if (!otherTag.getTagTitle().equals(this.get(i).getTagTitle())) {`
`46`	`46`	`return false;`
	`47`	`+ } else if (!otherTag.getTagDisplayName().equals(this.get(i).getTagDisplayName())) {`
	`48`	`+ return false;`
`47`	`49`	`}`
`48`	`50`	`}`
`49`	`51`