For #21275 - Sort items by how many times they were actually shown

upstream-sync
Mugurell 3 years ago committed by mergify[bot]
parent 4596d4f905
commit c1f0e5a611

@ -30,7 +30,9 @@ fun HomeFragmentState.getFilteredStories(
return pocketStoriesCategories
.find {
it.name == POCKET_STORIES_DEFAULT_CATEGORY_NAME
}?.stories?.take(neededStoriesCount) ?: emptyList()
}?.stories
?.sortedBy { it.timesShown }
?.take(neededStoriesCount) ?: emptyList()
}
val oldestSortedCategories = currentlySelectedCategories
@ -42,12 +44,17 @@ fun HomeFragmentState.getFilteredStories(
// 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?.take(neededTopups)
pocketStoriesCategories
.find { it.name == POCKET_STORIES_DEFAULT_CATEGORY_NAME }
?.stories
?.sortedBy { it.timesShown }
?.take(neededTopups)
} ?: emptyList()
return oldestSortedCategories
.flatMap { it.stories.take(filteredStoriesCount[it.name]!!) }
.plus(generalStoriesTopup)
.flatMap { category ->
category.stories.sortedBy { it.timesShown }.take(filteredStoriesCount[category.name]!!)
}.plus(generalStoriesTopup)
.take(neededStoriesCount)
}

@ -237,6 +237,11 @@ class HomeFragment : Fragment() {
showSetAsDefaultBrowserCard = components.settings.shouldShowSetAsDefaultBrowserCard(),
recentTabs = emptyList(),
historyMetadata = emptyList()
),
listOf(
PocketUpdatesMiddleware(
lifecycleScope, requireComponents.core.pocketStoriesService
)
)
)
}

