From 768cb0df5f1c54de09ff6f15ce4ccc3a9613189a Mon Sep 17 00:00:00 2001 From: Thomas Horta Date: Tue, 7 May 2024 17:32:00 -0300 Subject: [PATCH 1/4] Implement Lazy Loading in Tags Feed --- .../tagsfeed/ReaderTagsFeedUiStateMapper.kt | 24 +++- .../tagsfeed/ReaderTagsFeedViewModel.kt | 129 +++++++++++------- .../views/compose/tagsfeed/ReaderTagsFeed.kt | 14 +- .../viewmodels/ReaderTagsFeedViewModelTest.kt | 8 +- 4 files changed, 116 insertions(+), 59 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedUiStateMapper.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedUiStateMapper.kt index 341584d3e700..51829c1bd3b6 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedUiStateMapper.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedUiStateMapper.kt @@ -20,6 +20,7 @@ class ReaderTagsFeedUiStateMapper @Inject constructor( onPostCardClick: (TagsFeedPostItem) -> Unit, onPostLikeClick: () -> Unit, onPostMoreMenuClick: () -> Unit, + onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit, ) = ReaderTagsFeedViewModel.TagFeedItem( tagChip = ReaderTagsFeedViewModel.TagChip( tag = tag, @@ -51,6 +52,7 @@ class ReaderTagsFeedUiStateMapper @Inject constructor( ) } ), + onItemEnteredView = onItemEnteredView, ) fun mapErrorTagFeedItem( @@ -58,6 +60,7 @@ class ReaderTagsFeedUiStateMapper @Inject constructor( errorType: ReaderTagsFeedViewModel.ErrorType, onTagClick: (ReaderTag) -> Unit, onRetryClick: () -> Unit, + onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit, ): ReaderTagsFeedViewModel.TagFeedItem = ReaderTagsFeedViewModel.TagFeedItem( tagChip = ReaderTagsFeedViewModel.TagChip( @@ -68,11 +71,13 @@ class ReaderTagsFeedUiStateMapper @Inject constructor( type = errorType, onRetryClick = onRetryClick, ), + onItemEnteredView = onItemEnteredView, ) - fun mapLoadingPostsUiState( + fun mapInitialPostsUiState( tags: List, onTagClick: (ReaderTag) -> Unit, + onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit, ): ReaderTagsFeedViewModel.UiState.Loaded = ReaderTagsFeedViewModel.UiState.Loaded( tags.map { tag -> @@ -81,8 +86,23 @@ class ReaderTagsFeedUiStateMapper @Inject constructor( tag = tag, onTagClick = onTagClick, ), - postList = ReaderTagsFeedViewModel.PostList.Loading, + postList = ReaderTagsFeedViewModel.PostList.Initial, + onItemEnteredView = onItemEnteredView, ) } ) + + fun mapLoadingTagFeedItem( + tag: ReaderTag, + onTagClick: (ReaderTag) -> Unit, + onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit, + ): ReaderTagsFeedViewModel.TagFeedItem = + ReaderTagsFeedViewModel.TagFeedItem( + tagChip = ReaderTagsFeedViewModel.TagChip( + tag = tag, + onTagClick = onTagClick, + ), + postList = ReaderTagsFeedViewModel.PostList.Loading, + onItemEnteredView = onItemEnteredView, + ) } diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt index 0558624aa9d9..8f6b0bb9c716 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt @@ -1,10 +1,12 @@ package org.wordpress.android.ui.reader.viewmodels.tagsfeed +import android.util.Log import androidx.annotation.VisibleForTesting import androidx.lifecycle.LiveData import androidx.lifecycle.MediatorLiveData import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.update @@ -24,6 +26,7 @@ import org.wordpress.android.viewmodel.SingleLiveEvent import javax.inject.Inject import javax.inject.Named +@OptIn(ExperimentalCoroutinesApi::class) @HiltViewModel class ReaderTagsFeedViewModel @Inject constructor( @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, @@ -68,13 +71,7 @@ class ReaderTagsFeedViewModel @Inject constructor( // Initially add all tags to the list with the posts loading UI _uiStateFlow.update { - readerTagsFeedUiStateMapper.mapLoadingPostsUiState(tags, ::onTagClick) - } - // Fetch all posts and update the posts loading UI to either loaded or error when the request finishes - launch { - tags.forEach { - fetchTag(it) - } + readerTagsFeedUiStateMapper.mapInitialPostsUiState(tags, ::onTagClick, ::onItemEnteredView) } } @@ -97,64 +94,89 @@ class ReaderTagsFeedViewModel @Inject constructor( */ @Suppress("SwallowedException") private suspend fun fetchTag(tag: ReaderTag) { - val updatedLoadedData = getUpdatedLoadedData() - // At this point, all tag feed items already exist in the UI with the loading status. - // We need it's index to update it to either Loaded or Error when the request is finished. - val existingIndex = updatedLoadedData.indexOfFirst { it.tagChip.tag == tag } - // Remove the current row of this tag (which is loading). This will be used to later add an updated item with - // either Loaded or Error status, depending on the result of the request. - updatedLoadedData.removeAll { it.tagChip.tag == tag } - try { + // Set the tag to loading state + updateTagFeedItem( + readerTagsFeedUiStateMapper.mapLoadingTagFeedItem( + tag = tag, + onTagClick = ::onTagClick, + onItemEnteredView = ::onItemEnteredView, + ) + ) + + val updatedItem: TagFeedItem = try { // Fetch posts for tag val posts = readerPostRepository.fetchNewerPostsForTag(tag) if (posts.isNotEmpty()) { - updatedLoadedData.add( - existingIndex, - readerTagsFeedUiStateMapper.mapLoadedTagFeedItem( - tag = tag, - posts = posts, - onTagClick = ::onTagClick, - onSiteClick = ::onSiteClick, - onPostCardClick = ::onPostCardClick, - onPostLikeClick = ::onPostLikeClick, - onPostMoreMenuClick = ::onPostMoreMenuClick, - ) + readerTagsFeedUiStateMapper.mapLoadedTagFeedItem( + tag = tag, + posts = posts, + onTagClick = ::onTagClick, + onSiteClick = ::onSiteClick, + onPostCardClick = ::onPostCardClick, + onPostLikeClick = ::onPostLikeClick, + onPostMoreMenuClick = ::onPostMoreMenuClick, + onItemEnteredView = ::onItemEnteredView, ) } else { - updatedLoadedData.add( - existingIndex, - readerTagsFeedUiStateMapper.mapErrorTagFeedItem( - tag = tag, - errorType = ErrorType.NoContent, - onTagClick = ::onTagClick, - onRetryClick = ::onRetryClick, - ) - ) - } - } catch (e: ReaderPostFetchException) { - updatedLoadedData.add( - existingIndex, readerTagsFeedUiStateMapper.mapErrorTagFeedItem( tag = tag, - errorType = ErrorType.Default, + errorType = ErrorType.NoContent, onTagClick = ::onTagClick, onRetryClick = ::onRetryClick, + onItemEnteredView = ::onItemEnteredView, ) + } + } catch (e: ReaderPostFetchException) { + readerTagsFeedUiStateMapper.mapErrorTagFeedItem( + tag = tag, + errorType = ErrorType.Default, + onTagClick = ::onTagClick, + onRetryClick = ::onRetryClick, + onItemEnteredView = ::onItemEnteredView, ) } - _uiStateFlow.update { UiState.Loaded(updatedLoadedData) } + + updateTagFeedItem(updatedItem) } - private fun getUpdatedLoadedData(): MutableList { + private fun getLoadedData(uiState: UiState): MutableList { val updatedLoadedData = mutableListOf() - val currentUiState = _uiStateFlow.value - if (currentUiState is UiState.Loaded) { - val currentLoadedData = currentUiState.data - updatedLoadedData.addAll(currentLoadedData) + if (uiState is UiState.Loaded) { + updatedLoadedData.addAll(uiState.data) } return updatedLoadedData } + // Update the UI state for a single feed item, making sure to do it atomically so we don't lose any updates. + private fun updateTagFeedItem(updatedItem: TagFeedItem) { + _uiStateFlow.update { uiState -> + val updatedLoadedData = getLoadedData(uiState) + + // At this point, all tag feed items already exist in the UI. + // We need it's index to update it and keep it in place. + val existingIndex = updatedLoadedData.indexOfFirst { it.tagChip.tag == updatedItem.tagChip.tag } + + // Remove the current row(s) of this tag, so we don't have duplicates. + updatedLoadedData.removeAll { it.tagChip.tag == updatedItem.tagChip.tag } + + // Add the updated item in the correct position. + updatedLoadedData.add(existingIndex, updatedItem) + + UiState.Loaded(updatedLoadedData) + } + } + + private fun onItemEnteredView(item: TagFeedItem) { + if (item.postList != PostList.Initial) { + // do nothing as it's still loading or already loaded + return + } + + launch { + fetchTag(item.tagChip.tag) + } + } + private fun onOpenTagsListClick() { // TODO } @@ -211,10 +233,10 @@ class ReaderTagsFeedViewModel @Inject constructor( } sealed class UiState { - object Initial : UiState() + data object Initial : UiState() data class Loaded(val data: List) : UiState() - object Loading : UiState() + data object Loading : UiState() data class Empty(val onOpenTagsListClick: () -> Unit) : UiState() } @@ -222,7 +244,12 @@ class ReaderTagsFeedViewModel @Inject constructor( data class TagFeedItem( val tagChip: TagChip, val postList: PostList, - ) + private val onItemEnteredView: (TagFeedItem) -> Unit = {}, + ) { + fun onEnteredView() { + onItemEnteredView(this) + } + } data class TagChip( val tag: ReaderTag, @@ -230,9 +257,11 @@ class ReaderTagsFeedViewModel @Inject constructor( ) sealed class PostList { + data object Initial : PostList() + data class Loaded(val items: List) : PostList() - object Loading : PostList() + data object Loading : PostList() data class Error( val type: ErrorType, diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/tagsfeed/ReaderTagsFeed.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/tagsfeed/ReaderTagsFeed.kt index eba913b2840e..9aa3186797f4 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/tagsfeed/ReaderTagsFeed.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/views/compose/tagsfeed/ReaderTagsFeed.kt @@ -28,6 +28,7 @@ import androidx.compose.material.ripple.rememberRipple import androidx.compose.material3.Icon import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -84,7 +85,14 @@ private fun Loaded(uiState: UiState.Loaded) { ) { items( items = uiState.data, - ) { (tagChip, postList) -> + ) { item -> + val tagChip = item.tagChip + val postList = item.postList + + LaunchedEffect(Unit) { + item.onEnteredView() + } + val backgroundColor = if (isSystemInDarkTheme()) { AppColor.White.copy(alpha = 0.12F) } else { @@ -103,7 +111,7 @@ private fun Loaded(uiState: UiState.Loaded) { Spacer(modifier = Modifier.height(Margin.Large.value)) // Posts list UI when (postList) { - is PostList.Loading -> PostListLoading() + is PostList.Initial, is PostList.Loading -> PostListLoading() is PostList.Loaded -> PostListLoaded(postList, tagChip, backgroundColor) is PostList.Error -> PostListError(backgroundColor, tagChip, postList) } @@ -525,7 +533,7 @@ fun ReaderTagsFeedLoaded() { ), TagFeedItem( tagChip = TagChip(readerTag, {}), - postList = PostList.Loading, + postList = PostList.Initial, ), TagFeedItem( tagChip = TagChip(readerTag, {}), diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderTagsFeedViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderTagsFeedViewModelTest.kt index b9bda3144e48..125453190f92 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderTagsFeedViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderTagsFeedViewModelTest.kt @@ -282,7 +282,7 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { } private fun mockMapLoadingTagFeedItems() { - whenever(readerTagsFeedUiStateMapper.mapLoadingPostsUiState(any(), any())) + whenever(readerTagsFeedUiStateMapper.mapInitialPostsUiState(any(), any())) .thenAnswer { val tags = it.getArgument>(0) ReaderTagsFeedViewModel.UiState.Loaded( @@ -292,7 +292,7 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { tag = tag, onTagClick = {}, ), - postList = ReaderTagsFeedViewModel.PostList.Loading, + postList = ReaderTagsFeedViewModel.PostList.Initial, ) } ) @@ -300,14 +300,14 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { } private fun mockMapLoadedTagFeedItems() { - whenever(readerTagsFeedUiStateMapper.mapLoadedTagFeedItem(any(), any(), any(), any(), any(), any(), any())) + whenever(readerTagsFeedUiStateMapper.mapLoadedTagFeedItem(any(), any(), any(), any(), any(), any(), any(),)) .thenAnswer { getLoadedTagFeedItem(it.getArgument(0)) } } private fun mockMapErrorTagFeedItems() { - whenever(readerTagsFeedUiStateMapper.mapErrorTagFeedItem(any(), any(), any(), any())) + whenever(readerTagsFeedUiStateMapper.mapErrorTagFeedItem(any(), any(), any(), any(),)) .thenAnswer { getErrorTagFeedItem(it.getArgument(0)) } From 2b60f3aff214d28faad09da8a45dcd2ba31270d9 Mon Sep 17 00:00:00 2001 From: Thomas Horta Date: Thu, 9 May 2024 13:45:04 -0300 Subject: [PATCH 2/4] Fix existing unit tests --- .../tagsfeed/ReaderTagsFeedViewModel.kt | 12 ++-- .../viewmodels/ReaderTagsFeedViewModelTest.kt | 67 +++++++++++++------ .../ReaderTagsFeedUiStateMapperTest.kt | 18 +++-- 3 files changed, 64 insertions(+), 33 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt index 8f6b0bb9c716..c07c2cfa82d7 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt @@ -1,12 +1,10 @@ package org.wordpress.android.ui.reader.viewmodels.tagsfeed -import android.util.Log import androidx.annotation.VisibleForTesting import androidx.lifecycle.LiveData import androidx.lifecycle.MediatorLiveData import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.update @@ -26,7 +24,6 @@ import org.wordpress.android.viewmodel.SingleLiveEvent import javax.inject.Inject import javax.inject.Named -@OptIn(ExperimentalCoroutinesApi::class) @HiltViewModel class ReaderTagsFeedViewModel @Inject constructor( @Named(BG_THREAD) private val bgDispatcher: CoroutineDispatcher, @@ -89,8 +86,6 @@ class ReaderTagsFeedViewModel @Inject constructor( /** * Fetch posts for a single tag. This method will emit a new state to [uiStateFlow] for different [UiState]s: * [UiState.Initial], [UiState.Loaded], [UiState.Loading], [UiState.Empty], but only for the tag being fetched. - * - * Can be used for retrying a failed fetch, for instance. */ @Suppress("SwallowedException") private suspend fun fetchTag(tag: ReaderTag) { @@ -166,7 +161,8 @@ class ReaderTagsFeedViewModel @Inject constructor( } } - private fun onItemEnteredView(item: TagFeedItem) { + @VisibleForTesting + fun onItemEnteredView(item: TagFeedItem) { if (item.postList != PostList.Initial) { // do nothing as it's still loading or already loaded return @@ -181,7 +177,7 @@ class ReaderTagsFeedViewModel @Inject constructor( // TODO } - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + @VisibleForTesting fun onTagClick(readerTag: ReaderTag) { _actionEvents.value = ActionEvent.OpenTagPostsFeed(readerTag) } @@ -190,7 +186,7 @@ class ReaderTagsFeedViewModel @Inject constructor( // TODO } - @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + @VisibleForTesting fun onSiteClick(postItem: TagsFeedPostItem) { launch { findPost(postItem.postId, postItem.blogId)?.let { diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderTagsFeedViewModelTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderTagsFeedViewModelTest.kt index 125453190f92..281137a82de2 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderTagsFeedViewModelTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/ReaderTagsFeedViewModelTest.kt @@ -22,9 +22,9 @@ import org.wordpress.android.models.ReaderPost import org.wordpress.android.models.ReaderPostList import org.wordpress.android.models.ReaderTag import org.wordpress.android.models.ReaderTagType +import org.wordpress.android.ui.reader.ReaderTestUtils import org.wordpress.android.ui.reader.discover.ReaderNavigationEvents import org.wordpress.android.ui.reader.discover.ReaderPostCardActionsHandler -import org.wordpress.android.ui.reader.ReaderTestUtils import org.wordpress.android.ui.reader.exceptions.ReaderPostFetchException import org.wordpress.android.ui.reader.repository.ReaderPostRepository import org.wordpress.android.ui.reader.viewmodels.tagsfeed.ReaderTagsFeedUiStateMapper @@ -82,7 +82,7 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { } @Test - fun `given valid tag, when fetchTag, then UI state should update properly`() = testCollectingUiStates { + fun `given valid tag, when loaded, then UI state should update properly`() = testCollectingUiStates { // Given val tag = ReaderTestUtils.createTag("tag") val posts = ReaderPostList().apply { @@ -92,12 +92,15 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { delay(100) posts } + mockMapInitialTagFeedItems() mockMapLoadingTagFeedItems() mockMapLoadedTagFeedItems() // When viewModel.start(listOf(tag)) advanceUntilIdle() + viewModel.onItemEnteredView(getInitialTagFeedItem(tag)) + advanceUntilIdle() // Then assertThat(collectedUiStates).contains( @@ -108,7 +111,7 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { } @Test - fun `given invalid tag, when fetchTag, then UI state should update properly`() = testCollectingUiStates { + fun `given invalid tag, when loaded, then UI state should update properly`() = testCollectingUiStates { // Given val tag = ReaderTestUtils.createTag("tag") val error = ReaderPostFetchException("error") @@ -116,12 +119,15 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { delay(100) throw error } + mockMapInitialTagFeedItems() mockMapLoadingTagFeedItems() mockMapErrorTagFeedItems() // When viewModel.start(listOf(tag)) advanceUntilIdle() + viewModel.onItemEnteredView(getInitialTagFeedItem(tag)) + advanceUntilIdle() // Then assertThat(collectedUiStates).contains( @@ -133,7 +139,7 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { @Suppress("LongMethod") @Test - fun `given valid tags, when start, then UI state should update properly`() = testCollectingUiStates { + fun `given valid tags, when loaded, then UI state should update properly`() = testCollectingUiStates { // Given val tag1 = ReaderTestUtils.createTag("tag1") val tag2 = ReaderTestUtils.createTag("tag2") @@ -151,12 +157,17 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { delay(200) posts2 } + mockMapInitialTagFeedItems() mockMapLoadingTagFeedItems() mockMapLoadedTagFeedItems() // When viewModel.start(listOf(tag1, tag2)) advanceUntilIdle() + viewModel.onItemEnteredView(getInitialTagFeedItem(tag1)) + advanceUntilIdle() + viewModel.onItemEnteredView(getInitialTagFeedItem(tag2)) + advanceUntilIdle() // Then assertThat(collectedUiStates).contains( @@ -171,7 +182,7 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { @Suppress("LongMethod") @Test - fun `given valid and invalid tags, when fetchAll, then UI state should update properly`() = testCollectingUiStates { + fun `given valid and invalid tags, when loaded, then UI state should update properly`() = testCollectingUiStates { // Given val tag1 = ReaderTestUtils.createTag("tag1") val tag2 = ReaderTestUtils.createTag("tag2") @@ -187,6 +198,7 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { delay(200) throw error2 } + mockMapInitialTagFeedItems() mockMapLoadingTagFeedItems() mockMapLoadedTagFeedItems() mockMapErrorTagFeedItems() @@ -194,6 +206,10 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { // When viewModel.start(listOf(tag1, tag2)) advanceUntilIdle() + viewModel.onItemEnteredView(getInitialTagFeedItem(tag1)) + advanceUntilIdle() + viewModel.onItemEnteredView(getInitialTagFeedItem(tag2)) + advanceUntilIdle() // Then assertThat(collectedUiStates).contains( @@ -250,7 +266,7 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { delay(200) posts2 } - mockMapLoadingTagFeedItems() + mockMapInitialTagFeedItems() mockMapLoadedTagFeedItems() // When @@ -281,38 +297,47 @@ class ReaderTagsFeedViewModelTest : BaseUnitTest() { assertThat(collectedUiStates).last().isInstanceOf(ReaderTagsFeedViewModel.UiState.Empty::class.java) } - private fun mockMapLoadingTagFeedItems() { - whenever(readerTagsFeedUiStateMapper.mapInitialPostsUiState(any(), any())) + private fun mockMapInitialTagFeedItems() { + whenever(readerTagsFeedUiStateMapper.mapInitialPostsUiState(any(), any(), any())) .thenAnswer { val tags = it.getArgument>(0) ReaderTagsFeedViewModel.UiState.Loaded( - tags.map { tag -> - ReaderTagsFeedViewModel.TagFeedItem( - tagChip = ReaderTagsFeedViewModel.TagChip( - tag = tag, - onTagClick = {}, - ), - postList = ReaderTagsFeedViewModel.PostList.Initial, - ) - } + tags.map { tag -> getInitialTagFeedItem(tag) } ) } } - private fun mockMapLoadedTagFeedItems() { - whenever(readerTagsFeedUiStateMapper.mapLoadedTagFeedItem(any(), any(), any(), any(), any(), any(), any(),)) + private fun mockMapLoadingTagFeedItems() { + whenever(readerTagsFeedUiStateMapper.mapLoadingTagFeedItem(any(), any(), any())) .thenAnswer { - getLoadedTagFeedItem(it.getArgument(0)) + val tag = it.getArgument(0) + ReaderTagsFeedViewModel.TagFeedItem( + ReaderTagsFeedViewModel.TagChip(tag, {}), + ReaderTagsFeedViewModel.PostList.Loading + ) } } + private fun mockMapLoadedTagFeedItems() { + whenever( + readerTagsFeedUiStateMapper.mapLoadedTagFeedItem(any(), any(), any(), any(), any(), any(), any(), any()) + ).thenAnswer { + getLoadedTagFeedItem(it.getArgument(0)) + } + } + private fun mockMapErrorTagFeedItems() { - whenever(readerTagsFeedUiStateMapper.mapErrorTagFeedItem(any(), any(), any(), any(),)) + whenever(readerTagsFeedUiStateMapper.mapErrorTagFeedItem(any(), any(), any(), any(), any())) .thenAnswer { getErrorTagFeedItem(it.getArgument(0)) } } + private fun getInitialTagFeedItem(tag: ReaderTag) = ReaderTagsFeedViewModel.TagFeedItem( + ReaderTagsFeedViewModel.TagChip(tag, {}), + ReaderTagsFeedViewModel.PostList.Initial + ) + private fun getLoadedTagFeedItem(tag: ReaderTag) = ReaderTagsFeedViewModel.TagFeedItem( ReaderTagsFeedViewModel.TagChip(tag, {}), ReaderTagsFeedViewModel.PostList.Loaded(listOf()) diff --git a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedUiStateMapperTest.kt b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedUiStateMapperTest.kt index 737cd43ffe37..ee446059dc6d 100644 --- a/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedUiStateMapperTest.kt +++ b/WordPress/src/test/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedUiStateMapperTest.kt @@ -56,6 +56,7 @@ class ReaderTagsFeedUiStateMapperTest : BaseUnitTest() { val onPostCardClick: (TagsFeedPostItem) -> Unit = {} val onPostLikeClick = {} val onPostMoreMenuClick = {} + val onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit = {} val dateLine = "dateLine" val numberLikesText = "numberLikesText" @@ -79,6 +80,7 @@ class ReaderTagsFeedUiStateMapperTest : BaseUnitTest() { onPostCardClick = onPostCardClick, onPostLikeClick = onPostLikeClick, onPostMoreMenuClick = onPostMoreMenuClick, + onItemEnteredView = onItemEnteredView, ) // Then val expected = ReaderTagsFeedViewModel.TagFeedItem( @@ -106,6 +108,7 @@ class ReaderTagsFeedUiStateMapperTest : BaseUnitTest() { ) ) ), + onItemEnteredView = onItemEnteredView, ) assertEquals(expected, actual) } @@ -123,12 +126,14 @@ class ReaderTagsFeedUiStateMapperTest : BaseUnitTest() { val errorType = ReaderTagsFeedViewModel.ErrorType.Default val onTagClick: (ReaderTag) -> Unit = {} val onRetryClick = {} + val onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit = {} // When val actual = classToTest.mapErrorTagFeedItem( tag = readerTag, errorType = errorType, onTagClick = onTagClick, onRetryClick = onRetryClick, + onItemEnteredView = onItemEnteredView, ) // Then @@ -140,7 +145,8 @@ class ReaderTagsFeedUiStateMapperTest : BaseUnitTest() { postList = ReaderTagsFeedViewModel.PostList.Error( type = errorType, onRetryClick = onRetryClick, - ) + ), + onItemEnteredView = onItemEnteredView, ) assertEquals(expected, actual) } @@ -149,6 +155,7 @@ class ReaderTagsFeedUiStateMapperTest : BaseUnitTest() { fun `Should map loading posts UI state correctly`() { // Given val onTagClick: (ReaderTag) -> Unit = {} + val onItemEnteredView: (ReaderTagsFeedViewModel.TagFeedItem) -> Unit = {} val tag1 = ReaderTag( "tag", "tag", @@ -166,9 +173,10 @@ class ReaderTagsFeedUiStateMapperTest : BaseUnitTest() { val tags = listOf(tag1, tag2) // When - val actual = classToTest.mapLoadingPostsUiState( + val actual = classToTest.mapInitialPostsUiState( tags = tags, onTagClick = onTagClick, + onItemEnteredView = onItemEnteredView, ) // Then @@ -179,14 +187,16 @@ class ReaderTagsFeedUiStateMapperTest : BaseUnitTest() { tag = tag1, onTagClick = onTagClick, ), - postList = ReaderTagsFeedViewModel.PostList.Loading, + postList = ReaderTagsFeedViewModel.PostList.Initial, + onItemEnteredView = onItemEnteredView, ), ReaderTagsFeedViewModel.TagFeedItem( tagChip = ReaderTagsFeedViewModel.TagChip( tag = tag2, onTagClick = onTagClick, ), - postList = ReaderTagsFeedViewModel.PostList.Loading, + postList = ReaderTagsFeedViewModel.PostList.Initial, + onItemEnteredView = onItemEnteredView, ) ) ) From c977d71291de1a1ca1940ce667363343cae5a95a Mon Sep 17 00:00:00 2001 From: Thomas Horta Date: Thu, 9 May 2024 13:57:50 -0300 Subject: [PATCH 3/4] Move post fetching for tags to IO thread --- .../android/ui/reader/repository/ReaderPostRepository.kt | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/ReaderPostRepository.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/ReaderPostRepository.kt index 7fbb8e88556e..503682566dbb 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/ReaderPostRepository.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/repository/ReaderPostRepository.kt @@ -3,7 +3,9 @@ package org.wordpress.android.ui.reader.repository import com.android.volley.VolleyError import com.wordpress.rest.RestRequest import dagger.Reusable +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.suspendCancellableCoroutine +import kotlinx.coroutines.withContext import org.json.JSONObject import org.wordpress.android.WordPress.Companion.getRestClientUtilsV1_2 import org.wordpress.android.datasets.ReaderPostTable @@ -11,6 +13,7 @@ import org.wordpress.android.datasets.ReaderTagTable import org.wordpress.android.models.ReaderPostList import org.wordpress.android.models.ReaderTag import org.wordpress.android.models.ReaderTagType +import org.wordpress.android.modules.IO_THREAD import org.wordpress.android.ui.reader.ReaderConstants import org.wordpress.android.ui.reader.actions.ReaderActions import org.wordpress.android.ui.reader.actions.ReaderActions.UpdateResultListener @@ -23,6 +26,7 @@ import org.wordpress.android.util.LocaleManagerWrapper import org.wordpress.android.util.UrlUtils import java.util.Locale import javax.inject.Inject +import javax.inject.Named import kotlin.coroutines.resume import kotlin.coroutines.resumeWithException @@ -30,13 +34,14 @@ import kotlin.coroutines.resumeWithException class ReaderPostRepository @Inject constructor( private val localeManagerWrapper: LocaleManagerWrapper, private val localSource: ReaderPostLocalSource, + @Named(IO_THREAD) private val ioDispatcher: CoroutineDispatcher, ) { /** * Fetches and returns the most recent posts for the passed tag, respecting the maxPosts limit. * It always fetches the most recent posts, saves them to the local DB and returns the latest from that cache. */ - suspend fun fetchNewerPostsForTag(tag: ReaderTag, maxPosts: Int = 10): ReaderPostList { - return suspendCancellableCoroutine { cont -> + suspend fun fetchNewerPostsForTag(tag: ReaderTag, maxPosts: Int = 10): ReaderPostList = withContext(ioDispatcher) { + suspendCancellableCoroutine { cont -> val resultListener = UpdateResultListener { result -> if (result == ReaderActions.UpdateResult.FAILED) { cont.resumeWithException( From 4acb2b9248382fcdc9d32e5995b1103129bd0e7c Mon Sep 17 00:00:00 2001 From: Thomas Horta Date: Fri, 10 May 2024 12:39:13 -0300 Subject: [PATCH 4/4] Improve item update (review comment) --- .../tagsfeed/ReaderTagsFeedViewModel.kt | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt index 8f24e741b42d..139c0c1179cf 100644 --- a/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt +++ b/WordPress/src/main/java/org/wordpress/android/ui/reader/viewmodels/tagsfeed/ReaderTagsFeedViewModel.kt @@ -154,14 +154,13 @@ class ReaderTagsFeedViewModel @Inject constructor( val updatedLoadedData = getLoadedData(uiState) // At this point, all tag feed items already exist in the UI. - // We need it's index to update it and keep it in place. - val existingIndex = updatedLoadedData.indexOfFirst { it.tagChip.tag == updatedItem.tagChip.tag } - - // Remove the current row(s) of this tag, so we don't have duplicates. - updatedLoadedData.removeAll { it.tagChip.tag == updatedItem.tagChip.tag } - - // Add the updated item in the correct position. - updatedLoadedData.add(existingIndex, updatedItem) + // We need it's index to update it and keep it in the same place. + updatedLoadedData.indexOfFirst { it.tagChip.tag == updatedItem.tagChip.tag } + .takeIf { it >= 0 } + ?.let { existingIndex -> + // Update item + updatedLoadedData[existingIndex] = updatedItem + } UiState.Loaded(updatedLoadedData) }