For #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/543/head
Mugurell 2 years ago committed by mergify[bot]
parent 9aec04ede7
commit abff68a31b

@ -9,7 +9,6 @@ import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.ext.filterOutTab
import org.mozilla.fenix.ext.getFilteredStories
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.recenttabs.RecentTab
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.
updatedCategoriesState.copy(
pocketStories = updatedCategoriesState.getFilteredStories(
POCKET_STORIES_TO_SHOW_COUNT
)
pocketStories = updatedCategoriesState.getFilteredStories()
)
}
is AppAction.DeselectPocketStoriesCategory -> {
@ -124,18 +121,14 @@ internal object AppStoreReducer {
// Deselecting a category means the stories to be displayed needs to also be changed.
updatedCategoriesState.copy(
pocketStories = updatedCategoriesState.getFilteredStories(
POCKET_STORIES_TO_SHOW_COUNT
)
pocketStories = updatedCategoriesState.getFilteredStories()
)
}
is AppAction.PocketStoriesCategoriesChange -> {
val updatedCategoriesState = state.copy(pocketStoriesCategories = action.storiesCategories)
// Whenever categories change stories to be displayed needs to also be changed.
updatedCategoriesState.copy(
pocketStories = updatedCategoriesState.getFilteredStories(
POCKET_STORIES_TO_SHOW_COUNT
)
pocketStories = updatedCategoriesState.getFilteredStories()
)
}
is AppAction.PocketStoriesCategoriesSelectionsChange -> {
@ -145,9 +138,7 @@ internal object AppStoreReducer {
)
// Whenever categories change stories to be displayed needs to also be changed.
updatedCategoriesState.copy(
pocketStories = updatedCategoriesState.getFilteredStories(
POCKET_STORIES_TO_SHOW_COUNT
)
pocketStories = updatedCategoriesState.getFilteredStories()
)
}
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.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.
*
* @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.
*/
fun AppState.getFilteredStories(
neededStoriesCount: Int
): List<PocketRecommendedStory> {
fun AppState.getFilteredStories(): List<PocketRecommendedStory> {
if (pocketStoriesCategoriesSelections.isEmpty()) {
return pocketStoriesCategories
.find {
it.name == POCKET_STORIES_DEFAULT_CATEGORY_NAME
}?.stories
?.sortedBy { it.timesShown }
?.take(neededStoriesCount) ?: emptyList()
?.take(POCKET_STORIES_TO_SHOW_COUNT) ?: emptyList()
}
val oldestSortedCategories = pocketStoriesCategoriesSelections
@ -41,13 +39,13 @@ fun AppState.getFilteredStories(
}
val filteredStoriesCount = getFilteredStoriesCount(
oldestSortedCategories, neededStoriesCount
oldestSortedCategories, POCKET_STORIES_TO_SHOW_COUNT
)
return oldestSortedCategories
.flatMap { category ->
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.Theme
internal const val POCKET_STORIES_TO_SHOW_COUNT = 8
/**
* [RecyclerView.ViewHolder] for displaying the list of [PocketRecommendedStory]s from [AppStore].
*
@ -102,8 +100,9 @@ fun PocketStoriesViewHolderPreview() {
Spacer(Modifier.height(16.dp))
@Suppress("MagicNumber")
PocketStories(
stories = getFakePocketStories(POCKET_STORIES_TO_SHOW_COUNT),
stories = getFakePocketStories(8),
contentPadding = 0.dp,
onStoryClicked = { _, _ -> },
onDiscoverMoreClicked = {}

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

@ -36,48 +36,61 @@ class AppStateTest {
)
)
var result = state.getFilteredStories(2)
assertNull(result.firstOrNull { it.category != POCKET_STORIES_DEFAULT_CATEGORY_NAME })
val result = state.getFilteredStories()
result = state.getFilteredStories(5)
assertNull(result.firstOrNull { it.category != POCKET_STORIES_DEFAULT_CATEGORY_NAME })
}
@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(
pocketStoriesCategories = listOf(
otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategory
otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategoryWithManyStories
)
)
// Asking for fewer than available
var result = state.getFilteredStories(2)
assertEquals(2, result.size)
val result = state.getFilteredStories()
// Asking for more than available
result = state.getFilteredStories(5)
assertEquals(3, result.size)
assertEquals(POCKET_STORIES_TO_SHOW_COUNT, result.size)
}
@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(
pocketStoriesCategories = listOf(otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategory),
pocketStoriesCategoriesSelections = listOf(PocketRecommendedStoriesSelectedCategory(otherStoriesCategory.name))
)
var result = state.getFilteredStories(2)
assertEquals(2, result.size)
assertNull(result.firstOrNull { it.category != otherStoriesCategory.name })
val result = state.getFilteredStories()
result = state.getFilteredStories(3)
assertEquals(3, result.size)
assertNull(result.firstOrNull { it.category != otherStoriesCategory.name })
}
@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(
pocketStoriesCategories = listOf(otherStoriesCategory, anotherStoriesCategory, defaultStoriesCategory),
pocketStoriesCategoriesSelections = listOf(
@ -86,15 +99,7 @@ class AppStateTest {
)
)
var result = state.getFilteredStories(2)
assertEquals(2, result.size)
assertNull(
result.firstOrNull {
it.category != otherStoriesCategory.name && it.category != anotherStoriesCategory.name
}
)
result = state.getFilteredStories(6)
val result = state.getFilteredStories()
assertEquals(6, result.size)
assertNull(
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
fun `GIVEN no category is selected WHEN getFilteredStoriesCount is called THEN return an empty result`() {
val result = getFilteredStoriesCount(emptyList(), 1)
@ -250,7 +238,7 @@ class AppStateTest {
)
)
val result = state.getFilteredStories(6)
val result = state.getFilteredStories()
assertEquals(6, result.size)
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)
assertNull(result.firstOrNull { it.category != anotherStoriesCategory.name })

Loading…
Cancel
Save