@ -53,7 +53,9 @@ data class Tab(
* @property recentTabs The list of recent [RecentTab] in the [HomeFragment].
* @property recentBookmarks The list of recently saved [BookmarkNode]s to show on the [HomeFragment].
* @property historyMetadata The list of [HistoryMetadataGroup].
* @property pocketArticles The list of [PocketRecommendedStory].
* @property pocketStories Currently shown [PocketRecommendedStory]ies.
* @property pocketStoriesCategories All [PocketRecommendedStory] categories.
* Also serves as an in memory cache of all stories mapped by category allowing for quick stories filtering.
*/
data class HomeFragmentState(
val collections: List<TabCollection> = emptyList(),
@ -95,6 +97,7 @@ sealed class HomeFragmentAction : Action {
data class HistoryMetadataChange(val historyMetadata: List<HistoryMetadataGroup>) : HomeFragmentAction()
data class SelectPocketStoriesCategory(val categoryName: String) : HomeFragmentAction()
data class DeselectPocketStoriesCategory(val categoryName: String) : HomeFragmentAction()
data class PocketStoriesShown(val storiesShown: List<PocketRecommendedStory>) : HomeFragmentAction()
data class PocketStoriesChange(val pocketStories: List<PocketRecommendedStory>) : HomeFragmentAction()
data class PocketStoriesCategoriesChange(val storiesCategories: List<PocketRecommendedStoryCategory>) :
HomeFragmentAction()
@ -102,7 +105,7 @@ sealed class HomeFragmentAction : Action {
object RemoveSetDefaultBrowserCard : HomeFragmentAction()
}
@Suppress("ReturnCount")
@Suppress("ReturnCount", "LongMethod")
private fun homeFragmentStateReducer(
state: HomeFragmentState,
action: HomeFragmentAction
@ -174,10 +177,30 @@ private fun homeFragmentStateReducer(
val updatedCategoriesState = state.copy(pocketStoriesCategories = action.storiesCategories)
return updatedCategoriesState.copy(
pocketStories = updatedCategoriesState.getFilteredStories(POCKET_STORIES_TO_SHOW_COUNT)
).also {
println("just updated stories in the state")
}
)
}
is HomeFragmentAction.PocketStoriesChange -> state.copy(pocketStories = action.pocketStories)
is HomeFragmentAction.PocketStoriesShown -> {
var updatedCategories = state.pocketStoriesCategories
action.storiesShown.forEach { shownStory ->
updatedCategories = updatedCategories.map { category ->
when (category.name == shownStory.category) {
true -> {
category.copy(
stories = category.stories.map { story ->
when (story.title == shownStory.title) {
true -> story.copy(timesShown = story.timesShown.inc())
false -> story
}
}
)
}
false -> category
}
}
}
state.copy(pocketStoriesCategories = updatedCategories)
}
}
}

@ -0,0 +1,44 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.home
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import mozilla.components.lib.state.Action
import mozilla.components.lib.state.Middleware
import mozilla.components.lib.state.MiddlewareContext
import mozilla.components.service.pocket.PocketStoriesService
/**
* [HomeFragmentStore] middleware reacting in response to Pocket related [Action]s.
*/
class PocketUpdatesMiddleware(
private val coroutineScope: CoroutineScope,
private val pocketStoriesService: PocketStoriesService
) : Middleware<HomeFragmentState, HomeFragmentAction> {
override fun invoke(
context: MiddlewareContext<HomeFragmentState, HomeFragmentAction>,
next: (HomeFragmentAction) -> Unit,
action: HomeFragmentAction
) {
next(action)
// Post process actions
when (action) {
is HomeFragmentAction.PocketStoriesShown -> {
coroutineScope.launch {
pocketStoriesService.updateStoriesTimesShown(
action.storiesShown.map {
it.copy(timesShown = it.timesShown.inc())
}
)
}
}
else -> {
// no-op
}
}
}
}

@ -8,6 +8,7 @@ import mozilla.components.concept.storage.BookmarkNode
import mozilla.components.feature.tab.collections.Tab
import mozilla.components.feature.tab.collections.TabCollection
import mozilla.components.feature.top.sites.TopSite
import mozilla.components.service.pocket.PocketRecommendedStory
import org.mozilla.fenix.browser.browsingmode.BrowsingMode
import org.mozilla.fenix.components.tips.Tip
import org.mozilla.fenix.historymetadata.HistoryMetadataGroup
@ -388,4 +389,8 @@ class SessionControlInteractor(
override fun onCategoryClick(categoryClicked: PocketRecommendedStoryCategory) {
pocketStoriesController.handleCategoryClick(categoryClicked)
}
override fun onStoriesShown(storiesShown: List<PocketRecommendedStory>) {
pocketStoriesController.handleStoriesShown(storiesShown)
}
}

@ -480,7 +480,8 @@ private fun getFakePocketStories(limit: Int = 1): List<PocketRecommendedStory> {
url = "https://story$randomNumber.com",
imageUrl = "",
timeToRead = randomNumber,
category = "Category #$randomNumber"
category = "Category #$randomNumber",
timesShown = randomNumber.toLong()
)
)
}

@ -7,6 +7,7 @@ package org.mozilla.fenix.home.sessioncontrol.viewholders.pocket
import org.mozilla.fenix.home.HomeFragmentAction
import org.mozilla.fenix.home.HomeFragmentStore
import mozilla.components.lib.state.Store
import mozilla.components.service.pocket.PocketRecommendedStory
/**
* Contract for how all user interactions with the Pocket recommended stories feature are to be handled.
@ -18,6 +19,13 @@ interface PocketStoriesController {
* @param categoryClicked the just clicked [PocketRecommendedStoryCategory].
*/
fun handleCategoryClick(categoryClicked: PocketRecommendedStoryCategory): Unit
/**
* Callback to decide what should happen as an effect of a new list of stories being shown.
*
* @param storiesShown the new list of [PocketRecommendedStory]es shown to the user.
*/
fun handleStoriesShown(storiesShown: List<PocketRecommendedStory>)
}
/**
@ -62,4 +70,8 @@ internal class DefaultPocketStoriesController(
// Finally update the selection.
homeStore.dispatch(HomeFragmentAction.SelectPocketStoriesCategory(categoryClicked.name))
}
override fun handleStoriesShown(storiesShown: List<PocketRecommendedStory>) {
homeStore.dispatch(HomeFragmentAction.PocketStoriesShown(storiesShown))
}
}

@ -4,6 +4,8 @@
package org.mozilla.fenix.home.sessioncontrol.viewholders.pocket
import mozilla.components.service.pocket.PocketRecommendedStory
/**
* Contract for all possible user interactions with the Pocket recommended stories feature.
*/
@ -14,4 +16,11 @@ interface PocketStoriesInteractor {
* @param categoryClicked the just clicked [PocketRecommendedStoryCategory].
*/
fun onCategoryClick(categoryClicked: PocketRecommendedStoryCategory)
/**
* Callback for then new stories are shown to the user.
*
* @param storiesShown the new list of [PocketRecommendedStory]es shown to the user.
*/
fun onStoriesShown(storiesShown: List<PocketRecommendedStory>)
}

