For #21592 - Don't topup with general stories

upstream-sync
Mugurell 3 years ago committed by mergify[bot]
parent ea6d5e3d38
commit 53d4336939

@ -11,15 +11,12 @@ import org.mozilla.fenix.home.sessioncontrol.viewholders.pocket.POCKET_STORIES_D
import org.mozilla.fenix.home.sessioncontrol.viewholders.pocket.PocketRecommendedStoryCategory
/**
* Get the list of stories to be displayed.
* Either the stories from the [POCKET_STORIES_DEFAULT_CATEGORY_NAME] either
* filtered stories 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
* topped if necessary with stories from the [POCKET_STORIES_DEFAULT_CATEGORY_NAME] up to [neededStoriesCount].
* @return a list of [PocketRecommendedStory]es from the currently selected categories.
*/
fun HomeFragmentState.getFilteredStories(
neededStoriesCount: Int
@ -39,43 +36,27 @@ fun HomeFragmentState.getFilteredStories(
.sortedByDescending { it.lastInteractedWithTimestamp }
val filteredStoriesCount = getFilteredStoriesCount(
pocketStoriesCategories, oldestSortedCategories, neededStoriesCount
oldestSortedCategories, neededStoriesCount
)
// Add general stories at the end of the stories list to show until neededStoriesCount
val generalStoriesTopup = filteredStoriesCount[POCKET_STORIES_DEFAULT_CATEGORY_NAME]?.let { neededTopups ->
pocketStoriesCategories
.find { it.name == POCKET_STORIES_DEFAULT_CATEGORY_NAME }
?.stories
?.sortedBy { it.timesShown }
?.take(neededTopups)
} ?: emptyList()
return oldestSortedCategories
.flatMap { category ->
category.stories.sortedBy { it.timesShown }.take(filteredStoriesCount[category.name]!!)
}.plus(generalStoriesTopup)
.take(neededStoriesCount)
}.take(neededStoriesCount)
}
/**
* Get how many stories needs to be shown from each currently selected category.
*
* If the selected categories together don't have [neededStoriesCount] stories then
* the difference is added from the [POCKET_STORIES_DEFAULT_CATEGORY_NAME] category.
*
* @param allCategories the list of all Pocket stories categories.
* @param selectedCategories ordered list of categories from which to return results.
* @param neededStoriesCount how many stories are intended to be displayed.
* This impacts the results by guaranteeing an even spread of stories from each category in that stories count.
*
* @return a mapping of how many stories are to be shown from each category from [selectedCategories].
* The result is topped with stories counts from the [POCKET_STORIES_DEFAULT_CATEGORY_NAME] up to [neededStoriesCount].
*/
@VisibleForTesting
@Suppress("ReturnCount", "NestedBlockDepth")
internal fun getFilteredStoriesCount(
allCategories: List<PocketRecommendedStoryCategory>,
selectedCategories: List<PocketRecommendedStoryCategory>,
neededStoriesCount: Int
): Map<String, Int> {
@ -83,17 +64,8 @@ internal fun getFilteredStoriesCount(
availableStories + category.stories.size
}
when {
totalStoriesInFilteredCategories == neededStoriesCount -> {
return selectedCategories.map { it.name to it.stories.size }.toMap()
}
totalStoriesInFilteredCategories < neededStoriesCount -> {
return selectedCategories.map { it.name to it.stories.size }.toMap() +
allCategories.filter { it.name == POCKET_STORIES_DEFAULT_CATEGORY_NAME }.map {
it.name to (neededStoriesCount - totalStoriesInFilteredCategories).coerceAtMost(it.stories.size)
}.toMap()
}
else -> {
when (totalStoriesInFilteredCategories > neededStoriesCount) {
true -> {
val storiesCountFromEachCategory = mutableMapOf<String, Int>()
var currentFilteredStoriesCount = 0
@ -110,6 +82,9 @@ internal fun getFilteredStoriesCount(
}
}
}
false -> {
return selectedCategories.map { it.name to it.stories.size }.toMap()
}
}
return emptyMap()

@ -149,7 +149,7 @@ private fun homeFragmentStateReducer(
val updatedCategoriesState = state.copy(
pocketStoriesCategories = state.pocketStoriesCategories.map {
when (it.name == action.categoryName) {
true -> it.copy(isSelected = true)
true -> it.copy(isSelected = true, lastInteractedWithTimestamp = System.currentTimeMillis())
false -> it
}
}
@ -163,7 +163,7 @@ private fun homeFragmentStateReducer(
// Deselecting a category means the stories to be displayed needs to also be changed.
pocketStoriesCategories = state.pocketStoriesCategories.map {
when (it.name == action.categoryName) {
true -> it.copy(isSelected = false)
true -> it.copy(isSelected = false, lastInteractedWithTimestamp = System.currentTimeMillis())
false -> it
}
}

@ -50,6 +50,13 @@ import org.mozilla.fenix.theme.FirefoxTheme
import kotlin.math.roundToInt
import kotlin.random.Random
/**
* Placeholder [PocketRecommendedStory] allowing to combine other items in the same list that shows stories.
* It uses empty values for it's properties ensuring that no conflict is possible since real stories have
* mandatory values.
*/
private val placeholderStory = PocketRecommendedStory("", "", "", "", "", 0, 0)
/**
* Displays a single [PocketRecommendedStory].
*
@ -96,25 +103,28 @@ fun PocketStories(
client: Client,
onExternalLinkClicked: (String) -> Unit
) {
// Show stories in a 3 by 3 grid
val gridLength = 3
// Show stories in at most 3 rows but on any number of columns depending on the data received.
val maxRowsNo = 3
val storiesToShow = (stories + placeholderStory).chunked(maxRowsNo)
val listState = rememberLazyListState()
val flingBehavior = EagerFlingBehavior(lazyRowState = listState)
LazyRow(state = listState, flingBehavior = flingBehavior) {
itemsIndexed(stories.chunked(gridLength)) { rowIndex, columnItems ->
Column(Modifier.padding(end = if (rowIndex < gridLength - 1) 8.dp else 0.dp)) {
for (index in 0 until gridLength) {
columnItems.getOrNull(index)?.let { story ->
itemsIndexed(storiesToShow) { columnIndex, columnItems ->
Column(Modifier.padding(end = if (columnIndex < storiesToShow.size - 1) 8.dp else 0.dp)) {
columnItems.forEachIndexed { rowIndex, story ->
if (story == placeholderStory) {
ListItemTabLargePlaceholder(stringResource(R.string.pocket_stories_placeholder_text)) {
onExternalLinkClicked("http://getpocket.com/explore")
}
} else {
PocketStory(story, client) {
onExternalLinkClicked(story.url)
}
} ?: ListItemTabLargePlaceholder(stringResource(R.string.pocket_stories_placeholder_text)) {
onExternalLinkClicked("http://getpocket.com/explore")
}
// Add padding between all rows. Not also after the last.
if (index < gridLength - 1) {
if (rowIndex < maxRowsNo - 1) {
Spacer(modifier = Modifier.height(8.dp))
}
}
@ -137,7 +147,7 @@ fun PocketStoriesCategories(
StaggeredHorizontalGrid(
horizontalItemsSpacing = 16.dp
) {
categories.forEach { category ->
categories.filter { it.name != POCKET_STORIES_DEFAULT_CATEGORY_NAME }.forEach { category ->
SelectableChip(category.name, category.isSelected) {
onCategoryClick(category)
}

@ -74,24 +74,6 @@ class HomeFragmentStateTest {
assertNull(result.firstOrNull { it.category != otherStoriesCategory.name })
}
@Test
fun `GIVEN a category is selected WHEN getFilteredStories is called for more than in the category THEN results topped with ones from the default category are returned`() {
val homeState = HomeFragmentState(
pocketStoriesCategories = listOf(
otherStoriesCategory.copy(isSelected = true), anotherStoriesCategory, defaultStoriesCategory
)
)
val result = homeState.getFilteredStories(5)
assertEquals(5, result.size)
assertEquals(3, result.filter { it.category == otherStoriesCategory.name }.size)
assertEquals(
2,
result.filter { it.category == POCKET_STORIES_DEFAULT_CATEGORY_NAME }.size
)
}
@Test
fun `GIVEN two categories are selected WHEN getFilteredStories is called for fewer than in both THEN only stories from those categories are returned`() {
val homeState = HomeFragmentState(
@ -119,27 +101,6 @@ class HomeFragmentStateTest {
)
}
@Test
fun `GIVEN two categories are selected WHEN getFilteredStories is called for more than in the categories THEN results topped with ones from the default category are returned`() {
val homeState = HomeFragmentState(
pocketStoriesCategories = listOf(
otherStoriesCategory.copy(isSelected = true),
anotherStoriesCategory.copy(isSelected = true),
defaultStoriesCategory
)
)
val result = homeState.getFilteredStories(8)
assertEquals(8, result.size)
assertEquals(3, result.filter { it.category == otherStoriesCategory.name }.size)
assertEquals(3, result.filter { it.category == anotherStoriesCategory.name }.size)
assertEquals(
2,
result.filter { it.category == POCKET_STORIES_DEFAULT_CATEGORY_NAME }.size
)
}
@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 firstSelectedCategory = otherStoriesCategory.copy(lastInteractedWithTimestamp = 0, isSelected = true)
@ -158,59 +119,36 @@ class HomeFragmentStateTest {
}
@Test
fun `GIVEN no category is selected WHEN getFilteredStoriesCount is called THEN Pocket stories count only from the default category are returned`() {
val availableCategories = listOf(otherStoriesCategory, defaultStoriesCategory, anotherStoriesCategory)
var result = getFilteredStoriesCount(availableCategories, emptyList(), 2)
assertEquals(1, result.keys.size)
assertEquals(defaultStoriesCategory.name, result.entries.first().key)
assertEquals(2, result[defaultStoriesCategory.name])
fun `GIVEN no category is selected WHEN getFilteredStoriesCount is called THEN return an empty result`() {
val result = getFilteredStoriesCount(emptyList(), 1)
result = getFilteredStoriesCount(availableCategories, emptyList(), 5)
assertEquals(1, result.keys.size)
assertEquals(defaultStoriesCategory.name, result.entries.first().key)
assertEquals(3, result[defaultStoriesCategory.name])
assertTrue(result.isEmpty())
}
@Test
fun `GIVEN a category is selected WHEN getFilteredStoriesCount is called for at most the stories from this category THEN only stories count only from that category are returned`() {
val availableCategories = listOf(otherStoriesCategory, defaultStoriesCategory, anotherStoriesCategory)
var result = getFilteredStoriesCount(availableCategories, listOf(otherStoriesCategory), 2)
var result = getFilteredStoriesCount(listOf(otherStoriesCategory), 2)
assertEquals(1, result.keys.size)
assertEquals(otherStoriesCategory.name, result.entries.first().key)
assertEquals(2, result[otherStoriesCategory.name])
result = getFilteredStoriesCount(availableCategories, listOf(otherStoriesCategory), 3)
result = getFilteredStoriesCount(listOf(otherStoriesCategory), 3)
assertEquals(1, result.keys.size)
assertEquals(otherStoriesCategory.name, result.entries.first().key)
assertEquals(3, result[otherStoriesCategory.name])
}
@Test
fun `GIVEN a category is selected WHEN getFilteredStoriesCount is called for more stories than this category has THEN results topped with ones from the default category are returned`() {
val availableCategories = listOf(otherStoriesCategory, defaultStoriesCategory, anotherStoriesCategory)
val result = getFilteredStoriesCount(availableCategories, listOf(otherStoriesCategory), 5)
assertEquals(2, result.keys.size)
assertTrue(
result.keys.containsAll(
listOf(
defaultStoriesCategory.name,
otherStoriesCategory.name
)
)
)
fun `GIVEN a category is selected WHEN getFilteredStoriesCount is called for more stories than in this category THEN return only that`() {
var result = getFilteredStoriesCount(listOf(otherStoriesCategory), 4)
assertEquals(1, result.keys.size)
assertEquals(otherStoriesCategory.name, result.entries.first().key)
assertEquals(3, result[otherStoriesCategory.name])
assertEquals(2, result[defaultStoriesCategory.name])
}
@Test
fun `GIVEN two categories are selected WHEN getFilteredStoriesCount is called for at most the stories count in both THEN only stories counts from those categories are returned`() {
val availableCategories = listOf(otherStoriesCategory, defaultStoriesCategory, anotherStoriesCategory)
var result = getFilteredStoriesCount(availableCategories, listOf(otherStoriesCategory, anotherStoriesCategory), 2)
var result = getFilteredStoriesCount(listOf(otherStoriesCategory, anotherStoriesCategory), 2)
assertEquals(2, result.keys.size)
assertTrue(
result.keys.containsAll(
@ -223,7 +161,7 @@ class HomeFragmentStateTest {
assertEquals(1, result[otherStoriesCategory.name])
assertEquals(1, result[anotherStoriesCategory.name])
result = getFilteredStoriesCount(availableCategories, listOf(otherStoriesCategory, anotherStoriesCategory), 6)
result = getFilteredStoriesCount(listOf(otherStoriesCategory, anotherStoriesCategory), 6)
assertEquals(2, result.keys.size)
assertTrue(
result.keys.containsAll(
@ -238,16 +176,12 @@ class HomeFragmentStateTest {
}
@Test
fun `GIVEN two categories are selected WHEN getFilteredStoriesCount is called for more results than in those categories THEN results topped with ones from the default category are returned`() {
val availableCategories = listOf(otherStoriesCategory, defaultStoriesCategory, anotherStoriesCategory)
val result = getFilteredStoriesCount(availableCategories, listOf(otherStoriesCategory, anotherStoriesCategory), 8)
assertEquals(3, result.size)
fun `GIVEN two categories are selected WHEN getFilteredStoriesCount is called for more results than stories in both THEN only stories counts from those categories are returned`() {
val result = getFilteredStoriesCount(listOf(otherStoriesCategory, anotherStoriesCategory), 8)
assertEquals(2, result.keys.size)
assertTrue(
result.keys.containsAll(
listOf(
defaultStoriesCategory.name,
otherStoriesCategory.name,
anotherStoriesCategory.name
)
@ -255,15 +189,11 @@ class HomeFragmentStateTest {
)
assertEquals(3, result[otherStoriesCategory.name])
assertEquals(3, result[anotherStoriesCategory.name])
assertEquals(2, result[defaultStoriesCategory.name])
}
@Test
fun `GIVEN two categories are selected WHEN getFilteredStoriesCount is called for an odd number of results THEN there are more by one results from first selected category`() {
val availableCategories = listOf(otherStoriesCategory, defaultStoriesCategory, anotherStoriesCategory)
// The lastInteractedWithTimestamp is not checked in this method but the selected categories order
val result = getFilteredStoriesCount(availableCategories, listOf(otherStoriesCategory, anotherStoriesCategory), 5)
val result = getFilteredStoriesCount(listOf(otherStoriesCategory, anotherStoriesCategory), 5)
assertTrue(
result.keys.containsAll(

@ -208,10 +208,9 @@ class HomeFragmentStoreTest {
verify { any<HomeFragmentState>().getFilteredStories(POCKET_STORIES_TO_SHOW_COUNT) }
}
assertTrue(
listOf(otherStoriesCategory.copy(isSelected = true))
.containsAll(homeFragmentStore.state.pocketStoriesCategories.filter { it.isSelected })
)
val selectedCategories = homeFragmentStore.state.pocketStoriesCategories.filter { it.isSelected }
assertEquals(1, selectedCategories.size)
assertTrue(otherStoriesCategory.name === selectedCategories[0].name)
assertSame(filteredStories, homeFragmentStore.state.pocketStories)
}

Loading…
Cancel
Save