From 1788f2b4a236f3d2d2bc412a3e61ab73cf358bb6 Mon Sep 17 00:00:00 2001 From: Caren Chang Date: Fri, 23 Sep 2022 15:12:20 -0700 Subject: [PATCH 1/7] Enable bookmarks on topics page --- .../nowinandroid/feature/topic/TopicScreen.kt | 55 ++++--- .../feature/topic/TopicViewModel.kt | 154 ++++++++++++------ 2 files changed, 144 insertions(+), 65 deletions(-) diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt index 08cd7a8ad..be6efaf3e 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt @@ -52,6 +52,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaFilte import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaLoadingWheel import com.google.samples.apps.nowinandroid.core.designsystem.theme.NiaTheme import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic +import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.model.data.previewNewsResources import com.google.samples.apps.nowinandroid.core.model.data.previewTopics import com.google.samples.apps.nowinandroid.core.ui.newsResourceCardItems @@ -65,24 +66,27 @@ fun TopicRoute( modifier: Modifier = Modifier, viewModel: TopicViewModel = hiltViewModel(), ) { - val uiState: TopicScreenUiState by viewModel.uiState.collectAsStateWithLifecycle() + val topicUiState: TopicUiState by viewModel.topicUiState.collectAsStateWithLifecycle() + val newsUiState: NewsUiState by viewModel.newUiState.collectAsStateWithLifecycle() TopicScreen( - topicState = uiState.topicState, - newsState = uiState.newsState, + topicState = topicUiState, + newsUiState = newsUiState, modifier = modifier, onBackClick = onBackClick, onFollowClick = viewModel::followTopicToggle, - ) + onBookmarkChanged = viewModel::bookmarkNews, + ) } @VisibleForTesting @Composable internal fun TopicScreen( topicState: TopicUiState, - newsState: NewsUiState, + newsUiState: NewsUiState, onBackClick: () -> Unit, onFollowClick: (Boolean) -> Unit, + onBookmarkChanged: (String, Boolean) -> Unit, modifier: Modifier = Modifier, ) { LazyColumn( @@ -111,8 +115,9 @@ internal fun TopicScreen( TopicBody( name = topicState.followableTopic.topic.name, description = topicState.followableTopic.topic.longDescription, - news = newsState, - imageUrl = topicState.followableTopic.topic.imageUrl + news = newsUiState, + imageUrl = topicState.followableTopic.topic.imageUrl, + onBookmarkChanged = onBookmarkChanged ) } } @@ -126,14 +131,15 @@ private fun LazyListScope.TopicBody( name: String, description: String, news: NewsUiState, - imageUrl: String + imageUrl: String, + onBookmarkChanged: (String, Boolean) -> Unit ) { // TODO: Show icon if available item { TopicHeader(name, description, imageUrl) } - TopicCards(news) + TopicCards(news, onBookmarkChanged) } @Composable @@ -160,14 +166,17 @@ private fun TopicHeader(name: String, description: String, imageUrl: String) { } } -private fun LazyListScope.TopicCards(news: NewsUiState) { +private fun LazyListScope.TopicCards( + news: NewsUiState, + onBookmarkChanged: (String, Boolean) -> Unit +) { when (news) { is NewsUiState.Success -> { newsResourceCardItems( items = news.news, - newsResourceMapper = { it }, - isBookmarkedMapper = { /* TODO */ false }, - onToggleBookmark = { /* TODO */ }, + newsResourceMapper = { it.newsResource }, + isBookmarkedMapper = { it.isSaved }, + onToggleBookmark = { onBookmarkChanged(it.newsResource.id, !it.isSaved) }, itemModifier = Modifier.padding(24.dp) ) } @@ -187,7 +196,7 @@ private fun TopicBodyPreview() { LazyColumn { TopicBody( "Jetpack Compose", "Lorem ipsum maximum", - NewsUiState.Success(emptyList()), "" + NewsUiState.Success(emptyList()), "", { _, _ -> } ) } } @@ -238,9 +247,16 @@ fun TopicScreenPopulated() { NiaBackground { TopicScreen( topicState = TopicUiState.Success(FollowableTopic(previewTopics[0], false)), - newsState = NewsUiState.Success(previewNewsResources), - onBackClick = {}, - onFollowClick = {} + newsUiState = NewsUiState.Success( + previewNewsResources.mapIndexed { index, newsResource -> + SaveableNewsResource( + newsResource = newsResource, + isSaved = index % 2 == 0, + ) + } + ), onBackClick = {}, + onFollowClick = {}, + onBookmarkChanged = { _, _ -> }, ) } } @@ -256,9 +272,10 @@ fun TopicScreenLoading() { NiaBackground { TopicScreen( topicState = TopicUiState.Loading, - newsState = NewsUiState.Loading, + newsUiState = NewsUiState.Loading, onBackClick = {}, - onFollowClick = {} + onFollowClick = {}, + onBookmarkChanged = { _, _ -> }, ) } } diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt index d647e4bf7..823e1ec45 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt @@ -19,11 +19,15 @@ package com.google.samples.apps.nowinandroid.feature.topic import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.google.samples.apps.nowinandroid.core.data.repository.AuthorsRepository import com.google.samples.apps.nowinandroid.core.data.repository.NewsRepository import com.google.samples.apps.nowinandroid.core.data.repository.TopicsRepository import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository +import com.google.samples.apps.nowinandroid.core.model.data.Author +import com.google.samples.apps.nowinandroid.core.model.data.FollowableAuthor import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.NewsResource +import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.model.data.Topic import com.google.samples.apps.nowinandroid.core.result.Result import com.google.samples.apps.nowinandroid.core.result.asResult @@ -48,63 +52,126 @@ class TopicViewModel @Inject constructor( private val topicId: String = checkNotNull(savedStateHandle[TopicDestination.topicIdArg]) + val topicUiState: StateFlow = topicUiStateStream( + topicId = topicId, + userDataRepository = userDataRepository, + topicsRepository = topicsRepository + ) + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = TopicUiState.Loading + ) + + val newUiState: StateFlow = newsUiStateStream( + topicId = topicId, + userDataRepository = userDataRepository, + newsRepository = newsRepository + ) + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = NewsUiState.Loading + ) + + fun followTopicToggle(followed: Boolean) { + viewModelScope.launch { + userDataRepository.toggleFollowedTopicId(topicId, followed) + } + } + + fun bookmarkNews(newsResourceId: String, bookmarked: Boolean) { + viewModelScope.launch { + userDataRepository.updateNewsResourceBookmark(newsResourceId, bookmarked) + } + } +} + +private fun topicUiStateStream( + topicId: String, + userDataRepository: UserDataRepository, + topicsRepository: TopicsRepository, +): Flow { // Observe the followed topics, as they could change over time. - private val followedTopicIdsStream: Flow>> = + val followedTopicIdsStream: Flow> = userDataRepository.userDataStream .map { it.followedTopics } - .asResult() // Observe topic information - private val topic: Flow> = topicsRepository.getTopic(topicId).asResult() - - // Observe the News for this topic - private val newsStream: Flow>> = - newsRepository.getNewsResourcesStream( - filterTopicIds = setOf(element = topicId), - ).asResult() - - val uiState: StateFlow = - combine( - followedTopicIdsStream, - topic, - newsStream - ) { followedTopicsResult, topicResult, newsResult -> - val topic: TopicUiState = - if (topicResult is Result.Success && followedTopicsResult is Result.Success) { - val followed = followedTopicsResult.data.contains(topicId) + val topicStream: Flow = topicsRepository.getTopic( + id = topicId + ) + + return combine( + followedTopicIdsStream, + topicStream, + ::Pair + ) + .asResult() + .map { followedTopicToTopicResult -> + when (followedTopicToTopicResult) { + is Result.Success -> { + val (followedTopics, topic) = followedTopicToTopicResult.data + val followed = followedTopics.contains(topicId) TopicUiState.Success( followableTopic = FollowableTopic( - topic = topicResult.data, + topic = topic, isFollowed = followed ) ) - } else if ( - topicResult is Result.Loading || followedTopicsResult is Result.Loading - ) { + } + is Result.Loading -> { TopicUiState.Loading - } else { + } + is Result.Error -> { TopicUiState.Error } - - val news: NewsUiState = when (newsResult) { - is Result.Success -> NewsUiState.Success(newsResult.data) - is Result.Loading -> NewsUiState.Loading - is Result.Error -> NewsUiState.Error } - - TopicScreenUiState(topic, news) } - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = TopicScreenUiState(TopicUiState.Loading, NewsUiState.Loading) - ) +} - fun followTopicToggle(followed: Boolean) { - viewModelScope.launch { - userDataRepository.toggleFollowedTopicId(topicId, followed) +private fun newsUiStateStream( + topicId: String, + newsRepository: NewsRepository, + userDataRepository: UserDataRepository, +): Flow { + // Observe news + val newsStream: Flow> = newsRepository.getNewsResourcesStream( + filterAuthorIds = emptySet(), + filterTopicIds = setOf(element = topicId), + ) + + // Observe bookmarks + val bookmarkStream: Flow> = userDataRepository.userDataStream + .map { it.bookmarkedNewsResources } + + return combine( + newsStream, + bookmarkStream, + ::Pair + ) + .asResult() + .map { newsToBookmarksResult -> + when (newsToBookmarksResult) { + is Result.Success -> { + val (news, bookmarks) = newsToBookmarksResult.data + NewsUiState.Success( + news.map { newsResource -> + SaveableNewsResource( + newsResource, + isSaved = bookmarks.contains(newsResource.id) + ) + } + ) + } + is Result.Loading -> { + NewsUiState.Loading + } + is Result.Error -> { + NewsUiState.Error + } + } } - } } sealed interface TopicUiState { @@ -114,12 +181,7 @@ sealed interface TopicUiState { } sealed interface NewsUiState { - data class Success(val news: List) : NewsUiState + data class Success(val news: List) : NewsUiState object Error : NewsUiState object Loading : NewsUiState } - -data class TopicScreenUiState( - val topicState: TopicUiState, - val newsState: NewsUiState -) From 69f6ed602597f5c3edeea2f06c7f1a0c19c36ee2 Mon Sep 17 00:00:00 2001 From: Caren Chang Date: Fri, 23 Sep 2022 15:37:22 -0700 Subject: [PATCH 2/7] Fix tests --- .../feature/topic/TopicScreenTest.kt | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt b/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt index 855d753b9..f5535e532 100644 --- a/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt +++ b/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt @@ -27,6 +27,7 @@ import androidx.compose.ui.test.performScrollToNode import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.NewsResource import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video +import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.model.data.Topic import kotlinx.datetime.Instant import org.junit.Before @@ -57,9 +58,10 @@ class TopicScreenTest { composeTestRule.setContent { TopicScreen( topicState = TopicUiState.Loading, - newsState = NewsUiState.Loading, + newsUiState = NewsUiState.Loading, onBackClick = { }, - onFollowClick = { } + onFollowClick = { }, + onBookmarkChanged = { _, _ -> }, ) } @@ -74,9 +76,10 @@ class TopicScreenTest { composeTestRule.setContent { TopicScreen( topicState = TopicUiState.Success(testTopic), - newsState = NewsUiState.Loading, + newsUiState = NewsUiState.Loading, onBackClick = { }, - onFollowClick = { } + onFollowClick = { }, + onBookmarkChanged = { _, _ -> }, ) } @@ -96,9 +99,15 @@ class TopicScreenTest { composeTestRule.setContent { TopicScreen( topicState = TopicUiState.Loading, - newsState = NewsUiState.Success(sampleNewsResources), + newsUiState = NewsUiState.Success(sampleNewsResources.mapIndexed { index, newsResource -> + SaveableNewsResource( + newsResource = newsResource, + isSaved = index % 2 == 0, + ) + }), onBackClick = { }, - onFollowClick = { } + onFollowClick = { }, + onBookmarkChanged = { _, _ -> }, ) } @@ -107,15 +116,22 @@ class TopicScreenTest { .onNodeWithContentDescription(topicLoading) .assertExists() } + @Test fun news_whenSuccessAndTopicIsSuccess_isShown() { val testTopic = testTopics.first() composeTestRule.setContent { TopicScreen( topicState = TopicUiState.Success(testTopic), - newsState = NewsUiState.Success(sampleNewsResources), + newsUiState = NewsUiState.Success(sampleNewsResources.mapIndexed { index, newsResource -> + SaveableNewsResource( + newsResource = newsResource, + isSaved = index % 2 == 0, + ) + }), onBackClick = { }, - onFollowClick = { } + onFollowClick = { }, + onBookmarkChanged = { _, _ -> }, ) } From 2cb72d052541dabdcd56200571d0a0ec768a40b8 Mon Sep 17 00:00:00 2001 From: Caren Chang Date: Fri, 23 Sep 2022 15:38:58 -0700 Subject: [PATCH 3/7] Fix indents --- .../feature/topic/TopicScreenTest.kt | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt b/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt index f5535e532..35a5964ce 100644 --- a/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt +++ b/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt @@ -99,12 +99,13 @@ class TopicScreenTest { composeTestRule.setContent { TopicScreen( topicState = TopicUiState.Loading, - newsUiState = NewsUiState.Success(sampleNewsResources.mapIndexed { index, newsResource -> - SaveableNewsResource( - newsResource = newsResource, - isSaved = index % 2 == 0, - ) - }), + newsUiState = NewsUiState.Success( + sampleNewsResources.mapIndexed { index, newsResource -> + SaveableNewsResource( + newsResource = newsResource, + isSaved = index % 2 == 0, + ) + }), onBackClick = { }, onFollowClick = { }, onBookmarkChanged = { _, _ -> }, @@ -123,12 +124,13 @@ class TopicScreenTest { composeTestRule.setContent { TopicScreen( topicState = TopicUiState.Success(testTopic), - newsUiState = NewsUiState.Success(sampleNewsResources.mapIndexed { index, newsResource -> - SaveableNewsResource( - newsResource = newsResource, - isSaved = index % 2 == 0, - ) - }), + newsUiState = NewsUiState.Success( + sampleNewsResources.mapIndexed { index, newsResource -> + SaveableNewsResource( + newsResource = newsResource, + isSaved = index % 2 == 0, + ) + }), onBackClick = { }, onFollowClick = { }, onBookmarkChanged = { _, _ -> }, From ab46bca43faf2a82d174ed68e772986c70d5dc1c Mon Sep 17 00:00:00 2001 From: Caren Chang Date: Fri, 23 Sep 2022 15:40:04 -0700 Subject: [PATCH 4/7] Rename topicState -> topicUiState --- .../feature/topic/TopicScreenTest.kt | 8 ++++---- .../nowinandroid/feature/topic/TopicScreen.kt | 18 +++++++++--------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt b/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt index 35a5964ce..54a15c128 100644 --- a/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt +++ b/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt @@ -57,7 +57,7 @@ class TopicScreenTest { fun niaLoadingWheel_whenScreenIsLoading_showLoading() { composeTestRule.setContent { TopicScreen( - topicState = TopicUiState.Loading, + topicUiState = TopicUiState.Loading, newsUiState = NewsUiState.Loading, onBackClick = { }, onFollowClick = { }, @@ -75,7 +75,7 @@ class TopicScreenTest { val testTopic = testTopics.first() composeTestRule.setContent { TopicScreen( - topicState = TopicUiState.Success(testTopic), + topicUiState = TopicUiState.Success(testTopic), newsUiState = NewsUiState.Loading, onBackClick = { }, onFollowClick = { }, @@ -98,7 +98,7 @@ class TopicScreenTest { fun news_whenTopicIsLoading_isNotShown() { composeTestRule.setContent { TopicScreen( - topicState = TopicUiState.Loading, + topicUiState = TopicUiState.Loading, newsUiState = NewsUiState.Success( sampleNewsResources.mapIndexed { index, newsResource -> SaveableNewsResource( @@ -123,7 +123,7 @@ class TopicScreenTest { val testTopic = testTopics.first() composeTestRule.setContent { TopicScreen( - topicState = TopicUiState.Success(testTopic), + topicUiState = TopicUiState.Success(testTopic), newsUiState = NewsUiState.Success( sampleNewsResources.mapIndexed { index, newsResource -> SaveableNewsResource( diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt index be6efaf3e..b97fc7d7b 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt @@ -70,7 +70,7 @@ fun TopicRoute( val newsUiState: NewsUiState by viewModel.newUiState.collectAsStateWithLifecycle() TopicScreen( - topicState = topicUiState, + topicUiState = topicUiState, newsUiState = newsUiState, modifier = modifier, onBackClick = onBackClick, @@ -82,7 +82,7 @@ fun TopicRoute( @VisibleForTesting @Composable internal fun TopicScreen( - topicState: TopicUiState, + topicUiState: TopicUiState, newsUiState: NewsUiState, onBackClick: () -> Unit, onFollowClick: (Boolean) -> Unit, @@ -96,7 +96,7 @@ internal fun TopicScreen( item { Spacer(Modifier.windowInsetsTopHeight(WindowInsets.safeDrawing)) } - when (topicState) { + when (topicUiState) { Loading -> item { NiaLoadingWheel( modifier = modifier, @@ -109,14 +109,14 @@ internal fun TopicScreen( TopicToolbar( onBackClick = onBackClick, onFollowClick = onFollowClick, - uiState = topicState.followableTopic, + uiState = topicUiState.followableTopic, ) } TopicBody( - name = topicState.followableTopic.topic.name, - description = topicState.followableTopic.topic.longDescription, + name = topicUiState.followableTopic.topic.name, + description = topicUiState.followableTopic.topic.longDescription, news = newsUiState, - imageUrl = topicState.followableTopic.topic.imageUrl, + imageUrl = topicUiState.followableTopic.topic.imageUrl, onBookmarkChanged = onBookmarkChanged ) } @@ -246,7 +246,7 @@ fun TopicScreenPopulated() { NiaTheme { NiaBackground { TopicScreen( - topicState = TopicUiState.Success(FollowableTopic(previewTopics[0], false)), + topicUiState = TopicUiState.Success(FollowableTopic(previewTopics[0], false)), newsUiState = NewsUiState.Success( previewNewsResources.mapIndexed { index, newsResource -> SaveableNewsResource( @@ -271,7 +271,7 @@ fun TopicScreenLoading() { NiaTheme { NiaBackground { TopicScreen( - topicState = TopicUiState.Loading, + topicUiState = TopicUiState.Loading, newsUiState = NewsUiState.Loading, onBackClick = {}, onFollowClick = {}, From b678d2474557415da682a3804f8ccb2636b5ee54 Mon Sep 17 00:00:00 2001 From: Caren Chang Date: Fri, 23 Sep 2022 15:46:30 -0700 Subject: [PATCH 5/7] Run spotless --- .../apps/nowinandroid/feature/topic/TopicScreenTest.kt | 6 ++++-- .../samples/apps/nowinandroid/feature/topic/TopicScreen.kt | 5 +++-- .../apps/nowinandroid/feature/topic/TopicViewModel.kt | 3 --- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt b/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt index 54a15c128..b86797474 100644 --- a/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt +++ b/feature/topic/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreenTest.kt @@ -105,7 +105,8 @@ class TopicScreenTest { newsResource = newsResource, isSaved = index % 2 == 0, ) - }), + } + ), onBackClick = { }, onFollowClick = { }, onBookmarkChanged = { _, _ -> }, @@ -130,7 +131,8 @@ class TopicScreenTest { newsResource = newsResource, isSaved = index % 2 == 0, ) - }), + } + ), onBackClick = { }, onFollowClick = { }, onBookmarkChanged = { _, _ -> }, diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt index b97fc7d7b..e07072981 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt @@ -76,7 +76,7 @@ fun TopicRoute( onBackClick = onBackClick, onFollowClick = viewModel::followTopicToggle, onBookmarkChanged = viewModel::bookmarkNews, - ) + ) } @VisibleForTesting @@ -254,7 +254,8 @@ fun TopicScreenPopulated() { isSaved = index % 2 == 0, ) } - ), onBackClick = {}, + ), + onBackClick = {}, onFollowClick = {}, onBookmarkChanged = { _, _ -> }, ) diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt index 823e1ec45..3877fdb57 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt @@ -19,12 +19,9 @@ package com.google.samples.apps.nowinandroid.feature.topic import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import com.google.samples.apps.nowinandroid.core.data.repository.AuthorsRepository import com.google.samples.apps.nowinandroid.core.data.repository.NewsRepository import com.google.samples.apps.nowinandroid.core.data.repository.TopicsRepository import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository -import com.google.samples.apps.nowinandroid.core.model.data.Author -import com.google.samples.apps.nowinandroid.core.model.data.FollowableAuthor import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.NewsResource import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource From fa8a6c624b8fcf0f3cedc0f79eeb437212e6ac47 Mon Sep 17 00:00:00 2001 From: Caren Date: Fri, 23 Sep 2022 17:10:04 -0700 Subject: [PATCH 6/7] Fix tests --- .../feature/topic/TopicViewModelTest.kt | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt b/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt index e29b33054..9f24ae188 100644 --- a/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt +++ b/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt @@ -64,15 +64,15 @@ class TopicViewModelTest { } @Test - fun uiStateAuthor_whenSuccess_matchesTopicFromRepository() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + fun uiStateTopic_whenSuccess_matchesTopicFromRepository() = runTest { + val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.topicUiState.collect() } topicsRepository.sendTopics(testInputTopics.map(FollowableTopic::topic)) userDataRepository.setFollowedTopicIds(setOf(testInputTopics[1].topic.id)) - val item = viewModel.uiState.value - assertTrue(item.topicState is TopicUiState.Success) + val item = viewModel.topicUiState.value + assertTrue(item is TopicUiState.Success) - val successTopicState = item.topicState as TopicUiState.Success + val successTopicState = item as TopicUiState.Success val topicFromRepository = topicsRepository.getTopic( testInputTopics[0].topic.id ).first() @@ -84,20 +84,20 @@ class TopicViewModelTest { @Test fun uiStateNews_whenInitialized_thenShowLoading() = runTest { - assertEquals(NewsUiState.Loading, viewModel.uiState.value.newsState) + assertEquals(NewsUiState.Loading, viewModel.newUiState.value) } @Test fun uiStateTopic_whenInitialized_thenShowLoading() = runTest { - assertEquals(TopicUiState.Loading, viewModel.uiState.value.topicState) + assertEquals(TopicUiState.Loading, viewModel.topicUiState.value) } @Test fun uiStateTopic_whenFollowedIdsSuccessAndTopicLoading_thenShowLoading() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.topicUiState.collect() } userDataRepository.setFollowedTopicIds(setOf(testInputTopics[1].topic.id)) - assertEquals(TopicUiState.Loading, viewModel.uiState.value.topicState) + assertEquals(TopicUiState.Loading, viewModel.topicUiState.value) collectJob.cancel() } @@ -105,13 +105,15 @@ class TopicViewModelTest { @Test fun uiStateTopic_whenFollowedIdsSuccessAndTopicSuccess_thenTopicSuccessAndNewsLoading() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.topicUiState.collect() } topicsRepository.sendTopics(testInputTopics.map { it.topic }) userDataRepository.setFollowedTopicIds(setOf(testInputTopics[1].topic.id)) - val item = viewModel.uiState.value - assertTrue(item.topicState is TopicUiState.Success) - assertTrue(item.newsState is NewsUiState.Loading) + val topicUiState = viewModel.topicUiState.value + val newsUiState = viewModel.newUiState.value + + assertTrue(topicUiState is TopicUiState.Success) + assertTrue(newsUiState is NewsUiState.Loading) collectJob.cancel() } @@ -119,21 +121,23 @@ class TopicViewModelTest { @Test fun uiStateTopic_whenFollowedIdsSuccessAndTopicSuccessAndNewsIsSuccess_thenAllSuccess() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.topicUiState.collect() } topicsRepository.sendTopics(testInputTopics.map { it.topic }) userDataRepository.setFollowedTopicIds(setOf(testInputTopics[1].topic.id)) newsRepository.sendNewsResources(sampleNewsResources) - val item = viewModel.uiState.value - assertTrue(item.topicState is TopicUiState.Success) - assertTrue(item.newsState is NewsUiState.Success) + val topicUiState = viewModel.topicUiState.value + val newsUiState = viewModel.newUiState.value + + assertTrue(topicUiState is TopicUiState.Success) + assertTrue(newsUiState is NewsUiState.Success) collectJob.cancel() } @Test fun uiStateTopic_whenFollowingTopic_thenShowUpdatedTopic() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.topicUiState.collect() } topicsRepository.sendTopics(testInputTopics.map { it.topic }) // Set which topic IDs are followed, not including 0. @@ -143,7 +147,7 @@ class TopicViewModelTest { assertEquals( TopicUiState.Success(followableTopic = testOutputTopics[0]), - viewModel.uiState.value.topicState + viewModel.topicUiState.value ) collectJob.cancel() From 0c5d7526d785b1d04d3d12b3dcc741ddf07979a9 Mon Sep 17 00:00:00 2001 From: Caren Date: Sat, 24 Sep 2022 08:17:32 -0700 Subject: [PATCH 7/7] Fix test --- .../nowinandroid/feature/topic/TopicViewModelTest.kt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt b/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt index 9f24ae188..0300f2470 100644 --- a/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt +++ b/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt @@ -27,6 +27,7 @@ import com.google.samples.apps.nowinandroid.core.testing.repository.TestUserData import com.google.samples.apps.nowinandroid.core.testing.util.MainDispatcherRule import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicDestination import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.test.UnconfinedTestDispatcher @@ -121,8 +122,13 @@ class TopicViewModelTest { @Test fun uiStateTopic_whenFollowedIdsSuccessAndTopicSuccessAndNewsIsSuccess_thenAllSuccess() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.topicUiState.collect() } - + val collectJob = launch(UnconfinedTestDispatcher()) { + combine( + viewModel.topicUiState, + viewModel.newUiState, + ::Pair + ).collect() + } topicsRepository.sendTopics(testInputTopics.map { it.topic }) userDataRepository.setFollowedTopicIds(setOf(testInputTopics[1].topic.id)) newsRepository.sendNewsResources(sampleNewsResources)