[fenix] For https://github.com/mozilla-mobile/fenix/issues/25025 - Hardcode showing at max 8 Pocket stories

The default was already 8. This is now being moved closer to the source from
where the stories to be shown are emitted.
With the addition of sponsored stories at fixed positions having to support a
variable number of stories being returned from AppState#getFilteredStories
means increased complexity with no benefit.
pull/600/head
Mugurell 2 years ago committed by mergify[bot]
parent 2f0189a2eb
commit 15677b59b6

@ -9,7 +9,6 @@ import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.ext.filterOutTab import org.mozilla.fenix.ext.filterOutTab
import org.mozilla.fenix.ext.getFilteredStories import org.mozilla.fenix.ext.getFilteredStories
import org.mozilla.fenix.ext.recentSearchGroup import org.mozilla.fenix.ext.recentSearchGroup
import org.mozilla.fenix.home.pocket.POCKET_STORIES_TO_SHOW_COUNT
import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesSelectedCategory import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesSelectedCategory
import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.recenttabs.RecentTab
import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem import org.mozilla.fenix.home.recentvisits.RecentlyVisitedItem
@ -110,9 +109,7 @@ internal object AppStoreReducer {
// Selecting a category means the stories to be displayed needs to also be changed. // Selecting a category means the stories to be displayed needs to also be changed.
updatedCategoriesState.copy( updatedCategoriesState.copy(
pocketStories = updatedCategoriesState.getFilteredStories( pocketStories = updatedCategoriesState.getFilteredStories()
POCKET_STORIES_TO_SHOW_COUNT
)
) )
} }
is AppAction.DeselectPocketStoriesCategory -> { is AppAction.DeselectPocketStoriesCategory -> {
@ -124,18 +121,14 @@ internal object AppStoreReducer {
// Deselecting a category means the stories to be displayed needs to also be changed. // Deselecting a category means the stories to be displayed needs to also be changed.
updatedCategoriesState.copy( updatedCategoriesState.copy(
pocketStories = updatedCategoriesState.getFilteredStories( pocketStories = updatedCategoriesState.getFilteredStories()
POCKET_STORIES_TO_SHOW_COUNT
)
) )
} }
is AppAction.PocketStoriesCategoriesChange -> { is AppAction.PocketStoriesCategoriesChange -> {
val updatedCategoriesState = state.copy(pocketStoriesCategories = action.storiesCategories) val updatedCategoriesState = state.copy(pocketStoriesCategories = action.storiesCategories)
// Whenever categories change stories to be displayed needs to also be changed. // Whenever categories change stories to be displayed needs to also be changed.
updatedCategoriesState.copy( updatedCategoriesState.copy(
pocketStories = updatedCategoriesState.getFilteredStories( pocketStories = updatedCategoriesState.getFilteredStories()
POCKET_STORIES_TO_SHOW_COUNT
)
) )
} }
is AppAction.PocketStoriesCategoriesSelectionsChange -> { is AppAction.PocketStoriesCategoriesSelectionsChange -> {
@ -145,9 +138,7 @@ internal object AppStoreReducer {
) )
// Whenever categories change stories to be displayed needs to also be changed. // Whenever categories change stories to be displayed needs to also be changed.
updatedCategoriesState.copy( updatedCategoriesState.copy(
pocketStories = updatedCategoriesState.getFilteredStories( pocketStories = updatedCategoriesState.getFilteredStories()
POCKET_STORIES_TO_SHOW_COUNT
)
) )
} }
is AppAction.PocketStoriesChange -> state.copy(pocketStories = action.pocketStories) is AppAction.PocketStoriesChange -> state.copy(pocketStories = action.pocketStories)

