For mozilla-mobile#24710 - Remove Event.wrapper for RecentBookmarks t…

…elemetry
mcarare · Apr 14, 2022 · 1051a98 · 1051a98
1 parent 9ff4f37
commit 1051a98
Show file tree

Hide file tree

Showing 9 changed files with 35 additions and 58 deletions.
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
@@ -48,12 +48,6 @@ sealed class Event {
     object StartOnHomeEnterHomeScreen : Event()
     object StartOnHomeOpenTabsTray : Event()
 
-    // Recent bookmarks
-    object BookmarkClicked : Event()
-    object ShowAllBookmarks : Event()
-    object RecentBookmarksShown : Event()
-    data class RecentBookmarkCount(val count: Int) : Event()
-
     // Recently visited/Recent searches
     object RecentSearchesGroupDeleted : 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
@@ -16,7 +16,6 @@ import org.mozilla.fenix.GleanMetrics.HomeMenu
 import org.mozilla.fenix.GleanMetrics.HomeScreen
 import org.mozilla.fenix.GleanMetrics.Pings
 import org.mozilla.fenix.GleanMetrics.ProgressiveWebApp
-import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.GleanMetrics.RecentSearches
 import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage
 import org.mozilla.fenix.GleanMetrics.SearchTerms
@@ -158,25 +157,10 @@ private val Event.wrapper: EventWrapper<*>?
             { StartOnHome.openTabsTray.record(it) }
         )
 
-        is Event.BookmarkClicked -> EventWrapper<NoExtraKeys>(
-            { RecentBookmarks.bookmarkClicked.add() }
-        )
-
-        is Event.ShowAllBookmarks -> EventWrapper<NoExtraKeys>(
-            { RecentBookmarks.showAllBookmarks.add() }
-        )
-
         is Event.RecentSearchesGroupDeleted -> EventWrapper<NoExtraKeys>(
             { RecentSearches.groupDeleted.record(it) }
         )
 
