Skip to content

Commit

Permalink
For mozilla-mobile#21275 - Sort items by how many times they were act…
Browse files Browse the repository at this point in the history
…ually shown
  • Loading branch information
Mugurell authored and mergify[bot] committed Sep 28, 2021
1 parent 4596d4f commit c1f0e5a
Show file tree
Hide file tree
Showing 15 changed files with 231 additions and 17 deletions.
15 changes: 11 additions & 4 deletions app/src/main/java/org/mozilla/fenix/ext/HomeFragmentState.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}

Expand Down
5 changes: 5 additions & 0 deletions app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ class HomeFragment : Fragment() {
showSetAsDefaultBrowserCard = components.settings.shouldShowSetAsDefaultBrowserCard(),
recentTabs = emptyList(),
historyMetadata = emptyList()
),
listOf(
PocketUpdatesMiddleware(
lifecycleScope, requireComponents.core.pocketStoriesService
)
)
)
}
Expand Down
33 changes: 28 additions & 5 deletions app/src/main/java/org/mozilla/fenix/home/HomeFragmentStore.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -95,14 +97,15 @@ 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()
object RemoveCollectionsPlaceholder : HomeFragmentAction()
object RemoveSetDefaultBrowserCard : HomeFragmentAction()
}

@Suppress("ReturnCount")
@Suppress("ReturnCount", "LongMethod")
private fun homeFragmentStateReducer(
state: HomeFragmentState,
action: HomeFragmentAction
Expand Down Expand Up @@ -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)
}
}
}
Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -388,4 +389,8 @@ class SessionControlInteractor(
override fun onCategoryClick(categoryClicked: PocketRecommendedStoryCategory) {
pocketStoriesController.handleCategoryClick(categoryClicked)
}

override fun onStoriesShown(storiesShown: List<PocketRecommendedStory>) {
pocketStoriesController.handleStoriesShown(storiesShown)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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>)
}

/**
Expand Down Expand Up @@ -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))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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>)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -45,7 +46,7 @@ class PocketStoriesViewHolder(
ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed
)
composeView.setContent {
PocketStories(store, client) { interactor.onCategoryClick(it) }
PocketStories(store, client, interactor::onStoriesShown, interactor::onCategoryClick)
}
}

Expand All @@ -58,6 +59,7 @@ class PocketStoriesViewHolder(
fun PocketStories(
store: HomeFragmentStore,
client: Client,
onStoriesShown: (List<PocketRecommendedStory>) -> Unit,
onCategoryClick: (PocketRecommendedStoryCategory) -> Unit
) {
val stories = store
Expand All @@ -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()
Expand Down
50 changes: 49 additions & 1 deletion app/src/test/java/org/mozilla/fenix/ext/HomeFragmentStateTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -292,7 +339,8 @@ private fun getFakePocketStories(
url = "https://story$randomNumber.com",
imageUrl = "",
timeToRead = randomNumber,
category = category
category = category,
timesShown = index.toLong()
)
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
Original file line number Diff line number Diff line change
@@ -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))) }
}
}

0 comments on commit c1f0e5a

Please sign in to comment.