Skip to content

Commit

Permalink
Incorporate code review changes: Move UserNewsResourceRepository to data
Browse files Browse the repository at this point in the history
module; move UserNewsResource to model module. Implement unread dot for
bookmarked articles. Keep the flows cold in UserNewsResourceRepository.
  • Loading branch information
rosejr committed Mar 13, 2023
1 parent ebfbb5b commit 8194f7a
Show file tree
Hide file tree
Showing 45 changed files with 276 additions and 324 deletions.
2 changes: 0 additions & 2 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,9 @@ dependencies {
implementation(project(":feature:settings"))

implementation(project(":core:common"))
implementation(project(":core:domain"))
implementation(project(":core:ui"))
implementation(project(":core:designsystem"))
implementation(project(":core:data"))
implementation(project(":core:domain"))
implementation(project(":core:model"))
implementation(project(":core:analytics"))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.unit.DpSize
import androidx.compose.ui.unit.dp
import com.google.accompanist.testharness.TestHarness
import com.google.samples.apps.nowinandroid.core.data.repository.CompositeUserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.util.NetworkMonitor
import com.google.samples.apps.nowinandroid.core.domain.repository.CompositeUserNewsResourceRepository
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.uitesthiltmanifest.HiltComponentActivity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,11 @@ class NiaAppStateTest {
val navController = rememberTestNavController()
state = remember(navController) {
NiaAppState(
windowSizeClass = getCompactWindowClass(),
navController = navController,
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = getCompactWindowClass(),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -92,6 +93,7 @@ class NiaAppStateTest {
state = rememberNiaAppState(
windowSizeClass = getCompactWindowClass(),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -105,10 +107,11 @@ class NiaAppStateTest {
fun niaAppState_showBottomBar_compact() = runTest {
composeTestRule.setContent {
state = NiaAppState(
windowSizeClass = getCompactWindowClass(),
navController = NavHostController(LocalContext.current),
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = getCompactWindowClass(),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -120,10 +123,11 @@ class NiaAppStateTest {
fun niaAppState_showNavRail_medium() = runTest {
composeTestRule.setContent {
state = NiaAppState(
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(800.dp, 800.dp)),
navController = NavHostController(LocalContext.current),
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(800.dp, 800.dp)),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -135,10 +139,11 @@ class NiaAppStateTest {
fun niaAppState_showNavRail_large() = runTest {
composeTestRule.setContent {
state = NiaAppState(
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)),
navController = NavHostController(LocalContext.current),
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -150,10 +155,11 @@ class NiaAppStateTest {
fun stateIsOfflineWhenNetworkMonitorIsOffline() = runTest(UnconfinedTestDispatcher()) {
composeTestRule.setContent {
state = NiaAppState(
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)),
navController = NavHostController(LocalContext.current),
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ import com.google.samples.apps.nowinandroid.MainActivityUiState.Loading
import com.google.samples.apps.nowinandroid.MainActivityUiState.Success
import com.google.samples.apps.nowinandroid.core.analytics.AnalyticsHelper
import com.google.samples.apps.nowinandroid.core.analytics.LocalAnalyticsHelper
import com.google.samples.apps.nowinandroid.core.data.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.util.NetworkMonitor
import com.google.samples.apps.nowinandroid.core.designsystem.theme.NiaTheme
import com.google.samples.apps.nowinandroid.core.domain.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.model.data.DarkThemeConfig
import com.google.samples.apps.nowinandroid.core.model.data.ThemeBrand
import com.google.samples.apps.nowinandroid.ui.NiaApp
Expand Down
53 changes: 24 additions & 29 deletions app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.navigation.NavDestination
import androidx.navigation.NavDestination.Companion.hierarchy
import com.google.samples.apps.nowinandroid.R
import com.google.samples.apps.nowinandroid.core.data.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.util.NetworkMonitor
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaBackground
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaGradientBackground
Expand All @@ -70,7 +71,6 @@ import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.ImageVec
import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons
import com.google.samples.apps.nowinandroid.core.designsystem.theme.GradientColors
import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalGradientColors
import com.google.samples.apps.nowinandroid.core.domain.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.feature.settings.SettingsDialog
import com.google.samples.apps.nowinandroid.navigation.NiaNavHost
import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination
Expand All @@ -85,11 +85,12 @@ import com.google.samples.apps.nowinandroid.feature.settings.R as settingsR
fun NiaApp(
windowSizeClass: WindowSizeClass,
networkMonitor: NetworkMonitor,
userNewsResourceRepository: UserNewsResourceRepository,
appState: NiaAppState = rememberNiaAppState(
networkMonitor = networkMonitor,
windowSizeClass = windowSizeClass,
userNewsResourceRepository = userNewsResourceRepository,
),
userNewsResourceRepository: UserNewsResourceRepository,
) {
val shouldShowGradientBackground =
appState.currentTopLevelDestination == TopLevelDestination.FOR_YOU
Expand Down Expand Up @@ -133,14 +134,7 @@ fun NiaApp(
snackbarHost = { SnackbarHost(snackbarHostState) },
bottomBar = {
if (appState.shouldShowBottomBar) {
val forYouNewsResources by userNewsResourceRepository.getUserNewsResourcesForFollowedTopics()
.collectAsStateWithLifecycle(emptyList())
val unreadDestinations =
when {
forYouNewsResources.all { it.isViewed } -> emptySet()
else -> setOf(TopLevelDestination.FOR_YOU)
}

val unreadDestinations by appState.topLevelDestinationsWithUnreadResources.collectAsStateWithLifecycle()
NiaBottomBar(
destinations = appState.topLevelDestinations,
destinationsWithUnreadResources = unreadDestinations,
Expand Down Expand Up @@ -275,30 +269,31 @@ private fun NiaBottomBar(
}
},
label = { Text(stringResource(destination.iconTextId)) },
modifier = if (hasUnread) {
val tertiaryColor = MaterialTheme.colorScheme.tertiary
Modifier.drawWithContent {
drawContent()
drawCircle(
tertiaryColor,
radius = 5.dp.toPx(),
// This is based on the dimensions of the NavigationBar's "indicator pill";
// however, its parameters are private, so we must depend on them implicitly
// (NavigationBarTokens.ActiveIndicatorWidth = 64.dp)
center = center + Offset(
64.dp.toPx() * .45f,
32.dp.toPx() * -.45f - 6.dp.toPx(),
),
)
}
} else {
Modifier
},
modifier = if (hasUnread) notificationDot() else Modifier,
)
}
}
}

@Composable
private fun notificationDot(): Modifier {
val tertiaryColor = MaterialTheme.colorScheme.tertiary
return Modifier.drawWithContent {
drawContent()
drawCircle(
tertiaryColor,
radius = 5.dp.toPx(),
// This is based on the dimensions of the NavigationBar's "indicator pill";
// however, its parameters are private, so we must depend on them implicitly
// (NavigationBarTokens.ActiveIndicatorWidth = 64.dp)
center = center + Offset(
64.dp.toPx() * .45f,
32.dp.toPx() * -.45f - 6.dp.toPx(),
),
)
}
}

private fun NavDestination?.isTopLevelDestinationInHierarchy(destination: TopLevelDestination) =
this?.hierarchy?.any {
it.route?.contains(destination.name, true) ?: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ 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.data.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.util.NetworkMonitor
import com.google.samples.apps.nowinandroid.core.ui.TrackDisposableJank
import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.bookmarksRoute
Expand All @@ -47,19 +48,34 @@ import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.FOR_Y
import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.INTERESTS
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.stateIn

@Composable
fun rememberNiaAppState(
windowSizeClass: WindowSizeClass,
networkMonitor: NetworkMonitor,
userNewsResourceRepository: UserNewsResourceRepository,
coroutineScope: CoroutineScope = rememberCoroutineScope(),
navController: NavHostController = rememberNavController(),
): NiaAppState {
NavigationTrackingSideEffect(navController)
return remember(navController, coroutineScope, windowSizeClass, networkMonitor) {
NiaAppState(navController, coroutineScope, windowSizeClass, networkMonitor)
return remember(
navController,
coroutineScope,
windowSizeClass,
networkMonitor,
userNewsResourceRepository,
) {
NiaAppState(
navController,
coroutineScope,
windowSizeClass,
networkMonitor,
userNewsResourceRepository,
)
}
}

Expand All @@ -69,6 +85,7 @@ class NiaAppState(
val coroutineScope: CoroutineScope,
val windowSizeClass: WindowSizeClass,
networkMonitor: NetworkMonitor,
userNewsResourceRepository: UserNewsResourceRepository,
) {
val currentDestination: NavDestination?
@Composable get() = navController
Expand Down Expand Up @@ -105,6 +122,22 @@ class NiaAppState(
*/
val topLevelDestinations: List<TopLevelDestination> = TopLevelDestination.values().asList()

/**
* The top level destinations that have unread news resources.
*/
val topLevelDestinationsWithUnreadResources: StateFlow<Set<TopLevelDestination>> =
userNewsResourceRepository.getUserNewsResourcesForFollowedTopics()
.combine(userNewsResourceRepository.getBookmarkedUserNewsResources()) { forYouNewsResources, bookmarkedNewsResources ->
setOfNotNull(
FOR_YOU.takeIf { forYouNewsResources.any { !it.hasBeenViewed } },
BOOKMARKS.takeIf { bookmarkedNewsResources.any { !it.hasBeenViewed } },
)
}.stateIn(
coroutineScope,
SharingStarted.WhileSubscribed(5_000),
initialValue = emptySet(),
)

/**
* 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
* limitations under the License.
*/

package com.google.samples.apps.nowinandroid.core.domain.di
package com.google.samples.apps.nowinandroid.core.data.di

import com.google.samples.apps.nowinandroid.core.domain.repository.CompositeUserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.domain.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.repository.CompositeUserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.repository.UserNewsResourceRepository
import dagger.Binds
import dagger.Module
import dagger.hilt.InstallIn
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/*
* Copyright 2023 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.data.repository

import com.google.samples.apps.nowinandroid.core.model.data.UserNewsResource
import com.google.samples.apps.nowinandroid.core.model.data.mapToUserNewsResources
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.map
import javax.inject.Inject

/**
* Implements a [UserNewsResourceRepository] by combining a [NewsRepository] with a
* [UserDataRepository].
*/
class CompositeUserNewsResourceRepository @Inject constructor(
val newsRepository: NewsRepository,
val userDataRepository: UserDataRepository,
) : UserNewsResourceRepository {

/**
* Returns available news resources (joined with user data) matching the given query.
*/
override fun getUserNewsResources(
query: NewsResourceQuery,
): Flow<List<UserNewsResource>> =
newsRepository.getNewsResources(query)
.combine(userDataRepository.userData) { newsResources, userData ->
newsResources.mapToUserNewsResources(userData)
}

/**
* Returns available news resources (joined with user data) for the followed topics.
*/
override fun getUserNewsResourcesForFollowedTopics(): Flow<List<UserNewsResource>> =
userDataRepository.userData.map { it.followedTopics }.distinctUntilChanged()
.flatMapLatest { followedTopics ->
when {
followedTopics.isEmpty() -> flowOf(emptyList())
else -> getUserNewsResources(NewsResourceQuery(filterTopicIds = followedTopics))
}
}

override fun getBookmarkedUserNewsResources(): Flow<List<UserNewsResource>> =
userDataRepository.userData.map { it.bookmarkedNewsResources }.distinctUntilChanged()
.flatMapLatest { bookmarkedNewsResources ->
when {
bookmarkedNewsResources.isEmpty() -> flowOf(emptyList())
else -> getUserNewsResources(NewsResourceQuery(filterNewsIds = bookmarkedNewsResources))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class OfflineFirstUserDataRepository @Inject constructor(
)
}

override suspend fun updateNewsResourceViewed(newsResourceId: String, viewed: Boolean) =
niaPreferencesDataSource.toggleNewsResourceViewed(newsResourceId, viewed)
override suspend fun setNewsResourceViewed(newsResourceId: String, viewed: Boolean) =
niaPreferencesDataSource.setNewsResourceViewed(newsResourceId, viewed)

override suspend fun setThemeBrand(themeBrand: ThemeBrand) {
niaPreferencesDataSource.setThemeBrand(themeBrand)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ interface UserDataRepository {
/**
* Updates the viewed status for a news resource
*/
suspend fun updateNewsResourceViewed(newsResourceId: String, viewed: Boolean)
suspend fun setNewsResourceViewed(newsResourceId: String, viewed: Boolean)

/**
* Sets the desired theme brand.
Expand Down
Loading

0 comments on commit 8194f7a

Please sign in to comment.