@ -11,6 +11,7 @@ import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.platform.ViewCompositionStrategy
@ -45,7 +46,7 @@ class PocketStoriesViewHolder(
ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed
)
composeView.setContent {
PocketStories(store, client) { interactor.onCategoryClick(it) }
PocketStories(store, client, interactor::onStoriesShown, interactor::onCategoryClick)
}
}
@ -58,6 +59,7 @@ class PocketStoriesViewHolder(
fun PocketStories(
store: HomeFragmentStore,
client: Client,
onStoriesShown: (List<PocketRecommendedStory>) -> Unit,
onCategoryClick: (PocketRecommendedStoryCategory) -> Unit
) {
val stories = store
@ -66,6 +68,14 @@ fun PocketStories(
val categories = store
.observeAsComposableState { state -> state.pocketStoriesCategories }.value
LaunchedEffect(stories) {
// We should report back when a certain story is actually being displayed.
// Cannot do it reliably so for now we'll just mass report everything as being displayed.
stories?.let {
onStoriesShown(it)
}
}
ExpandableCard(
Modifier
.fillMaxWidth()

@ -7,6 +7,7 @@ package org.mozilla.fenix.ext
import mozilla.components.service.pocket.PocketRecommendedStory
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertSame
import org.junit.Assert.assertTrue
import org.junit.Test
import org.mozilla.fenix.home.HomeFragmentState
@ -275,6 +276,52 @@ class HomeFragmentStateTest {
assertEquals(3, result[otherStoriesCategory.name])
assertEquals(2, result[anotherStoriesCategory.name])
}
@Test
fun `GIVEN two categories selected with more than needed stories WHEN getFilteredStories is called THEN the results are sorted in the order of least shown`() {
val firstCategory = PocketRecommendedStoryCategory(
"first", getFakePocketStories(3, "first"), true, 222
).run {
// Avoid the first item also being the oldest to eliminate a potential bug in code
// that would still get the expected result.
copy(
stories = stories.mapIndexed { index, story ->
when (index) {
0 -> story.copy(timesShown = 333)
1 -> story.copy(timesShown = 0)
else -> story.copy(timesShown = 345)
}
}
)
}
val secondCategory = PocketRecommendedStoryCategory(
"second", getFakePocketStories(3, "second"), true, 0
).run {
// Avoid the first item also being the oldest to eliminate a potential bug in code
// that would still get the expected result.
copy(
stories = stories.mapIndexed { index, story ->
when (index) {
0 -> story.copy(timesShown = 222)
1 -> story.copy(timesShown = 111)
else -> story.copy(timesShown = 11)
}
}
)
}
val homeState = HomeFragmentState(pocketStoriesCategories = listOf(firstCategory, secondCategory))
val result = homeState.getFilteredStories(6)
assertEquals(6, result.size)
assertSame(secondCategory.stories[2], result.first())
assertSame(secondCategory.stories[1], result[1])
assertSame(secondCategory.stories[0], result[2])
assertSame(firstCategory.stories[1], result[3])
assertSame(firstCategory.stories[0], result[4])
assertSame(firstCategory.stories[2], result[5])
}
}
private fun getFakePocketStories(
@ -292,7 +339,8 @@ private fun getFakePocketStories(
url = "https://story$randomNumber.com",
imageUrl = "",
timeToRead = randomNumber,
category = category
category = category,
timesShown = index.toLong()
)
)
}

@ -245,7 +245,7 @@ class HomeFragmentStoreTest {
@Test
fun `Test updating the list of Pocket recommended stories`() = runBlocking {
val story1 = PocketRecommendedStory("title1", "publisher", "url", "imageUrl", 1, "category")
val story1 = PocketRecommendedStory("title1", "url", "imageUrl", "publisher", "category", 1, 1)
val story2 = story1.copy("title2")
homeFragmentStore = HomeFragmentStore(HomeFragmentState())

@ -0,0 +1,37 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.fenix.home
import io.mockk.coVerify
import io.mockk.mockk
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.TestCoroutineScope
import mozilla.components.service.pocket.PocketRecommendedStory
import mozilla.components.service.pocket.PocketStoriesService
import mozilla.components.support.test.ext.joinBlocking
import org.junit.Test
class PocketUpdatesMiddlewareTest {
@ExperimentalCoroutinesApi
@Test
fun `WHEN PocketStoriesShown is dispatched THEN update PocketStoriesService`() {
val story1 = PocketRecommendedStory("title", "url1", "imageUrl", "publisher", "category", 0, timesShown = 0)
val story2 = story1.copy("title2", "url2")
val story3 = story1.copy("title3", "url3")
val coroutineScope = TestCoroutineScope()
val pocketService: PocketStoriesService = mockk(relaxed = true)
val pocketMiddleware = PocketUpdatesMiddleware(coroutineScope, pocketService)
val homeStore = HomeFragmentStore(
HomeFragmentState(
pocketStories = listOf(story1, story2, story3)
),
listOf(pocketMiddleware)
)
homeStore.dispatch(HomeFragmentAction.PocketStoriesShown(listOf(story2))).joinBlocking()
coVerify { pocketService.updateStoriesTimesShown(listOf(story2.copy(timesShown = 1))) }
}
}

@ -68,7 +68,7 @@ class SessionControlViewTest {
@Test
fun `GIVEN pocketArticles WHEN calling shouldShowHomeOnboardingDialog THEN show the dialog `() {
val pocketArticles = listOf(PocketRecommendedStory("", "", "", "", 0, ""))
val pocketArticles = listOf(PocketRecommendedStory("", "", "", "", "", 0, 0))
val settings: Settings = mockk()
every { settings.hasShownHomeOnboardingDialog } returns false
@ -80,7 +80,7 @@ class SessionControlViewTest {
@Test
fun `GIVEN the home onboading dialog has been shown before WHEN calling shouldShowHomeOnboardingDialog THEN DO NOT showthe dialog `() {
val pocketArticles = listOf(PocketRecommendedStory("", "", "", "", 0, ""))
val pocketArticles = listOf(PocketRecommendedStory("", "", "", "", "", 0, 0))
val settings: Settings = mockk()
every { settings.hasShownHomeOnboardingDialog } returns true
@ -228,7 +228,7 @@ class SessionControlViewTest {
val recentBookmarks = listOf<BookmarkNode>()
val recentTabs = emptyList<RecentTab.Tab>()
val historyMetadata = emptyList<HistoryMetadataGroup>()
val pocketArticles = listOf(PocketRecommendedStory("", "", "", "", 0, ""))
val pocketArticles = listOf(PocketRecommendedStory("", "", "", "", "", 1, 1))
val context = spyk(testContext)
val settings: Settings = mockk()

@ -4,8 +4,10 @@
package org.mozilla.fenix.home.sessioncontrol.viewholders.pocket
import io.mockk.mockk
import io.mockk.spyk
import io.mockk.verify
import mozilla.components.service.pocket.PocketRecommendedStory
import org.junit.Test
import org.mozilla.fenix.home.HomeFragmentAction
import org.mozilla.fenix.home.HomeFragmentState
@ -90,4 +92,15 @@ class DefaultPocketStoriesControllerTest {
verify(exactly = 0) { store.dispatch(HomeFragmentAction.DeselectPocketStoriesCategory(oldestSelectedCategory.name)) }
verify { store.dispatch(HomeFragmentAction.SelectPocketStoriesCategory(newSelectedCategory.name)) }
}
@Test
fun `WHEN new stories are shown THEN update the State`() {
val store = spyk(HomeFragmentStore())
val controller = DefaultPocketStoriesController(store)
val storiesShown: List<PocketRecommendedStory> = mockk()
controller.handleStoriesShown(storiesShown)
verify { store.dispatch(HomeFragmentAction.PocketStoriesShown(storiesShown)) }
}
}

@ -3,5 +3,5 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
object AndroidComponents {
const val VERSION = "94.0.20210927143215"
const val VERSION = "94.0.20210928143135"
}

Loading…
Cancel
Save