Make the events into counters in the metrics ping

Update tests to reflect new metrics Add data review link for new metrics Mock new settings for startup metrics tests Update metrics Add test for recent bookmark glean events
mozilla-mobile · Aug 19, 2021 · 64a18f0 · 64a18f0
1 parent 89f1580
commit 64a18f0
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 28 deletions.
diff --git a/app/metrics.yaml b/app/metrics.yaml
@@ -5809,31 +5809,41 @@ recent_tabs:
     notification_emails:
       - [email protected]
     expires: "2022-06-23"
+
 recent_bookmarks:
   bookmark_clicked:
-    type: event
+    type: counter
+    lifetime: application
     description: |
-      A recently saved bookmark was clicked on the home screen.
+      A counter that indicates the number of times that a user
+      has clicked on a recently saved bookmark from the home
+      screen.
+    send_in_pings:
+      - metrics
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/19931
     data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/TBD...
+      - https://github.com/mozilla-mobile/fenix/pull/20316#issuecomment-888291843
     data_sensitivity:
       - interaction
     notification_emails:
       - [email protected]
-    expires: "2022-01-31"
-  show_all_clicked:
-    type: event
+    expires: "2022-02-01"
+  show_all_bookmarks:
+    type: counter
+    lifetime: application
     description: |
-      The show all button was clicked on the recently saved
-      bookmarks section of the home screen.
+      A counter that indicates the number of times that a user
+      has clicked the show all button for recently saved bookmarks
+      on the home screen.
+    send_in_pings:
+      - metrics
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/19931
     data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/TBD...
+      - https://github.com/mozilla-mobile/fenix/pull/20316#issuecomment-888291843
     data_sensitivity:
       - interaction
     notification_emails:
       - [email protected]
-    expires: "2022-01-31"
+    expires: "2022-02-01"
diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt
@@ -242,6 +242,10 @@ sealed class Event {
     object OpenRecentTab : Event()
     object OpenInProgressMediaTab : Event()
 
+    // Recent bookmarks
+    object BookmarkClicked : Event()
+    object ShowAllBookmarks : Event()
+
     // Android Autofill
     object AndroidAutofillUnlockSuccessful : Event()
     object AndroidAutofillUnlockCanceled : Event()
@@ -252,10 +256,6 @@ sealed class Event {
     object AndroidAutofillRequestWithLogins : Event()
     object AndroidAutofillRequestWithoutLogins : Event()
 
-    // Recent bookmarks on Home
-    object RecentBookmarkClicked : Event()
-    object RecentBookmarkShowAll : Event()
-
     // Interaction events with extras
 
     data class TopSiteSwipeCarousel(val page: Int) : Event() {

diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt
@@ -840,6 +840,14 @@ private val Event.wrapper: EventWrapper<*>?
             { RecentTabs.showAllClicked.record(it) }
         )
 
+        is Event.BookmarkClicked -> EventWrapper<NoExtraKeys>(
+            { RecentBookmarks.bookmarkClicked.add() }
+        )
+
+        is Event.ShowAllBookmarks -> EventWrapper<NoExtraKeys>(
+            { RecentBookmarks.showAllBookmarks.add() }
+        )
+
         is Event.AndroidAutofillRequestWithLogins -> EventWrapper<NoExtraKeys>(
             { AndroidAutofill.requestMatchingLogins.record(it) }
         )
@@ -865,14 +873,6 @@ private val Event.wrapper: EventWrapper<*>?
             { AndroidAutofill.confirmSuccessful.record(it) }
         )
 
-        is Event.RecentBookmarkClicked -> EventWrapper<NoExtraKeys>(
-            { RecentBookmarks.bookmarkClicked.record(it) }
-        )
-
-        is Event.RecentBookmarkShowAll -> EventWrapper<NoExtraKeys>(
-            { RecentBookmarks.showAllClicked.record(it) }
-        )
-
         // Don't record other events in Glean:
         is Event.AddBookmark -> null
         is Event.OpenedAppFirstRun -> null

diff --git a/.../main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt b/.../main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt
@@ -14,7 +14,6 @@ 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.ext.nav
 import org.mozilla.fenix.home.HomeFragmentDirections
 import org.mozilla.fenix.home.recentbookmarks.interactor.RecentBookmarksInteractor
 
@@ -50,11 +49,11 @@ class DefaultRecentBookmarksController(
             newTab = true,
             from = BrowserDirection.FromHome
         )
-        activity.components.core.metrics.track(Event.RecentBookmarkClicked)
+        activity.components.core.metrics.track(Event.BookmarkClicked)
     }
 
     override fun handleShowAllBookmarksClicked() {
-        activity.components.core.metrics.track(Event.RecentBookmarkShowAll)
+        activity.components.core.metrics.track(Event.ShowAllBookmarks)
         dismissSearchDialogIfDisplayed()
         navController.navigate(
             HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id)

diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt
@@ -19,6 +19,7 @@ import org.mozilla.fenix.GleanMetrics.Awesomebar
 import org.mozilla.fenix.GleanMetrics.BookmarksManagement
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.History
+import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.GleanMetrics.SyncedTabs
 import org.mozilla.fenix.GleanMetrics.TabsTray
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@@ -261,4 +262,15 @@ class GleanMetricsServiceTest {
         gleanService.track(Event.DefaultBrowserNotifTapped)
         assertTrue(Events.defaultBrowserNotifTapped.testHasValue())
     }
+
+    @Test
+    fun `Home screen recent bookmarks events are correctly recorded`() {
+        assertFalse(RecentBookmarks.bookmarkClicked.testHasValue())
+        gleanService.track(Event.BookmarkClicked)
+        assertTrue(RecentBookmarks.bookmarkClicked.testHasValue())
+
+        assertFalse(RecentBookmarks.showAllBookmarks.testHasValue())
+        gleanService.track(Event.ShowAllBookmarks)
+        assertTrue(RecentBookmarks.showAllBookmarks.testHasValue())
+    }
 }
