Merge pull request #552 from android/dt/refactor-fyvm-2

Alternative approach to refactoring the ForYouViewModel

Author: dturner

feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt

@@ -23,12 +23,14 @@ 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.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
@@ -40,7 +42,7 @@ import javax.inject.Inject
 class ForYouViewModel @Inject constructor(
     syncStatusMonitor: SyncStatusMonitor,
     private val userDataRepository: UserDataRepository,
-    private val getSaveableNewsResources: GetUserNewsResourcesUseCase,
+    getUserNewsResources: GetUserNewsResourcesUseCase,
     getFollowableTopics: GetFollowableTopicsUseCase,
 ) : ViewModel() {
 
@@ -55,26 +57,8 @@ class ForYouViewModel @Inject constructor(
         )
 
     val feedState: StateFlow<NewsFeedUiState> =
-        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<List<UserNewsResource>, 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 }
+        userDataRepository.getFollowedUserNewsResources(getUserNewsResources)
+            .map(NewsFeedUiState::Success)
             .stateIn(
                 scope = viewModelScope,
                 started = SharingStarted.WhileSubscribed(5_000),
@@ -116,3 +100,45 @@ class ForYouViewModel @Inject constructor(
         }
     }
 }
+
+/**
+ * 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<List<UserNewsResource>> = 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())
+        } 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()

feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt

@@ -60,6 +60,7 @@ class ForYouViewModelTest {
         newsRepository = newsRepository,
         userDataRepository = userDataRepository,
     )
+
     private val getFollowableTopicsUseCase = GetFollowableTopicsUseCase(
         topicsRepository = topicsRepository,
         userDataRepository = userDataRepository,
@@ -71,7 +72,7 @@ class ForYouViewModelTest {
         viewModel = ForYouViewModel(
             syncStatusMonitor = syncStatusMonitor,
             userDataRepository = userDataRepository,
-            getSaveableNewsResources = getUserNewsResourcesUseCase,
+            getUserNewsResources = getUserNewsResourcesUseCase,
             getFollowableTopics = getFollowableTopicsUseCase,
         )
     }