From 5b069b2ece8c2a01eb0b28b066de33ae24446c6a Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Wed, 3 Nov 2021 01:00:01 -0400 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/22211 - Use Hero images for Recent Bookmarks --- .../mozilla/fenix/components/Components.kt | 3 +- .../org/mozilla/fenix/components/UseCases.kt | 6 +- .../components/bookmarks/BookmarksUseCase.kt | 55 ++++++-- .../mozilla/fenix/home/HomeFragmentStore.kt | 9 +- .../recentbookmarks/RecentBookmarksFeature.kt | 28 ++-- .../controller/RecentBookmarksController.kt | 6 +- .../interactor/RecentBookmarksInteractor.kt | 4 +- .../recentbookmarks/view/RecentBookmarks.kt | 128 +++++++++--------- .../SessionControlInteractor.kt | 10 +- .../home/sessioncontrol/SessionControlView.kt | 4 +- .../fenix/components/TestComponents.kt | 3 +- .../org/mozilla/fenix/components/TestCore.kt | 2 + .../bookmarks/BookmarksUseCaseTest.kt | 73 +++++++--- .../DefaultSessionControlControllerTest.kt | 4 +- .../fenix/home/HomeFragmentStoreTest.kt | 6 +- .../home/SessionControlInteractorTest.kt | 18 +-- .../DefaultRecentBookmarksControllerTest.kt | 12 +- .../RecentBookmarksFeatureTest.kt | 10 +- .../sessioncontrol/SessionControlViewTest.kt | 19 ++- 19 files changed, 232 insertions(+), 168 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt index 954f27cfd3..a5ca685525 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -74,7 +74,8 @@ class Components(private val context: Context) { core.store, core.webAppShortcutManager, core.topSitesStorage, - core.bookmarksStorage + core.bookmarksStorage, + core.historyStorage ) } diff --git a/app/src/main/java/org/mozilla/fenix/components/UseCases.kt b/app/src/main/java/org/mozilla/fenix/components/UseCases.kt index 63ca8f2113..efd7bbd52e 100644 --- a/app/src/main/java/org/mozilla/fenix/components/UseCases.kt +++ b/app/src/main/java/org/mozilla/fenix/components/UseCases.kt @@ -8,6 +8,7 @@ import android.content.Context import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.Engine import mozilla.components.concept.storage.BookmarksStorage +import mozilla.components.concept.storage.HistoryStorage import mozilla.components.feature.app.links.AppLinksUseCases import mozilla.components.feature.contextmenu.ContextMenuUseCases import mozilla.components.feature.downloads.DownloadsUseCases @@ -38,7 +39,8 @@ class UseCases( private val store: BrowserStore, private val shortcutManager: WebAppShortcutManager, private val topSitesStorage: TopSitesStorage, - private val bookmarksStorage: BookmarksStorage + private val bookmarksStorage: BookmarksStorage, + private val historyStorage: HistoryStorage ) { /** * Use cases that provide engine interactions for a given browser session. @@ -98,5 +100,5 @@ class UseCases( /** * Use cases that provide bookmark management. */ - val bookmarksUseCases by lazyMonitored { BookmarksUseCase(bookmarksStorage) } + val bookmarksUseCases by lazyMonitored { BookmarksUseCase(bookmarksStorage, historyStorage) } } diff --git a/app/src/main/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCase.kt b/app/src/main/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCase.kt index 895dda3be4..a86d7fa724 100644 --- a/app/src/main/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCase.kt +++ b/app/src/main/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCase.kt @@ -6,14 +6,18 @@ package org.mozilla.fenix.components.bookmarks import androidx.annotation.WorkerThread import mozilla.appservices.places.BookmarkRoot -import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarksStorage +import mozilla.components.concept.storage.HistoryStorage +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import java.util.concurrent.TimeUnit /** * Use cases that allow for modifying and retrieving bookmarks. */ -class BookmarksUseCase(storage: BookmarksStorage) { +class BookmarksUseCase( + bookmarksStorage: BookmarksStorage, + historyStorage: HistoryStorage, +) { class AddBookmarksUseCase internal constructor(private val storage: BookmarksStorage) { @@ -40,29 +44,62 @@ class BookmarksUseCase(storage: BookmarksStorage) { } } + /** + * Uses for retrieving recently added bookmarks. + * + * @param bookmarksStorage [BookmarksStorage] to retrieve the bookmark data. + * @param historyStorage Optional [HistoryStorage] to retrieve the preview image of a visited + * page associated with a bookmark. + */ class RetrieveRecentBookmarksUseCase internal constructor( - private val storage: BookmarksStorage + private val bookmarksStorage: BookmarksStorage, + private val historyStorage: HistoryStorage? = null ) { /** * Retrieves a list of recently added bookmarks, if any, up to maximum. + * + * @param count The number of recent bookmarks to return. + * @param maxAgeInMs The maximum age (ms) of a recently added bookmark to return. + * @return a list of [RecentBookmark] that were added no older than specify by [maxAgeInMs], + * if any, up to a number specified by [count]. */ @WorkerThread suspend operator fun invoke( count: Int = DEFAULT_BOOKMARKS_TO_RETRIEVE, maxAgeInMs: Long = TimeUnit.DAYS.toMillis(DEFAULT_BOOKMARKS_DAYS_AGE_TO_RETRIEVE) - ): List { - return storage.getRecentBookmarks( - count, - maxAgeInMs + ): List { + val currentTime = System.currentTimeMillis() + + // Fetch visit information within the time range of now and the specified maximum age. + val history = historyStorage?.getDetailedVisits( + start = currentTime - maxAgeInMs, + end = currentTime ) + + return bookmarksStorage + .getRecentBookmarks(count, maxAgeInMs) + .map { bookmark -> + RecentBookmark( + title = bookmark.title, + url = bookmark.url, + previewImageUrl = history?.find { bookmark.url == it.url }?.previewImageUrl + ) + } } } - val addBookmark by lazy { AddBookmarksUseCase(storage) } - val retrieveRecentBookmarks by lazy { RetrieveRecentBookmarksUseCase(storage) } + val addBookmark by lazy { AddBookmarksUseCase(bookmarksStorage) } + val retrieveRecentBookmarks by lazy { + RetrieveRecentBookmarksUseCase( + bookmarksStorage, + historyStorage + ) + } companion object { + // Number of recent bookmarks to retrieve. const val DEFAULT_BOOKMARKS_TO_RETRIEVE = 4 + // The maximum age in days of a recent bookmarks to retrieve. const val DEFAULT_BOOKMARKS_DAYS_AGE_TO_RETRIEVE = 10L } } diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragmentStore.kt index c15cb95e9a..a76dbcf126 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragmentStore.kt @@ -18,10 +18,11 @@ import org.mozilla.fenix.components.tips.Tip import org.mozilla.fenix.ext.getFilteredStories import org.mozilla.fenix.ext.recentSearchGroup import org.mozilla.fenix.historymetadata.HistoryMetadataGroup -import org.mozilla.fenix.home.recenttabs.RecentTab 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 +import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.recenttabs.RecentTab.SearchGroup /** @@ -70,7 +71,7 @@ data class HomeFragmentState( val showCollectionPlaceholder: Boolean = false, val showSetAsDefaultBrowserCard: Boolean = false, val recentTabs: List = emptyList(), - val recentBookmarks: List = emptyList(), + val recentBookmarks: List = emptyList(), val historyMetadata: List = emptyList(), val pocketStories: List = emptyList(), val pocketStoriesCategories: List = emptyList(), @@ -85,7 +86,7 @@ sealed class HomeFragmentAction : Action { val tip: Tip? = null, val showCollectionPlaceholder: Boolean, val recentTabs: List, - val recentBookmarks: List, + val recentBookmarks: List, val historyMetadata: List ) : HomeFragmentAction() @@ -98,7 +99,7 @@ sealed class HomeFragmentAction : Action { data class TopSitesChange(val topSites: List) : HomeFragmentAction() data class RemoveTip(val tip: Tip) : HomeFragmentAction() data class RecentTabsChange(val recentTabs: List) : HomeFragmentAction() - data class RecentBookmarksChange(val recentBookmarks: List) : HomeFragmentAction() + data class RecentBookmarksChange(val recentBookmarks: List) : HomeFragmentAction() data class HistoryMetadataChange(val historyMetadata: List) : HomeFragmentAction() data class DisbandSearchGroupAction(val searchTerm: String) : HomeFragmentAction() data class SelectPocketStoriesCategory(val categoryName: String) : HomeFragmentAction() diff --git a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/RecentBookmarksFeature.kt b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/RecentBookmarksFeature.kt index 26c45c708f..fca0d31f7e 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/RecentBookmarksFeature.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/RecentBookmarksFeature.kt @@ -16,14 +16,14 @@ import org.mozilla.fenix.home.HomeFragmentAction import org.mozilla.fenix.home.HomeFragmentStore /** - * View-bound feature that retrieves a list of recently added [BookmarkNode]s and dispatches - * updates to the [HomeFragmentStore]. + * View-bound feature that retrieves a list of recently added [BookmarkNode]s and dispatches + * updates to the [HomeFragmentStore]. * - * @param homeStore the [HomeFragmentStore] - * @param bookmarksUseCase the [BookmarksUseCase] for retrieving the list of recently saved -* bookmarks from storage. - * @param scope the [CoroutineScope] used to fetch the bookmarks list - * @param ioDispatcher the [CoroutineDispatcher] for performing read/write operations. + * @param homeStore the [HomeFragmentStore] + * @param bookmarksUseCase the [BookmarksUseCase] for retrieving the list of recently saved + * bookmarks from storage. + * @param scope the [CoroutineScope] used to fetch the bookmarks list + * @param ioDispatcher the [CoroutineDispatcher] for performing read/write operations. */ class RecentBookmarksFeature( private val homeStore: HomeFragmentStore, @@ -36,7 +36,6 @@ class RecentBookmarksFeature( override fun start() { job = scope.launch(ioDispatcher) { val bookmarks = bookmarksUseCase.retrieveRecentBookmarks() - homeStore.dispatch(HomeFragmentAction.RecentBookmarksChange(bookmarks)) } } @@ -45,3 +44,16 @@ class RecentBookmarksFeature( job?.cancel() } } + +/** + * A bookmark that was recently added. + * + * @param title The title of the bookmark. + * @param url The url of the bookmark. + * @param previewImageUrl A preview image of the page (a.k.a. the hero image), if available. + */ +data class RecentBookmark( + val title: String? = null, + val url: String? = null, + val previewImageUrl: String? = null +) diff --git a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt index 6c2346baac..f18bacf281 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt @@ -10,13 +10,13 @@ import androidx.navigation.NavController import mozilla.appservices.places.BookmarkRoot import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL -import mozilla.components.concept.storage.BookmarkNode import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.home.HomeFragmentDirections +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recentbookmarks.interactor.RecentBookmarksInteractor /** @@ -28,7 +28,7 @@ interface RecentBookmarksController { /** * @see [RecentBookmarksInteractor.onRecentBookmarkClicked] */ - fun handleBookmarkClicked(bookmark: BookmarkNode) + fun handleBookmarkClicked(bookmark: RecentBookmark) /** * @see [RecentBookmarksInteractor.onShowAllBookmarksClicked] @@ -44,7 +44,7 @@ class DefaultRecentBookmarksController( private val navController: NavController ) : RecentBookmarksController { - override fun handleBookmarkClicked(bookmark: BookmarkNode) { + override fun handleBookmarkClicked(bookmark: RecentBookmark) { dismissSearchDialogIfDisplayed() activity.openToBrowserAndLoad( searchTermOrURL = bookmark.url!!, diff --git a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/interactor/RecentBookmarksInteractor.kt b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/interactor/RecentBookmarksInteractor.kt index a27a143f2c..4040c24569 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/interactor/RecentBookmarksInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/interactor/RecentBookmarksInteractor.kt @@ -4,7 +4,7 @@ package org.mozilla.fenix.home.recentbookmarks.interactor -import mozilla.components.concept.storage.BookmarkNode +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.sessioncontrol.SessionControlInteractor /** @@ -18,7 +18,7 @@ interface RecentBookmarksInteractor { * * @param bookmark The bookmark that will be opened. */ - fun onRecentBookmarkClicked(bookmark: BookmarkNode) + fun onRecentBookmarkClicked(bookmark: RecentBookmark) /** * Navigates to bookmark list. Called when an user clicks on the "Show all" button for diff --git a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarks.kt b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarks.kt index 777de3c5a8..e6dd29cd7b 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarks.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarks.kt @@ -34,22 +34,22 @@ import androidx.compose.ui.unit.sp import mozilla.components.browser.icons.compose.Loader import mozilla.components.browser.icons.compose.Placeholder import mozilla.components.browser.icons.compose.WithIcon -import mozilla.components.concept.storage.BookmarkNode -import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.ui.colors.PhotonColors import org.mozilla.fenix.components.components +import org.mozilla.fenix.compose.Image +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.theme.FirefoxTheme /** * A list of recent bookmarks. * - * @param bookmarks List of [BookmarkNode]s to display. + * @param bookmarks List of [RecentBookmark]s to display. * @param onRecentBookmarkClick Invoked when the user clicks on a recent bookmark. */ @Composable fun RecentBookmarks( - bookmarks: List, - onRecentBookmarkClick: (BookmarkNode) -> Unit = {} + bookmarks: List, + onRecentBookmarkClick: (RecentBookmark) -> Unit = {} ) { LazyRow( contentPadding = PaddingValues(horizontal = 16.dp), @@ -67,13 +67,13 @@ fun RecentBookmarks( /** * A recent bookmark item. * - * @param bookmark The [BookmarkNode] to display. + * @param bookmark The [RecentBookmark] to display. * @param onRecentBookmarkClick Invoked when the user clicks on the recent bookmark item. */ @Composable private fun RecentBookmarkItem( - bookmark: BookmarkNode, - onRecentBookmarkClick: (BookmarkNode) -> Unit = {} + bookmark: RecentBookmark, + onRecentBookmarkClick: (RecentBookmark) -> Unit = {} ) { Column( modifier = Modifier @@ -86,36 +86,7 @@ private fun RecentBookmarkItem( .height(96.dp), elevation = 6.dp ) { - if (bookmark.url != null) { - components.core.icons.Loader(bookmark.url!!) { - Placeholder { - Box( - modifier = Modifier.background( - color = when (isSystemInDarkTheme()) { - true -> PhotonColors.DarkGrey30 - false -> PhotonColors.LightGrey30 - } - ) - ) - } - - WithIcon { icon -> - Box( - modifier = Modifier.size(36.dp), - contentAlignment = Alignment.Center - ) { - Image( - painter = icon.painter, - contentDescription = null, - modifier = Modifier - .size(36.dp) - .clip(RoundedCornerShape(8.dp)), - contentScale = ContentScale.Fit - ) - } - } - } - } + RecentBookmarkImage(bookmark) } Spacer(modifier = Modifier.height(8.dp)) @@ -130,51 +101,76 @@ private fun RecentBookmarkItem( } } +@Composable +private fun RecentBookmarkImage(bookmark: RecentBookmark) { + when { + !bookmark.previewImageUrl.isNullOrEmpty() -> { + Image( + url = bookmark.previewImageUrl, + modifier = Modifier + .size(156.dp, 96.dp), + targetSize = 156.dp, + contentScale = ContentScale.Crop + ) + } + !bookmark.url.isNullOrEmpty() -> { + components.core.icons.Loader(bookmark.url) { + Placeholder { + Box( + modifier = Modifier.background( + color = when (isSystemInDarkTheme()) { + true -> PhotonColors.DarkGrey30 + false -> PhotonColors.LightGrey30 + } + ) + ) + } + + WithIcon { icon -> + Box( + modifier = Modifier.size(36.dp), + contentAlignment = Alignment.Center + ) { + Image( + painter = icon.painter, + contentDescription = null, + modifier = Modifier + .size(36.dp) + .clip(RoundedCornerShape(8.dp)), + contentScale = ContentScale.Fit + ) + } + } + } + } + } +} + @Composable @Preview private fun RecentBookmarksPreview() { FirefoxTheme { RecentBookmarks( bookmarks = listOf( - BookmarkNode( - type = BookmarkNodeType.ITEM, - guid = "1", - parentGuid = null, - position = null, + RecentBookmark( title = "Other Bookmark Title", url = "https://www.example.com", - dateAdded = 0, - children = null + previewImageUrl = null ), - BookmarkNode( - type = BookmarkNodeType.ITEM, - guid = "2", - parentGuid = null, - position = null, + RecentBookmark( title = "Other Bookmark Title", url = "https://www.example.com", - dateAdded = 0, - children = null + previewImageUrl = null ), - BookmarkNode( - type = BookmarkNodeType.ITEM, - guid = "3", - parentGuid = null, - position = null, + RecentBookmark( title = "Other Bookmark Title", url = "https://www.example.com", - dateAdded = 0, - children = null + previewImageUrl = null ), - BookmarkNode( - type = BookmarkNodeType.ITEM, - guid = "4", - parentGuid = null, - position = null, + RecentBookmark( title = "Other Bookmark Title", url = "https://www.example.com", - dateAdded = 0, - children = null + previewImageUrl = null ) ) ) diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlInteractor.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlInteractor.kt index d8844b405a..43e3445737 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlInteractor.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlInteractor.kt @@ -4,7 +4,6 @@ package org.mozilla.fenix.home.sessioncontrol -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 @@ -15,13 +14,14 @@ import org.mozilla.fenix.historymetadata.HistoryMetadataGroup import org.mozilla.fenix.historymetadata.controller.HistoryMetadataController import org.mozilla.fenix.historymetadata.interactor.HistoryMetadataInteractor import org.mozilla.fenix.home.HomeFragmentState +import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory +import org.mozilla.fenix.home.pocket.PocketStoriesController +import org.mozilla.fenix.home.pocket.PocketStoriesInteractor +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recentbookmarks.controller.RecentBookmarksController import org.mozilla.fenix.home.recentbookmarks.interactor.RecentBookmarksInteractor import org.mozilla.fenix.home.recenttabs.controller.RecentTabController import org.mozilla.fenix.home.recenttabs.interactor.RecentTabInteractor -import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory -import org.mozilla.fenix.home.pocket.PocketStoriesController -import org.mozilla.fenix.home.pocket.PocketStoriesInteractor /** * Interface for tab related actions in the [SessionControlInteractor]. @@ -373,7 +373,7 @@ class SessionControlInteractor( recentTabController.handleRecentTabShowAllClicked() } - override fun onRecentBookmarkClicked(bookmark: BookmarkNode) { + override fun onRecentBookmarkClicked(bookmark: RecentBookmark) { recentBookmarksController.handleBookmarkClicked(bookmark) } diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt index f58da37f96..94db921708 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlView.kt @@ -10,7 +10,6 @@ import androidx.lifecycle.LifecycleOwner import androidx.recyclerview.widget.ItemTouchHelper import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.RecyclerView -import mozilla.components.concept.storage.BookmarkNode import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.top.sites.TopSite import mozilla.components.service.pocket.PocketRecommendedStory @@ -22,6 +21,7 @@ import org.mozilla.fenix.home.HomeFragmentState import org.mozilla.fenix.home.HomeFragmentStore import org.mozilla.fenix.home.Mode import org.mozilla.fenix.home.OnboardingState +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.onboarding.JumpBackInCFRDialog import org.mozilla.fenix.utils.Settings @@ -35,7 +35,7 @@ internal fun normalModeAdapterItems( collections: List, expandedCollections: Set, tip: Tip?, - recentBookmarks: List, + recentBookmarks: List, showCollectionsPlaceholder: Boolean, showSetAsDefaultBrowserCard: Boolean, recentTabs: List, diff --git a/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt b/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt index 7d0c8e2d76..3529c3519e 100644 --- a/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt +++ b/app/src/test/java/org/mozilla/fenix/components/TestComponents.kt @@ -24,7 +24,8 @@ class TestComponents(private val context: Context) : Components(context) { core.store, core.webAppShortcutManager, core.topSitesStorage, - core.bookmarksStorage + core.bookmarksStorage, + core.historyStorage ) } override val intentProcessors by lazy { mockk(relaxed = true) } diff --git a/app/src/test/java/org/mozilla/fenix/components/TestCore.kt b/app/src/test/java/org/mozilla/fenix/components/TestCore.kt index c7ac58e9a9..b25a15a12f 100644 --- a/app/src/test/java/org/mozilla/fenix/components/TestCore.kt +++ b/app/src/test/java/org/mozilla/fenix/components/TestCore.kt @@ -9,6 +9,7 @@ import io.mockk.every import io.mockk.mockk import mozilla.components.browser.state.store.BrowserStore import mozilla.components.browser.storage.sync.PlacesBookmarksStorage +import mozilla.components.browser.storage.sync.PlacesHistoryStorage import mozilla.components.browser.thumbnails.storage.ThumbnailStorage import mozilla.components.concept.base.crash.CrashReporting import mozilla.components.concept.engine.Engine @@ -32,4 +33,5 @@ class TestCore(context: Context, crashReporter: CrashReporting) : Core( override val thumbnailStorage = mockk() override val topSitesStorage = mockk() override val bookmarksStorage = mockk() + override val historyStorage = mockk() } diff --git a/app/src/test/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCaseTest.kt b/app/src/test/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCaseTest.kt index 2e18ad64e7..e099564513 100644 --- a/app/src/test/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCaseTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/bookmarks/BookmarksUseCaseTest.kt @@ -11,23 +11,29 @@ import io.mockk.mockk import kotlinx.coroutines.test.runBlockingTest import mozilla.appservices.places.BookmarkRoot import mozilla.components.concept.storage.BookmarkNode +import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.concept.storage.BookmarksStorage +import mozilla.components.concept.storage.HistoryStorage +import mozilla.components.concept.storage.VisitInfo +import mozilla.components.concept.storage.VisitType import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import java.util.concurrent.TimeUnit class BookmarksUseCaseTest { @Test fun `WHEN adding existing bookmark THEN no new item is stored`() = runBlockingTest { - val storage = mockk() + val bookmarksStorage = mockk() + val historyStorage = mockk() val bookmarkNode = mockk() - val useCase = BookmarksUseCase(storage) + val useCase = BookmarksUseCase(bookmarksStorage, historyStorage) every { bookmarkNode.url }.answers { "https://mozilla.org" } - coEvery { storage.getBookmarksWithUrl(any()) }.coAnswers { listOf(bookmarkNode) } + coEvery { bookmarksStorage.getBookmarksWithUrl(any()) }.coAnswers { listOf(bookmarkNode) } val result = useCase.addBookmark("https://mozilla.org", "Mozilla") @@ -36,26 +42,49 @@ class BookmarksUseCaseTest { @Test fun `WHEN adding bookmark THEN new item is stored`() = runBlockingTest { - val storage = mockk(relaxed = true) - val useCase = BookmarksUseCase(storage) + val bookmarksStorage = mockk(relaxed = true) + val historyStorage = mockk(relaxed = true) + val useCase = BookmarksUseCase(bookmarksStorage, historyStorage) - coEvery { storage.getBookmarksWithUrl(any()) }.coAnswers { emptyList() } + coEvery { bookmarksStorage.getBookmarksWithUrl(any()) }.coAnswers { emptyList() } val result = useCase.addBookmark("https://mozilla.org", "Mozilla") assertTrue(result) - coVerify { storage.addItem(BookmarkRoot.Mobile.id, "https://mozilla.org", "Mozilla", null) } + coVerify { bookmarksStorage.addItem(BookmarkRoot.Mobile.id, "https://mozilla.org", "Mozilla", null) } } @Test fun `WHEN recently saved bookmarks exist THEN retrieve the list from storage`() = runBlockingTest { - val storage = mockk(relaxed = true) - val useCase = BookmarksUseCase(storage) - val bookmarkNode = mockk() + val bookmarksStorage = mockk(relaxed = true) + val historyStorage = mockk(relaxed = true) + val useCase = BookmarksUseCase(bookmarksStorage, historyStorage) + + val visitInfo = VisitInfo( + url = "https://www.firefox.com", + title = "firefox", + visitTime = 2, + visitType = VisitType.LINK, + previewImageUrl = "http://firefox.com/image1" + ) + val bookmarkNode = BookmarkNode( + BookmarkNodeType.ITEM, + "987", + "123", + 2, + "Firefox", + "https://www.firefox.com", + 0, + null + ) coEvery { - storage.getRecentBookmarks( + historyStorage.getDetailedVisits(any(), any()) + }.coAnswers { listOf(visitInfo) } + + coEvery { + bookmarksStorage.getRecentBookmarks( any(), any(), any() @@ -64,10 +93,19 @@ class BookmarksUseCaseTest { val result = useCase.retrieveRecentBookmarks(BookmarksUseCase.DEFAULT_BOOKMARKS_TO_RETRIEVE, 22) - assertEquals(listOf(bookmarkNode), result) + assertEquals( + listOf( + RecentBookmark( + title = bookmarkNode.title, + url = bookmarkNode.url, + previewImageUrl = visitInfo.previewImageUrl + ) + ), + result + ) coVerify { - storage.getRecentBookmarks( + bookmarksStorage.getRecentBookmarks( BookmarksUseCase.DEFAULT_BOOKMARKS_TO_RETRIEVE, 22, any() @@ -77,17 +115,18 @@ class BookmarksUseCaseTest { @Test fun `WHEN there are no recently saved bookmarks THEN retrieve the empty list from storage`() = runBlockingTest { - val storage = mockk(relaxed = true) - val useCase = BookmarksUseCase(storage) + val bookmarksStorage = mockk(relaxed = true) + val historyStorage = mockk(relaxed = true) + val useCase = BookmarksUseCase(bookmarksStorage, historyStorage) - coEvery { storage.getRecentBookmarks(any(), any(), any()) }.coAnswers { listOf() } + coEvery { bookmarksStorage.getRecentBookmarks(any(), any(), any()) }.coAnswers { listOf() } val result = useCase.retrieveRecentBookmarks(BookmarksUseCase.DEFAULT_BOOKMARKS_TO_RETRIEVE) assertEquals(listOf(), result) coVerify { - storage.getRecentBookmarks( + bookmarksStorage.getRecentBookmarks( BookmarksUseCase.DEFAULT_BOOKMARKS_TO_RETRIEVE, TimeUnit.DAYS.toMillis(BookmarksUseCase.DEFAULT_BOOKMARKS_DAYS_AGE_TO_RETRIEVE), any() diff --git a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt index 749f568406..07a7d1d579 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -29,7 +29,6 @@ import mozilla.components.browser.state.state.recover.RecoverableTab import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import mozilla.components.browser.state.store.BrowserStore import mozilla.components.concept.engine.Engine -import mozilla.components.concept.storage.BookmarkNode import mozilla.components.feature.session.SessionUseCases import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.tabs.TabsUseCases @@ -56,6 +55,7 @@ import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.tips.Tip import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.sessioncontrol.DefaultSessionControlController import org.mozilla.fenix.settings.SupportUtils @@ -856,7 +856,7 @@ class DefaultSessionControlControllerTest { @Test fun `WHEN handleReportSessionMetrics is called AND there is at least one recent bookmark THEN report Event#RecentBookmarkCount(1)`() { - val recentBookmark: BookmarkNode = mockk(relaxed = true) + val recentBookmark: RecentBookmark = mockk(relaxed = true) every { homeFragmentState.recentBookmarks } returns listOf(recentBookmark) every { homeFragmentState.recentTabs } returns emptyList() createController().handleReportSessionMetrics(homeFragmentState) diff --git a/app/src/test/java/org/mozilla/fenix/home/HomeFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/home/HomeFragmentStoreTest.kt index 50bcf30107..089c5c35b8 100644 --- a/app/src/test/java/org/mozilla/fenix/home/HomeFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/HomeFragmentStoreTest.kt @@ -10,7 +10,6 @@ import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.verify import kotlinx.coroutines.runBlocking -import mozilla.components.concept.storage.BookmarkNode import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.top.sites.TopSite import mozilla.components.service.fxa.manager.FxaAccountManager @@ -26,10 +25,11 @@ import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getFilteredStories import org.mozilla.fenix.historymetadata.HistoryMetadataGroup -import org.mozilla.fenix.home.recenttabs.RecentTab 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 +import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.onboarding.FenixOnboarding class HomeFragmentStoreTest { @@ -202,7 +202,7 @@ class HomeFragmentStoreTest { val collections: List = listOf(mockk()) val topSites: List = listOf(mockk(), mockk()) val recentTabs: List = listOf(mockk(), recentGroup, mockk()) - val recentBookmarks: List = listOf(mockk(), mockk()) + val recentBookmarks: List = listOf(mockk(), mockk()) val g1 = HistoryMetadataGroup(title = "test One") val g2 = HistoryMetadataGroup(title = recentGroup.searchTerm.lowercase()) val g3 = HistoryMetadataGroup(title = "test two") diff --git a/app/src/test/java/org/mozilla/fenix/home/SessionControlInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/home/SessionControlInteractorTest.kt index 9924615587..1a88874739 100644 --- a/app/src/test/java/org/mozilla/fenix/home/SessionControlInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/SessionControlInteractorTest.kt @@ -7,8 +7,6 @@ package org.mozilla.fenix.home import io.mockk.every import io.mockk.mockk import io.mockk.verify -import mozilla.components.concept.storage.BookmarkNode -import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.feature.tab.collections.Tab import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.service.pocket.PocketRecommendedStory @@ -16,12 +14,13 @@ import org.junit.Before import org.junit.Test import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.historymetadata.controller.HistoryMetadataController +import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory +import org.mozilla.fenix.home.pocket.PocketStoriesController +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recentbookmarks.controller.RecentBookmarksController import org.mozilla.fenix.home.recenttabs.controller.RecentTabController import org.mozilla.fenix.home.sessioncontrol.DefaultSessionControlController import org.mozilla.fenix.home.sessioncontrol.SessionControlInteractor -import org.mozilla.fenix.home.pocket.PocketRecommendedStoriesCategory -import org.mozilla.fenix.home.pocket.PocketStoriesController class SessionControlInteractorTest { @@ -173,16 +172,7 @@ class SessionControlInteractorTest { @Test fun `WHEN a recently saved bookmark is clicked THEN the selected bookmark is handled`() { - val bookmark = BookmarkNode( - type = BookmarkNodeType.ITEM, - guid = "guid#${Math.random() * 1000}", - parentGuid = null, - position = null, - title = null, - url = null, - dateAdded = 0, - children = null - ) + val bookmark = RecentBookmark() interactor.onRecentBookmarkClicked(bookmark) verify { recentBookmarksController.handleBookmarkClicked(bookmark) } diff --git a/app/src/test/java/org/mozilla/fenix/home/recentbookmarks/DefaultRecentBookmarksControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/recentbookmarks/DefaultRecentBookmarksControllerTest.kt index e69bf59f4e..2bf84c29d1 100644 --- a/app/src/test/java/org/mozilla/fenix/home/recentbookmarks/DefaultRecentBookmarksControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/recentbookmarks/DefaultRecentBookmarksControllerTest.kt @@ -17,8 +17,6 @@ import kotlinx.coroutines.test.runBlockingTest import mozilla.appservices.places.BookmarkRoot import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL -import mozilla.components.concept.storage.BookmarkNode -import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.After import org.junit.Before @@ -76,15 +74,9 @@ class DefaultRecentBookmarksControllerTest { every { id } returns R.id.homeFragment } - val bookmark = BookmarkNode( - type = BookmarkNodeType.ITEM, - guid = "guid#${Math.random() * 1000}", - parentGuid = null, - position = null, + val bookmark = RecentBookmark( title = null, - url = "https://www.example.com", - dateAdded = 0, - children = null + url = "https://www.example.com" ) controller.handleBookmarkClicked(bookmark) diff --git a/app/src/test/java/org/mozilla/fenix/home/recentbookmarks/RecentBookmarksFeatureTest.kt b/app/src/test/java/org/mozilla/fenix/home/recentbookmarks/RecentBookmarksFeatureTest.kt index 5f44407212..7e8fe24873 100644 --- a/app/src/test/java/org/mozilla/fenix/home/recentbookmarks/RecentBookmarksFeatureTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/recentbookmarks/RecentBookmarksFeatureTest.kt @@ -12,7 +12,6 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.TestCoroutineDispatcher import kotlinx.coroutines.test.runBlockingTest import mozilla.components.concept.storage.BookmarkNode -import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.support.test.libstate.ext.waitUntilIdle import mozilla.components.support.test.middleware.CaptureActionsMiddleware import mozilla.components.support.test.rule.MainCoroutineRule @@ -33,15 +32,10 @@ class RecentBookmarksFeatureTest { private val homeStore = HomeFragmentStore(middlewares = listOf(middleware)) private val bookmarksUseCases: BookmarksUseCase = mockk(relaxed = true) private val testDispatcher = TestCoroutineDispatcher() - private val bookmark = BookmarkNode( - type = BookmarkNodeType.ITEM, - guid = "guid#${Math.random() * 1000}", - parentGuid = null, - position = null, + private val bookmark = RecentBookmark( title = null, url = "https://www.example.com", - dateAdded = 0, - children = null + previewImageUrl = null ) @get:Rule diff --git a/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/SessionControlViewTest.kt b/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/SessionControlViewTest.kt index 4e642fdaa2..7dfc896f3f 100644 --- a/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/SessionControlViewTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/sessioncontrol/SessionControlViewTest.kt @@ -8,8 +8,6 @@ import androidx.recyclerview.widget.RecyclerView import io.mockk.every import io.mockk.mockk import io.mockk.verify -import mozilla.components.concept.storage.BookmarkNode -import mozilla.components.concept.storage.BookmarkNodeType import mozilla.components.feature.tab.collections.TabCollection import mozilla.components.feature.top.sites.TopSite import mozilla.components.service.pocket.PocketRecommendedStory @@ -22,6 +20,7 @@ import org.junit.runner.RunWith import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.historymetadata.HistoryMetadataGroup import org.mozilla.fenix.home.HomeFragmentState +import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.utils.Settings @@ -30,8 +29,7 @@ class SessionControlViewTest { @Test fun `GIVEN recent Bookmarks WHEN calling shouldShowHomeOnboardingDialog THEN show the dialog `() { - val recentBookmarks = - listOf(BookmarkNode(BookmarkNodeType.ITEM, "guid", null, null, null, null, 0, null)) + val recentBookmarks = listOf(RecentBookmark()) val settings: Settings = mockk() every { settings.hasShownHomeOnboardingDialog } returns false @@ -135,8 +133,7 @@ class SessionControlViewTest { val topSites = emptyList() val collections = emptyList() val expandedCollections = emptySet() - val recentBookmarks = - listOf(BookmarkNode(BookmarkNodeType.ITEM, "guid", null, null, null, null, 0, null)) + val recentBookmarks = listOf(RecentBookmark()) val recentTabs = emptyList() val historyMetadata = emptyList() val pocketArticles = emptyList() @@ -165,7 +162,7 @@ class SessionControlViewTest { val topSites = emptyList() val collections = emptyList() val expandedCollections = emptySet() - val recentBookmarks = listOf() + val recentBookmarks = listOf() val recentTabs = listOf(mockk()) val historyMetadata = emptyList() val pocketArticles = emptyList() @@ -194,7 +191,7 @@ class SessionControlViewTest { val topSites = emptyList() val collections = emptyList() val expandedCollections = emptySet() - val recentBookmarks = listOf() + val recentBookmarks = listOf() val recentTabs = emptyList() val historyMetadata = listOf(HistoryMetadataGroup("title", emptyList())) val pocketArticles = emptyList() @@ -223,7 +220,7 @@ class SessionControlViewTest { val topSites = emptyList() val collections = emptyList() val expandedCollections = emptySet() - val recentBookmarks = listOf() + val recentBookmarks = listOf() val recentTabs = emptyList() val historyMetadata = emptyList() val pocketArticles = listOf(PocketRecommendedStory("", "", "", "", "", 1, 1)) @@ -251,7 +248,7 @@ class SessionControlViewTest { val topSites = emptyList() val collections = emptyList() val expandedCollections = emptySet() - val recentBookmarks = listOf() + val recentBookmarks = listOf() val recentTabs = emptyList() val historyMetadata = emptyList() val pocketArticles = emptyList() @@ -280,7 +277,7 @@ class SessionControlViewTest { val topSites = listOf(mockk()) val collections = listOf(collection) val expandedCollections = emptySet() - val recentBookmarks = listOf(mockk()) + val recentBookmarks = listOf(mockk()) val recentTabs = listOf(mockk()) val historyMetadata = listOf(mockk()) val pocketArticles = listOf(mockk())