diff --git a/.../test/java/org/mozilla/fenix/home/recentbookmarks/DefaultRecentBookmarksControllerTest.kt b/.../test/java/org/mozilla/fenix/home/recentbookmarks/DefaultRecentBookmarksControllerTest.kt
@@ -22,14 +22,17 @@ import org.junit.After
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
+import org.junit.Assert.assertEquals
 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.components.metrics.MetricController
 import org.mozilla.fenix.ext.components
+import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.home.HomeFragmentDirections
 import org.mozilla.fenix.home.recentbookmarks.controller.DefaultRecentBookmarksController
+import org.mozilla.fenix.utils.Settings
 
 @OptIn(ExperimentalCoroutinesApi::class)
 class DefaultRecentBookmarksControllerTest {
@@ -49,6 +52,7 @@ class DefaultRecentBookmarksControllerTest {
     fun setup() {
         every { activity.openToBrowserAndLoad(any(), any(), any()) } just Runs
         every { activity.components.core.metrics } returns metrics
+
         every { navController.currentDestination } returns mockk {
             every { id } returns R.id.homeFragment
         }
@@ -93,7 +97,7 @@ class DefaultRecentBookmarksControllerTest {
                 newTab = true,
                 from = BrowserDirection.FromHome
             )
-            metrics.track(Event.RecentBookmarkClicked)
+            activity.components.core.metrics.track(Event.BookmarkClicked)
         }
         verify(exactly = 0) {
             navController.navigateUp()
@@ -109,9 +113,8 @@ class DefaultRecentBookmarksControllerTest {
         controller.handleShowAllBookmarksClicked()
 
         val directions = HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id)
-
         verify {
-            metrics.track(Event.RecentBookmarkShowAll)
+            activity.components.core.metrics.track(Event.ShowAllBookmarks)
             navController.navigate(directions)
         }
         verify(exactly = 0) {
@@ -129,6 +132,7 @@ class DefaultRecentBookmarksControllerTest {
 
         val directions = HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id)
         verify {
+            activity.components.core.metrics.track(Event.ShowAllBookmarks)
             controller.dismissSearchDialogIfDisplayed()
             navController.navigateUp()
             navController.navigate(directions)