From a9bf21aaf4cc6ad07ab5d5b17065bf681778bffd Mon Sep 17 00:00:00 2001 From: Don Turner Date: Fri, 27 Jan 2023 17:37:49 +0000 Subject: [PATCH 1/6] Add UseCase for getting followed news resources, refactor ForYouVM --- .../domain/GetUserNewsResourcesUseCase.kt | 41 +++++++++++++++++++ .../feature/foryou/ForYouViewModel.kt | 29 +++---------- .../feature/foryou/ForYouViewModelTest.kt | 8 +++- 3 files changed, 53 insertions(+), 25 deletions(-) diff --git a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt index db274bbbd..72361eeed 100644 --- a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt +++ b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt @@ -24,7 +24,11 @@ import com.google.samples.apps.nowinandroid.core.model.data.NewsResource import com.google.samples.apps.nowinandroid.core.model.data.UserData import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterNot +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map import javax.inject.Inject /** @@ -51,6 +55,43 @@ class GetUserNewsResourcesUseCase @Inject constructor( }.mapToUserNewsResources(userDataRepository.userData) } +class GetFollowedUserNewsResourcesUseCase @Inject constructor( + private val userDataRepository: UserDataRepository, + val getUserNewsResources: GetUserNewsResourcesUseCase, +) { + /** + * Returns a list of UserNewsResources which the user is following + */ + operator fun invoke(): Flow> = + userDataRepository.userData.map { userData -> + if (shouldShowEmptyFeed(userData)) { + null + } else { + userData.followedTopics + } + } + .distinctUntilChanged() + .flatMapLatest { followedTopics -> + if (followedTopics == null) { + flowOf(emptyList()) + } else { + getUserNewsResources(filterTopicIds = followedTopics) + } + } + + /** + * If the user hasn't completed the onboarding and hasn't selected any interests + * show an empty news list to clearly demonstrate that their selections affect the + * news articles they will see. + * + * Note: It should not be possible for the user to get into a state where the onboarding + * is not displayed AND they haven't followed any topics, however, this method is to safeguard + * against that scenario in future. + */ + private fun shouldShowEmptyFeed(userData: UserData) = + !userData.shouldHideOnboarding && userData.followedTopics.isEmpty() +} + private fun Flow>.mapToUserNewsResources( userDataStream: Flow, ): Flow> = diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt index d75965778..84c4d11a3 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt @@ -21,7 +21,7 @@ import androidx.lifecycle.viewModelScope import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository import com.google.samples.apps.nowinandroid.core.data.util.SyncStatusMonitor import com.google.samples.apps.nowinandroid.core.domain.GetFollowableTopicsUseCase -import com.google.samples.apps.nowinandroid.core.domain.GetUserNewsResourcesUseCase +import com.google.samples.apps.nowinandroid.core.domain.GetFollowedUserNewsResourcesUseCase import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState import dagger.hilt.android.lifecycle.HiltViewModel @@ -29,8 +29,6 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.flatMapLatest -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -40,7 +38,7 @@ import javax.inject.Inject class ForYouViewModel @Inject constructor( syncStatusMonitor: SyncStatusMonitor, private val userDataRepository: UserDataRepository, - private val getSaveableNewsResources: GetUserNewsResourcesUseCase, + getFollowedUserNewsResources: GetFollowedUserNewsResourcesUseCase, getFollowableTopics: GetFollowableTopicsUseCase, ) : ViewModel() { @@ -55,26 +53,9 @@ class ForYouViewModel @Inject constructor( ) val feedState: StateFlow = - userDataRepository.userData - .map { userData -> - // If the user hasn't completed the onboarding and hasn't selected any interests - // show an empty news list to clearly demonstrate that their selections affect the - // news articles they will see. - if (!userData.shouldHideOnboarding && - userData.followedTopics.isEmpty() - ) { - flowOf(NewsFeedUiState.Success(emptyList())) - } else { - getSaveableNewsResources( - filterTopicIds = userData.followedTopics, - ) - .map, NewsFeedUiState>(NewsFeedUiState::Success) - } - } - // Flatten the feed flows. - // As the selected topics and topic state changes, this will cancel the old feed - // monitoring and start the new one. - .flatMapLatest { it } + getFollowedUserNewsResources().map { + NewsFeedUiState.Success(it) + } .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(5_000), diff --git a/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt b/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt index 1f6e010d5..c335f7603 100644 --- a/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt +++ b/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt @@ -17,6 +17,7 @@ package com.google.samples.apps.nowinandroid.feature.foryou import com.google.samples.apps.nowinandroid.core.domain.GetFollowableTopicsUseCase +import com.google.samples.apps.nowinandroid.core.domain.GetFollowedUserNewsResourcesUseCase import com.google.samples.apps.nowinandroid.core.domain.GetUserNewsResourcesUseCase import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource @@ -60,6 +61,11 @@ class ForYouViewModelTest { newsRepository = newsRepository, userDataRepository = userDataRepository, ) + private val getFollowedUserNewsResourcesUseCase = GetFollowedUserNewsResourcesUseCase( + userDataRepository = userDataRepository, + getUserNewsResources = getUserNewsResourcesUseCase, + ) + private val getFollowableTopicsUseCase = GetFollowableTopicsUseCase( topicsRepository = topicsRepository, userDataRepository = userDataRepository, @@ -71,7 +77,7 @@ class ForYouViewModelTest { viewModel = ForYouViewModel( syncStatusMonitor = syncStatusMonitor, userDataRepository = userDataRepository, - getSaveableNewsResources = getUserNewsResourcesUseCase, + getFollowedUserNewsResources = getFollowedUserNewsResourcesUseCase, getFollowableTopics = getFollowableTopicsUseCase, ) } From 5a29414b7ae65a59bdd6d91bc4b431bbed114bfb Mon Sep 17 00:00:00 2001 From: Don Turner Date: Fri, 27 Jan 2023 17:53:58 +0000 Subject: [PATCH 2/6] Fix spotless --- .../samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt index 84c4d11a3..85268e894 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt @@ -22,7 +22,6 @@ import com.google.samples.apps.nowinandroid.core.data.repository.UserDataReposit import com.google.samples.apps.nowinandroid.core.data.util.SyncStatusMonitor import com.google.samples.apps.nowinandroid.core.domain.GetFollowableTopicsUseCase import com.google.samples.apps.nowinandroid.core.domain.GetFollowedUserNewsResourcesUseCase -import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.Flow From a98dd001b2dc5c775e1a88c23fa5582a74cd6651 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Fri, 27 Jan 2023 18:05:40 +0000 Subject: [PATCH 3/6] Add comment describing flow transformation functions --- .../domain/GetUserNewsResourcesUseCase.kt | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt index 72361eeed..d4ef20ca9 100644 --- a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt +++ b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt @@ -60,16 +60,30 @@ class GetFollowedUserNewsResourcesUseCase @Inject constructor( val getUserNewsResources: GetUserNewsResourcesUseCase, ) { /** - * Returns a list of UserNewsResources which the user is following + * Returns a list of UserNewsResources for topics which the user is following */ operator fun invoke(): Flow> = - userDataRepository.userData.map { userData -> - if (shouldShowEmptyFeed(userData)) { - null - } else { - userData.followedTopics + + /** + * This sequence of flow transformation functions does the following: + * + * - map: maps the user data into a set of followed topic IDs or null if we should return + * an empty list + * - distinctUntilChanged: will only emit a set of followed topic IDs if it's changed. This + * avoids calling potentially expensive operations (like setting up a new flow) when nothing + * has changed. + * - flatMapLatest: getUserNewsResources returns a flow, so we end up with a flow inside a + * flow. flatMapLatest solves this and cancels any previous flows created by + * getUserNewsResources. + */ + userDataRepository.userData + .map { userData -> + if (shouldShowEmptyFeed(userData)) { + null + } else { + userData.followedTopics + } } - } .distinctUntilChanged() .flatMapLatest { followedTopics -> if (followedTopics == null) { From e86cdea381d4233221ad57d342b67ecd7600c80c Mon Sep 17 00:00:00 2001 From: Don Turner Date: Mon, 30 Jan 2023 12:54:11 +0000 Subject: [PATCH 4/6] Add unit tests and comments --- .../domain/GetUserNewsResourcesUseCase.kt | 7 ++- .../domain/GetUserNewsResourcesUseCaseTest.kt | 49 +++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt index d4ef20ca9..fdb9eb6d9 100644 --- a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt +++ b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt @@ -63,7 +63,6 @@ class GetFollowedUserNewsResourcesUseCase @Inject constructor( * Returns a list of UserNewsResources for topics which the user is following */ operator fun invoke(): Flow> = - /** * This sequence of flow transformation functions does the following: * @@ -72,9 +71,9 @@ class GetFollowedUserNewsResourcesUseCase @Inject constructor( * - distinctUntilChanged: will only emit a set of followed topic IDs if it's changed. This * avoids calling potentially expensive operations (like setting up a new flow) when nothing * has changed. - * - flatMapLatest: getUserNewsResources returns a flow, so we end up with a flow inside a - * flow. flatMapLatest solves this and cancels any previous flows created by - * getUserNewsResources. + * - flatMapLatest: getUserNewsResources returns a flow, so we have a flow inside a + * flow. flatMapLatest moves the inner flow (the one we want to return) to the outer flow + * and cancels any previous flows created by getUserNewsResources. */ userDataRepository.userData .map { userData -> diff --git a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt index 32ee8773c..8674c4026 100644 --- a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt +++ b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt @@ -83,6 +83,55 @@ class GetUserNewsResourcesUseCaseTest { } } +class GetFollowedUserNewsResourcesUseCaseTest { + + @get:Rule + val mainDispatcherRule = MainDispatcherRule() + + private val newsRepository = TestNewsRepository() + private val userDataRepository = TestUserDataRepository() + private val getUserNewsResourcesUseCase = + GetUserNewsResourcesUseCase(newsRepository, userDataRepository) + + val useCase = + GetFollowedUserNewsResourcesUseCase(userDataRepository, getUserNewsResourcesUseCase) + + @Test + fun whenOnboardingShownAndNoTopicsFollowed_emptyListIsReturned() = runTest { + val followedNewsResources = useCase() + + // Send some news resources and empty user data + newsRepository.sendNewsResources(sampleNewsResources) + userDataRepository.setUserData(emptyUserData) + + // Check that an empty list is returned + assertEquals( + emptyList(), + followedNewsResources.first(), + ) + } + + @Test + fun whenTopicsAreFollowed_correctNewsResourcesAreReturned() = runTest { + val followedNewsResources = useCase() + + // Send some news resources and user data with a followed topic + newsRepository.sendNewsResources(sampleNewsResources) + + val userData = emptyUserData.copy( + followedTopics = setOf(sampleTopic1.id), + ) + userDataRepository.setUserData(userData) + + assertEquals( + sampleNewsResources + .filter { it.topics.contains(sampleTopic1) } + .mapToUserNewsResources(userData), + followedNewsResources.first(), + ) + } +} + private val sampleTopic1 = Topic( id = "Topic1", name = "Headlines", From e536c396f9bf29278a5da62d3e7cf37e6ebec6d3 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Mon, 30 Jan 2023 15:12:19 +0000 Subject: [PATCH 5/6] Move new UseCase and tests into separate files Change-Id: I4c337473ca0a60a5fccbb8aa640735cf7d616e71 --- .../domain/GetUserNewsResourcesUseCase.kt | 54 ------------ .../domain/GetUserNewsResourcesUseCaseTest.kt | 49 ----------- .../feature/foryou/ForYouViewModel.kt | 87 ++++++++++++++++--- .../feature/foryou/ForYouViewModelTest.kt | 7 +- 4 files changed, 77 insertions(+), 120 deletions(-) diff --git a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt index fdb9eb6d9..db274bbbd 100644 --- a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt +++ b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCase.kt @@ -24,11 +24,7 @@ import com.google.samples.apps.nowinandroid.core.model.data.NewsResource import com.google.samples.apps.nowinandroid.core.model.data.UserData import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine -import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterNot -import kotlinx.coroutines.flow.flatMapLatest -import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.flow.map import javax.inject.Inject /** @@ -55,56 +51,6 @@ class GetUserNewsResourcesUseCase @Inject constructor( }.mapToUserNewsResources(userDataRepository.userData) } -class GetFollowedUserNewsResourcesUseCase @Inject constructor( - private val userDataRepository: UserDataRepository, - val getUserNewsResources: GetUserNewsResourcesUseCase, -) { - /** - * Returns a list of UserNewsResources for topics which the user is following - */ - operator fun invoke(): Flow> = - /** - * This sequence of flow transformation functions does the following: - * - * - map: maps the user data into a set of followed topic IDs or null if we should return - * an empty list - * - distinctUntilChanged: will only emit a set of followed topic IDs if it's changed. This - * avoids calling potentially expensive operations (like setting up a new flow) when nothing - * has changed. - * - flatMapLatest: getUserNewsResources returns a flow, so we have a flow inside a - * flow. flatMapLatest moves the inner flow (the one we want to return) to the outer flow - * and cancels any previous flows created by getUserNewsResources. - */ - userDataRepository.userData - .map { userData -> - if (shouldShowEmptyFeed(userData)) { - null - } else { - userData.followedTopics - } - } - .distinctUntilChanged() - .flatMapLatest { followedTopics -> - if (followedTopics == null) { - flowOf(emptyList()) - } else { - getUserNewsResources(filterTopicIds = followedTopics) - } - } - - /** - * If the user hasn't completed the onboarding and hasn't selected any interests - * show an empty news list to clearly demonstrate that their selections affect the - * news articles they will see. - * - * Note: It should not be possible for the user to get into a state where the onboarding - * is not displayed AND they haven't followed any topics, however, this method is to safeguard - * against that scenario in future. - */ - private fun shouldShowEmptyFeed(userData: UserData) = - !userData.shouldHideOnboarding && userData.followedTopics.isEmpty() -} - private fun Flow>.mapToUserNewsResources( userDataStream: Flow, ): Flow> = diff --git a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt index 8674c4026..32ee8773c 100644 --- a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt +++ b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt @@ -83,55 +83,6 @@ class GetUserNewsResourcesUseCaseTest { } } -class GetFollowedUserNewsResourcesUseCaseTest { - - @get:Rule - val mainDispatcherRule = MainDispatcherRule() - - private val newsRepository = TestNewsRepository() - private val userDataRepository = TestUserDataRepository() - private val getUserNewsResourcesUseCase = - GetUserNewsResourcesUseCase(newsRepository, userDataRepository) - - val useCase = - GetFollowedUserNewsResourcesUseCase(userDataRepository, getUserNewsResourcesUseCase) - - @Test - fun whenOnboardingShownAndNoTopicsFollowed_emptyListIsReturned() = runTest { - val followedNewsResources = useCase() - - // Send some news resources and empty user data - newsRepository.sendNewsResources(sampleNewsResources) - userDataRepository.setUserData(emptyUserData) - - // Check that an empty list is returned - assertEquals( - emptyList(), - followedNewsResources.first(), - ) - } - - @Test - fun whenTopicsAreFollowed_correctNewsResourcesAreReturned() = runTest { - val followedNewsResources = useCase() - - // Send some news resources and user data with a followed topic - newsRepository.sendNewsResources(sampleNewsResources) - - val userData = emptyUserData.copy( - followedTopics = setOf(sampleTopic1.id), - ) - userDataRepository.setUserData(userData) - - assertEquals( - sampleNewsResources - .filter { it.topics.contains(sampleTopic1) } - .mapToUserNewsResources(userData), - followedNewsResources.first(), - ) - } -} - private val sampleTopic1 = Topic( id = "Topic1", name = "Headlines", diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt index 85268e894..259cc31f2 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt @@ -21,13 +21,18 @@ import androidx.lifecycle.viewModelScope import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository import com.google.samples.apps.nowinandroid.core.data.util.SyncStatusMonitor import com.google.samples.apps.nowinandroid.core.domain.GetFollowableTopicsUseCase -import com.google.samples.apps.nowinandroid.core.domain.GetFollowedUserNewsResourcesUseCase +import com.google.samples.apps.nowinandroid.core.domain.GetUserNewsResourcesUseCase +import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource +import com.google.samples.apps.nowinandroid.core.model.data.UserData import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -37,7 +42,7 @@ import javax.inject.Inject class ForYouViewModel @Inject constructor( syncStatusMonitor: SyncStatusMonitor, private val userDataRepository: UserDataRepository, - getFollowedUserNewsResources: GetFollowedUserNewsResourcesUseCase, + private val getUserNewsResources: GetUserNewsResourcesUseCase, getFollowableTopics: GetFollowableTopicsUseCase, ) : ViewModel() { @@ -51,15 +56,13 @@ class ForYouViewModel @Inject constructor( initialValue = false, ) - val feedState: StateFlow = - getFollowedUserNewsResources().map { - NewsFeedUiState.Success(it) - } - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = NewsFeedUiState.Loading, - ) + val feedState: StateFlow = getFollowedUserNewsResources() + .map(NewsFeedUiState::Success) + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = NewsFeedUiState.Loading, + ) val onboardingUiState: StateFlow = combine( @@ -95,4 +98,66 @@ class ForYouViewModel @Inject constructor( userDataRepository.setShouldHideOnboarding(true) } } + + /** + * This sequence of flow transformation functions does the following: + * + * - map: maps the user data into a set of followed topic IDs or null if we should return + * an empty list + * - distinctUntilChanged: will only emit a set of followed topic IDs if it's changed. This + * avoids calling potentially expensive operations (like setting up a new flow) when nothing + * has changed. + * - flatMapLatest: getUserNewsResources returns a flow, so we have a flow inside a + * flow. flatMapLatest moves the inner flow (the one we want to return) to the outer flow + * and cancels any previous flows created by getUserNewsResources. + */ + private fun getFollowedUserNewsResources(): Flow> = + userDataRepository.userData + .map { userData -> + if (userData.shouldShowEmptyFeed()) { + null + } else { + userData.followedTopics + } + } + .distinctUntilChanged() + .flatMapLatest { followedTopics -> + if (followedTopics == null) { + flowOf(emptyList()) + } else { + getUserNewsResources(filterTopicIds = followedTopics) + } + } } + +// Alternative approach (not currently being called) +private fun Flow.getFollowedUserNewsResources( + getUserNewsResources: GetUserNewsResourcesUseCase, +): Flow> = + map { userData -> + if (userData.shouldShowEmptyFeed()) { + null + } else { + userData.followedTopics + } + } + .distinctUntilChanged() + .flatMapLatest { followedTopics -> + if (followedTopics == null) { + flowOf(emptyList()) + } else { + getUserNewsResources(filterTopicIds = followedTopics) + } + } + +/** + * If the user hasn't completed the onboarding and hasn't selected any interests + * show an empty news list to clearly demonstrate that their selections affect the + * news articles they will see. + * + * Note: It should not be possible for the user to get into a state where the onboarding + * is not displayed AND they haven't followed any topics, however, this method is to safeguard + * against that scenario in future. + */ +private fun UserData.shouldShowEmptyFeed() = + !shouldHideOnboarding && followedTopics.isEmpty() diff --git a/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt b/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt index c335f7603..9e51758f0 100644 --- a/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt +++ b/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt @@ -17,7 +17,6 @@ package com.google.samples.apps.nowinandroid.feature.foryou import com.google.samples.apps.nowinandroid.core.domain.GetFollowableTopicsUseCase -import com.google.samples.apps.nowinandroid.core.domain.GetFollowedUserNewsResourcesUseCase import com.google.samples.apps.nowinandroid.core.domain.GetUserNewsResourcesUseCase import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource @@ -61,10 +60,6 @@ class ForYouViewModelTest { newsRepository = newsRepository, userDataRepository = userDataRepository, ) - private val getFollowedUserNewsResourcesUseCase = GetFollowedUserNewsResourcesUseCase( - userDataRepository = userDataRepository, - getUserNewsResources = getUserNewsResourcesUseCase, - ) private val getFollowableTopicsUseCase = GetFollowableTopicsUseCase( topicsRepository = topicsRepository, @@ -77,7 +72,7 @@ class ForYouViewModelTest { viewModel = ForYouViewModel( syncStatusMonitor = syncStatusMonitor, userDataRepository = userDataRepository, - getFollowedUserNewsResources = getFollowedUserNewsResourcesUseCase, + getUserNewsResources = getUserNewsResourcesUseCase, getFollowableTopics = getFollowableTopicsUseCase, ) } From f9dcdb4390061a609accf726dd011f480a39ee93 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Mon, 30 Jan 2023 20:17:39 +0000 Subject: [PATCH 6/6] More refactoring --- .../feature/foryou/ForYouViewModel.kt | 65 +++++++------------ 1 file changed, 23 insertions(+), 42 deletions(-) diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt index 259cc31f2..cd029b4af 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt @@ -42,7 +42,7 @@ import javax.inject.Inject class ForYouViewModel @Inject constructor( syncStatusMonitor: SyncStatusMonitor, private val userDataRepository: UserDataRepository, - private val getUserNewsResources: GetUserNewsResourcesUseCase, + getUserNewsResources: GetUserNewsResourcesUseCase, getFollowableTopics: GetFollowableTopicsUseCase, ) : ViewModel() { @@ -56,13 +56,14 @@ class ForYouViewModel @Inject constructor( initialValue = false, ) - val feedState: StateFlow = getFollowedUserNewsResources() - .map(NewsFeedUiState::Success) - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = NewsFeedUiState.Loading, - ) + val feedState: StateFlow = + userDataRepository.getFollowedUserNewsResources(getUserNewsResources) + .map(NewsFeedUiState::Success) + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = NewsFeedUiState.Loading, + ) val onboardingUiState: StateFlow = combine( @@ -98,50 +99,30 @@ class ForYouViewModel @Inject constructor( userDataRepository.setShouldHideOnboarding(true) } } - - /** - * This sequence of flow transformation functions does the following: - * - * - map: maps the user data into a set of followed topic IDs or null if we should return - * an empty list - * - distinctUntilChanged: will only emit a set of followed topic IDs if it's changed. This - * avoids calling potentially expensive operations (like setting up a new flow) when nothing - * has changed. - * - flatMapLatest: getUserNewsResources returns a flow, so we have a flow inside a - * flow. flatMapLatest moves the inner flow (the one we want to return) to the outer flow - * and cancels any previous flows created by getUserNewsResources. - */ - private fun getFollowedUserNewsResources(): Flow> = - userDataRepository.userData - .map { userData -> - if (userData.shouldShowEmptyFeed()) { - null - } else { - userData.followedTopics - } - } - .distinctUntilChanged() - .flatMapLatest { followedTopics -> - if (followedTopics == null) { - flowOf(emptyList()) - } else { - getUserNewsResources(filterTopicIds = followedTopics) - } - } } -// Alternative approach (not currently being called) -private fun Flow.getFollowedUserNewsResources( +/** + * Obtain a flow of user news resources whose topics match those the user is following. + * + * getUserNewsResources: The `UseCase` used to obtain the flow of user news resources. + */ +private fun UserDataRepository.getFollowedUserNewsResources( getUserNewsResources: GetUserNewsResourcesUseCase, -): Flow> = - map { userData -> +): Flow> = userData + // Map the user data into a set of followed topic IDs or null if we should return an empty list. + .map { userData -> if (userData.shouldShowEmptyFeed()) { null } else { userData.followedTopics } } + // Only emit a set of followed topic IDs if it's changed. This avoids calling potentially + // expensive operations (like setting up a new flow) when nothing has changed. .distinctUntilChanged() + // getUserNewsResources returns a flow, so we have a flow inside a flow. flatMapLatest moves + // the inner flow (the one we want to return) to the outer flow and cancels any previous flows + // created by getUserNewsResources. .flatMapLatest { followedTopics -> if (followedTopics == null) { flowOf(emptyList())