From 54549c49ec0e9d8636e656d13a594c029ec02e8d Mon Sep 17 00:00:00 2001
From: Christian Sadilek <christian.sadilek@gmail.com>
Date: Thu, 23 Sep 2021 16:01:59 -0400
Subject: [PATCH] Fix mapping between history visits and groups to use
 createdAt

We currently have a 15s buffer to match metadata to its corresponding
visit. However, a existing metadata record can be updated more than
15s after it was created e.g. when closing the tab and updating
the view time.
---
 .../history/PagedHistoryProvider.kt           |  3 ++-
 .../history/PagedHistoryProviderTest.kt       | 19 ++++++++++---------
 2 files changed, 12 insertions(+), 10 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 42c410ada..90f17434f 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
@@ -54,13 +54,14 @@ class DefaultPagedHistoryProvider(
                 if (historyGroups == null || offset == 0L) {
 
                     historyGroups = historyStorage.getHistoryMetadataSince(Long.MIN_VALUE)
+                        .sortedByDescending { it.createdAt }
                         .filter { it.key.searchTerm != null }
                         .groupBy { it.key.searchTerm!! }
                         .map { (searchTerm, items) ->
                             History.Group(
                                 id = items.first().createdAt.toInt(),
                                 title = searchTerm,
-                                visitedAt = items.first().updatedAt,
+                                visitedAt = items.first().createdAt,
                                 items = items.map { it.toHistoryMetadata() }
                             )
                         }
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 d4f8d6781..e01fbe04b 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
@@ -57,7 +57,7 @@ class PagedHistoryProviderTest {
             key = historyMetadataKey1,
             title = "mozilla",
             createdAt = 5,
-            updatedAt = 5,
+            updatedAt = 10,
             totalViewTime = 10,
             documentType = DocumentType.Regular,
             previewImageUrl = null
@@ -67,19 +67,19 @@ class PagedHistoryProviderTest {
             key = historyMetadataKey2,
             title = "firefox",
             createdAt = 2,
-            updatedAt = 2,
+            updatedAt = 11,
             totalViewTime = 20,
             documentType = DocumentType.Regular,
             previewImageUrl = null
         )
 
-        // Adding a third entry with same url to test deduping
+        // Adding a third entry with same url to test de-duping
         val historyMetadataKey3 = HistoryMetadataKey("http://www.firefox.com", "mozilla", null)
         val historyEntry3 = HistoryMetadata(
             key = historyMetadataKey3,
             title = "firefox",
             createdAt = 3,
-            updatedAt = 3,
+            updatedAt = 12,
             totalViewTime = 30,
             documentType = DocumentType.Regular,
             previewImageUrl = null
@@ -114,6 +114,7 @@ class PagedHistoryProviderTest {
                 id = historyEntry1.createdAt.toInt(),
                 title = historyEntry1.key.searchTerm!!,
                 visitedAt = historyEntry1.createdAt,
+                // Results are de-duped by URL and sorted descending by createdAt/visitedAt
                 items = listOf(
                     History.Metadata(
                         id = historyEntry1.createdAt.toInt(),
@@ -124,11 +125,11 @@ class PagedHistoryProviderTest {
                         historyMetadataKey = historyMetadataKey1
                     ),
                     History.Metadata(
-                        id = historyEntry2.createdAt.toInt(),
-                        title = historyEntry2.title!!,
-                        url = historyEntry2.key.url,
-                        visitedAt = historyEntry2.createdAt,
-                        totalViewTime = historyEntry2.totalViewTime,
+                        id = historyEntry3.createdAt.toInt(),
+                        title = historyEntry3.title!!,
+                        url = historyEntry3.key.url,
+                        visitedAt = historyEntry3.createdAt,
+                        totalViewTime = historyEntry3.totalViewTime,
                         historyMetadataKey = historyMetadataKey2
                     )
                 )