From 1b420ea1986698079b11e5a4da5368f66c9b15d7 Mon Sep 17 00:00:00 2001 From: Roger Yang Date: Mon, 17 Jan 2022 14:29:48 -0500 Subject: [PATCH] [fenix] Close https://github.com/mozilla-mobile/fenix/issues/23250: Remove duplicate sites within a time period in history --- .../java/org/mozilla/fenix/FeatureFlags.kt | 5 + .../history/PagedHistoryProvider.kt | 37 +++++++- .../fenix/library/history/HistoryAdapter.kt | 70 ++------------ .../library/history/HistoryDataSource.kt | 11 ++- .../library/history/HistoryFragmentStore.kt | 8 ++ .../library/history/HistoryItemTimeGroup.kt | 57 ++++++++++++ .../history/PagedHistoryProviderTest.kt | 4 +- .../library/history/HistoryControllerTest.kt | 2 +- .../library/history/HistoryDataSourceTest.kt | 9 +- .../history/HistoryFragmentStoreTest.kt | 4 +- .../library/history/HistoryInteractorTest.kt | 2 +- ...terTest.kt => HistoryItemTimeGroupTest.kt} | 92 +++++++++++-------- .../HistoryMetadataGroupFragmentStoreTest.kt | 3 + .../HistoryMetadataGroupControllerTest.kt | 3 + .../HistoryMetadataGroupItemViewHolderTest.kt | 2 + 15 files changed, 192 insertions(+), 117 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/library/history/HistoryItemTimeGroup.kt rename app/src/test/java/org/mozilla/fenix/library/history/{HistoryAdapterTest.kt => HistoryItemTimeGroupTest.kt} (57%) diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index 1ea8daf61c..91ff59002d 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -96,4 +96,9 @@ object FeatureFlags { * Enables the Contile top sites. */ const val contileFeature = false + + /** + * Enables history improvement features. + */ + val historyImprovementFeatures = Config.channel.isNightlyOrDebug } diff --git a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt index 06379d6df5..75ac49f357 100644 --- a/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt +++ b/app/src/main/java/org/mozilla/fenix/components/history/PagedHistoryProvider.kt @@ -12,7 +12,10 @@ import mozilla.components.concept.storage.VisitInfo import mozilla.components.concept.storage.VisitType import mozilla.components.support.ktx.kotlin.tryGetHostFromUrl import org.mozilla.fenix.FeatureFlags +import org.mozilla.fenix.FeatureFlags.historyImprovementFeatures import org.mozilla.fenix.library.history.History +import org.mozilla.fenix.library.history.HistoryDataSource +import org.mozilla.fenix.library.history.HistoryItemTimeGroup import org.mozilla.fenix.perf.runBlockingIncrement import kotlin.math.abs @@ -27,6 +30,9 @@ sealed class HistoryDB { abstract val title: String abstract val visitedAt: Long abstract val selected: Boolean + val historyTimeGroup: HistoryItemTimeGroup by lazy { + HistoryItemTimeGroup.timeGroupForTimestamp(visitedAt) + } data class Regular( override val title: String, @@ -84,6 +90,9 @@ class DefaultPagedHistoryProvider( private val historyStorage: PlacesHistoryStorage, private val showHistorySearchGroups: Boolean = FeatureFlags.showHistorySearchGroups, ) : PagedHistoryProvider { + + val urlSet = Array>(HistoryItemTimeGroup.values().size) { mutableSetOf() } + /** * Types of visits we currently do not display in the History UI. */ @@ -113,10 +122,14 @@ class DefaultPagedHistoryProvider( numberOfItems: Int, onComplete: (List) -> Unit, ) { + if (offset == HistoryDataSource.INITIAL_OFFSET) { + urlSet.map { it.clear() } + } + // A PagedList DataSource runs on a background thread automatically. // If we run this in our own coroutineScope it breaks the PagedList runBlockingIncrement { - val history: List + var history: List if (showHistorySearchGroups) { // We need to re-fetch all the history metadata if the offset resets back at 0 @@ -144,6 +157,13 @@ class DefaultPagedHistoryProvider( excludeTypes = excludedVisitTypes ) .map { transformVisitInfoToHistoryItem(it) } + .distinctBy { Pair(it.historyTimeGroup, it.url) } + + if (historyImprovementFeatures) { + history = history.distinctBy { Pair(it.historyTimeGroup, it.url) } + .filter { !urlSet[it.historyTimeGroup.ordinal].contains(it.url) } + history.map { urlSet[it.historyTimeGroup.ordinal].add(it.url) } + } } onComplete(history) @@ -191,7 +211,7 @@ class DefaultPagedHistoryProvider( numberOfItems: Int, ): List { val result = mutableListOf() - val history: List = historyStorage + var history: List = historyStorage .getVisitsPaginated( offset.toLong(), numberOfItems.toLong(), @@ -234,6 +254,12 @@ class DefaultPagedHistoryProvider( } val historyMetadata = historyGroupsInOffset.flatMap { it.items } + if (historyImprovementFeatures) { + history = history.distinctBy { Pair(it.historyTimeGroup, it.url) } + .filter { !urlSet[it.historyTimeGroup.ordinal].contains(it.url) } + history.map { urlSet[it.historyTimeGroup.ordinal].add(it.url) } + } + // Add all history items that are not in a group filtering out any matches with a history // metadata item. result.addAll(history.filter { item -> historyMetadata.find { it.url == item.url } == null }) @@ -249,8 +275,11 @@ class DefaultPagedHistoryProvider( } ) - return result.removeConsecutiveDuplicates() - .sortedByDescending { it.visitedAt } + return if (historyImprovementFeatures) { + result.sortedByDescending { it.visitedAt } + } else { + result.removeConsecutiveDuplicates().sortedByDescending { it.visitedAt } + } } private fun transformVisitInfoToHistoryItem(visit: VisitInfo): HistoryDB.Regular { diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt index 2e9c1e5d69..277886a0d3 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryAdapter.kt @@ -4,29 +4,12 @@ package org.mozilla.fenix.library.history -import android.content.Context import android.view.LayoutInflater import android.view.ViewGroup -import androidx.annotation.VisibleForTesting import androidx.paging.PagedListAdapter import androidx.recyclerview.widget.DiffUtil -import org.mozilla.fenix.R import org.mozilla.fenix.selection.SelectionHolder import org.mozilla.fenix.library.history.viewholders.HistoryListItemViewHolder -import java.util.Calendar -import java.util.Date - -enum class HistoryItemTimeGroup { - Today, Yesterday, ThisWeek, ThisMonth, Older; - - fun humanReadable(context: Context): String = when (this) { - Today -> context.getString(R.string.history_today) - Yesterday -> context.getString(R.string.history_yesterday) - ThisWeek -> context.getString(R.string.history_7_days) - ThisMonth -> context.getString(R.string.history_30_days) - Older -> context.getString(R.string.history_older) - } -} class HistoryAdapter( private val historyInteractor: HistoryInteractor, @@ -54,25 +37,24 @@ class HistoryAdapter( override fun onBindViewHolder(holder: HistoryListItemViewHolder, position: Int) { val current = getItem(position) ?: return - val headerForCurrentItem = timeGroupForHistoryItem(current) val isPendingDeletion = pendingDeletionIds.contains(current.visitedAt) var timeGroup: HistoryItemTimeGroup? = null // Add or remove the header and position to the map depending on it's deletion status - if (itemsWithHeaders.containsKey(headerForCurrentItem)) { - if (isPendingDeletion && itemsWithHeaders[headerForCurrentItem] == position) { - itemsWithHeaders.remove(headerForCurrentItem) - } else if (isPendingDeletion && itemsWithHeaders[headerForCurrentItem] != position) { + if (itemsWithHeaders.containsKey(current.historyTimeGroup)) { + if (isPendingDeletion && itemsWithHeaders[current.historyTimeGroup] == position) { + itemsWithHeaders.remove(current.historyTimeGroup) + } else if (isPendingDeletion && itemsWithHeaders[current.historyTimeGroup] != position) { // do nothing } else { - if (position <= itemsWithHeaders[headerForCurrentItem] as Int) { - itemsWithHeaders[headerForCurrentItem] = position - timeGroup = headerForCurrentItem + if (position <= itemsWithHeaders[current.historyTimeGroup] as Int) { + itemsWithHeaders[current.historyTimeGroup] = position + timeGroup = current.historyTimeGroup } } } else if (!isPendingDeletion) { - itemsWithHeaders[headerForCurrentItem] = position - timeGroup = headerForCurrentItem + itemsWithHeaders[current.historyTimeGroup] = position + timeGroup = current.historyTimeGroup } holder.bind(current, timeGroup, position == 0, mode, isPendingDeletion) @@ -83,40 +65,6 @@ class HistoryAdapter( } companion object { - private const val zeroDays = 0 - private const val oneDay = 1 - private const val sevenDays = 7 - private const val thirtyDays = 30 - private val today = getDaysAgo(zeroDays).time - private val yesterday = getDaysAgo(oneDay).time - private val sevenDaysAgo = getDaysAgo(sevenDays).time - private val thirtyDaysAgo = getDaysAgo(thirtyDays).time - private val todayRange = LongRange(today, Long.MAX_VALUE) // all future time is considered today - private val yesterdayRange = LongRange(yesterday, today) - private val lastWeekRange = LongRange(sevenDaysAgo, yesterday) - private val lastMonthRange = LongRange(thirtyDaysAgo, sevenDaysAgo) - - private fun getDaysAgo(daysAgo: Int): Date { - return Calendar.getInstance().apply { - set(Calendar.HOUR_OF_DAY, 0) - set(Calendar.MINUTE, 0) - set(Calendar.SECOND, 0) - set(Calendar.MILLISECOND, 0) - add(Calendar.DAY_OF_YEAR, -daysAgo) - }.time - } - - @VisibleForTesting - internal fun timeGroupForHistoryItem(item: History): HistoryItemTimeGroup { - return when { - todayRange.contains(item.visitedAt) -> HistoryItemTimeGroup.Today - yesterdayRange.contains(item.visitedAt) -> HistoryItemTimeGroup.Yesterday - lastWeekRange.contains(item.visitedAt) -> HistoryItemTimeGroup.ThisWeek - lastMonthRange.contains(item.visitedAt) -> HistoryItemTimeGroup.ThisMonth - else -> HistoryItemTimeGroup.Older - } - } - private val historyDiffCallback = object : DiffUtil.ItemCallback() { override fun areItemsTheSame(oldItem: History, newItem: History): Boolean { return oldItem == newItem diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryDataSource.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryDataSource.kt index edb4898da4..87a732f4fe 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryDataSource.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryDataSource.kt @@ -36,7 +36,7 @@ class HistoryDataSource( override fun loadBefore(params: LoadParams, callback: LoadCallback) { /* noop */ } companion object { - private const val INITIAL_OFFSET = 0 + internal const val INITIAL_OFFSET = 0 } } @@ -78,7 +78,8 @@ private fun HistoryDB.Group.positioned(position: Int): History.Group { position = position, items = this.items.mapIndexed { index, item -> item.positioned(index) }, title = this.title, - visitedAt = this.visitedAt + visitedAt = this.visitedAt, + historyTimeGroup = this.historyTimeGroup, ) } @@ -89,7 +90,8 @@ private fun HistoryDB.Metadata.positioned(position: Int): History.Metadata { title = this.title, totalViewTime = this.totalViewTime, url = this.url, - visitedAt = this.visitedAt + visitedAt = this.visitedAt, + historyTimeGroup = this.historyTimeGroup, ) } @@ -98,6 +100,7 @@ private fun HistoryDB.Regular.positioned(position: Int): History.Regular { position = position, title = this.title, url = this.url, - visitedAt = this.visitedAt + visitedAt = this.visitedAt, + historyTimeGroup = this.historyTimeGroup, ) } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt index d061ef1ca0..d407af1648 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragmentStore.kt @@ -20,6 +20,7 @@ sealed class History : Parcelable { abstract val position: Int abstract val title: String abstract val visitedAt: Long + abstract val historyTimeGroup: HistoryItemTimeGroup abstract val selected: Boolean /** @@ -29,6 +30,7 @@ sealed class History : Parcelable { * @property title Title of the history item. * @property url URL of the history item. * @property visitedAt Timestamp of when this history item was visited. + * @property historyTimeGroup [HistoryItemTimeGroup] of the history item. * @property selected Whether or not the history item is selected. */ @Parcelize data class Regular( @@ -36,6 +38,7 @@ sealed class History : Parcelable { override val title: String, val url: String, override val visitedAt: Long, + override val historyTimeGroup: HistoryItemTimeGroup, override val selected: Boolean = false ) : History() @@ -46,6 +49,7 @@ sealed class History : Parcelable { * @property title Title of the history metadata item. * @property url URL of the history metadata item. * @property visitedAt Timestamp of when this history metadata item was visited. + * @property historyTimeGroup [HistoryItemTimeGroup] of the history item. * @property totalViewTime Total time the user viewed the page associated with this record. * @property historyMetadataKey The [HistoryMetadataKey] of the new tab in case this tab * was opened from history. @@ -56,6 +60,7 @@ sealed class History : Parcelable { override val title: String, val url: String, override val visitedAt: Long, + override val historyTimeGroup: HistoryItemTimeGroup, val totalViewTime: Int, val historyMetadataKey: HistoryMetadataKey, override val selected: Boolean = false @@ -67,6 +72,7 @@ sealed class History : Parcelable { * @property position Position of this item in a result list of other [History] items. * @property title Title of the history metadata group. * @property visitedAt Timestamp of when this history metadata group was visited. + * @property historyTimeGroup [HistoryItemTimeGroup] of the history item. * @property items List of history metadata items associated with the group. * @property selected Whether or not the history group is selected. */ @@ -74,6 +80,7 @@ sealed class History : Parcelable { override val position: Int, override val title: String, override val visitedAt: Long, + override val historyTimeGroup: HistoryItemTimeGroup, val items: List, override val selected: Boolean = false ) : History() @@ -89,6 +96,7 @@ fun HistoryMetadata.toHistoryMetadata(position: Int): History.Metadata { ?: key.url.tryGetHostFromUrl(), url = key.url, visitedAt = createdAt, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(createdAt), totalViewTime = totalViewTime, historyMetadataKey = key ) diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryItemTimeGroup.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryItemTimeGroup.kt new file mode 100644 index 0000000000..a882c257af --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryItemTimeGroup.kt @@ -0,0 +1,57 @@ +/* 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.library.history + +import android.content.Context +import org.mozilla.fenix.R +import java.util.Calendar +import java.util.Date + +enum class HistoryItemTimeGroup { + Today, Yesterday, ThisWeek, ThisMonth, Older; + + fun humanReadable(context: Context): String = when (this) { + Today -> context.getString(R.string.history_today) + Yesterday -> context.getString(R.string.history_yesterday) + ThisWeek -> context.getString(R.string.history_7_days) + ThisMonth -> context.getString(R.string.history_30_days) + Older -> context.getString(R.string.history_older) + } + + companion object { + private const val zeroDays = 0 + private const val oneDay = 1 + private const val sevenDays = 7 + private const val thirtyDays = 30 + private val today = getDaysAgo(zeroDays).time + private val yesterday = getDaysAgo(oneDay).time + private val sevenDaysAgo = getDaysAgo(sevenDays).time + private val thirtyDaysAgo = getDaysAgo(thirtyDays).time + private val todayRange = LongRange(today, Long.MAX_VALUE) // all future time is considered today + private val yesterdayRange = LongRange(yesterday, today) + private val lastWeekRange = LongRange(sevenDaysAgo, yesterday) + private val lastMonthRange = LongRange(thirtyDaysAgo, sevenDaysAgo) + + private fun getDaysAgo(daysAgo: Int): Date { + return Calendar.getInstance().apply { + set(Calendar.HOUR_OF_DAY, 0) + set(Calendar.MINUTE, 0) + set(Calendar.SECOND, 0) + set(Calendar.MILLISECOND, 0) + add(Calendar.DAY_OF_YEAR, -daysAgo) + }.time + } + + internal fun timeGroupForTimestamp(timestamp: Long): HistoryItemTimeGroup { + return when { + todayRange.contains(timestamp) -> Today + yesterdayRange.contains(timestamp) -> Yesterday + lastWeekRange.contains(timestamp) -> ThisWeek + lastMonthRange.contains(timestamp) -> ThisMonth + else -> Older + } + } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt index c4014be159..3907ef57e7 100644 --- a/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/history/PagedHistoryProviderTest.kt @@ -137,7 +137,7 @@ class PagedHistoryProviderTest { HistoryDB.Regular( title = visitInfo3.title!!, url = visitInfo3.url, - visitedAt = visitInfo3.visitTime + visitedAt = visitInfo3.visitTime, ) ) assertEquals(results, actualResults) @@ -495,7 +495,7 @@ class PagedHistoryProviderTest { HistoryDB.Regular( title = "No duplicate item", url = "url", - visitedAt = 0 + visitedAt = 0, ), ) assertEquals(expectedList, results) diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt index ea198bbab5..7a13c98350 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryControllerTest.kt @@ -22,7 +22,7 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @RunWith(FenixRobolectricTestRunner::class) class HistoryControllerTest { - private val historyItem = History.Regular(0, "title", "url", 0.toLong()) + private val historyItem = History.Regular(0, "title", "url", 0.toLong(), HistoryItemTimeGroup.timeGroupForTimestamp(0)) private val scope = TestCoroutineScope() private val store: HistoryFragmentStore = mockk(relaxed = true) private val state: HistoryFragmentState = mockk(relaxed = true) diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryDataSourceTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryDataSourceTest.kt index 737c642d65..9e59311bcf 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryDataSourceTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryDataSourceTest.kt @@ -171,7 +171,11 @@ class HistoryDataSourceTest { return this.map { when (it) { is TestHistory.Regular -> { - HistoryDB.Regular(title = it.url, url = it.url, visitedAt = 0) + HistoryDB.Regular( + title = it.url, + url = it.url, + visitedAt = 0 + ) } is TestHistory.Metadata -> { HistoryDB.Metadata( @@ -184,7 +188,8 @@ class HistoryDataSourceTest { } is TestHistory.Group -> { HistoryDB.Group( - title = it.title, visitedAt = 0, + title = it.title, + visitedAt = 0, items = it.items.map { item -> HistoryDB.Metadata( title = item, diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt index 135636a2cc..4c4588280a 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryFragmentStoreTest.kt @@ -10,8 +10,8 @@ import org.junit.Assert.assertNotSame import org.junit.Test class HistoryFragmentStoreTest { - private val historyItem = History.Regular(0, "title", "url", 0.toLong()) - private val newHistoryItem = History.Regular(1, "title", "url", 0.toLong()) + private val historyItem = History.Regular(0, "title", "url", 0.toLong(), HistoryItemTimeGroup.timeGroupForTimestamp(0)) + private val newHistoryItem = History.Regular(1, "title", "url", 0.toLong(), HistoryItemTimeGroup.timeGroupForTimestamp(0)) @Test fun exitEditMode() = runBlocking { diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt index 57880bf387..7fee97707b 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryInteractorTest.kt @@ -11,7 +11,7 @@ import org.junit.Assert.assertTrue import org.junit.Test class HistoryInteractorTest { - private val historyItem = History.Regular(0, "title", "url", 0.toLong()) + private val historyItem = History.Regular(0, "title", "url", 0.toLong(), HistoryItemTimeGroup.timeGroupForTimestamp(0)) val controller: HistoryController = mockk(relaxed = true) val interactor = DefaultHistoryInteractor(controller) diff --git a/app/src/test/java/org/mozilla/fenix/library/history/HistoryAdapterTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/HistoryItemTimeGroupTest.kt similarity index 57% rename from app/src/test/java/org/mozilla/fenix/library/history/HistoryAdapterTest.kt rename to app/src/test/java/org/mozilla/fenix/library/history/HistoryItemTimeGroupTest.kt index 6ec8a96348..7dee9b4827 100644 --- a/app/src/test/java/org/mozilla/fenix/library/history/HistoryAdapterTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/history/HistoryItemTimeGroupTest.kt @@ -9,32 +9,34 @@ import org.junit.Assert.assertEquals import org.junit.Test import java.util.Calendar -class HistoryAdapterTest { +class HistoryItemTimeGroupTest { @Test fun `WHEN grouping history item with future date THEN item is grouped to today`() { + val time = System.currentTimeMillis() + DateUtils.WEEK_IN_MILLIS val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = System.currentTimeMillis() + DateUtils.WEEK_IN_MILLIS + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.Today, timeGroup) + assertEquals(HistoryItemTimeGroup.Today, history.historyTimeGroup) } @Test fun `WHEN grouping history item with today's date THEN item is grouped to today`() { + val time = System.currentTimeMillis() + DateUtils.MINUTE_IN_MILLIS val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = System.currentTimeMillis() - DateUtils.MINUTE_IN_MILLIS + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.Today, timeGroup) + assertEquals(HistoryItemTimeGroup.Today, history.historyTimeGroup) } @Test @@ -49,11 +51,11 @@ class HistoryAdapterTest { position = 1, title = "test item", url = "url", - visitedAt = calendar.timeInMillis + visitedAt = calendar.timeInMillis, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(calendar.timeInMillis) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.Today, timeGroup) + assertEquals(HistoryItemTimeGroup.Today, history.historyTimeGroup) } @Test @@ -64,15 +66,17 @@ class HistoryAdapterTest { set(Calendar.SECOND, 0) } + val time = calendar.timeInMillis - DateUtils.HOUR_IN_MILLIS + val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = calendar.timeInMillis - DateUtils.HOUR_IN_MILLIS + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.Yesterday, timeGroup) + assertEquals(HistoryItemTimeGroup.Yesterday, history.historyTimeGroup) } @Test @@ -82,15 +86,17 @@ class HistoryAdapterTest { calendar.set(Calendar.MINUTE, 0) calendar.set(Calendar.SECOND, 0) + val time = calendar.timeInMillis - (DateUtils.HOUR_IN_MILLIS * 23) + val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = calendar.timeInMillis - (DateUtils.HOUR_IN_MILLIS * 23) + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.Yesterday, timeGroup) + assertEquals(HistoryItemTimeGroup.Yesterday, history.historyTimeGroup) } @Test @@ -101,80 +107,86 @@ class HistoryAdapterTest { set(Calendar.SECOND, 0) } + val time = calendar.timeInMillis - (DateUtils.HOUR_IN_MILLIS * 25) val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = calendar.timeInMillis - (DateUtils.HOUR_IN_MILLIS * 25) + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.ThisWeek, timeGroup) + assertEquals(HistoryItemTimeGroup.ThisWeek, history.historyTimeGroup) } @Test fun `WHEN grouping history item with 3 days ago date THEN item is grouped to this week`() { + val time = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 3) val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 3) + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.ThisWeek, timeGroup) + assertEquals(HistoryItemTimeGroup.ThisWeek, history.historyTimeGroup) } @Test fun `WHEN grouping history item with 6 days ago date THEN item is grouped to this week`() { + val time = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 6) val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 6) + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.ThisWeek, timeGroup) + assertEquals(HistoryItemTimeGroup.ThisWeek, history.historyTimeGroup) } @Test fun `WHEN grouping history item with 8 days ago date THEN item is grouped to this month`() { + val time = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 8) val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 8) + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.ThisMonth, timeGroup) + assertEquals(HistoryItemTimeGroup.ThisMonth, history.historyTimeGroup) } @Test fun `WHEN grouping history item with 29 days ago date THEN item is grouped to this month`() { + val time = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 29) val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 29) + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.ThisMonth, timeGroup) + assertEquals(HistoryItemTimeGroup.ThisMonth, history.historyTimeGroup) } @Test fun `WHEN grouping history item with 31 days ago date THEN item is grouped to older`() { + val time = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 31) val history = History.Regular( position = 1, title = "test item", url = "url", - visitedAt = System.currentTimeMillis() - (DateUtils.DAY_IN_MILLIS * 31) + visitedAt = time, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(time) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.Older, timeGroup) + assertEquals(HistoryItemTimeGroup.Older, history.historyTimeGroup) } @Test @@ -183,11 +195,11 @@ class HistoryAdapterTest { position = 1, title = "test item", url = "url", - visitedAt = 0 + visitedAt = 0, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(0) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.Older, timeGroup) + assertEquals(HistoryItemTimeGroup.Older, history.historyTimeGroup) } @Test @@ -196,10 +208,10 @@ class HistoryAdapterTest { position = 1, title = "test item", url = "url", - visitedAt = -100 + visitedAt = -100, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(-100) ) - val timeGroup = HistoryAdapter.timeGroupForHistoryItem(history as History) - assertEquals(HistoryItemTimeGroup.Older, timeGroup) + assertEquals(HistoryItemTimeGroup.Older, history.historyTimeGroup) } } diff --git a/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt index 88ab71188e..fca2949d94 100644 --- a/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/historymetadata/HistoryMetadataGroupFragmentStoreTest.kt @@ -12,6 +12,7 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.mozilla.fenix.library.history.History +import org.mozilla.fenix.library.history.HistoryItemTimeGroup class HistoryMetadataGroupFragmentStoreTest { @@ -23,6 +24,7 @@ class HistoryMetadataGroupFragmentStoreTest { title = "Mozilla", url = "mozilla.org", visitedAt = 0, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(0), totalViewTime = 0, historyMetadataKey = HistoryMetadataKey("http://www.mozilla.com", "mozilla", null) ) @@ -31,6 +33,7 @@ class HistoryMetadataGroupFragmentStoreTest { title = "Firefox", url = "firefox.com", visitedAt = 0, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(0), totalViewTime = 0, historyMetadataKey = HistoryMetadataKey("http://www.firefox.com", "mozilla", null) ) diff --git a/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt b/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt index 7ca26f8bb4..04e01d7e4b 100644 --- a/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/historymetadata/controller/HistoryMetadataGroupControllerTest.kt @@ -30,6 +30,7 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.directionsEq import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.library.history.History +import org.mozilla.fenix.library.history.HistoryItemTimeGroup import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragmentAction import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragmentDirections import org.mozilla.fenix.library.historymetadata.HistoryMetadataGroupFragmentStore @@ -55,6 +56,7 @@ class HistoryMetadataGroupControllerTest { title = "Mozilla", url = "mozilla.org", visitedAt = 0, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(0), totalViewTime = 1, historyMetadataKey = historyMetadataKey ) @@ -63,6 +65,7 @@ class HistoryMetadataGroupControllerTest { title = "Firefox", url = "firefox.com", visitedAt = 0, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(0), totalViewTime = 1, historyMetadataKey = historyMetadataKey ) diff --git a/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt index 6e1afbdac3..f2f26b724f 100644 --- a/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/library/historymetadata/view/HistoryMetadataGroupItemViewHolderTest.kt @@ -18,6 +18,7 @@ import org.mozilla.fenix.databinding.HistoryMetadataGroupListItemBinding import org.mozilla.fenix.ext.components import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.library.history.History +import org.mozilla.fenix.library.history.HistoryItemTimeGroup import org.mozilla.fenix.library.historymetadata.interactor.HistoryMetadataGroupInteractor import org.mozilla.fenix.selection.SelectionHolder @@ -33,6 +34,7 @@ class HistoryMetadataGroupItemViewHolderTest { title = "Mozilla", url = "mozilla.org", visitedAt = 0, + historyTimeGroup = HistoryItemTimeGroup.timeGroupForTimestamp(0), totalViewTime = 0, historyMetadataKey = HistoryMetadataKey("http://www.mozilla.com", "mozilla", null) )