@ -12,24 +12,22 @@ import org.mozilla.fenix.home.pocket.POCKET_STORIES_DEFAULT_CATEGORY_NAME
import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory
import org.mozilla.fenix.home.recenttabs.RecentTab.SearchGroup import org.mozilla.fenix.home.recenttabs.RecentTab.SearchGroup
@VisibleForTesting
internal const val POCKET_STORIES_TO_SHOW_COUNT = 8
/** /**
* Get the list of stories to be displayed based on the user selected categories. * Get the list of stories to be displayed based on the user selected categories.
* *
* @param neededStoriesCount how many stories are intended to be displayed.
* This only impacts filtered results guaranteeing an even spread of stories from each category.
*
* @return a list of [PocketRecommendedStory]es from the currently selected categories. * @return a list of [PocketRecommendedStory]es from the currently selected categories.
*/ */
fun AppState.getFilteredStories( fun AppState.getFilteredStories(): List<PocketRecommendedStory> {
neededStoriesCount: Int
): List<PocketRecommendedStory> {
if (pocketStoriesCategoriesSelections.isEmpty()) { if (pocketStoriesCategoriesSelections.isEmpty()) {
return pocketStoriesCategories return pocketStoriesCategories
.find { .find {
it.name == POCKET_STORIES_DEFAULT_CATEGORY_NAME it.name == POCKET_STORIES_DEFAULT_CATEGORY_NAME
}?.stories }?.stories
?.sortedBy { it.timesShown } ?.sortedBy { it.timesShown }
?.take(neededStoriesCount) ?: emptyList() ?.take(POCKET_STORIES_TO_SHOW_COUNT) ?: emptyList()
} }
val oldestSortedCategories = pocketStoriesCategoriesSelections val oldestSortedCategories = pocketStoriesCategoriesSelections
@ -41,13 +39,13 @@ fun AppState.getFilteredStories(
} }
val filteredStoriesCount = getFilteredStoriesCount( val filteredStoriesCount = getFilteredStoriesCount(
oldestSortedCategories, neededStoriesCount oldestSortedCategories, POCKET_STORIES_TO_SHOW_COUNT
) )
return oldestSortedCategories return oldestSortedCategories
.flatMap { category -> .flatMap { category ->
category.stories.sortedBy { it.timesShown }.take(filteredStoriesCount[category.name]!!) category.stories.sortedBy { it.timesShown }.take(filteredStoriesCount[category.name]!!)
}.take(neededStoriesCount) }.take(POCKET_STORIES_TO_SHOW_COUNT)
} }
/** /**

@ -32,8 +32,6 @@ import org.mozilla.fenix.compose.SectionHeader
import org.mozilla.fenix.theme.FirefoxTheme import org.mozilla.fenix.theme.FirefoxTheme
import org.mozilla.fenix.theme.Theme import org.mozilla.fenix.theme.Theme
internal const val POCKET_STORIES_TO_SHOW_COUNT = 8
/** /**
* [RecyclerView.ViewHolder] for displaying the list of [PocketRecommendedStory]s from [AppStore]. * [RecyclerView.ViewHolder] for displaying the list of [PocketRecommendedStory]s from [AppStore].
* *
@ -102,8 +100,9 @@ fun PocketStoriesViewHolderPreview() {
Spacer(Modifier.height(16.dp)) Spacer(Modifier.height(16.dp))
@Suppress("MagicNumber")
PocketStories( PocketStories(
stories = getFakePocketStories(POCKET_STORIES_TO_SHOW_COUNT), stories = getFakePocketStories(8),
contentPadding = 0.dp, contentPadding = 0.dp,
onStoryClicked = { _, _ -> }, onStoryClicked = { _, _ -> },
onDiscoverMoreClicked = {} onDiscoverMoreClicked = {}

@ -33,7 +33,6 @@ import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.getFilteredStories import org.mozilla.fenix.ext.getFilteredStories
import org.mozilla.fenix.home.CurrentMode import org.mozilla.fenix.home.CurrentMode
import org.mozilla.fenix.home.Mode import org.mozilla.fenix.home.Mode
import org.mozilla.fenix.home.pocket.POCKET_STORIES_TO_SHOW_COUNT
import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory
import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesSelectedCategory import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesSelectedCategory
import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recentbookmarks.RecentBookmark
@ -300,11 +299,11 @@ class AppStoreTest {
) )
mockkStatic("org.mozilla.fenix.ext.AppStateKt") { mockkStatic("org.mozilla.fenix.ext.AppStateKt") {
every { any<AppState>().getFilteredStories(any()) } returns filteredStories every { any<AppState>().getFilteredStories() } returns filteredStories
appStore.dispatch(AppAction.SelectPocketStoriesCategory("another")).join() appStore.dispatch(AppAction.SelectPocketStoriesCategory("another")).join()
verify { any<AppState>().getFilteredStories(POCKET_STORIES_TO_SHOW_COUNT) } verify { any<AppState>().getFilteredStories() }
} }
val selectedCategories = appStore.state.pocketStoriesCategoriesSelections val selectedCategories = appStore.state.pocketStoriesCategoriesSelections
@ -329,11 +328,11 @@ class AppStoreTest {
) )
mockkStatic("org.mozilla.fenix.ext.AppStateKt") { mockkStatic("org.mozilla.fenix.ext.AppStateKt") {
every { any<AppState>().getFilteredStories(any()) } returns filteredStories every { any<AppState>().getFilteredStories() } returns filteredStories
appStore.dispatch(AppAction.DeselectPocketStoriesCategory("other")).join() appStore.dispatch(AppAction.DeselectPocketStoriesCategory("other")).join()
verify { any<AppState>().getFilteredStories(POCKET_STORIES_TO_SHOW_COUNT) } verify { any<AppState>().getFilteredStories() }
} }
val selectedCategories = appStore.state.pocketStoriesCategoriesSelections val selectedCategories = appStore.state.pocketStoriesCategoriesSelections
@ -365,12 +364,12 @@ class AppStoreTest {
mockkStatic("org.mozilla.fenix.ext.AppStateKt") { mockkStatic("org.mozilla.fenix.ext.AppStateKt") {
val firstFilteredStories = listOf(mockk<PocketRecommendedStory>()) val firstFilteredStories = listOf(mockk<PocketRecommendedStory>())
every { any<AppState>().getFilteredStories(any()) } returns firstFilteredStories every { any<AppState>().getFilteredStories() } returns firstFilteredStories
appStore.dispatch( appStore.dispatch(
AppAction.PocketStoriesCategoriesChange(listOf(otherStoriesCategory, anotherStoriesCategory)) AppAction.PocketStoriesCategoriesChange(listOf(otherStoriesCategory, anotherStoriesCategory))
).join() ).join()
verify { any<AppState>().getFilteredStories(POCKET_STORIES_TO_SHOW_COUNT) } verify { any<AppState>().getFilteredStories() }
assertTrue( assertTrue(
appStore.state.pocketStoriesCategories.containsAll( appStore.state.pocketStoriesCategories.containsAll(
listOf(otherStoriesCategory, anotherStoriesCategory) listOf(otherStoriesCategory, anotherStoriesCategory)
@ -380,13 +379,13 @@ class AppStoreTest {
val updatedCategories = listOf(PocketRecommendedStoriesCategory("yetAnother")) val updatedCategories = listOf(PocketRecommendedStoriesCategory("yetAnother"))
val secondFilteredStories = listOf(mockk<PocketRecommendedStory>()) val secondFilteredStories = listOf(mockk<PocketRecommendedStory>())
every { any<AppState>().getFilteredStories(any()) } returns secondFilteredStories every { any<AppState>().getFilteredStories() } returns secondFilteredStories
appStore.dispatch( appStore.dispatch(
AppAction.PocketStoriesCategoriesChange( AppAction.PocketStoriesCategoriesChange(
updatedCategories updatedCategories
) )
).join() ).join()
verify(exactly = 2) { any<AppState>().getFilteredStories(POCKET_STORIES_TO_SHOW_COUNT) } verify(exactly = 2) { any<AppState>().getFilteredStories() }
assertTrue(updatedCategories.containsAll(appStore.state.pocketStoriesCategories)) assertTrue(updatedCategories.containsAll(appStore.state.pocketStoriesCategories))
assertSame(secondFilteredStories, appStore.state.pocketStories) assertSame(secondFilteredStories, appStore.state.pocketStories)
} }
@ -401,7 +400,7 @@ class AppStoreTest {
mockkStatic("org.mozilla.fenix.ext.AppStateKt") { mockkStatic("org.mozilla.fenix.ext.AppStateKt") {
val firstFilteredStories = listOf(mockk<PocketRecommendedStory>()) val firstFilteredStories = listOf(mockk<PocketRecommendedStory>())
every { any<AppState>().getFilteredStories(any()) } returns firstFilteredStories every { any<AppState>().getFilteredStories() } returns firstFilteredStories
appStore.dispatch( appStore.dispatch(
AppAction.PocketStoriesCategoriesSelectionsChange( AppAction.PocketStoriesCategoriesSelectionsChange(
@ -409,7 +408,7 @@ class AppStoreTest {
categoriesSelected = listOf(selectedCategory) categoriesSelected = listOf(selectedCategory)
) )
).join() ).join()
verify { any<AppState>().getFilteredStories(POCKET_STORIES_TO_SHOW_COUNT) } verify { any<AppState>().getFilteredStories() }
assertTrue( assertTrue(
appStore.state.pocketStoriesCategories.containsAll( appStore.state.pocketStoriesCategories.containsAll(
listOf(otherStoriesCategory, anotherStoriesCategory) listOf(otherStoriesCategory, anotherStoriesCategory)

@ -36,48 +36,61 @@ class AppStateTest {
) )
) )
var result = state.getFilteredStories(2) val result = state.getFilteredStories()
assertNull(result.firstOrNull { it.category != POCKET_STORIES_DEFAULT_CATEGORY_NAME })
result = state.getFilteredStories(5)
assertNull(result.firstOrNull { it.category != POCKET_STORIES_DEFAULT_CATEGORY_NAME }) assertNull(result.firstOrNull { it.category != POCKET_STORIES_DEFAULT_CATEGORY_NAME })
} }
@Test @Test
fun `GIVEN no category is selected WHEN getFilteredStories is called THEN no more than the indicated number of stories are returned`() { fun `GIVEN no category is selected WHEN getFilteredStories is called THEN no more than the default stories number are returned from the default category`() {
val defaultStoriesCategoryWithManyStories = PocketRecommendedStoriesCategory(
POCKET_STORIES_DEFAULT_CATEGORY_NAME,
getFakePocketStories(POCKET_STORIES_TO_SHOW_COUNT + 2)
)
val state = AppState( val state = AppState(
pocketStoriesCategories = listOf( pocketStoriesCategories = listOf(
otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategory otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategoryWithManyStories
) )
) )
// Asking for fewer than available val result = state.getFilteredStories()
var result = state.getFilteredStories(2)
assertEquals(2, result.size)
// Asking for more than available assertEquals(POCKET_STORIES_TO_SHOW_COUNT, result.size)
result = state.getFilteredStories(5)
assertEquals(3, result.size)
} }
@Test @Test
fun `GIVEN a category is selected WHEN getFilteredStories is called for fewer than in the category THEN only stories from that category are returned`() { fun `GIVEN a category is selected WHEN getFilteredStories is called THEN only stories from that category are returned`() {
val state = AppState( val state = AppState(
pocketStoriesCategories = listOf(otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategory), pocketStoriesCategories = listOf(otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategory),
pocketStoriesCategoriesSelections = listOf(PocketRecommendedStoriesSelectedCategory(otherStoriesCategory.name)) pocketStoriesCategoriesSelections = listOf(PocketRecommendedStoriesSelectedCategory(otherStoriesCategory.name))
) )
var result = state.getFilteredStories(2) val result = state.getFilteredStories()
assertEquals(2, result.size)
assertNull(result.firstOrNull { it.category != otherStoriesCategory.name })
result = state.getFilteredStories(3)
assertEquals(3, result.size)
assertNull(result.firstOrNull { it.category != otherStoriesCategory.name }) assertNull(result.firstOrNull { it.category != otherStoriesCategory.name })
} }
@Test @Test
fun `GIVEN two categories are selected WHEN getFilteredStories is called for fewer than in both THEN only stories from those categories are returned`() { fun `GIVEN a category is selected WHEN getFilteredStories is called THEN no more than the default stories number are returned from the selected category`() {
val otherStoriesCategoryWithManyStories =
PocketRecommendedStoriesCategory(
"other",
getFakePocketStories(POCKET_STORIES_TO_SHOW_COUNT + 2, "other")
)
val state = AppState(
pocketStoriesCategories =
listOf(otherStoriesCategoryWithManyStories, anotherStoriesCategory, defaultStoriesCategory),
pocketStoriesCategoriesSelections =
listOf(PocketRecommendedStoriesSelectedCategory(otherStoriesCategoryWithManyStories.name))
)
val result = state.getFilteredStories()
assertEquals(POCKET_STORIES_TO_SHOW_COUNT, result.size)
}
@Test
fun `GIVEN two categories are selected WHEN getFilteredStories is called THEN only stories from those categories are returned`() {
val state = AppState( val state = AppState(
pocketStoriesCategories = listOf(otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategory), pocketStoriesCategories = listOf(otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategory),
pocketStoriesCategoriesSelections = listOf( pocketStoriesCategoriesSelections = listOf(
@ -86,15 +99,7 @@ class AppStateTest {
) )
) )
var result = state.getFilteredStories(2) val result = state.getFilteredStories()
assertEquals(2, result.size)
assertNull(
result.firstOrNull {
it.category != otherStoriesCategory.name && it.category != anotherStoriesCategory.name
}
)
result = state.getFilteredStories(6)
assertEquals(6, result.size) assertEquals(6, result.size)
assertNull( assertNull(
result.firstOrNull { result.firstOrNull {
@ -103,23 +108,6 @@ class AppStateTest {
) )
} }
@Test
fun `GIVEN two categories are selected WHEN getFilteredStories is called for an odd number of stories THEN there are more by one stories from the newest category`() {
val state = AppState(
pocketStoriesCategories = listOf(otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategory),
pocketStoriesCategoriesSelections = listOf(
PocketRecommendedStoriesSelectedCategory(otherStoriesCategory.name, selectionTimestamp = 0),
PocketRecommendedStoriesSelectedCategory(anotherStoriesCategory.name, selectionTimestamp = 1)
)
)
val result = state.getFilteredStories(5)
assertEquals(5, result.size)
assertEquals(2, result.filter { it.category == otherStoriesCategory.name }.size)
assertEquals(3, result.filter { it.category == anotherStoriesCategory.name }.size)
}
@Test @Test
fun `GIVEN no category is selected WHEN getFilteredStoriesCount is called THEN return an empty result`() { fun `GIVEN no category is selected WHEN getFilteredStoriesCount is called THEN return an empty result`() {
val result = getFilteredStoriesCount(emptyList(), 1) val result = getFilteredStoriesCount(emptyList(), 1)
@ -250,7 +238,7 @@ class AppStateTest {
) )
) )
val result = state.getFilteredStories(6) val result = state.getFilteredStories()
assertEquals(6, result.size) assertEquals(6, result.size)
assertSame(secondCategory.stories[2], result.first()) assertSame(secondCategory.stories[2], result.first())
@ -271,7 +259,7 @@ class AppStateTest {
) )
) )
val result = state.getFilteredStories(6) val result = state.getFilteredStories()
assertEquals(3, result.size) assertEquals(3, result.size)
assertNull(result.firstOrNull { it.category != anotherStoriesCategory.name }) assertNull(result.firstOrNull { it.category != anotherStoriesCategory.name })

Loading…
Cancel
Save