From d108459bda50d84b383072b0fe0d9d670729775f Mon Sep 17 00:00:00 2001 From: Manuel Vivo Date: Thu, 6 Oct 2022 08:33:15 +0200 Subject: [PATCH 1/6] Updates Navigation approach with new guidance Change-Id: I2b5f536e56366210196fd5a64c2653235c5fd44b --- app/build.gradle.kts | 5 +- .../apps/nowinandroid/ui/NiaAppStateTest.kt | 6 +- .../nowinandroid/navigation/NiaNavHost.kt | 38 ++++---- .../navigation/TopLevelDestination.kt | 29 +++++- .../samples/apps/nowinandroid/ui/NiaApp.kt | 21 +++-- .../apps/nowinandroid/ui/NiaAppState.kt | 91 +++++++------------ .../kotlin/AndroidFeatureConventionPlugin.kt | 3 +- .../core/decoder/StringDecoder.kt} | 18 +--- .../nowinandroid/core/decoder/UriDecoder.kt | 24 +++++ .../core/decoder/di/StringDecoderModule.kt | 31 +++++++ core/navigation/.gitignore | 1 - core/navigation/README.md | 3 - core/navigation/src/main/AndroidManifest.xml | 19 ---- .../navigation/NiaNavigationDestination.kt | 38 -------- .../core/testing/decoder/FakeStringDecoder.kt | 24 +++++ .../testing/di/TestStringDecoderModule.kt | 35 +++++++ docs/ModularizationLearningJourney.md | 8 -- .../feature/author/AuthorScreen.kt | 2 +- .../feature/author/AuthorViewModel.kt | 14 +-- .../author/navigation/AuthorNavigation.kt | 39 ++++---- .../feature/author/AuthorViewModelTest.kt | 6 +- .../feature/bookmarks/BookmarksScreen.kt | 2 +- .../navigation/BookmarksNavigation.kt | 14 +-- .../feature/foryou/ForYouScreen.kt | 4 +- .../foryou/navigation/ForYouNavigation.kt | 14 +-- .../feature/interests/InterestsScreen.kt | 4 +- .../navigation/InterestsNavigation.kt | 18 ++-- .../nowinandroid/feature/topic/TopicScreen.kt | 2 +- .../feature/topic/TopicViewModel.kt | 12 ++- .../topic/navigation/TopicNavigation.kt | 39 ++++---- .../feature/topic/TopicViewModelTest.kt | 7 +- settings.gradle.kts | 3 +- 32 files changed, 296 insertions(+), 278 deletions(-) rename core/{navigation/build.gradle.kts => common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt} (59%) create mode 100644 core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt create mode 100644 core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt delete mode 100644 core/navigation/.gitignore delete mode 100644 core/navigation/README.md delete mode 100644 core/navigation/src/main/AndroidManifest.xml delete mode 100644 core/navigation/src/main/java/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigationDestination.kt create mode 100644 core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt create mode 100644 core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 10bfe9c22..fe1f1cb64 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -85,7 +85,6 @@ dependencies { implementation(project(":core:ui")) implementation(project(":core:designsystem")) - implementation(project(":core:navigation")) implementation(project(":sync:work")) implementation(project(":sync:sync-test")) @@ -104,6 +103,8 @@ dependencies { implementation(libs.androidx.compose.runtime) implementation(libs.androidx.compose.runtime.tracing) implementation(libs.androidx.compose.material3.windowSizeClass) + implementation(libs.androidx.hilt.navigation.compose) + implementation(libs.androidx.navigation.compose) implementation(libs.androidx.window.manager) implementation(libs.androidx.profileinstaller) @@ -118,4 +119,4 @@ configurations.configureEach { // Temporary workaround for https://issuetracker.google.com/174733673 force("org.objenesis:objenesis:2.6") } -} \ No newline at end of file +} diff --git a/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NiaAppStateTest.kt b/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NiaAppStateTest.kt index 2814fdbc9..c2e17700f 100644 --- a/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NiaAppStateTest.kt +++ b/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NiaAppStateTest.kt @@ -82,9 +82,9 @@ class NiaAppStateTest { } assertEquals(3, state.topLevelDestinations.size) - assertTrue(state.topLevelDestinations[0].destination.contains("for_you")) - assertTrue(state.topLevelDestinations[1].destination.contains("bookmarks")) - assertTrue(state.topLevelDestinations[2].destination.contains("interests")) + assertTrue(state.topLevelDestinations[0].name.contains("for_you", true)) + assertTrue(state.topLevelDestinations[1].name.contains("bookmarks", true)) + assertTrue(state.topLevelDestinations[2].name.contains("interests", true)) } @Test diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/NiaNavHost.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/NiaNavHost.kt index 6d2206743..10fafddd9 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/NiaNavHost.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/NiaNavHost.kt @@ -20,15 +20,14 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.navigation.NavHostController import androidx.navigation.compose.NavHost -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination -import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorDestination -import com.google.samples.apps.nowinandroid.feature.author.navigation.authorGraph -import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.bookmarksGraph -import com.google.samples.apps.nowinandroid.feature.foryou.navigation.ForYouDestination -import com.google.samples.apps.nowinandroid.feature.foryou.navigation.forYouGraph +import com.google.samples.apps.nowinandroid.feature.author.navigation.authorScreen +import com.google.samples.apps.nowinandroid.feature.author.navigation.navigateToAuthor +import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.bookmarksScreen +import com.google.samples.apps.nowinandroid.feature.foryou.navigation.forYouNavigationRoute +import com.google.samples.apps.nowinandroid.feature.foryou.navigation.forYouScreen import com.google.samples.apps.nowinandroid.feature.interests.navigation.interestsGraph -import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicDestination -import com.google.samples.apps.nowinandroid.feature.topic.navigation.topicGraph +import com.google.samples.apps.nowinandroid.feature.topic.navigation.navigateToTopic +import com.google.samples.apps.nowinandroid.feature.topic.navigation.topicScreen /** * Top-level navigation graph. Navigation is organized as explained at @@ -40,32 +39,27 @@ import com.google.samples.apps.nowinandroid.feature.topic.navigation.topicGraph @Composable fun NiaNavHost( navController: NavHostController, - onNavigateToDestination: (NiaNavigationDestination, String) -> Unit, onBackClick: () -> Unit, modifier: Modifier = Modifier, - startDestination: String = ForYouDestination.route + startDestination: String = forYouNavigationRoute ) { NavHost( navController = navController, startDestination = startDestination, modifier = modifier, ) { - forYouGraph() - bookmarksGraph() + forYouScreen() + bookmarksScreen() interestsGraph( - navigateToTopic = { - onNavigateToDestination( - TopicDestination, TopicDestination.createNavigationRoute(it) - ) + navigateToTopic = { topicId -> + navController.navigateToTopic(topicId) }, - navigateToAuthor = { - onNavigateToDestination( - AuthorDestination, AuthorDestination.createNavigationRoute(it) - ) + navigateToAuthor = { authorId -> + navController.navigateToAuthor(authorId) }, nestedGraphs = { - topicGraph(onBackClick) - authorGraph(onBackClick) + topicScreen(onBackClick) + authorScreen(onBackClick) } ) } diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/TopLevelDestination.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/TopLevelDestination.kt index 4a2523bb5..566997920 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/TopLevelDestination.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/TopLevelDestination.kt @@ -17,17 +17,36 @@ package com.google.samples.apps.nowinandroid.navigation import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination +import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.DrawableResourceIcon +import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.ImageVectorIcon +import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons +import com.google.samples.apps.nowinandroid.feature.bookmarks.R as bookmarksR +import com.google.samples.apps.nowinandroid.feature.foryou.R as forYouR +import com.google.samples.apps.nowinandroid.feature.interests.R as interestsR /** * Type for the top level destinations in the application. Each of these destinations * can contain one or more screens (based on the window size). Navigation from one screen to the * next within a single destination will be handled directly in composables. */ -data class TopLevelDestination( - override val route: String, - override val destination: String, +enum class TopLevelDestination( val selectedIcon: Icon, val unselectedIcon: Icon, val iconTextId: Int -) : NiaNavigationDestination +) { + FOR_YOU( + selectedIcon = DrawableResourceIcon(NiaIcons.Upcoming), + unselectedIcon = DrawableResourceIcon(NiaIcons.UpcomingBorder), + iconTextId = forYouR.string.for_you + ), + BOOKMARKS( + selectedIcon = DrawableResourceIcon(NiaIcons.Bookmarks), + unselectedIcon = DrawableResourceIcon(NiaIcons.BookmarksBorder), + iconTextId = bookmarksR.string.saved + ), + INTERESTS( + selectedIcon = ImageVectorIcon(NiaIcons.Grid3x3), + unselectedIcon = ImageVectorIcon(NiaIcons.Grid3x3), + iconTextId = interestsR.string.interests + ) +} diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt index 1d5ec6e98..213374100 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt @@ -52,7 +52,6 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaNavig import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.DrawableResourceIcon import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.ImageVectorIcon import com.google.samples.apps.nowinandroid.core.designsystem.theme.NiaTheme -import com.google.samples.apps.nowinandroid.feature.foryou.navigation.ForYouDestination import com.google.samples.apps.nowinandroid.navigation.NiaNavHost import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination @@ -69,7 +68,9 @@ fun NiaApp( NiaTheme { val background: @Composable (@Composable () -> Unit) -> Unit = when (appState.currentDestination?.route) { - ForYouDestination.route -> { content -> NiaGradientBackground(content = content) } + TopLevelDestination.FOR_YOU.name -> { content -> + NiaGradientBackground(content = content) + } else -> { content -> NiaBackground(content = content) } } @@ -85,7 +86,7 @@ fun NiaApp( if (appState.shouldShowBottomBar) { NiaBottomBar( destinations = appState.topLevelDestinations, - onNavigateToDestination = appState::navigate, + onNavigateToDestination = appState::navigateToTopLevelDestination, currentDestination = appState.currentDestination ) } @@ -103,7 +104,7 @@ fun NiaApp( if (appState.shouldShowNavRail) { NiaNavRail( destinations = appState.topLevelDestinations, - onNavigateToDestination = appState::navigate, + onNavigateToDestination = appState::navigateToTopLevelDestination, currentDestination = appState.currentDestination, modifier = Modifier.safeDrawingPadding() ) @@ -112,7 +113,6 @@ fun NiaApp( NiaNavHost( navController = appState.navController, onBackClick = appState::onBackClick, - onNavigateToDestination = appState::navigate, modifier = Modifier .padding(padding) .consumedWindowInsets(padding) @@ -132,8 +132,7 @@ private fun NiaNavRail( ) { NiaNavigationRail(modifier = modifier) { destinations.forEach { destination -> - val selected = - currentDestination?.hierarchy?.any { it.route == destination.route } == true + val selected = currentDestination.isTopLevelDestinationInHierarchy(destination) NiaNavigationRailItem( selected = selected, onClick = { onNavigateToDestination(destination) }, @@ -168,8 +167,7 @@ private fun NiaBottomBar( ) { NiaNavigationBar { destinations.forEach { destination -> - val selected = - currentDestination?.hierarchy?.any { it.route == destination.route } == true + val selected = currentDestination.isTopLevelDestinationInHierarchy(destination) NiaNavigationBarItem( selected = selected, onClick = { onNavigateToDestination(destination) }, @@ -195,3 +193,8 @@ private fun NiaBottomBar( } } } + +private fun NavDestination?.isTopLevelDestinationInHierarchy(destination: TopLevelDestination) = + this?.hierarchy?.any { + it.route?.contains(destination.name, true) ?: false + } ?: false diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt index 127c3e206..04e2f395f 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt @@ -28,19 +28,16 @@ import androidx.navigation.NavGraph.Companion.findStartDestination import androidx.navigation.NavHostController import androidx.navigation.compose.currentBackStackEntryAsState import androidx.navigation.compose.rememberNavController +import androidx.navigation.navOptions import androidx.tracing.trace -import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.DrawableResourceIcon -import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.ImageVectorIcon -import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination import com.google.samples.apps.nowinandroid.core.ui.TrackDisposableJank -import com.google.samples.apps.nowinandroid.feature.bookmarks.R as bookmarksR -import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.BookmarksDestination -import com.google.samples.apps.nowinandroid.feature.foryou.R as forYouR -import com.google.samples.apps.nowinandroid.feature.foryou.navigation.ForYouDestination -import com.google.samples.apps.nowinandroid.feature.interests.R as interestsR -import com.google.samples.apps.nowinandroid.feature.interests.navigation.InterestsDestination +import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.navigateToBookmarks +import com.google.samples.apps.nowinandroid.feature.foryou.navigation.navigateToForYou +import com.google.samples.apps.nowinandroid.feature.interests.navigation.navigateToInterestsGraph import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination +import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.BOOKMARKS +import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.FOR_YOU +import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.INTERESTS @Composable fun rememberNiaAppState( @@ -72,61 +69,35 @@ class NiaAppState( /** * Top level destinations to be used in the BottomBar and NavRail */ - val topLevelDestinations: List = listOf( - TopLevelDestination( - route = ForYouDestination.route, - destination = ForYouDestination.destination, - selectedIcon = DrawableResourceIcon(NiaIcons.Upcoming), - unselectedIcon = DrawableResourceIcon(NiaIcons.UpcomingBorder), - iconTextId = forYouR.string.for_you - ), - TopLevelDestination( - route = BookmarksDestination.route, - destination = BookmarksDestination.destination, - selectedIcon = DrawableResourceIcon(NiaIcons.Bookmarks), - unselectedIcon = DrawableResourceIcon(NiaIcons.BookmarksBorder), - iconTextId = bookmarksR.string.saved - ), - TopLevelDestination( - route = InterestsDestination.route, - destination = InterestsDestination.destination, - selectedIcon = ImageVectorIcon(NiaIcons.Grid3x3), - unselectedIcon = ImageVectorIcon(NiaIcons.Grid3x3), - iconTextId = interestsR.string.interests - ) - ) + val topLevelDestinations: List = TopLevelDestination.values().asList() /** - * UI logic for navigating to a particular destination in the app. The NavigationOptions to - * navigate with are based on the type of destination, which could be a top level destination or - * just a regular destination. + * UI logic for navigating to a top level destination in the app. Top level destinations have + * only one copy of the destination of the back stack, and save and restore state whenever you + * navigate to and from it. * - * Top level destinations have only one copy of the destination of the back stack, and save and - * restore state whenever you navigate to and from it. - * Regular destinations can have multiple copies in the back stack and state isn't saved nor - * restored. - * - * @param destination: The [NiaNavigationDestination] the app needs to navigate to. - * @param route: Optional route to navigate to in case the destination contains arguments. + * @param topLevelDestination: The destination the app needs to navigate to. */ - fun navigate(destination: NiaNavigationDestination, route: String? = null) { - trace("Navigation: ${destination.route}") { - if (destination is TopLevelDestination) { - navController.navigate(route ?: destination.route) { - // Pop up to the start destination of the graph to - // avoid building up a large stack of destinations - // on the back stack as users select items - popUpTo(navController.graph.findStartDestination().id) { - saveState = true - } - // Avoid multiple copies of the same destination when - // reselecting the same item - launchSingleTop = true - // Restore state when reselecting a previously selected item - restoreState = true + fun navigateToTopLevelDestination(topLevelDestination: TopLevelDestination) { + trace("Navigation: ${topLevelDestination.name}") { + val topLevelNavOptions = navOptions { + // Pop up to the start destination of the graph to + // avoid building up a large stack of destinations + // on the back stack as users select items + popUpTo(navController.graph.findStartDestination().id) { + saveState = true } - } else { - navController.navigate(route ?: destination.route) + // Avoid multiple copies of the same destination when + // reselecting the same item + launchSingleTop = true + // Restore state when reselecting a previously selected item + restoreState = true + } + + when (topLevelDestination) { + FOR_YOU -> navController.navigateToForYou(topLevelNavOptions) + BOOKMARKS -> navController.navigateToBookmarks(topLevelNavOptions) + INTERESTS -> navController.navigateToInterestsGraph(topLevelNavOptions) } } } diff --git a/build-logic/convention/src/main/kotlin/AndroidFeatureConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidFeatureConventionPlugin.kt index e6ad9c031..7dd9b8972 100644 --- a/build-logic/convention/src/main/kotlin/AndroidFeatureConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidFeatureConventionPlugin.kt @@ -44,7 +44,6 @@ class AndroidFeatureConventionPlugin : Plugin { add("implementation", project(":core:designsystem")) add("implementation", project(":core:data")) add("implementation", project(":core:common")) - add("implementation", project(":core:navigation")) add("implementation", project(":core:domain")) add("testImplementation", project(":core:testing")) @@ -68,4 +67,4 @@ class AndroidFeatureConventionPlugin : Plugin { } } } -} \ No newline at end of file +} diff --git a/core/navigation/build.gradle.kts b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt similarity index 59% rename from core/navigation/build.gradle.kts rename to core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt index f762d5b7d..29437cc71 100644 --- a/core/navigation/build.gradle.kts +++ b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt @@ -13,19 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -// TODO: Remove once https://youtrack.jetbrains.com/issue/KTIJ-19369 is fixed -@Suppress("DSL_SCOPE_VIOLATION") -plugins { - id("nowinandroid.android.library") - id("nowinandroid.android.library.jacoco") - id("nowinandroid.android.hilt") -} -android { - namespace = "com.google.samples.apps.nowinandroid.core.navigation" -} +package com.google.samples.apps.nowinandroid.core.decoder -dependencies { - api(libs.androidx.hilt.navigation.compose) - api(libs.androidx.navigation.compose) -} \ No newline at end of file +interface StringDecoder { + fun decodeString(encodedString: String): String +} diff --git a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt new file mode 100644 index 000000000..b114bdab6 --- /dev/null +++ b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt @@ -0,0 +1,24 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.samples.apps.nowinandroid.core.decoder + +import android.net.Uri +import javax.inject.Inject + +class UriDecoder @Inject constructor() : StringDecoder { + override fun decodeString(encodedString: String): String = Uri.decode(encodedString) +} diff --git a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt new file mode 100644 index 000000000..e39b0e1f8 --- /dev/null +++ b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt @@ -0,0 +1,31 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.samples.apps.nowinandroid.core.decoder.di + +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder +import com.google.samples.apps.nowinandroid.core.decoder.UriDecoder +import dagger.Binds +import dagger.Module +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent + +@Module +@InstallIn(SingletonComponent::class) +abstract class StringDecoderModule { + @Binds + abstract fun bindStringDecoder(uriDecoder: UriDecoder): StringDecoder +} diff --git a/core/navigation/.gitignore b/core/navigation/.gitignore deleted file mode 100644 index 42afabfd2..000000000 --- a/core/navigation/.gitignore +++ /dev/null @@ -1 +0,0 @@ -/build \ No newline at end of file diff --git a/core/navigation/README.md b/core/navigation/README.md deleted file mode 100644 index a61f10aee..000000000 --- a/core/navigation/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# :core:navigation module - -![Dependency graph](../../docs/images/graphs/dep_graph_core_navigation.png) diff --git a/core/navigation/src/main/AndroidManifest.xml b/core/navigation/src/main/AndroidManifest.xml deleted file mode 100644 index ec921f928..000000000 --- a/core/navigation/src/main/AndroidManifest.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - \ No newline at end of file diff --git a/core/navigation/src/main/java/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigationDestination.kt b/core/navigation/src/main/java/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigationDestination.kt deleted file mode 100644 index af3303f18..000000000 --- a/core/navigation/src/main/java/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigationDestination.kt +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.core.navigation - -/** - * Interface for describing the Now in Android navigation destinations - */ - -interface NiaNavigationDestination { - /** - * Defines a specific route this destination belongs to. - * Route is a String that defines the path to your composable. - * You can think of it as an implicit deep link that leads to a specific destination. - * Each destination should have a unique route. - */ - val route: String - - /** - * Defines a specific destination ID. - * This is needed when using nested graphs via the navigation DLS, to differentiate a specific - * destination's route from the route of the entire nested graph it belongs to. - */ - val destination: String -} diff --git a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt new file mode 100644 index 000000000..7282661cc --- /dev/null +++ b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt @@ -0,0 +1,24 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.samples.apps.nowinandroid.core.testing.decoder + +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder +import javax.inject.Inject + +class FakeStringDecoder @Inject constructor() : StringDecoder { + override fun decodeString(encodedString: String): String = encodedString +} diff --git a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt new file mode 100644 index 000000000..0873ee546 --- /dev/null +++ b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt @@ -0,0 +1,35 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.samples.apps.nowinandroid.core.testing.di + +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder +import com.google.samples.apps.nowinandroid.core.decoder.di.StringDecoderModule +import com.google.samples.apps.nowinandroid.core.testing.decoder.FakeStringDecoder +import dagger.Binds +import dagger.Module +import dagger.hilt.components.SingletonComponent +import dagger.hilt.testing.TestInstallIn + +@Module +@TestInstallIn( + components = [SingletonComponent::class], + replaces = [StringDecoderModule::class], +) +abstract class TestStringDecoderModule { + @Binds + abstract fun bindsStringDecoder(fakeStringDecoder: FakeStringDecoder): StringDecoder +} diff --git a/docs/ModularizationLearningJourney.md b/docs/ModularizationLearningJourney.md index 56461ce73..fbb0ac791 100644 --- a/docs/ModularizationLearningJourney.md +++ b/docs/ModularizationLearningJourney.md @@ -224,14 +224,6 @@ Using the above modularization strategy, the Now in Android app has the followin NewsResource - - core:navigation - - Navigation dependencies and shared navigation classes. - - NiaNavigationDestination - - diff --git a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt index ecebb2ac5..114b4c438 100644 --- a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt +++ b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt @@ -65,7 +65,7 @@ import com.google.samples.apps.nowinandroid.core.ui.newsResourceCardItems @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun AuthorRoute( +internal fun AuthorRoute( onBackClick: () -> Unit, modifier: Modifier = Modifier, viewModel: AuthorViewModel = hiltViewModel(), diff --git a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt index 137285193..2f63dbc11 100644 --- a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt +++ b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt @@ -21,13 +21,14 @@ 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.UserDataRepository +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder import com.google.samples.apps.nowinandroid.core.domain.GetSaveableNewsResourcesStreamUseCase import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor import com.google.samples.apps.nowinandroid.core.domain.model.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.model.data.Author import com.google.samples.apps.nowinandroid.core.result.Result import com.google.samples.apps.nowinandroid.core.result.asResult -import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorDestination +import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorArgs import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.coroutines.flow.Flow @@ -41,17 +42,16 @@ import kotlinx.coroutines.launch @HiltViewModel class AuthorViewModel @Inject constructor( savedStateHandle: SavedStateHandle, + stringDecoder: StringDecoder, private val userDataRepository: UserDataRepository, authorsRepository: AuthorsRepository, getSaveableNewsResourcesStream: GetSaveableNewsResourcesStreamUseCase ) : ViewModel() { - private val authorId: String = checkNotNull( - savedStateHandle[AuthorDestination.authorIdArg] - ) + private val authorArgs: AuthorArgs = AuthorArgs(savedStateHandle, stringDecoder) val authorUiState: StateFlow = authorUiStateStream( - authorId = authorId, + authorId = authorArgs.authorId, userDataRepository = userDataRepository, authorsRepository = authorsRepository ) @@ -62,7 +62,7 @@ class AuthorViewModel @Inject constructor( ) val newsUiState: StateFlow = - getSaveableNewsResourcesStream.newsUiStateStream(authorId = authorId) + getSaveableNewsResourcesStream.newsUiStateStream(authorId = authorArgs.authorId) .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(5_000), @@ -71,7 +71,7 @@ class AuthorViewModel @Inject constructor( fun followAuthorToggle(followed: Boolean) { viewModelScope.launch { - userDataRepository.toggleFollowedAuthorId(authorId, followed) + userDataRepository.toggleFollowedAuthorId(authorArgs.authorId, followed) } } diff --git a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/navigation/AuthorNavigation.kt b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/navigation/AuthorNavigation.kt index ee37bab95..59b68ce91 100644 --- a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/navigation/AuthorNavigation.kt +++ b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/navigation/AuthorNavigation.kt @@ -17,43 +17,36 @@ package com.google.samples.apps.nowinandroid.feature.author.navigation import android.net.Uri -import androidx.navigation.NavBackStackEntry +import androidx.annotation.VisibleForTesting +import androidx.lifecycle.SavedStateHandle +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder import androidx.navigation.NavType import androidx.navigation.compose.composable import androidx.navigation.navArgument -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder import com.google.samples.apps.nowinandroid.feature.author.AuthorRoute -object AuthorDestination : NiaNavigationDestination { - const val authorIdArg = "authorId" - override val route = "author_route/{$authorIdArg}" - override val destination = "author_destination" +@VisibleForTesting +internal const val authorIdArg = "authorId" - /** - * Creates destination route for an authorId that could include special characters - */ - fun createNavigationRoute(authorIdArg: String): String { - val encodedId = Uri.encode(authorIdArg) - return "author_route/$encodedId" - } +internal class AuthorArgs(val authorId: String) { + constructor(savedStateHandle: SavedStateHandle, stringDecoder: StringDecoder) : + this(stringDecoder.decodeString(checkNotNull(savedStateHandle[authorIdArg]))) +} - /** - * Returns the authorId from a [NavBackStackEntry] after an author destination navigation call - */ - fun fromNavArgs(entry: NavBackStackEntry): String { - val encodedId = entry.arguments?.getString(authorIdArg)!! - return Uri.decode(encodedId) - } +fun NavController.navigateToAuthor(authorId: String) { + val encodedString = Uri.encode(authorId) + this.navigate("author_route/$encodedString") } -fun NavGraphBuilder.authorGraph( +fun NavGraphBuilder.authorScreen( onBackClick: () -> Unit ) { composable( - route = AuthorDestination.route, + route = "author_route/{$authorIdArg}", arguments = listOf( - navArgument(AuthorDestination.authorIdArg) { type = NavType.StringType } + navArgument(authorIdArg) { type = NavType.StringType } ) ) { AuthorRoute(onBackClick = onBackClick) diff --git a/feature/author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt b/feature/author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt index cb6c17076..aac9617a4 100644 --- a/feature/author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt +++ b/feature/author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt @@ -22,11 +22,12 @@ import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor import com.google.samples.apps.nowinandroid.core.model.data.Author 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.testing.decoder.FakeStringDecoder import com.google.samples.apps.nowinandroid.core.testing.repository.TestAuthorsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestNewsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestUserDataRepository import com.google.samples.apps.nowinandroid.core.testing.util.MainDispatcherRule -import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorDestination +import com.google.samples.apps.nowinandroid.feature.author.navigation.authorIdArg import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first @@ -63,9 +64,10 @@ class AuthorViewModelTest { viewModel = AuthorViewModel( savedStateHandle = SavedStateHandle( mapOf( - AuthorDestination.authorIdArg to testInputAuthors[0].author.id + authorIdArg to testInputAuthors[0].author.id ) ), + stringDecoder = FakeStringDecoder(), userDataRepository = userDataRepository, authorsRepository = authorsRepository, getSaveableNewsResourcesStream = getSaveableNewsResourcesStreamUseCase diff --git a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt index dba490a73..849291df3 100644 --- a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt +++ b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt @@ -54,7 +54,7 @@ import com.google.samples.apps.nowinandroid.core.ui.newsFeed @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun BookmarksRoute( +internal fun BookmarksRoute( modifier: Modifier = Modifier, viewModel: BookmarksViewModel = hiltViewModel() ) { diff --git a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt index bf64eb34c..0d530019d 100644 --- a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt +++ b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt @@ -16,18 +16,20 @@ package com.google.samples.apps.nowinandroid.feature.bookmarks.navigation +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder +import androidx.navigation.NavOptions import androidx.navigation.compose.composable -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination import com.google.samples.apps.nowinandroid.feature.bookmarks.BookmarksRoute -object BookmarksDestination : NiaNavigationDestination { - override val route = "bookmarks_route" - override val destination = "bookmarks_destination" +private const val bookmarksRoute = "bookmarks_route" + +fun NavController.navigateToBookmarks(navOptions: NavOptions? = null) { + this.navigate(bookmarksRoute, navOptions) } -fun NavGraphBuilder.bookmarksGraph() { - composable(route = BookmarksDestination.route) { +fun NavGraphBuilder.bookmarksScreen() { + composable(route = bookmarksRoute) { BookmarksRoute() } } diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt index 24de3548f..252afb1ce 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt @@ -100,7 +100,7 @@ import com.google.samples.apps.nowinandroid.core.ui.newsFeed @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun ForYouRoute( +internal fun ForYouRoute( modifier: Modifier = Modifier, viewModel: ForYouViewModel = hiltViewModel() ) { @@ -124,7 +124,7 @@ fun ForYouRoute( @OptIn(ExperimentalMaterial3Api::class, ExperimentalLayoutApi::class) @Composable -fun ForYouScreen( +internal fun ForYouScreen( isOffline: Boolean, isSyncing: Boolean, interestsSelectionState: ForYouInterestsSelectionUiState, diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt index 2c0dccccb..f57deab90 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt @@ -16,18 +16,20 @@ package com.google.samples.apps.nowinandroid.feature.foryou.navigation +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder +import androidx.navigation.NavOptions import androidx.navigation.compose.composable -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination import com.google.samples.apps.nowinandroid.feature.foryou.ForYouRoute -object ForYouDestination : NiaNavigationDestination { - override val route = "for_you_route" - override val destination = "for_you_destination" +const val forYouNavigationRoute = "for_you_route" + +fun NavController.navigateToForYou(navOptions: NavOptions? = null) { + this.navigate(forYouNavigationRoute, navOptions) } -fun NavGraphBuilder.forYouGraph() { - composable(route = ForYouDestination.route) { +fun NavGraphBuilder.forYouScreen() { + composable(route = forYouNavigationRoute) { ForYouRoute() } } diff --git a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt index 843a45e49..d92579723 100644 --- a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt +++ b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt @@ -45,7 +45,7 @@ import com.google.samples.apps.nowinandroid.core.ui.TrackDisposableJank @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun InterestsRoute( +internal fun InterestsRoute( navigateToAuthor: (String) -> Unit, navigateToTopic: (String) -> Unit, modifier: Modifier = Modifier, @@ -76,7 +76,7 @@ fun InterestsRoute( @OptIn(ExperimentalMaterial3Api::class) @Composable -fun InterestsScreen( +internal fun InterestsScreen( uiState: InterestsUiState, tabState: InterestsTabState, followAuthor: (String, Boolean) -> Unit, diff --git a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/navigation/InterestsNavigation.kt b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/navigation/InterestsNavigation.kt index 87612beeb..19e04b8b5 100644 --- a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/navigation/InterestsNavigation.kt +++ b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/navigation/InterestsNavigation.kt @@ -16,28 +16,30 @@ package com.google.samples.apps.nowinandroid.feature.interests.navigation +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder +import androidx.navigation.NavOptions import androidx.navigation.compose.composable import androidx.navigation.navigation -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination import com.google.samples.apps.nowinandroid.feature.interests.InterestsRoute -object InterestsDestination : NiaNavigationDestination { - override val route = "interests_route" - override val destination = "interests_destination" +private const val interestsGraphRoutePattern = "interests_graph" +private const val interestsRoute = "interests_route" + +fun NavController.navigateToInterestsGraph(navOptions: NavOptions? = null) { + this.navigate(interestsGraphRoutePattern, navOptions) } fun NavGraphBuilder.interestsGraph( navigateToTopic: (String) -> Unit, navigateToAuthor: (String) -> Unit, nestedGraphs: NavGraphBuilder.() -> Unit - ) { navigation( - route = InterestsDestination.route, - startDestination = InterestsDestination.destination + route = interestsGraphRoutePattern, + startDestination = interestsRoute ) { - composable(route = InterestsDestination.destination) { + composable(route = interestsRoute) { InterestsRoute( navigateToTopic = navigateToTopic, navigateToAuthor = navigateToAuthor, 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 0adc66b40..58c572ade 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 @@ -64,7 +64,7 @@ import com.google.samples.apps.nowinandroid.feature.topic.TopicUiState.Loading @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun TopicRoute( +internal fun TopicRoute( onBackClick: () -> Unit, modifier: Modifier = Modifier, viewModel: TopicViewModel = hiltViewModel(), 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 862e130f1..160bcfb3b 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 @@ -21,13 +21,14 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope 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.decoder.StringDecoder import com.google.samples.apps.nowinandroid.core.domain.GetSaveableNewsResourcesStreamUseCase import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic import com.google.samples.apps.nowinandroid.core.domain.model.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 -import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicDestination +import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicArgs import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.coroutines.flow.Flow @@ -41,16 +42,17 @@ import kotlinx.coroutines.launch @HiltViewModel class TopicViewModel @Inject constructor( savedStateHandle: SavedStateHandle, + stringDecoder: StringDecoder, private val userDataRepository: UserDataRepository, topicsRepository: TopicsRepository, // newsRepository: NewsRepository, getSaveableNewsResourcesStream: GetSaveableNewsResourcesStreamUseCase ) : ViewModel() { - private val topicId: String = checkNotNull(savedStateHandle[TopicDestination.topicIdArg]) + private val topicArgs: TopicArgs = TopicArgs(savedStateHandle, stringDecoder) val topicUiState: StateFlow = topicUiStateStream( - topicId = topicId, + topicId = topicArgs.topicId, userDataRepository = userDataRepository, topicsRepository = topicsRepository ) @@ -61,7 +63,7 @@ class TopicViewModel @Inject constructor( ) val newUiState: StateFlow = newsUiStateStream( - topicId = topicId, + topicId = topicArgs.topicId, userDataRepository = userDataRepository, getSaveableNewsResourcesStream = getSaveableNewsResourcesStream ) @@ -73,7 +75,7 @@ class TopicViewModel @Inject constructor( fun followTopicToggle(followed: Boolean) { viewModelScope.launch { - userDataRepository.toggleFollowedTopicId(topicId, followed) + userDataRepository.toggleFollowedTopicId(topicArgs.topicId, followed) } } diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt index f3d4d021b..808143275 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt @@ -17,43 +17,36 @@ package com.google.samples.apps.nowinandroid.feature.topic.navigation import android.net.Uri -import androidx.navigation.NavBackStackEntry +import androidx.annotation.VisibleForTesting +import androidx.lifecycle.SavedStateHandle +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder import androidx.navigation.NavType import androidx.navigation.compose.composable import androidx.navigation.navArgument -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder import com.google.samples.apps.nowinandroid.feature.topic.TopicRoute -object TopicDestination : NiaNavigationDestination { - const val topicIdArg = "topicId" - override val route = "topic_route/{$topicIdArg}" - override val destination = "topic_destination" +@VisibleForTesting +internal const val topicIdArg = "topicId" - /** - * Creates destination route for a topicId that could include special characters - */ - fun createNavigationRoute(topicIdArg: String): String { - val encodedId = Uri.encode(topicIdArg) - return "topic_route/$encodedId" - } +internal class TopicArgs(val topicId: String) { + constructor(savedStateHandle: SavedStateHandle, stringDecoder: StringDecoder) : + this(stringDecoder.decodeString(checkNotNull(savedStateHandle[topicIdArg]))) +} - /** - * Returns the topicId from a [NavBackStackEntry] after a topic destination navigation call - */ - fun fromNavArgs(entry: NavBackStackEntry): String { - val encodedId = entry.arguments?.getString(topicIdArg)!! - return Uri.decode(encodedId) - } +fun NavController.navigateToTopic(topicId: String) { + val encodedId = Uri.encode(topicId) + this.navigate("topic_route/$encodedId") } -fun NavGraphBuilder.topicGraph( +fun NavGraphBuilder.topicScreen( onBackClick: () -> Unit ) { composable( - route = TopicDestination.route, + route = "topic_route/{$topicIdArg}", arguments = listOf( - navArgument(TopicDestination.topicIdArg) { type = NavType.StringType } + navArgument(topicIdArg) { type = NavType.StringType } ) ) { TopicRoute(onBackClick = onBackClick) 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 94e0e9337..b914856be 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 @@ -22,11 +22,12 @@ import com.google.samples.apps.nowinandroid.core.domain.model.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.Topic +import com.google.samples.apps.nowinandroid.core.testing.decoder.FakeStringDecoder import com.google.samples.apps.nowinandroid.core.testing.repository.TestNewsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestTopicsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestUserDataRepository import com.google.samples.apps.nowinandroid.core.testing.util.MainDispatcherRule -import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicDestination +import com.google.samples.apps.nowinandroid.feature.topic.navigation.topicIdArg import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first @@ -61,8 +62,8 @@ class TopicViewModelTest { @Before fun setup() { viewModel = TopicViewModel( - savedStateHandle = - SavedStateHandle(mapOf(TopicDestination.topicIdArg to testInputTopics[0].topic.id)), + savedStateHandle = SavedStateHandle(mapOf(topicIdArg to testInputTopics[0].topic.id)), + stringDecoder = FakeStringDecoder(), userDataRepository = userDataRepository, topicsRepository = topicsRepository, getSaveableNewsResourcesStream = getSaveableNewsResourcesStreamUseCase diff --git a/settings.gradle.kts b/settings.gradle.kts index 253fa23d4..c6e2d3bf5 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -43,7 +43,6 @@ include(":core:datastore-test") include(":core:designsystem") include(":core:domain") include(":core:model") -include(":core:navigation") include(":core:network") include(":core:ui") include(":core:testing") @@ -54,4 +53,4 @@ include(":feature:bookmarks") include(":feature:topic") include(":lint") include(":sync:work") -include(":sync:sync-test") \ No newline at end of file +include(":sync:sync-test") From 2dd2f83026330ab81bda24aae897144c03f11355 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Mon, 31 Oct 2022 16:57:36 +0000 Subject: [PATCH 2/6] Remove applicationIdSuffix from demo variant, add it to prod instead Change-Id: I890beefbd79de04a9f5fa1153eb8b1ab3ee6eaf4 --- .../kotlin/com/google/samples/apps/nowinandroid/Flavor.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build-logic/convention/src/main/kotlin/com/google/samples/apps/nowinandroid/Flavor.kt b/build-logic/convention/src/main/kotlin/com/google/samples/apps/nowinandroid/Flavor.kt index f3a12db4f..e251267ce 100644 --- a/build-logic/convention/src/main/kotlin/com/google/samples/apps/nowinandroid/Flavor.kt +++ b/build-logic/convention/src/main/kotlin/com/google/samples/apps/nowinandroid/Flavor.kt @@ -14,8 +14,8 @@ enum class FlavorDimension { // purposes, or from a production backend server which supplies up-to-date, real content. // These two product flavors reflect this behaviour. enum class Flavor (val dimension : FlavorDimension, val applicationIdSuffix : String? = null) { - demo(FlavorDimension.contentType, ".demo"), - prod(FlavorDimension.contentType) + demo(FlavorDimension.contentType), + prod(FlavorDimension.contentType, ".prod") } fun Project.configureFlavors( From d616bf3a42bbcae41047de634d60e5b62591acdc Mon Sep 17 00:00:00 2001 From: Don Turner Date: Fri, 4 Nov 2022 20:43:11 +0000 Subject: [PATCH 3/6] New version to trigger new build on server Change-Id: I19e606f29c4d7449c09184282ba1e646ead52517 --- app/build.gradle.kts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b67a3f965..c4d60e202 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -26,8 +26,8 @@ plugins { android { defaultConfig { applicationId = "com.google.samples.apps.nowinandroid" - versionCode = 2 - versionName = "0.0.2" // X.Y.Z; X = Major, Y = minor, Z = Patch level + versionCode = 3 + versionName = "0.0.3" // X.Y.Z; X = Major, Y = minor, Z = Patch level // Custom test runner to set up Hilt dependency graph testInstrumentationRunner = "com.google.samples.apps.nowinandroid.core.testing.NiaTestRunner" From 6654403498eeffef6c1feed4f925679831ae03b2 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Thu, 10 Nov 2022 13:20:14 +0000 Subject: [PATCH 4/6] Refactor onboarding to save interests to user data Change-Id: I97772a8b65e1e94a95187ebb7c514104af2d3a08 --- .../OfflineFirstUserDataRepository.kt | 3 + .../data/repository/UserDataRepository.kt | 5 + .../repository/fake/FakeUserDataRepository.kt | 4 + .../OfflineFirstUserDataRepositoryTest.kt | 14 +- core/datastore/build.gradle.kts | 1 + .../datastore/NiaPreferencesDataSource.kt | 22 +- .../nowinandroid/data/user_preferences.proto | 5 + .../datastore/NiaPreferencesDataSourceTest.kt | 137 +++++ .../GetFollowableTopicsStreamUseCase.kt | 19 +- ...entSortedFollowableAuthorsStreamUseCase.kt | 48 -- ...GetSortedFollowableAuthorsStreamUseCase.kt | 29 +- .../GetFollowableTopicsStreamUseCaseTest.kt | 26 - ...ortedFollowableAuthorsStreamUseCaseTest.kt | 95 ---- ...ortedFollowableAuthorsStreamUseCaseTest.kt | 12 +- .../nowinandroid/core/model/data/UserData.kt | 1 + .../repository/TestUserDataRepository.kt | 9 +- .../feature/foryou/ForYouScreenTest.kt | 24 +- .../foryou/FollowedInterestsUiState.kt | 41 -- .../feature/foryou/ForYouScreen.kt | 58 +-- .../feature/foryou/ForYouViewModel.kt | 157 ++---- ...lectionUiState.kt => OnboardingUiState.kt} | 26 +- .../feature/foryou/ForYouViewModelTest.kt | 477 ++---------------- .../feature/interests/InterestsViewModel.kt | 6 +- .../interests/InterestsViewModelTest.kt | 8 +- 24 files changed, 379 insertions(+), 848 deletions(-) create mode 100644 core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSourceTest.kt delete mode 100644 core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetPersistentSortedFollowableAuthorsStreamUseCase.kt delete mode 100644 core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetPersistentSortedFollowableAuthorsStreamUseCaseTest.kt delete mode 100644 feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsUiState.kt rename feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/{ForYouInterestsSelectionUiState.kt => OnboardingUiState.kt} (59%) diff --git a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepository.kt b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepository.kt index 8af0d3711..0bf4d6438 100644 --- a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepository.kt +++ b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepository.kt @@ -50,4 +50,7 @@ class OfflineFirstUserDataRepository @Inject constructor( override suspend fun setDarkThemeConfig(darkThemeConfig: DarkThemeConfig) = niaPreferencesDataSource.setDarkThemeConfig(darkThemeConfig) + + override suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) = + niaPreferencesDataSource.setHasDismissedOnboarding(hasDismissedOnboarding) } diff --git a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/UserDataRepository.kt b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/UserDataRepository.kt index 7554d3f03..86d26a47e 100644 --- a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/UserDataRepository.kt +++ b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/UserDataRepository.kt @@ -62,4 +62,9 @@ interface UserDataRepository { * Sets the desired dark theme config. */ suspend fun setDarkThemeConfig(darkThemeConfig: DarkThemeConfig) + + /** + * Sets whether the user has completed the onboarding process. + */ + suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) } diff --git a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/fake/FakeUserDataRepository.kt b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/fake/FakeUserDataRepository.kt index 58714cf40..e87b59a91 100644 --- a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/fake/FakeUserDataRepository.kt +++ b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/fake/FakeUserDataRepository.kt @@ -63,4 +63,8 @@ class FakeUserDataRepository @Inject constructor( override suspend fun setDarkThemeConfig(darkThemeConfig: DarkThemeConfig) { niaPreferencesDataSource.setDarkThemeConfig(darkThemeConfig) } + + override suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) { + niaPreferencesDataSource.setHasDismissedOnboarding(hasDismissedOnboarding) + } } diff --git a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepositoryTest.kt b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepositoryTest.kt index fd4b2ccde..6f0f9394d 100644 --- a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepositoryTest.kt +++ b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepositoryTest.kt @@ -58,7 +58,8 @@ class OfflineFirstUserDataRepositoryTest { followedTopics = emptySet(), followedAuthors = emptySet(), themeBrand = ThemeBrand.DEFAULT, - darkThemeConfig = DarkThemeConfig.FOLLOW_SYSTEM + darkThemeConfig = DarkThemeConfig.FOLLOW_SYSTEM, + hasDismissedOnboarding = false ), subject.userDataStream.first() ) @@ -187,4 +188,15 @@ class OfflineFirstUserDataRepositoryTest { .first() ) } + + @Test + fun whenUserCompletesOnboarding_thenRemovesAllInterests_hasDismissedOnboardingIsFalse() = + runTest { + subject.setFollowedTopicIds(setOf("1")) + subject.setHasDismissedOnboarding(true) + assertEquals(true, subject.userDataStream.first().hasDismissedOnboarding) + + subject.setFollowedTopicIds(emptySet()) + assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + } } diff --git a/core/datastore/build.gradle.kts b/core/datastore/build.gradle.kts index 5d33f2bfb..8f3d7ece6 100644 --- a/core/datastore/build.gradle.kts +++ b/core/datastore/build.gradle.kts @@ -59,6 +59,7 @@ dependencies { implementation(project(":core:model")) testImplementation(project(":core:testing")) + testImplementation(project(":core:datastore-test")) implementation(libs.kotlinx.coroutines.android) diff --git a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt index 433130679..be129a3bd 100644 --- a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt +++ b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt @@ -52,7 +52,8 @@ class NiaPreferencesDataSource @Inject constructor( DarkThemeConfigProto.DARK_THEME_CONFIG_LIGHT -> DarkThemeConfig.LIGHT DarkThemeConfigProto.DARK_THEME_CONFIG_DARK -> DarkThemeConfig.DARK - } + }, + hasDismissedOnboarding = it.hasDismissedOnboarding ) } @@ -62,6 +63,7 @@ class NiaPreferencesDataSource @Inject constructor( it.copy { followedTopicIds.clear() followedTopicIds.putAll(topicIds.associateWith { true }) + updateHasDismissedOnboardingIfNecessary() } } } catch (ioException: IOException) { @@ -78,6 +80,7 @@ class NiaPreferencesDataSource @Inject constructor( } else { followedTopicIds.remove(topicId) } + updateHasDismissedOnboardingIfNecessary() } } } catch (ioException: IOException) { @@ -91,6 +94,7 @@ class NiaPreferencesDataSource @Inject constructor( it.copy { followedAuthorIds.clear() followedAuthorIds.putAll(authorIds.associateWith { true }) + updateHasDismissedOnboardingIfNecessary() } } } catch (ioException: IOException) { @@ -107,6 +111,7 @@ class NiaPreferencesDataSource @Inject constructor( } else { followedAuthorIds.remove(authorId) } + updateHasDismissedOnboardingIfNecessary() } } } catch (ioException: IOException) { @@ -188,4 +193,19 @@ class NiaPreferencesDataSource @Inject constructor( Log.e("NiaPreferences", "Failed to update user preferences", ioException) } } + + suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) { + userPreferences.updateData { + it.copy { + this.hasDismissedOnboarding = hasDismissedOnboarding + } + } + } +} + +fun UserPreferencesKt.Dsl.updateHasDismissedOnboardingIfNecessary() { + + if (followedTopicIds.isEmpty() && followedAuthorIds.isEmpty()) { + hasDismissedOnboarding = false + } } diff --git a/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto b/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto index f0eca3d32..f771ac1ac 100644 --- a/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto +++ b/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto @@ -43,4 +43,9 @@ message UserPreferences { ThemeBrandProto theme_brand = 16; DarkThemeConfigProto dark_theme_config = 17; + + // Note that proto3 only allows a default value of `false` for boolean types. This means that + // whilst the name `should_show_onboarding` would be preferable here we are forced to choose a + // name which works with this default value constraint. + bool has_dismissed_onboarding = 18; } diff --git a/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSourceTest.kt b/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSourceTest.kt new file mode 100644 index 000000000..8bb552521 --- /dev/null +++ b/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSourceTest.kt @@ -0,0 +1,137 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.samples.apps.nowinandroid.core.datastore + +import com.google.samples.apps.nowinandroid.core.datastore.test.testUserPreferencesDataStore +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder + +class NiaPreferencesDataSourceTest { + private lateinit var subject: NiaPreferencesDataSource + + @get:Rule + val tmpFolder: TemporaryFolder = TemporaryFolder.builder().assureDeletion().build() + + @Before + fun setup() { + subject = NiaPreferencesDataSource( + tmpFolder.testUserPreferencesDataStore() + ) + } + + @Test + fun hasDismissedOnboardingIsFalseByDefault() = runTest { + assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + } + + @Test + fun userHasDismissedOnboardingIsTrueWhenSet() = runTest { + subject.setHasDismissedOnboarding(true) + assertEquals(true, subject.userDataStream.first().hasDismissedOnboarding) + } + + @Test + fun userHasDismissedOnboarding_unfollowsLastAuthor_hasDismissedOnboardingIsFalse() = runTest { + + // Given: user completes onboarding by selecting a single author. + subject.toggleFollowedAuthorId("1", true) + subject.setHasDismissedOnboarding(true) + + // When: they unfollow that author. + subject.toggleFollowedAuthorId("1", false) + + // Then: onboarding should be shown again + assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + } + + @Test + fun userHasDismissedOnboarding_unfollowsLastTopic_hasDismissedOnboardingIsFalse() = runTest { + + // Given: user completes onboarding by selecting a single topic. + subject.toggleFollowedTopicId("1", true) + subject.setHasDismissedOnboarding(true) + + // When: they unfollow that topic. + subject.toggleFollowedTopicId("1", false) + + // Then: onboarding should be shown again + assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + } + + @Test + fun userHasDismissedOnboarding_unfollowsAllAuthors_hasDismissedOnboardingIsFalse() = runTest { + + // Given: user completes onboarding by selecting several authors. + subject.setFollowedAuthorIds(setOf("1", "2")) + subject.setHasDismissedOnboarding(true) + + // When: they unfollow those authors. + subject.setFollowedAuthorIds(emptySet()) + + // Then: onboarding should be shown again + assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + } + + @Test + fun userHasDismissedOnboarding_unfollowsAllTopics_hasDismissedOnboardingIsFalse() = runTest { + + // Given: user completes onboarding by selecting several topics. + subject.setFollowedTopicIds(setOf("1", "2")) + subject.setHasDismissedOnboarding(true) + + // When: they unfollow those topics. + subject.setFollowedTopicIds(emptySet()) + + // Then: onboarding should be shown again + assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + } + + @Test + fun userHasDismissedOnboarding_unfollowsAllTopicsButNotAuthors_hasDismissedOnboardingIsTrue() = + runTest { + // Given: user completes onboarding by selecting several topics and authors. + subject.setFollowedTopicIds(setOf("1", "2")) + subject.setFollowedAuthorIds(setOf("3", "4")) + subject.setHasDismissedOnboarding(true) + + // When: they unfollow just the topics. + subject.setFollowedTopicIds(emptySet()) + + // Then: onboarding should still be dismissed + assertEquals(true, subject.userDataStream.first().hasDismissedOnboarding) + } + + @Test + fun userHasDismissedOnboarding_unfollowsAllAuthorsButNotTopics_hasDismissedOnboardingIsTrue() = + runTest { + // Given: user completes onboarding by selecting several topics and authors. + subject.setFollowedTopicIds(setOf("1", "2")) + subject.setFollowedAuthorIds(setOf("3", "4")) + subject.setHasDismissedOnboarding(true) + + // When: they unfollow just the authors. + subject.setFollowedAuthorIds(emptySet()) + + // Then: onboarding should still be dismissed + assertEquals(true, subject.userDataStream.first().hasDismissedOnboarding) + } +} diff --git a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetFollowableTopicsStreamUseCase.kt b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetFollowableTopicsStreamUseCase.kt index 80dea1898..2f1fada9d 100644 --- a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetFollowableTopicsStreamUseCase.kt +++ b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetFollowableTopicsStreamUseCase.kt @@ -36,29 +36,18 @@ class GetFollowableTopicsStreamUseCase @Inject constructor( /** * Returns a list of topics with their associated followed state. * - * @param followedTopicIdsStream - the set of topic ids which are currently being followed. By - * default the followed topic ids are supplied from the user data repository, but in certain - * scenarios, such as when creating a temporary set of followed topics, you may wish to override - * this parameter to supply your own list of topic ids. @see ForYouViewModel for an example of - * this. * @param sortBy - the field used to sort the topics. Default NONE = no sorting. */ - operator fun invoke( - followedTopicIdsStream: Flow> = - userDataRepository.userDataStream.map { userdata -> - userdata.followedTopics - }, - sortBy: TopicSortField = NONE - ): Flow> { + operator fun invoke(sortBy: TopicSortField = NONE): Flow> { return combine( - followedTopicIdsStream, + userDataRepository.userDataStream, topicsRepository.getTopicsStream() - ) { followedIds, topics -> + ) { userData, topics -> val followedTopics = topics .map { topic -> FollowableTopic( topic = topic, - isFollowed = topic.id in followedIds + isFollowed = topic.id in userData.followedTopics ) } when (sortBy) { diff --git a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetPersistentSortedFollowableAuthorsStreamUseCase.kt b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetPersistentSortedFollowableAuthorsStreamUseCase.kt deleted file mode 100644 index fda0b4728..000000000 --- a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetPersistentSortedFollowableAuthorsStreamUseCase.kt +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.core.domain - -import com.google.samples.apps.nowinandroid.core.data.repository.AuthorsRepository -import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository -import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor -import javax.inject.Inject -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.flatMapLatest -import kotlinx.coroutines.flow.map - -/** - * A use case which obtains a sorted list of authors with their followed state obtained from user - * data. - */ -class GetPersistentSortedFollowableAuthorsStreamUseCase @Inject constructor( - authorsRepository: AuthorsRepository, - private val userDataRepository: UserDataRepository -) { - private val getSortedFollowableAuthorsStream = - GetSortedFollowableAuthorsStreamUseCase(authorsRepository) - - /** - * Returns a list of authors with their associated followed state sorted alphabetically by name. - */ - operator fun invoke(): Flow> { - return userDataRepository.userDataStream.map { userdata -> - userdata.followedAuthors - }.flatMapLatest { - getSortedFollowableAuthorsStream(it) - } - } -} diff --git a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetSortedFollowableAuthorsStreamUseCase.kt b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetSortedFollowableAuthorsStreamUseCase.kt index e1709e4cd..498558c2f 100644 --- a/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetSortedFollowableAuthorsStreamUseCase.kt +++ b/core/domain/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetSortedFollowableAuthorsStreamUseCase.kt @@ -17,33 +17,34 @@ package com.google.samples.apps.nowinandroid.core.domain import com.google.samples.apps.nowinandroid.core.data.repository.AuthorsRepository +import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor import javax.inject.Inject import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.combine /** * A use case which obtains a list of authors sorted alphabetically by name with their followed * state. */ class GetSortedFollowableAuthorsStreamUseCase @Inject constructor( - private val authorsRepository: AuthorsRepository + private val authorsRepository: AuthorsRepository, + private val userDataRepository: UserDataRepository ) { /** * Returns a list of authors with their associated followed state sorted alphabetically by name. - * - * @param followedTopicIds - the set of topic ids which are currently being followed. */ - operator fun invoke(followedAuthorIds: Set): Flow> { - return authorsRepository.getAuthorsStream().map { authors -> - authors - .map { author -> - FollowableAuthor( - author = author, - isFollowed = author.id in followedAuthorIds - ) - } + operator fun invoke(): Flow> = + combine( + authorsRepository.getAuthorsStream(), + userDataRepository.userDataStream + ) { authors, userData -> + authors.map { author -> + FollowableAuthor( + author = author, + isFollowed = author.id in userData.followedAuthors + ) + } .sortedBy { it.author.name } } - } } diff --git a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetFollowableTopicsStreamUseCaseTest.kt b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetFollowableTopicsStreamUseCaseTest.kt index c406de47b..101590605 100644 --- a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetFollowableTopicsStreamUseCaseTest.kt +++ b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetFollowableTopicsStreamUseCaseTest.kt @@ -16,7 +16,6 @@ package com.google.samples.apps.nowinandroid.core.domain -import androidx.compose.runtime.snapshotFlow import com.google.samples.apps.nowinandroid.core.domain.TopicSortField.NAME import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.Topic @@ -63,31 +62,6 @@ class GetFollowableTopicsStreamUseCaseTest { ) } - @Test - fun whenFollowedTopicIdsSupplied_differentFollowedTopicsAreReturned() = runTest { - - // Obtain a stream of followable topics, specifying a list of topic ids which are currently - // followed. - val followableTopics = useCase( - followedTopicIdsStream = snapshotFlow { setOf(testTopics[1].id) } - ) - - // Send some test topics and their followed state. - topicsRepository.sendTopics(testTopics) - userDataRepository.setFollowedTopicIds(setOf(testTopics[0].id)) - - // Check that the topic ids supplied to the use case are used for the bookmark state, not - // the topic ids in the user data repository. - assertEquals( - followableTopics.first(), - listOf( - FollowableTopic(testTopics[0], false), - FollowableTopic(testTopics[1], true), - FollowableTopic(testTopics[2], false), - ) - ) - } - @Test fun whenSortOrderIsByName_topicsSortedByNameAreReturned() = runTest { diff --git a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetPersistentSortedFollowableAuthorsStreamUseCaseTest.kt b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetPersistentSortedFollowableAuthorsStreamUseCaseTest.kt deleted file mode 100644 index 250a5e89d..000000000 --- a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetPersistentSortedFollowableAuthorsStreamUseCaseTest.kt +++ /dev/null @@ -1,95 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.core.domain - -import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor -import com.google.samples.apps.nowinandroid.core.model.data.Author -import com.google.samples.apps.nowinandroid.core.testing.repository.TestAuthorsRepository -import com.google.samples.apps.nowinandroid.core.testing.repository.TestUserDataRepository -import com.google.samples.apps.nowinandroid.core.testing.util.MainDispatcherRule -import kotlinx.coroutines.flow.first -import kotlinx.coroutines.test.runTest -import org.junit.Assert.assertEquals -import org.junit.Rule -import org.junit.Test - -class GetPersistentSortedFollowableAuthorsStreamUseCaseTest { - - @get:Rule - val mainDispatcherRule = MainDispatcherRule() - - private val authorsRepository = TestAuthorsRepository() - private val userDataRepository = TestUserDataRepository() - - val useCase = GetPersistentSortedFollowableAuthorsStreamUseCase( - authorsRepository = authorsRepository, - userDataRepository = userDataRepository - ) - - @Test - fun whenFollowedAuthorsSupplied_sortedFollowableAuthorsAreReturned() = runTest { - - // Obtain the stream of authors. - val followableAuthorsStream = useCase() - - // Supply some authors and their followed state in user data. - authorsRepository.sendAuthors(sampleAuthors) - userDataRepository.setFollowedAuthorIds(setOf(sampleAuthor1.id, sampleAuthor3.id)) - - // Check that the authors have been sorted, and that the followed state is correct. - assertEquals( - listOf( - FollowableAuthor(sampleAuthor2, false), - FollowableAuthor(sampleAuthor1, true), - FollowableAuthor(sampleAuthor3, true) - ), - followableAuthorsStream.first() - ) - } -} - -private val sampleAuthor1 = - Author( - id = "Author1", - name = "Mandy", - imageUrl = "", - twitter = "", - mediumPage = "", - bio = "", - ) - -private val sampleAuthor2 = - Author( - id = "Author2", - name = "Andy", - imageUrl = "", - twitter = "", - mediumPage = "", - bio = "", - ) - -private val sampleAuthor3 = - Author( - id = "Author3", - name = "Sandy", - imageUrl = "", - twitter = "", - mediumPage = "", - bio = "", - ) - -private val sampleAuthors = listOf(sampleAuthor1, sampleAuthor2, sampleAuthor3) diff --git a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetSortedFollowableAuthorsStreamUseCaseTest.kt b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetSortedFollowableAuthorsStreamUseCaseTest.kt index c1ae1e961..a08ca1b3a 100644 --- a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetSortedFollowableAuthorsStreamUseCaseTest.kt +++ b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetSortedFollowableAuthorsStreamUseCaseTest.kt @@ -19,6 +19,7 @@ package com.google.samples.apps.nowinandroid.core.domain import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor import com.google.samples.apps.nowinandroid.core.model.data.Author import com.google.samples.apps.nowinandroid.core.testing.repository.TestAuthorsRepository +import com.google.samples.apps.nowinandroid.core.testing.repository.TestUserDataRepository import com.google.samples.apps.nowinandroid.core.testing.util.MainDispatcherRule import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest @@ -32,14 +33,21 @@ class GetSortedFollowableAuthorsStreamUseCaseTest { val mainDispatcherRule = MainDispatcherRule() private val authorsRepository = TestAuthorsRepository() + private val userDataRepository = TestUserDataRepository() - val useCase = GetSortedFollowableAuthorsStreamUseCase(authorsRepository) + val useCase = GetSortedFollowableAuthorsStreamUseCase( + authorsRepository = authorsRepository, + userDataRepository = userDataRepository + ) @Test fun whenFollowedAuthorsSupplied_sortedFollowableAuthorsAreReturned() = runTest { + // Specify some authors which the user is following. + userDataRepository.setFollowedAuthorIds(setOf(sampleAuthor1.id)) + // Obtain the stream of authors, specifying their followed state. - val followableAuthorsStream = useCase(followedAuthorIds = setOf(sampleAuthor1.id)) + val followableAuthorsStream = useCase() // Supply some authors. authorsRepository.sendAuthors(sampleAuthors) diff --git a/core/model/src/main/java/com/google/samples/apps/nowinandroid/core/model/data/UserData.kt b/core/model/src/main/java/com/google/samples/apps/nowinandroid/core/model/data/UserData.kt index 13f1dd737..3ce222a54 100644 --- a/core/model/src/main/java/com/google/samples/apps/nowinandroid/core/model/data/UserData.kt +++ b/core/model/src/main/java/com/google/samples/apps/nowinandroid/core/model/data/UserData.kt @@ -25,4 +25,5 @@ data class UserData( val followedAuthors: Set, val themeBrand: ThemeBrand, val darkThemeConfig: DarkThemeConfig, + val hasDismissedOnboarding: Boolean ) diff --git a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/repository/TestUserDataRepository.kt b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/repository/TestUserDataRepository.kt index 4dfd573a1..ae910e58c 100644 --- a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/repository/TestUserDataRepository.kt +++ b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/repository/TestUserDataRepository.kt @@ -30,7 +30,8 @@ private val emptyUserData = UserData( followedTopics = emptySet(), followedAuthors = emptySet(), themeBrand = ThemeBrand.DEFAULT, - darkThemeConfig = DarkThemeConfig.FOLLOW_SYSTEM + darkThemeConfig = DarkThemeConfig.FOLLOW_SYSTEM, + hasDismissedOnboarding = false ) class TestUserDataRepository : UserDataRepository { @@ -90,6 +91,12 @@ class TestUserDataRepository : UserDataRepository { } } + override suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) { + currentUserData.let { current -> + _userData.tryEmit(current.copy(hasDismissedOnboarding = hasDismissedOnboarding)) + } + } + /** * A test-only API to allow setting/unsetting of bookmarks. * diff --git a/feature/foryou/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt b/feature/foryou/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt index af5f07372..d89f5d91f 100644 --- a/feature/foryou/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt +++ b/feature/foryou/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt @@ -54,7 +54,7 @@ class ForYouScreenTest { BoxWithConstraints { ForYouScreen( isSyncing = false, - interestsSelectionState = ForYouInterestsSelectionUiState.Loading, + onboardingUiState = OnboardingUiState.Loading, feedState = NewsFeedUiState.Loading, onTopicCheckedChanged = { _, _ -> }, onAuthorCheckedChanged = { _, _ -> }, @@ -77,7 +77,7 @@ class ForYouScreenTest { BoxWithConstraints { ForYouScreen( isSyncing = true, - interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, + onboardingUiState = OnboardingUiState.NotShown, feedState = NewsFeedUiState.Success(emptyList()), onTopicCheckedChanged = { _, _ -> }, onAuthorCheckedChanged = { _, _ -> }, @@ -100,8 +100,8 @@ class ForYouScreenTest { BoxWithConstraints { ForYouScreen( isSyncing = false, - interestsSelectionState = - ForYouInterestsSelectionUiState.WithInterestsSelection( + onboardingUiState = + OnboardingUiState.Shown( topics = testTopics, authors = testAuthors ), @@ -149,8 +149,8 @@ class ForYouScreenTest { BoxWithConstraints { ForYouScreen( isSyncing = false, - interestsSelectionState = - ForYouInterestsSelectionUiState.WithInterestsSelection( + onboardingUiState = + OnboardingUiState.Shown( // Follow one topic topics = testTopics.mapIndexed { index, testTopic -> testTopic.copy(isFollowed = index == 1) @@ -201,8 +201,8 @@ class ForYouScreenTest { BoxWithConstraints { ForYouScreen( isSyncing = false, - interestsSelectionState = - ForYouInterestsSelectionUiState.WithInterestsSelection( + onboardingUiState = + OnboardingUiState.Shown( // Follow one topic topics = testTopics, authors = testAuthors.mapIndexed { index, testAuthor -> @@ -253,8 +253,8 @@ class ForYouScreenTest { BoxWithConstraints { ForYouScreen( isSyncing = false, - interestsSelectionState = - ForYouInterestsSelectionUiState.WithInterestsSelection( + onboardingUiState = + OnboardingUiState.Shown( topics = testTopics, authors = testAuthors ), @@ -280,7 +280,7 @@ class ForYouScreenTest { BoxWithConstraints { ForYouScreen( isSyncing = false, - interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, + onboardingUiState = OnboardingUiState.NotShown, feedState = NewsFeedUiState.Loading, onTopicCheckedChanged = { _, _ -> }, onAuthorCheckedChanged = { _, _ -> }, @@ -302,7 +302,7 @@ class ForYouScreenTest { composeTestRule.setContent { ForYouScreen( isSyncing = false, - interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, + onboardingUiState = OnboardingUiState.NotShown, feedState = NewsFeedUiState.Success( feed = previewNewsResources.map { SaveableNewsResource(it, false) diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsUiState.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsUiState.kt deleted file mode 100644 index 744cebbaa..000000000 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsUiState.kt +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.feature.foryou - -/** - * A sealed hierarchy for the user's current followed interests state. - */ -sealed interface FollowedInterestsUiState { - - /** - * The current state is unknown (hasn't loaded yet) - */ - object Unknown : FollowedInterestsUiState - - /** - * The user hasn't followed any interests yet. - */ - object None : FollowedInterestsUiState - - /** - * The user has followed the given (non-empty) set of [topicIds] or [authorIds]. - */ - data class FollowedInterests( - val topicIds: Set, - val authorIds: Set - ) : FollowedInterestsUiState -} diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt index bcff09fae..4651e1204 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt @@ -94,17 +94,17 @@ internal fun ForYouRoute( modifier: Modifier = Modifier, viewModel: ForYouViewModel = hiltViewModel() ) { - val interestsSelectionState by viewModel.interestsSelectionUiState.collectAsStateWithLifecycle() + val onboardingUiState by viewModel.onboardingUiState.collectAsStateWithLifecycle() val feedState by viewModel.feedState.collectAsStateWithLifecycle() val isSyncing by viewModel.isSyncing.collectAsStateWithLifecycle() ForYouScreen( isSyncing = isSyncing, - interestsSelectionState = interestsSelectionState, + onboardingUiState = onboardingUiState, feedState = feedState, onTopicCheckedChanged = viewModel::updateTopicSelection, onAuthorCheckedChanged = viewModel::updateAuthorSelection, - saveFollowedTopics = viewModel::saveFollowedInterests, + saveFollowedTopics = viewModel::dismissOnboarding, onNewsResourcesCheckedChanged = viewModel::updateNewsResourceSaved, modifier = modifier ) @@ -113,7 +113,7 @@ internal fun ForYouRoute( @Composable internal fun ForYouScreen( isSyncing: Boolean, - interestsSelectionState: ForYouInterestsSelectionUiState, + onboardingUiState: OnboardingUiState, feedState: NewsFeedUiState, onTopicCheckedChanged: (String, Boolean) -> Unit, onAuthorCheckedChanged: (String, Boolean) -> Unit, @@ -125,11 +125,11 @@ internal fun ForYouScreen( // Workaround to call Activity.reportFullyDrawn from Jetpack Compose. // This code should be called when the UI is ready for use // and relates to Time To Full Display. - val interestsLoaded = - interestsSelectionState !is ForYouInterestsSelectionUiState.Loading + val onboardingLoaded = + onboardingUiState !is OnboardingUiState.Loading val feedLoaded = feedState !is NewsFeedUiState.Loading - if (interestsLoaded && feedLoaded) { + if (onboardingLoaded && feedLoaded) { val localView = LocalView.current // We use Unit to call reportFullyDrawn only on the first recomposition, // however it will be called again if this composable goes out of scope. @@ -156,8 +156,8 @@ internal fun ForYouScreen( .testTag("forYou:feed"), state = state ) { - interestsSelection( - interestsSelectionState = interestsSelectionState, + onboarding( + onboardingUiState = onboardingUiState, onAuthorCheckedChanged = onAuthorCheckedChanged, onTopicCheckedChanged = onTopicCheckedChanged, saveFollowedTopics = saveFollowedTopics, @@ -193,7 +193,7 @@ internal fun ForYouScreen( AnimatedVisibility( visible = isSyncing || feedState is NewsFeedUiState.Loading || - interestsSelectionState is ForYouInterestsSelectionUiState.Loading + onboardingUiState is OnboardingUiState.Loading ) { val loadingContentDescription = stringResource(id = R.string.for_you_loading) Box( @@ -208,23 +208,23 @@ internal fun ForYouScreen( } /** - * An extension on [LazyListScope] defining the interests selection portion of the for you screen. - * Depending on the [interestsSelectionState], this might emit no items. + * An extension on [LazyListScope] defining the onboarding portion of the for you screen. + * Depending on the [onboardingUiState], this might emit no items. * */ -private fun LazyGridScope.interestsSelection( - interestsSelectionState: ForYouInterestsSelectionUiState, +private fun LazyGridScope.onboarding( + onboardingUiState: OnboardingUiState, onAuthorCheckedChanged: (String, Boolean) -> Unit, onTopicCheckedChanged: (String, Boolean) -> Unit, saveFollowedTopics: () -> Unit, interestsItemModifier: Modifier = Modifier ) { - when (interestsSelectionState) { - ForYouInterestsSelectionUiState.Loading, - ForYouInterestsSelectionUiState.LoadFailed, - ForYouInterestsSelectionUiState.NoInterestsSelection -> Unit + when (onboardingUiState) { + OnboardingUiState.Loading, + OnboardingUiState.LoadFailed, + OnboardingUiState.NotShown -> Unit - is ForYouInterestsSelectionUiState.WithInterestsSelection -> { + is OnboardingUiState.Shown -> { item(span = { GridItemSpan(maxLineSpan) }) { Column(modifier = interestsItemModifier) { Text( @@ -244,14 +244,14 @@ private fun LazyGridScope.interestsSelection( style = MaterialTheme.typography.bodyMedium ) AuthorsCarousel( - authors = interestsSelectionState.authors, + authors = onboardingUiState.authors, onAuthorClick = onAuthorCheckedChanged, modifier = Modifier .fillMaxWidth() .padding(vertical = 8.dp) ) TopicSelection( - interestsSelectionState, + onboardingUiState, onTopicCheckedChanged, Modifier.padding(bottom = 8.dp) ) @@ -262,7 +262,7 @@ private fun LazyGridScope.interestsSelection( ) { NiaFilledButton( onClick = saveFollowedTopics, - enabled = interestsSelectionState.canSaveInterests, + enabled = onboardingUiState.isDismissable, modifier = Modifier .padding(horizontal = 40.dp) .width(364.dp) @@ -280,7 +280,7 @@ private fun LazyGridScope.interestsSelection( @Composable private fun TopicSelection( - interestsSelectionState: ForYouInterestsSelectionUiState.WithInterestsSelection, + onboardingUiState: OnboardingUiState.Shown, onTopicCheckedChanged: (String, Boolean) -> Unit, modifier: Modifier = Modifier ) = trace("TopicSelection") { @@ -306,7 +306,7 @@ private fun TopicSelection( .heightIn(max = max(240.dp, with(LocalDensity.current) { 240.sp.toDp() })) .fillMaxWidth() ) { - items(interestsSelectionState.topics) { + items(onboardingUiState.topics) { SingleTopicButton( name = it.topic.name, topicId = it.topic.id, @@ -396,7 +396,7 @@ fun ForYouScreenPopulatedFeed() { NiaTheme { ForYouScreen( isSyncing = false, - interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, + onboardingUiState = OnboardingUiState.NotShown, feedState = NewsFeedUiState.Success( feed = previewNewsResources.map { SaveableNewsResource(it, false) @@ -418,7 +418,7 @@ fun ForYouScreenOfflinePopulatedFeed() { NiaTheme { ForYouScreen( isSyncing = false, - interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, + onboardingUiState = OnboardingUiState.NotShown, feedState = NewsFeedUiState.Success( feed = previewNewsResources.map { SaveableNewsResource(it, false) @@ -440,7 +440,7 @@ fun ForYouScreenTopicSelection() { NiaTheme { ForYouScreen( isSyncing = false, - interestsSelectionState = ForYouInterestsSelectionUiState.WithInterestsSelection( + onboardingUiState = OnboardingUiState.Shown( topics = previewTopics.map { FollowableTopic(it, false) }, authors = previewAuthors.map { FollowableAuthor(it, false) } ), @@ -465,7 +465,7 @@ fun ForYouScreenLoading() { NiaTheme { ForYouScreen( isSyncing = false, - interestsSelectionState = ForYouInterestsSelectionUiState.Loading, + onboardingUiState = OnboardingUiState.Loading, feedState = NewsFeedUiState.Loading, onTopicCheckedChanged = { _, _ -> }, onAuthorCheckedChanged = { _, _ -> }, @@ -483,7 +483,7 @@ fun ForYouScreenPopulatedAndLoading() { NiaTheme { ForYouScreen( isSyncing = true, - interestsSelectionState = ForYouInterestsSelectionUiState.Loading, + onboardingUiState = OnboardingUiState.Loading, feedState = NewsFeedUiState.Success( feed = previewNewsResources.map { SaveableNewsResource(it, false) 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 548e786fe..3ea5b9226 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 @@ -16,14 +16,9 @@ package com.google.samples.apps.nowinandroid.feature.foryou -import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.snapshotFlow -import androidx.compose.runtime.snapshots.Snapshot.Companion.withMutableSnapshot -import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import androidx.lifecycle.viewmodel.compose.SavedStateHandleSaveableApi -import androidx.lifecycle.viewmodel.compose.saveable 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.GetFollowableTopicsStreamUseCase @@ -31,9 +26,6 @@ import com.google.samples.apps.nowinandroid.core.domain.GetSaveableNewsResources import com.google.samples.apps.nowinandroid.core.domain.GetSortedFollowableAuthorsStreamUseCase import com.google.samples.apps.nowinandroid.core.domain.model.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState -import com.google.samples.apps.nowinandroid.feature.foryou.FollowedInterestsUiState.FollowedInterests -import com.google.samples.apps.nowinandroid.feature.foryou.FollowedInterestsUiState.None -import com.google.samples.apps.nowinandroid.feature.foryou.FollowedInterestsUiState.Unknown import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.coroutines.flow.Flow @@ -41,56 +33,22 @@ 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.onStart import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch -@OptIn(SavedStateHandleSaveableApi::class) @HiltViewModel class ForYouViewModel @Inject constructor( syncStatusMonitor: SyncStatusMonitor, private val userDataRepository: UserDataRepository, private val getSaveableNewsResourcesStream: GetSaveableNewsResourcesStreamUseCase, getSortedFollowableAuthorsStream: GetSortedFollowableAuthorsStreamUseCase, - getFollowableTopicsStream: GetFollowableTopicsStreamUseCase, - savedStateHandle: SavedStateHandle + getFollowableTopicsStream: GetFollowableTopicsStreamUseCase ) : ViewModel() { - private val followedInterestsUiState: StateFlow = - userDataRepository.userDataStream - .map { userData -> - if (userData.followedAuthors.isEmpty() && userData.followedTopics.isEmpty()) { - None - } else { - FollowedInterests( - authorIds = userData.followedAuthors, - topicIds = userData.followedTopics - ) - } - } - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = Unknown - ) - - /** - * The in-progress set of topics to be selected, persisted through process death with a - * [SavedStateHandle]. - */ - private var inProgressTopicSelection by savedStateHandle.saveable { - mutableStateOf>(emptySet()) - } - - /** - * The in-progress set of authors to be selected, persisted through process death with a - * [SavedStateHandle]. - */ - private var inProgressAuthorSelection by savedStateHandle.saveable { - mutableStateOf>(emptySet()) - } + private val shouldShowOnboarding: Flow = + userDataRepository.userDataStream.map { !it.hasDismissedOnboarding } val isSyncing = syncStatusMonitor.isSyncing .stateIn( @@ -100,35 +58,23 @@ class ForYouViewModel @Inject constructor( ) val feedState: StateFlow = - combine( - followedInterestsUiState, - snapshotFlow { inProgressTopicSelection }, - snapshotFlow { inProgressAuthorSelection } - ) { followedInterestsUiState, inProgressTopicSelection, inProgressAuthorSelection -> - when (followedInterestsUiState) { - // If we don't know the current selection state, emit loading. - Unknown -> flowOf(NewsFeedUiState.Loading) - // If the user has followed topics, use those followed topics to populate the feed - is FollowedInterests -> { + userDataRepository.userDataStream + .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.hasDismissedOnboarding && + userData.followedAuthors.isEmpty() && + userData.followedTopics.isEmpty() + ) { + snapshotFlow { NewsFeedUiState.Success(emptyList()) } + } else { getSaveableNewsResourcesStream( - filterTopicIds = followedInterestsUiState.topicIds, - filterAuthorIds = followedInterestsUiState.authorIds + filterTopicIds = userData.followedTopics, + filterAuthorIds = userData.followedAuthors ).mapToFeedState() } - // If the user hasn't followed interests yet, show a realtime populated feed based - // on the in-progress interests selections, if there are any. - None -> { - if (inProgressTopicSelection.isEmpty() && inProgressAuthorSelection.isEmpty()) { - flowOf(NewsFeedUiState.Success(emptyList())) - } else { - getSaveableNewsResourcesStream( - filterTopicIds = inProgressTopicSelection, - filterAuthorIds = inProgressAuthorSelection - ).mapToFeedState() - } - } } - } // Flatten the feed flows. // As the selected topics and topic state changes, this will cancel the old feed // monitoring and start the new one. @@ -139,58 +85,36 @@ class ForYouViewModel @Inject constructor( initialValue = NewsFeedUiState.Loading ) - val interestsSelectionUiState: StateFlow = + val onboardingUiState: StateFlow = combine( - followedInterestsUiState, - getFollowableTopicsStream( - followedTopicIdsStream = snapshotFlow { inProgressTopicSelection } - ), - snapshotFlow { inProgressAuthorSelection }.flatMapLatest { - getSortedFollowableAuthorsStream(it) - } - ) { followedInterestsUiState, topics, authors -> - when (followedInterestsUiState) { - Unknown -> ForYouInterestsSelectionUiState.Loading - is FollowedInterests -> ForYouInterestsSelectionUiState.NoInterestsSelection - None -> { - if (topics.isEmpty() && authors.isEmpty()) { - ForYouInterestsSelectionUiState.LoadFailed - } else { - ForYouInterestsSelectionUiState.WithInterestsSelection( - topics = topics, - authors = authors - ) - } - } + shouldShowOnboarding, + getFollowableTopicsStream(), + getSortedFollowableAuthorsStream() + ) { shouldShowOnboarding, topics, authors -> + if (shouldShowOnboarding) { + OnboardingUiState.Shown( + topics = topics, + authors = authors + ) + } else { + OnboardingUiState.NotShown } } .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(5_000), - initialValue = ForYouInterestsSelectionUiState.Loading + initialValue = OnboardingUiState.Loading ) fun updateTopicSelection(topicId: String, isChecked: Boolean) { - withMutableSnapshot { - inProgressTopicSelection = - // Update the in-progress selection based on whether the topic id was checked - if (isChecked) { - inProgressTopicSelection + topicId - } else { - inProgressTopicSelection - topicId - } + viewModelScope.launch { + userDataRepository.toggleFollowedTopicId(topicId, isChecked) } } fun updateAuthorSelection(authorId: String, isChecked: Boolean) { - withMutableSnapshot { - inProgressAuthorSelection = - // Update the in-progress selection based on whether the author id was checked - if (isChecked) { - inProgressAuthorSelection + authorId - } else { - inProgressAuthorSelection - authorId - } + viewModelScope.launch { + userDataRepository.toggleFollowedAuthorId(authorId, isChecked) } } @@ -200,20 +124,9 @@ class ForYouViewModel @Inject constructor( } } - fun saveFollowedInterests() { - // Don't attempt to save anything if nothing is selected - if (inProgressTopicSelection.isEmpty() && inProgressAuthorSelection.isEmpty()) { - return - } - + fun dismissOnboarding() { viewModelScope.launch { - userDataRepository.setFollowedTopicIds(inProgressTopicSelection) - userDataRepository.setFollowedAuthorIds(inProgressAuthorSelection) - // Clear out the old selection, in case we return to onboarding - withMutableSnapshot { - inProgressTopicSelection = emptySet() - inProgressAuthorSelection = emptySet() - } + userDataRepository.setHasDismissedOnboarding(true) } } } diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouInterestsSelectionUiState.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/OnboardingUiState.kt similarity index 59% rename from feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouInterestsSelectionUiState.kt rename to feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/OnboardingUiState.kt index e1e0d56d0..4760a4d13 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouInterestsSelectionUiState.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/OnboardingUiState.kt @@ -20,35 +20,35 @@ import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic /** - * A sealed hierarchy describing the interests selection state for the for you screen. + * A sealed hierarchy describing the onboarding state for the for you screen. */ -sealed interface ForYouInterestsSelectionUiState { +sealed interface OnboardingUiState { /** - * The interests selection state is loading. + * The onboarding state is loading. */ - object Loading : ForYouInterestsSelectionUiState + object Loading : OnboardingUiState /** - * The interests selection state was unable to load. + * The onboarding state was unable to load. */ - object LoadFailed : ForYouInterestsSelectionUiState + object LoadFailed : OnboardingUiState /** - * There is no interests selection state. + * There is no onboarding state. */ - object NoInterestsSelection : ForYouInterestsSelectionUiState + object NotShown : OnboardingUiState /** - * There is a interests selection state, with the given lists of topics and authors. + * There is a onboarding state, with the given lists of topics and authors. */ - data class WithInterestsSelection( + data class Shown( val topics: List, val authors: List - ) : ForYouInterestsSelectionUiState { + ) : OnboardingUiState { /** - * True if the current in-progress selection can be saved. + * True if the onboarding can be dismissed. */ - val canSaveInterests: Boolean get() = + val isDismissable: Boolean get() = topics.any { it.isFollowed } || authors.any { it.isFollowed } } } 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 4a6ce9c12..c71813b68 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 @@ -16,7 +16,6 @@ package com.google.samples.apps.nowinandroid.feature.foryou -import androidx.lifecycle.SavedStateHandle import com.google.samples.apps.nowinandroid.core.domain.GetFollowableTopicsStreamUseCase import com.google.samples.apps.nowinandroid.core.domain.GetSaveableNewsResourcesStreamUseCase import com.google.samples.apps.nowinandroid.core.domain.GetSortedFollowableAuthorsStreamUseCase @@ -65,7 +64,8 @@ class ForYouViewModelTest { userDataRepository = userDataRepository ) private val getSortedFollowableAuthorsStream = GetSortedFollowableAuthorsStreamUseCase( - authorsRepository = authorsRepository + authorsRepository = authorsRepository, + userDataRepository = userDataRepository ) private val getFollowableTopicsStreamUseCase = GetFollowableTopicsStreamUseCase( topicsRepository = topicsRepository, @@ -80,16 +80,15 @@ class ForYouViewModelTest { userDataRepository = userDataRepository, getSaveableNewsResourcesStream = getSaveableNewsResourcesStreamUseCase, getSortedFollowableAuthorsStream = getSortedFollowableAuthorsStream, - getFollowableTopicsStream = getFollowableTopicsStreamUseCase, - savedStateHandle = SavedStateHandle() + getFollowableTopicsStream = getFollowableTopicsStreamUseCase ) } @Test fun stateIsInitiallyLoading() = runTest { assertEquals( - ForYouInterestsSelectionUiState.Loading, - viewModel.interestsSelectionUiState.value + OnboardingUiState.Loading, + viewModel.onboardingUiState.value ) assertEquals(NewsFeedUiState.Loading, viewModel.feedState.value) } @@ -97,14 +96,14 @@ class ForYouViewModelTest { @Test fun stateIsLoadingWhenFollowedTopicsAreLoading() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } topicsRepository.sendTopics(sampleTopics) assertEquals( - ForYouInterestsSelectionUiState.Loading, - viewModel.interestsSelectionUiState.value + OnboardingUiState.Loading, + viewModel.onboardingUiState.value ) assertEquals(NewsFeedUiState.Loading, viewModel.feedState.value) @@ -130,14 +129,14 @@ class ForYouViewModelTest { @Test fun stateIsLoadingWhenFollowedAuthorsAreLoading() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } authorsRepository.sendAuthors(sampleAuthors) assertEquals( - ForYouInterestsSelectionUiState.Loading, - viewModel.interestsSelectionUiState.value + OnboardingUiState.Loading, + viewModel.onboardingUiState.value ) assertEquals(NewsFeedUiState.Loading, viewModel.feedState.value) @@ -146,16 +145,16 @@ class ForYouViewModelTest { } @Test - fun stateIsLoadingWhenTopicsAreLoading() = runTest { + fun onboardingStateIsLoadingWhenTopicsAreLoading() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } userDataRepository.setFollowedTopicIds(emptySet()) assertEquals( - ForYouInterestsSelectionUiState.Loading, - viewModel.interestsSelectionUiState.value + OnboardingUiState.Loading, + viewModel.onboardingUiState.value ) assertEquals(NewsFeedUiState.Success(emptyList()), viewModel.feedState.value) @@ -164,16 +163,16 @@ class ForYouViewModelTest { } @Test - fun stateIsLoadingWhenAuthorsAreLoading() = runTest { + fun onboardingStateIsLoadingWhenAuthorsAreLoading() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } userDataRepository.setFollowedAuthorIds(emptySet()) assertEquals( - ForYouInterestsSelectionUiState.Loading, - viewModel.interestsSelectionUiState.value + OnboardingUiState.Loading, + viewModel.onboardingUiState.value ) assertEquals(NewsFeedUiState.Success(emptyList()), viewModel.feedState.value) @@ -182,9 +181,9 @@ class ForYouViewModelTest { } @Test - fun stateIsInterestsSelectionWhenNewsResourcesAreLoading() = runTest { + fun onboardingIsShownWhenNewsResourcesAreLoading() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } topicsRepository.sendTopics(sampleTopics) @@ -193,7 +192,7 @@ class ForYouViewModelTest { userDataRepository.setFollowedAuthorIds(emptySet()) assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( + OnboardingUiState.Shown( topics = listOf( FollowableTopic( topic = Topic( @@ -265,7 +264,7 @@ class ForYouViewModelTest { ) ), ), - viewModel.interestsSelectionUiState.value + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( @@ -279,9 +278,9 @@ class ForYouViewModelTest { } @Test - fun stateIsInterestsSelectionAfterLoadingEmptyFollowedTopicsAndAuthors() = runTest { + fun onboardingIsShownAfterLoadingEmptyFollowedTopicsAndAuthors() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } topicsRepository.sendTopics(sampleTopics) @@ -291,7 +290,7 @@ class ForYouViewModelTest { newsRepository.sendNewsResources(sampleNewsResources) assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( + OnboardingUiState.Shown( topics = listOf( FollowableTopic( topic = Topic( @@ -363,7 +362,7 @@ class ForYouViewModelTest { ) ), ), - viewModel.interestsSelectionUiState.value + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( @@ -378,27 +377,28 @@ class ForYouViewModelTest { } @Test - fun stateIsWithoutInterestsSelectionAfterLoadingFollowedTopics() = runTest { + fun onboardingIsNotShownAfterUserDismissesOnboarding() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } authorsRepository.sendAuthors(sampleAuthors) userDataRepository.setFollowedAuthorIds(emptySet()) topicsRepository.sendTopics(sampleTopics) userDataRepository.setFollowedTopicIds(setOf("0", "1")) + viewModel.dismissOnboarding() assertEquals( - ForYouInterestsSelectionUiState.NoInterestsSelection, - viewModel.interestsSelectionUiState.value + OnboardingUiState.NotShown, + viewModel.onboardingUiState.value ) assertEquals(NewsFeedUiState.Loading, viewModel.feedState.value) newsRepository.sendNewsResources(sampleNewsResources) assertEquals( - ForYouInterestsSelectionUiState.NoInterestsSelection, - viewModel.interestsSelectionUiState.value + OnboardingUiState.NotShown, + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( @@ -417,52 +417,10 @@ class ForYouViewModelTest { collectJob2.cancel() } - @Test - fun stateIsWithoutInterestsSelectionAfterLoadingFollowedAuthors() = runTest { - val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } - val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } - - authorsRepository.sendAuthors(sampleAuthors) - userDataRepository.setFollowedAuthorIds(setOf("0", "1")) - topicsRepository.sendTopics(sampleTopics) - userDataRepository.setFollowedTopicIds(emptySet()) - - assertEquals( - ForYouInterestsSelectionUiState.NoInterestsSelection, - viewModel.interestsSelectionUiState.value - ) - assertEquals( - NewsFeedUiState.Loading, - viewModel.feedState.value - ) - - newsRepository.sendNewsResources(sampleNewsResources) - - assertEquals( - ForYouInterestsSelectionUiState.NoInterestsSelection, - viewModel.interestsSelectionUiState.value - ) - assertEquals( - NewsFeedUiState.Success( - feed = sampleNewsResources.map { - SaveableNewsResource( - newsResource = it, - isSaved = false - ) - } - ), - viewModel.feedState.value - ) - - collectJob1.cancel() - collectJob2.cancel() - } - @Test fun topicSelectionUpdatesAfterSelectingTopic() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } topicsRepository.sendTopics(sampleTopics) @@ -472,7 +430,7 @@ class ForYouViewModelTest { newsRepository.sendNewsResources(sampleNewsResources) assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( + OnboardingUiState.Shown( topics = listOf( FollowableTopic( topic = Topic( @@ -544,7 +502,7 @@ class ForYouViewModelTest { ) ), ), - viewModel.interestsSelectionUiState.value + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( @@ -556,7 +514,7 @@ class ForYouViewModelTest { viewModel.updateTopicSelection("1", isChecked = true) assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( + OnboardingUiState.Shown( topics = listOf( FollowableTopic( topic = Topic( @@ -628,7 +586,7 @@ class ForYouViewModelTest { ) ), ), - viewModel.interestsSelectionUiState.value + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( @@ -651,9 +609,9 @@ class ForYouViewModelTest { } @Test - fun topicSelectionUpdatesAfterSelectingAuthor() = runTest { + fun authorSelectionUpdatesAfterSelectingAuthor() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } topicsRepository.sendTopics(sampleTopics) @@ -663,7 +621,7 @@ class ForYouViewModelTest { newsRepository.sendNewsResources(sampleNewsResources) assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( + OnboardingUiState.Shown( topics = listOf( FollowableTopic( topic = Topic( @@ -735,7 +693,7 @@ class ForYouViewModelTest { ) ), ), - viewModel.interestsSelectionUiState.value + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( @@ -747,7 +705,7 @@ class ForYouViewModelTest { viewModel.updateAuthorSelection("1", isChecked = true) assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( + OnboardingUiState.Shown( topics = listOf( FollowableTopic( topic = Topic( @@ -819,7 +777,7 @@ class ForYouViewModelTest { ) ), ), - viewModel.interestsSelectionUiState.value + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( @@ -844,7 +802,7 @@ class ForYouViewModelTest { @Test fun topicSelectionUpdatesAfterUnselectingTopic() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } topicsRepository.sendTopics(sampleTopics) @@ -857,7 +815,7 @@ class ForYouViewModelTest { advanceUntilIdle() assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( + OnboardingUiState.Shown( topics = listOf( FollowableTopic( topic = Topic( @@ -929,7 +887,7 @@ class ForYouViewModelTest { ) ), ), - viewModel.interestsSelectionUiState.value + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( @@ -943,9 +901,9 @@ class ForYouViewModelTest { } @Test - fun topicSelectionUpdatesAfterUnselectingAuthor() = runTest { + fun authorSelectionUpdatesAfterUnselectingAuthor() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } topicsRepository.sendTopics(sampleTopics) @@ -958,7 +916,7 @@ class ForYouViewModelTest { assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( + OnboardingUiState.Shown( topics = listOf( FollowableTopic( topic = Topic( @@ -1030,331 +988,7 @@ class ForYouViewModelTest { ) ), ), - viewModel.interestsSelectionUiState.value - ) - assertEquals( - NewsFeedUiState.Success( - feed = emptyList() - ), - viewModel.feedState.value - ) - - collectJob1.cancel() - collectJob2.cancel() - } - - @Test - fun topicSelectionUpdatesAfterSavingTopicsOnly() = runTest { - val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } - val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } - - topicsRepository.sendTopics(sampleTopics) - userDataRepository.setFollowedTopicIds(emptySet()) - authorsRepository.sendAuthors(sampleAuthors) - userDataRepository.setFollowedAuthorIds(emptySet()) - newsRepository.sendNewsResources(sampleNewsResources) - viewModel.updateTopicSelection("1", isChecked = true) - - viewModel.saveFollowedInterests() - - assertEquals( - ForYouInterestsSelectionUiState.NoInterestsSelection, - viewModel.interestsSelectionUiState.value - ) - assertEquals( - NewsFeedUiState.Success( - feed = listOf( - SaveableNewsResource( - newsResource = sampleNewsResources[1], - isSaved = false, - ), - SaveableNewsResource( - newsResource = sampleNewsResources[2], - isSaved = false, - ) - ) - ), - viewModel.feedState.value - ) - assertEquals(setOf("1"), userDataRepository.getCurrentFollowedTopics()) - assertEquals(emptySet(), userDataRepository.getCurrentFollowedAuthors()) - - collectJob1.cancel() - collectJob2.cancel() - } - - @Test - fun topicSelectionUpdatesAfterSavingAuthorsOnly() = runTest { - val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } - val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } - - topicsRepository.sendTopics(sampleTopics) - userDataRepository.setFollowedTopicIds(emptySet()) - authorsRepository.sendAuthors(sampleAuthors) - userDataRepository.setFollowedAuthorIds(emptySet()) - newsRepository.sendNewsResources(sampleNewsResources) - viewModel.updateAuthorSelection("0", isChecked = true) - - viewModel.saveFollowedInterests() - - assertEquals( - ForYouInterestsSelectionUiState.NoInterestsSelection, - viewModel.interestsSelectionUiState.value - ) - assertEquals( - NewsFeedUiState.Success( - feed = listOf( - SaveableNewsResource( - newsResource = sampleNewsResources[0], - isSaved = false - ), - ) - ), - viewModel.feedState.value - ) - assertEquals(emptySet(), userDataRepository.getCurrentFollowedTopics()) - assertEquals(setOf("0"), userDataRepository.getCurrentFollowedAuthors()) - - collectJob1.cancel() - collectJob2.cancel() - } - - @Test - fun topicSelectionUpdatesAfterSavingAuthorsAndTopics() = runTest { - val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } - val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } - - topicsRepository.sendTopics(sampleTopics) - userDataRepository.setFollowedTopicIds(emptySet()) - authorsRepository.sendAuthors(sampleAuthors) - userDataRepository.setFollowedAuthorIds(emptySet()) - newsRepository.sendNewsResources(sampleNewsResources) - viewModel.updateAuthorSelection("1", isChecked = true) - viewModel.updateTopicSelection("1", isChecked = true) - - viewModel.saveFollowedInterests() - - assertEquals( - ForYouInterestsSelectionUiState.NoInterestsSelection, - viewModel.interestsSelectionUiState.value - ) - assertEquals( - NewsFeedUiState.Success( - feed = listOf( - SaveableNewsResource( - newsResource = sampleNewsResources[1], - isSaved = false - ), - SaveableNewsResource( - newsResource = sampleNewsResources[2], - isSaved = false - ) - ) - ), - viewModel.feedState.value - ) - assertEquals(setOf("1"), userDataRepository.getCurrentFollowedTopics()) - assertEquals(setOf("1"), userDataRepository.getCurrentFollowedAuthors()) - - collectJob1.cancel() - collectJob2.cancel() - } - - @Test - fun topicSelectionIsResetAfterSavingTopicsAndRemovingThem() = runTest { - val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } - val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } - - topicsRepository.sendTopics(sampleTopics) - userDataRepository.setFollowedTopicIds(emptySet()) - authorsRepository.sendAuthors(sampleAuthors) - userDataRepository.setFollowedAuthorIds(emptySet()) - newsRepository.sendNewsResources(sampleNewsResources) - viewModel.updateTopicSelection("1", isChecked = true) - viewModel.saveFollowedInterests() - - userDataRepository.setFollowedTopicIds(emptySet()) - - assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( - topics = listOf( - FollowableTopic( - topic = Topic( - id = "0", - name = "Headlines", - shortDescription = "", - longDescription = "long description", - url = "URL", - imageUrl = "image URL", - ), - isFollowed = false - ), - FollowableTopic( - topic = Topic( - id = "1", - name = "UI", - shortDescription = "", - longDescription = "long description", - url = "URL", - imageUrl = "image URL", - ), - isFollowed = false - ), - FollowableTopic( - topic = Topic( - id = "2", - name = "Tools", - shortDescription = "", - longDescription = "long description", - url = "URL", - imageUrl = "image URL", - ), - isFollowed = false - ) - ), - authors = listOf( - FollowableAuthor( - author = Author( - id = "0", - name = "Android Dev", - imageUrl = "", - twitter = "", - mediumPage = "", - bio = "", - ), - isFollowed = false - ), - FollowableAuthor( - author = Author( - id = "1", - name = "Android Dev 2", - imageUrl = "", - twitter = "", - mediumPage = "", - bio = "", - ), - isFollowed = false - ), - FollowableAuthor( - author = Author( - id = "2", - name = "Android Dev 3", - imageUrl = "", - twitter = "", - mediumPage = "", - bio = "", - ), - isFollowed = false - ) - ) - ), - viewModel.interestsSelectionUiState.value - ) - assertEquals( - NewsFeedUiState.Success( - feed = emptyList() - ), - viewModel.feedState.value - ) - - collectJob1.cancel() - collectJob2.cancel() - } - - @Test - fun authorSelectionIsResetAfterSavingAuthorsAndRemovingThem() = runTest { - val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } - val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } - - topicsRepository.sendTopics(sampleTopics) - userDataRepository.setFollowedTopicIds(emptySet()) - authorsRepository.sendAuthors(sampleAuthors) - userDataRepository.setFollowedAuthorIds(emptySet()) - newsRepository.sendNewsResources(sampleNewsResources) - viewModel.updateAuthorSelection("1", isChecked = true) - viewModel.saveFollowedInterests() - - userDataRepository.setFollowedAuthorIds(emptySet()) - - assertEquals( - ForYouInterestsSelectionUiState.WithInterestsSelection( - topics = listOf( - FollowableTopic( - topic = Topic( - id = "0", - name = "Headlines", - shortDescription = "", - longDescription = "long description", - url = "URL", - imageUrl = "image URL", - ), - isFollowed = false - ), - FollowableTopic( - topic = Topic( - id = "1", - name = "UI", - shortDescription = "", - longDescription = "long description", - url = "URL", - imageUrl = "image URL", - ), - isFollowed = false - ), - FollowableTopic( - topic = Topic( - id = "2", - name = "Tools", - shortDescription = "", - longDescription = "long description", - url = "URL", - imageUrl = "image URL", - ), - isFollowed = false - ) - ), - authors = listOf( - FollowableAuthor( - author = Author( - id = "0", - name = "Android Dev", - imageUrl = "", - twitter = "", - mediumPage = "", - bio = "", - ), - isFollowed = false - ), - FollowableAuthor( - author = Author( - id = "1", - name = "Android Dev 2", - imageUrl = "", - twitter = "", - mediumPage = "", - bio = "", - ), - isFollowed = false - ), - FollowableAuthor( - author = Author( - id = "2", - name = "Android Dev 3", - imageUrl = "", - twitter = "", - mediumPage = "", - bio = "", - ), - isFollowed = false - ) - ) - ), - viewModel.interestsSelectionUiState.value + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( @@ -1370,19 +1004,20 @@ class ForYouViewModelTest { @Test fun newsResourceSelectionUpdatesAfterLoadingFollowedTopics() = runTest { val collectJob1 = - launch(UnconfinedTestDispatcher()) { viewModel.interestsSelectionUiState.collect() } + launch(UnconfinedTestDispatcher()) { viewModel.onboardingUiState.collect() } val collectJob2 = launch(UnconfinedTestDispatcher()) { viewModel.feedState.collect() } topicsRepository.sendTopics(sampleTopics) userDataRepository.setFollowedTopicIds(setOf("1")) authorsRepository.sendAuthors(sampleAuthors) userDataRepository.setFollowedAuthorIds(setOf("1")) + userDataRepository.setHasDismissedOnboarding(true) newsRepository.sendNewsResources(sampleNewsResources) viewModel.updateNewsResourceSaved("2", true) assertEquals( - ForYouInterestsSelectionUiState.NoInterestsSelection, - viewModel.interestsSelectionUiState.value + OnboardingUiState.NotShown, + viewModel.onboardingUiState.value ) assertEquals( NewsFeedUiState.Success( diff --git a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsViewModel.kt b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsViewModel.kt index dcf46caf2..01d8b0d6e 100644 --- a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsViewModel.kt +++ b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsViewModel.kt @@ -20,7 +20,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository import com.google.samples.apps.nowinandroid.core.domain.GetFollowableTopicsStreamUseCase -import com.google.samples.apps.nowinandroid.core.domain.GetPersistentSortedFollowableAuthorsStreamUseCase +import com.google.samples.apps.nowinandroid.core.domain.GetSortedFollowableAuthorsStreamUseCase import com.google.samples.apps.nowinandroid.core.domain.TopicSortField import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic @@ -39,7 +39,7 @@ import kotlinx.coroutines.launch class InterestsViewModel @Inject constructor( val userDataRepository: UserDataRepository, getFollowableTopicsStream: GetFollowableTopicsStreamUseCase, - getPersistentSortedFollowableAuthorsStream: GetPersistentSortedFollowableAuthorsStreamUseCase + getSortedFollowableAuthorsStream: GetSortedFollowableAuthorsStreamUseCase ) : ViewModel() { private val _tabState = MutableStateFlow( @@ -51,7 +51,7 @@ class InterestsViewModel @Inject constructor( val tabState: StateFlow = _tabState.asStateFlow() val uiState: StateFlow = combine( - getPersistentSortedFollowableAuthorsStream(), + getSortedFollowableAuthorsStream(), getFollowableTopicsStream(sortBy = TopicSortField.NAME), InterestsUiState::Interests ).stateIn( diff --git a/feature/interests/src/test/java/com/google/samples/apps/nowinandroid/interests/InterestsViewModelTest.kt b/feature/interests/src/test/java/com/google/samples/apps/nowinandroid/interests/InterestsViewModelTest.kt index 862faec6b..6ecf3930a 100644 --- a/feature/interests/src/test/java/com/google/samples/apps/nowinandroid/interests/InterestsViewModelTest.kt +++ b/feature/interests/src/test/java/com/google/samples/apps/nowinandroid/interests/InterestsViewModelTest.kt @@ -17,7 +17,7 @@ package com.google.samples.apps.nowinandroid.interests import com.google.samples.apps.nowinandroid.core.domain.GetFollowableTopicsStreamUseCase -import com.google.samples.apps.nowinandroid.core.domain.GetPersistentSortedFollowableAuthorsStreamUseCase +import com.google.samples.apps.nowinandroid.core.domain.GetSortedFollowableAuthorsStreamUseCase import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.Author @@ -53,8 +53,8 @@ class InterestsViewModelTest { topicsRepository = topicsRepository, userDataRepository = userDataRepository ) - private val getPersistentSortedFollowableAuthorsStream = - GetPersistentSortedFollowableAuthorsStreamUseCase( + private val getSortedFollowableAuthorsStream = + GetSortedFollowableAuthorsStreamUseCase( authorsRepository = authorsRepository, userDataRepository = userDataRepository ) @@ -65,7 +65,7 @@ class InterestsViewModelTest { viewModel = InterestsViewModel( userDataRepository = userDataRepository, getFollowableTopicsStream = getFollowableTopicsStreamUseCase, - getPersistentSortedFollowableAuthorsStream = getPersistentSortedFollowableAuthorsStream + getSortedFollowableAuthorsStream = getSortedFollowableAuthorsStream ) } From 5b8c284c740b135c2b8e3925bbb21610f9a2cd16 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Fri, 11 Nov 2022 11:09:59 +0000 Subject: [PATCH 5/6] Rename hasDismissedOnboarding to shouldHideOnboarding Change-Id: I8a6b35f7216f9944652d8da346e7458614311e08 --- .../OfflineFirstUserDataRepository.kt | 4 +- .../data/repository/UserDataRepository.kt | 2 +- .../repository/fake/FakeUserDataRepository.kt | 4 +- .../OfflineFirstUserDataRepositoryTest.kt | 10 ++-- .../datastore/NiaPreferencesDataSource.kt | 18 ++++---- .../nowinandroid/data/user_preferences.proto | 5 +- .../datastore/NiaPreferencesDataSourceTest.kt | 46 +++++++++---------- .../nowinandroid/core/model/data/UserData.kt | 2 +- .../repository/TestUserDataRepository.kt | 6 +-- .../feature/foryou/ForYouViewModel.kt | 6 +-- .../feature/foryou/ForYouViewModelTest.kt | 2 +- 11 files changed, 51 insertions(+), 54 deletions(-) diff --git a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepository.kt b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepository.kt index 0bf4d6438..e95c13460 100644 --- a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepository.kt +++ b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepository.kt @@ -51,6 +51,6 @@ class OfflineFirstUserDataRepository @Inject constructor( override suspend fun setDarkThemeConfig(darkThemeConfig: DarkThemeConfig) = niaPreferencesDataSource.setDarkThemeConfig(darkThemeConfig) - override suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) = - niaPreferencesDataSource.setHasDismissedOnboarding(hasDismissedOnboarding) + override suspend fun setShouldHideOnboarding(shouldHideOnboarding: Boolean) = + niaPreferencesDataSource.setShouldHideOnboarding(shouldHideOnboarding) } diff --git a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/UserDataRepository.kt b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/UserDataRepository.kt index 86d26a47e..8cd22c18f 100644 --- a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/UserDataRepository.kt +++ b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/UserDataRepository.kt @@ -66,5 +66,5 @@ interface UserDataRepository { /** * Sets whether the user has completed the onboarding process. */ - suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) + suspend fun setShouldHideOnboarding(shouldHideOnboarding: Boolean) } diff --git a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/fake/FakeUserDataRepository.kt b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/fake/FakeUserDataRepository.kt index e87b59a91..4752cd7a1 100644 --- a/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/fake/FakeUserDataRepository.kt +++ b/core/data/src/main/java/com/google/samples/apps/nowinandroid/core/data/repository/fake/FakeUserDataRepository.kt @@ -64,7 +64,7 @@ class FakeUserDataRepository @Inject constructor( niaPreferencesDataSource.setDarkThemeConfig(darkThemeConfig) } - override suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) { - niaPreferencesDataSource.setHasDismissedOnboarding(hasDismissedOnboarding) + override suspend fun setShouldHideOnboarding(shouldHideOnboarding: Boolean) { + niaPreferencesDataSource.setShouldHideOnboarding(shouldHideOnboarding) } } diff --git a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepositoryTest.kt b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepositoryTest.kt index 6f0f9394d..c1d740602 100644 --- a/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepositoryTest.kt +++ b/core/data/src/test/java/com/google/samples/apps/nowinandroid/core/data/repository/OfflineFirstUserDataRepositoryTest.kt @@ -59,7 +59,7 @@ class OfflineFirstUserDataRepositoryTest { followedAuthors = emptySet(), themeBrand = ThemeBrand.DEFAULT, darkThemeConfig = DarkThemeConfig.FOLLOW_SYSTEM, - hasDismissedOnboarding = false + shouldHideOnboarding = false ), subject.userDataStream.first() ) @@ -190,13 +190,13 @@ class OfflineFirstUserDataRepositoryTest { } @Test - fun whenUserCompletesOnboarding_thenRemovesAllInterests_hasDismissedOnboardingIsFalse() = + fun whenUserCompletesOnboarding_thenRemovesAllInterests_shouldHideOnboardingIsFalse() = runTest { subject.setFollowedTopicIds(setOf("1")) - subject.setHasDismissedOnboarding(true) - assertEquals(true, subject.userDataStream.first().hasDismissedOnboarding) + subject.setShouldHideOnboarding(true) + assertEquals(true, subject.userDataStream.first().shouldHideOnboarding) subject.setFollowedTopicIds(emptySet()) - assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + assertEquals(false, subject.userDataStream.first().shouldHideOnboarding) } } diff --git a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt index be129a3bd..b4442528c 100644 --- a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt +++ b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt @@ -53,7 +53,7 @@ class NiaPreferencesDataSource @Inject constructor( DarkThemeConfig.LIGHT DarkThemeConfigProto.DARK_THEME_CONFIG_DARK -> DarkThemeConfig.DARK }, - hasDismissedOnboarding = it.hasDismissedOnboarding + shouldHideOnboarding = it.shouldHideOnboarding ) } @@ -63,7 +63,7 @@ class NiaPreferencesDataSource @Inject constructor( it.copy { followedTopicIds.clear() followedTopicIds.putAll(topicIds.associateWith { true }) - updateHasDismissedOnboardingIfNecessary() + updateShouldHideOnboardingIfNecessary() } } } catch (ioException: IOException) { @@ -80,7 +80,7 @@ class NiaPreferencesDataSource @Inject constructor( } else { followedTopicIds.remove(topicId) } - updateHasDismissedOnboardingIfNecessary() + updateShouldHideOnboardingIfNecessary() } } } catch (ioException: IOException) { @@ -94,7 +94,7 @@ class NiaPreferencesDataSource @Inject constructor( it.copy { followedAuthorIds.clear() followedAuthorIds.putAll(authorIds.associateWith { true }) - updateHasDismissedOnboardingIfNecessary() + updateShouldHideOnboardingIfNecessary() } } } catch (ioException: IOException) { @@ -111,7 +111,7 @@ class NiaPreferencesDataSource @Inject constructor( } else { followedAuthorIds.remove(authorId) } - updateHasDismissedOnboardingIfNecessary() + updateShouldHideOnboardingIfNecessary() } } } catch (ioException: IOException) { @@ -194,18 +194,18 @@ class NiaPreferencesDataSource @Inject constructor( } } - suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) { + suspend fun setShouldHideOnboarding(shouldHideOnboarding: Boolean) { userPreferences.updateData { it.copy { - this.hasDismissedOnboarding = hasDismissedOnboarding + this.shouldHideOnboarding = shouldHideOnboarding } } } } -fun UserPreferencesKt.Dsl.updateHasDismissedOnboardingIfNecessary() { +fun UserPreferencesKt.Dsl.updateShouldHideOnboardingIfNecessary() { if (followedTopicIds.isEmpty() && followedAuthorIds.isEmpty()) { - hasDismissedOnboarding = false + shouldHideOnboarding = false } } diff --git a/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto b/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto index f771ac1ac..b7d33dcaf 100644 --- a/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto +++ b/core/datastore/src/main/proto/com/google/samples/apps/nowinandroid/data/user_preferences.proto @@ -44,8 +44,5 @@ message UserPreferences { ThemeBrandProto theme_brand = 16; DarkThemeConfigProto dark_theme_config = 17; - // Note that proto3 only allows a default value of `false` for boolean types. This means that - // whilst the name `should_show_onboarding` would be preferable here we are forced to choose a - // name which works with this default value constraint. - bool has_dismissed_onboarding = 18; + bool should_hide_onboarding = 18; } diff --git a/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSourceTest.kt b/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSourceTest.kt index 8bb552521..d5e4be841 100644 --- a/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSourceTest.kt +++ b/core/datastore/src/test/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSourceTest.kt @@ -39,99 +39,99 @@ class NiaPreferencesDataSourceTest { } @Test - fun hasDismissedOnboardingIsFalseByDefault() = runTest { - assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + fun shouldHideOnboardingIsFalseByDefault() = runTest { + assertEquals(false, subject.userDataStream.first().shouldHideOnboarding) } @Test - fun userHasDismissedOnboardingIsTrueWhenSet() = runTest { - subject.setHasDismissedOnboarding(true) - assertEquals(true, subject.userDataStream.first().hasDismissedOnboarding) + fun userShouldHideOnboardingIsTrueWhenSet() = runTest { + subject.setShouldHideOnboarding(true) + assertEquals(true, subject.userDataStream.first().shouldHideOnboarding) } @Test - fun userHasDismissedOnboarding_unfollowsLastAuthor_hasDismissedOnboardingIsFalse() = runTest { + fun userShouldHideOnboarding_unfollowsLastAuthor_shouldHideOnboardingIsFalse() = runTest { // Given: user completes onboarding by selecting a single author. subject.toggleFollowedAuthorId("1", true) - subject.setHasDismissedOnboarding(true) + subject.setShouldHideOnboarding(true) // When: they unfollow that author. subject.toggleFollowedAuthorId("1", false) // Then: onboarding should be shown again - assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + assertEquals(false, subject.userDataStream.first().shouldHideOnboarding) } @Test - fun userHasDismissedOnboarding_unfollowsLastTopic_hasDismissedOnboardingIsFalse() = runTest { + fun userShouldHideOnboarding_unfollowsLastTopic_shouldHideOnboardingIsFalse() = runTest { // Given: user completes onboarding by selecting a single topic. subject.toggleFollowedTopicId("1", true) - subject.setHasDismissedOnboarding(true) + subject.setShouldHideOnboarding(true) // When: they unfollow that topic. subject.toggleFollowedTopicId("1", false) // Then: onboarding should be shown again - assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + assertEquals(false, subject.userDataStream.first().shouldHideOnboarding) } @Test - fun userHasDismissedOnboarding_unfollowsAllAuthors_hasDismissedOnboardingIsFalse() = runTest { + fun userShouldHideOnboarding_unfollowsAllAuthors_shouldHideOnboardingIsFalse() = runTest { // Given: user completes onboarding by selecting several authors. subject.setFollowedAuthorIds(setOf("1", "2")) - subject.setHasDismissedOnboarding(true) + subject.setShouldHideOnboarding(true) // When: they unfollow those authors. subject.setFollowedAuthorIds(emptySet()) // Then: onboarding should be shown again - assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + assertEquals(false, subject.userDataStream.first().shouldHideOnboarding) } @Test - fun userHasDismissedOnboarding_unfollowsAllTopics_hasDismissedOnboardingIsFalse() = runTest { + fun userShouldHideOnboarding_unfollowsAllTopics_shouldHideOnboardingIsFalse() = runTest { // Given: user completes onboarding by selecting several topics. subject.setFollowedTopicIds(setOf("1", "2")) - subject.setHasDismissedOnboarding(true) + subject.setShouldHideOnboarding(true) // When: they unfollow those topics. subject.setFollowedTopicIds(emptySet()) // Then: onboarding should be shown again - assertEquals(false, subject.userDataStream.first().hasDismissedOnboarding) + assertEquals(false, subject.userDataStream.first().shouldHideOnboarding) } @Test - fun userHasDismissedOnboarding_unfollowsAllTopicsButNotAuthors_hasDismissedOnboardingIsTrue() = + fun userShouldHideOnboarding_unfollowsAllTopicsButNotAuthors_shouldHideOnboardingIsTrue() = runTest { // Given: user completes onboarding by selecting several topics and authors. subject.setFollowedTopicIds(setOf("1", "2")) subject.setFollowedAuthorIds(setOf("3", "4")) - subject.setHasDismissedOnboarding(true) + subject.setShouldHideOnboarding(true) // When: they unfollow just the topics. subject.setFollowedTopicIds(emptySet()) // Then: onboarding should still be dismissed - assertEquals(true, subject.userDataStream.first().hasDismissedOnboarding) + assertEquals(true, subject.userDataStream.first().shouldHideOnboarding) } @Test - fun userHasDismissedOnboarding_unfollowsAllAuthorsButNotTopics_hasDismissedOnboardingIsTrue() = + fun userShouldHideOnboarding_unfollowsAllAuthorsButNotTopics_shouldHideOnboardingIsTrue() = runTest { // Given: user completes onboarding by selecting several topics and authors. subject.setFollowedTopicIds(setOf("1", "2")) subject.setFollowedAuthorIds(setOf("3", "4")) - subject.setHasDismissedOnboarding(true) + subject.setShouldHideOnboarding(true) // When: they unfollow just the authors. subject.setFollowedAuthorIds(emptySet()) // Then: onboarding should still be dismissed - assertEquals(true, subject.userDataStream.first().hasDismissedOnboarding) + assertEquals(true, subject.userDataStream.first().shouldHideOnboarding) } } diff --git a/core/model/src/main/java/com/google/samples/apps/nowinandroid/core/model/data/UserData.kt b/core/model/src/main/java/com/google/samples/apps/nowinandroid/core/model/data/UserData.kt index 3ce222a54..c3941cb0f 100644 --- a/core/model/src/main/java/com/google/samples/apps/nowinandroid/core/model/data/UserData.kt +++ b/core/model/src/main/java/com/google/samples/apps/nowinandroid/core/model/data/UserData.kt @@ -25,5 +25,5 @@ data class UserData( val followedAuthors: Set, val themeBrand: ThemeBrand, val darkThemeConfig: DarkThemeConfig, - val hasDismissedOnboarding: Boolean + val shouldHideOnboarding: Boolean ) diff --git a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/repository/TestUserDataRepository.kt b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/repository/TestUserDataRepository.kt index ae910e58c..507fa1f7b 100644 --- a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/repository/TestUserDataRepository.kt +++ b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/repository/TestUserDataRepository.kt @@ -31,7 +31,7 @@ private val emptyUserData = UserData( followedAuthors = emptySet(), themeBrand = ThemeBrand.DEFAULT, darkThemeConfig = DarkThemeConfig.FOLLOW_SYSTEM, - hasDismissedOnboarding = false + shouldHideOnboarding = false ) class TestUserDataRepository : UserDataRepository { @@ -91,9 +91,9 @@ class TestUserDataRepository : UserDataRepository { } } - override suspend fun setHasDismissedOnboarding(hasDismissedOnboarding: Boolean) { + override suspend fun setShouldHideOnboarding(shouldHideOnboarding: Boolean) { currentUserData.let { current -> - _userData.tryEmit(current.copy(hasDismissedOnboarding = hasDismissedOnboarding)) + _userData.tryEmit(current.copy(shouldHideOnboarding = shouldHideOnboarding)) } } 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 3ea5b9226..c79c9d777 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 @@ -48,7 +48,7 @@ class ForYouViewModel @Inject constructor( ) : ViewModel() { private val shouldShowOnboarding: Flow = - userDataRepository.userDataStream.map { !it.hasDismissedOnboarding } + userDataRepository.userDataStream.map { !it.shouldHideOnboarding } val isSyncing = syncStatusMonitor.isSyncing .stateIn( @@ -63,7 +63,7 @@ class ForYouViewModel @Inject constructor( // 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.hasDismissedOnboarding && + if (!userData.shouldHideOnboarding && userData.followedAuthors.isEmpty() && userData.followedTopics.isEmpty() ) { @@ -126,7 +126,7 @@ class ForYouViewModel @Inject constructor( fun dismissOnboarding() { viewModelScope.launch { - userDataRepository.setHasDismissedOnboarding(true) + userDataRepository.setShouldHideOnboarding(true) } } } 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 c71813b68..5f21f9d06 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 @@ -1011,7 +1011,7 @@ class ForYouViewModelTest { userDataRepository.setFollowedTopicIds(setOf("1")) authorsRepository.sendAuthors(sampleAuthors) userDataRepository.setFollowedAuthorIds(setOf("1")) - userDataRepository.setHasDismissedOnboarding(true) + userDataRepository.setShouldHideOnboarding(true) newsRepository.sendNewsResources(sampleNewsResources) viewModel.updateNewsResourceSaved("2", true) From 86ef9ff594ef0f40f92f4aea2e813be286924de8 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Fri, 11 Nov 2022 15:57:18 +0000 Subject: [PATCH 6/6] Make updateShouldHideOnboardingIfNecessary private Change-Id: I7917b025d258a07604e98950396ec3e119e2d9a7 --- .../nowinandroid/core/datastore/NiaPreferencesDataSource.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt index b4442528c..0f0931e6b 100644 --- a/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt +++ b/core/datastore/src/main/java/com/google/samples/apps/nowinandroid/core/datastore/NiaPreferencesDataSource.kt @@ -203,7 +203,7 @@ class NiaPreferencesDataSource @Inject constructor( } } -fun UserPreferencesKt.Dsl.updateShouldHideOnboardingIfNecessary() { +private fun UserPreferencesKt.Dsl.updateShouldHideOnboardingIfNecessary() { if (followedTopicIds.isEmpty() && followedAuthorIds.isEmpty()) { shouldHideOnboarding = false