Skip to content

Commit

Permalink
Merge pull request #20216 from wordpress-mobile/issue/20010-reader-ia…
Browse files Browse the repository at this point in the history
…-subscription-infinite-loading

[Reader][0 blogs/0 tags] Fix Subscription feed infinite loading and fetch when tapping filters
  • Loading branch information
Thomas Horta authored Feb 21, 2024
2 parents ab225e5 + c702921 commit a3b6de7
Show file tree
Hide file tree
Showing 15 changed files with 76 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -2333,7 +2335,7 @@ public void run() {
mActionableEmptyView.button.setVisibility(View.GONE);
mActionableEmptyView.subtitle.setVisibility(View.GONE);
showEmptyView();
} else {
} else if (!isPostAdapterEmpty()) {
hideEmptyView();
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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 -> {
Expand All @@ -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;
}

Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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));

Expand All @@ -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
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
Loading

0 comments on commit a3b6de7

Please sign in to comment.