From a6a6b6f257be41fbfcbe4439ddb5fa774032cc80 Mon Sep 17 00:00:00 2001
From: Christian Sadilek <christian.sadilek@gmail.com>
Date: Thu, 23 Sep 2021 10:58:52 -0400
Subject: [PATCH] [fenix] For
 https://github.com/mozilla-mobile/fenix/issues/20893: Dedupe urls/site in
 history groups

---
 .../components/history/PagedHistoryProvider.kt     |  8 ++++++--
 .../components/history/PagedHistoryProviderTest.kt | 14 +++++++++++++-
 2 files changed, 19 insertions(+), 3 deletions(-)

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 df89e27c84..42c410ada0 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
@@ -132,10 +132,14 @@ class DefaultPagedHistoryProvider(
         // item.
         result.addAll(history.filter { item -> historyMetadata.find { it.url == item.url } == null })
 
-        // Filter history metadata items with no view time.
+        // Filter history metadata items with no view time and dedupe by url.
+        // Note that distinctBy is sufficient here as it keeps the order of the source
+        // collection, and we're only sorting by visitedAt (=updatedAt) currently.
+        // If we needed the view time we'd have to aggregate it for entries with the same
+        // url, but we don't have a use case for this currently in the history view.
         result.addAll(
             historyGroupsInOffset.map { group ->
-                group.copy(items = group.items.filter { it.totalViewTime > 0 })
+                group.copy(items = group.items.filter { it.totalViewTime > 0 }.distinctBy { it.url })
             }
         )
 
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 562821c7cc..d4f8d67815 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
@@ -73,8 +73,20 @@ class PagedHistoryProviderTest {
             previewImageUrl = null
         )
 
+        // Adding a third entry with same url to test deduping
+        val historyMetadataKey3 = HistoryMetadataKey("http://www.firefox.com", "mozilla", null)
+        val historyEntry3 = HistoryMetadata(
+            key = historyMetadataKey3,
+            title = "firefox",
+            createdAt = 3,
+            updatedAt = 3,
+            totalViewTime = 30,
+            documentType = DocumentType.Regular,
+            previewImageUrl = null
+        )
+
         coEvery { storage.getVisitsPaginated(any(), any(), any()) } returns listOf(visitInfo1, visitInfo2, visitInfo3)
-        coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1, historyEntry2)
+        coEvery { storage.getHistoryMetadataSince(any()) } returns listOf(historyEntry1, historyEntry2, historyEntry3)
 
         var actualResults: List<History>? = null
         provider.getHistory(10L, 5) {