-        is Event.RecentBookmarksShown -> EventWrapper<NoExtraKeys>(
-            { RecentBookmarks.shown.record(it) }
-        )
-
-        is Event.RecentBookmarkCount -> EventWrapper<NoExtraKeys>(
-            { RecentBookmarks.recentBookmarksCount.set(this.count.toLong()) },
-        )
         is Event.SearchTermGroupCount -> EventWrapper(
             { SearchTerms.numberOfSearchTermGroup.record(it) },
             { SearchTerms.numberOfSearchTermGroupKeys.valueOf(it) }

diff --git a/.../main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt b/.../main/java/org/mozilla/fenix/home/recentbookmarks/controller/RecentBookmarksController.kt
@@ -11,12 +11,11 @@ import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL
 import org.mozilla.fenix.BrowserDirection
+import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.AppStore
 import org.mozilla.fenix.components.appstate.AppAction
-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
@@ -60,11 +59,11 @@ class DefaultRecentBookmarksController(
             from = BrowserDirection.FromHome,
             flags = EngineSession.LoadUrlFlags.select(ALLOW_JAVASCRIPT_URL)
         )
-        activity.components.core.metrics.track(Event.BookmarkClicked)
+        RecentBookmarks.bookmarkClicked.add()
     }
 
     override fun handleShowAllBookmarksClicked() {
-        activity.components.core.metrics.track(Event.ShowAllBookmarks)
+        RecentBookmarks.showAllBookmarks.add()
         dismissSearchDialogIfDisplayed()
         navController.navigate(
             HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id)

diff --git a/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarksViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/recentbookmarks/view/RecentBookmarksViewHolder.kt
@@ -10,22 +10,21 @@ import androidx.compose.ui.platform.ComposeView
 import androidx.compose.ui.res.stringResource
 import androidx.lifecycle.LifecycleOwner
 import mozilla.components.lib.state.ext.observeAsComposableState
+import mozilla.components.service.glean.private.NoExtras
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.components
-import org.mozilla.fenix.components.metrics.Event
-import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.compose.ComposeViewHolder
 import org.mozilla.fenix.home.recentbookmarks.interactor.RecentBookmarksInteractor
+import org.mozilla.fenix.GleanMetrics.RecentBookmarks as RecentBookmarksMetrics
 
 class RecentBookmarksViewHolder(
     composeView: ComposeView,
     viewLifecycleOwner: LifecycleOwner,
     val interactor: RecentBookmarksInteractor,
-    val metrics: MetricController
 ) : ComposeViewHolder(composeView, viewLifecycleOwner) {
 
     init {
-        metrics.track(Event.RecentBookmarksShown)
+        RecentBookmarksMetrics.shown.record(NoExtras())
     }
 
     companion object {

diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlAdapter.kt
@@ -241,7 +241,6 @@ class SessionControlAdapter(
                 composeView = ComposeView(parent.context),
                 viewLifecycleOwner = viewLifecycleOwner,
                 interactor = interactor,
-                metrics = components.analytics.metrics
             )
             RecentTabViewHolder.LAYOUT_ID -> return RecentTabViewHolder(
                 composeView = ComposeView(parent.context),

diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt
@@ -35,6 +35,7 @@ import org.mozilla.fenix.GleanMetrics.Collections
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.Pings
 import org.mozilla.fenix.GleanMetrics.Pocket
+import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.GleanMetrics.RecentTabs
 import org.mozilla.fenix.GleanMetrics.TopSites
 import org.mozilla.fenix.HomeActivity
@@ -654,6 +655,6 @@ class DefaultSessionControlController(
             RecentTabs.sectionVisible.set(true)
         }
 
-        metrics.track(Event.RecentBookmarkCount(state.recentBookmarks.size))
+        RecentBookmarks.recentBookmarksCount.set(state.recentBookmarks.size.toLong())
     }
 }
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
@@ -13,7 +13,6 @@ import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mozilla.fenix.GleanMetrics.Awesomebar
-import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage
 import org.mozilla.fenix.GleanMetrics.SyncedTabs
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@@ -64,21 +63,6 @@ class GleanMetricsServiceTest {
         assertTrue(Awesomebar.openedTabSuggestionClicked.testHasValue())
     }
 
-    @Test
-    fun `Home screen recent bookmarks events are correctly recorded`() {
-        assertFalse(RecentBookmarks.shown.testHasValue())
-        gleanService.track(Event.RecentBookmarksShown)
-        assertTrue(RecentBookmarks.shown.testHasValue())
-
-        assertFalse(RecentBookmarks.bookmarkClicked.testHasValue())
-        gleanService.track(Event.BookmarkClicked)
-        assertTrue(RecentBookmarks.bookmarkClicked.testHasValue())
-
-        assertFalse(RecentBookmarks.showAllBookmarks.testHasValue())
-        gleanService.track(Event.ShowAllBookmarks)
-        assertTrue(RecentBookmarks.showAllBookmarks.testHasValue())
-    }
-
     @Test
     fun `Home screen recently visited events are correctly recorded`() {
         assertFalse(RecentlyVisitedHomepage.historyHighlightOpened.testHasValue())

diff --git a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt
@@ -52,6 +52,7 @@ import org.mozilla.fenix.GleanMetrics.Collections
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.Pings
 import org.mozilla.fenix.GleanMetrics.RecentTabs
+import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.GleanMetrics.TopSites
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
@@ -1127,21 +1128,25 @@ class DefaultSessionControlControllerTest {
     fun `WHEN handleReportSessionMetrics is called AND there are zero recent bookmarks THEN report Event#RecentBookmarkCount(0)`() {
         every { appState.recentBookmarks } returns emptyList()
         every { appState.recentTabs } returns emptyList()
+        assertFalse(RecentBookmarks.recentBookmarksCount.testHasValue())
+
         createController().handleReportSessionMetrics(appState)
-        verify {
-            metrics.track(Event.RecentBookmarkCount(0))
-        }
+
+        assertTrue(RecentBookmarks.recentBookmarksCount.testHasValue())
+        assertEquals(0, RecentBookmarks.recentBookmarksCount.testGetValue())
     }
 
     @Test
     fun `WHEN handleReportSessionMetrics is called AND there is at least one recent bookmark THEN report Event#RecentBookmarkCount(1)`() {
         val recentBookmark: RecentBookmark = mockk(relaxed = true)
         every { appState.recentBookmarks } returns listOf(recentBookmark)
         every { appState.recentTabs } returns emptyList()
+        assertFalse(RecentBookmarks.recentBookmarksCount.testHasValue())
+
         createController().handleReportSessionMetrics(appState)
-        verify {
-            metrics.track(Event.RecentBookmarkCount(1))
-        }
+
+        assertTrue(RecentBookmarks.recentBookmarksCount.testHasValue())
+        assertEquals(1, RecentBookmarks.recentBookmarksCount.testGetValue())
     }
 
     @Test

diff --git a/.../test/java/org/mozilla/fenix/home/recentbookmarks/DefaultRecentBookmarksControllerTest.kt b/.../test/java/org/mozilla/fenix/home/recentbookmarks/DefaultRecentBookmarksControllerTest.kt
@@ -16,22 +16,31 @@ 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.support.test.robolectric.testContext
 import mozilla.components.support.test.rule.MainCoroutineRule
+import mozilla.telemetry.glean.testing.GleanTestRule
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
+import org.junit.runner.RunWith
 import org.mozilla.fenix.BrowserDirection
+import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 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.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.home.HomeFragmentDirections
 import org.mozilla.fenix.home.recentbookmarks.controller.DefaultRecentBookmarksController
 
 @OptIn(ExperimentalCoroutinesApi::class)
+@RunWith(FenixRobolectricTestRunner::class)
 class DefaultRecentBookmarksControllerTest {
 
+    @get:Rule
+    val gleanTestRule = GleanTestRule(testContext)
     @get:Rule
     val coroutinesTestRule = MainCoroutineRule()
 
@@ -65,6 +74,7 @@ class DefaultRecentBookmarksControllerTest {
         every { navController.currentDestination } returns mockk {
             every { id } returns R.id.homeFragment
         }
+        assertFalse(RecentBookmarks.bookmarkClicked.testHasValue())
 
         val bookmark = RecentBookmark(
             title = null,
@@ -82,7 +92,7 @@ class DefaultRecentBookmarksControllerTest {
                 from = BrowserDirection.FromHome
             )
         }
-        verify { metrics.track(Event.BookmarkClicked) }
+        assertTrue(RecentBookmarks.bookmarkClicked.testHasValue())
         verify(exactly = 0) {
             navController.navigateUp()
         }
@@ -93,14 +103,15 @@ class DefaultRecentBookmarksControllerTest {
         every { navController.currentDestination } returns mockk {
             every { id } returns R.id.homeFragment
         }
+        assertFalse(RecentBookmarks.showAllBookmarks.testHasValue())
 
         controller.handleShowAllBookmarksClicked()
 
         val directions = HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id)
         verify {
             navController.navigate(directions)
-            metrics.track(Event.ShowAllBookmarks)
         }
+        assertTrue(RecentBookmarks.showAllBookmarks.testHasValue())
         verify(exactly = 0) {
             navController.navigateUp()
         }
@@ -111,6 +122,7 @@ class DefaultRecentBookmarksControllerTest {
         every { navController.currentDestination } returns mockk {
             every { id } returns R.id.searchDialogFragment
         }
+        assertFalse(RecentBookmarks.showAllBookmarks.testHasValue())
 
         controller.handleShowAllBookmarksClicked()
 
@@ -120,7 +132,7 @@ class DefaultRecentBookmarksControllerTest {
             controller.dismissSearchDialogIfDisplayed()
             navController.navigateUp()
             navController.navigate(directions)
-            metrics.track(Event.ShowAllBookmarks)
         }
+        assertTrue(RecentBookmarks.showAllBookmarks.testHasValue())
     }
 }