For mozilla-mobile#24210: Remove wrapper from "default browser" events.

mcarare · Mar 30, 2022 · dbbfca4 · dbbfca4
1 parent f653bd4
commit dbbfca4
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 29 deletions.
diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt
@@ -68,6 +68,7 @@ import mozilla.components.support.locale.LocaleAwareAppCompatActivity
 import mozilla.components.support.utils.SafeIntent
 import mozilla.components.support.utils.toSafeIntent
 import mozilla.components.support.webextensions.WebExtensionPopupFeature
+import mozilla.telemetry.glean.private.NoExtras
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.Metrics
 import org.mozilla.fenix.addons.AddonDetailsFragmentDirections
@@ -172,7 +173,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
             StartSearchIntentProcessor(components.analytics.metrics),
             OpenBrowserIntentProcessor(this, ::getIntentSessionId),
             OpenSpecificTabIntentProcessor(this),
-            DefaultBrowserIntentProcessor(this, components.analytics.metrics)
+            DefaultBrowserIntentProcessor(this)
         )
     }
 
@@ -346,7 +347,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
             }
 
             if (settings().checkIfFenixIsDefaultBrowserOnAppResume()) {
-                metrics.track(Event.ChangedToDefaultBrowser)
+                Events.defaultBrowserChanged.record(NoExtras())
             }
 
             DefaultBrowserNotificationWorker.setDefaultBrowserNotificationIfNeeded(applicationContext)

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
@@ -177,8 +177,6 @@ sealed class Event {
     object AddonsOpenInSettings : Event()
     object VoiceSearchTapped : Event()
     object SearchWidgetInstalled : Event()
-    object ChangedToDefaultBrowser : Event()
-    object DefaultBrowserNotifTapped : Event()
 
     object ProgressiveWebAppOpenFromHomescreenTap : Event()
     object ProgressiveWebAppInstallAsShortcut : 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
@@ -129,12 +129,7 @@ private val Event.wrapper: EventWrapper<*>?
         is Event.ToolbarMenuShown -> EventWrapper<NoExtraKeys>(
             { Events.toolbarMenuVisible.record(it) }
         )
-        is Event.ChangedToDefaultBrowser -> EventWrapper<NoExtraKeys>(
-            { Events.defaultBrowserChanged.record(it) }
-        )
-        is Event.DefaultBrowserNotifTapped -> EventWrapper<NoExtraKeys>(
-            { Events.defaultBrowserNotifTapped.record(it) }
-        )
+
         is Event.OpenedBookmark -> EventWrapper<NoExtraKeys>(
             { BookmarksManagement.open.record(it) }
         )

diff --git a/app/src/main/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessor.kt b/app/src/main/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessor.kt
@@ -6,9 +6,9 @@ package org.mozilla.fenix.home.intent
 
 import android.content.Intent
 import androidx.navigation.NavController
+import mozilla.telemetry.glean.private.NoExtras
+import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.HomeActivity
-import org.mozilla.fenix.components.metrics.Event
-import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.ext.openSetDefaultBrowserOption
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.onboarding.DefaultBrowserNotificationWorker.Companion.isDefaultBrowserNotificationIntent
@@ -21,13 +21,12 @@ import org.mozilla.fenix.onboarding.DefaultBrowserNotificationWorker.Companion.i
  */
 class DefaultBrowserIntentProcessor(
     private val activity: HomeActivity,
-    private val metrics: MetricController
 ) : HomeIntentProcessor {
 
     override fun process(intent: Intent, navController: NavController, out: Intent): Boolean {
         return if (isDefaultBrowserNotificationIntent(intent)) {
             activity.openSetDefaultBrowserOption()
-            metrics.track(Event.DefaultBrowserNotifTapped)
+            Events.defaultBrowserNotifTapped.record(NoExtras())
             true
         } else {
             false

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
@@ -17,7 +17,6 @@ import org.mozilla.fenix.GleanMetrics.Addons
 import org.mozilla.fenix.GleanMetrics.Awesomebar
 import org.mozilla.fenix.GleanMetrics.BookmarksManagement
 import org.mozilla.fenix.GleanMetrics.CreditCards
-import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.History
 import org.mozilla.fenix.GleanMetrics.RecentBookmarks
 import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage
@@ -203,17 +202,6 @@ class GleanMetricsServiceTest {
         assertEquals("123", events[0].extra!!["addon_id"])
     }
 
-    @Test
-    fun `default browser events are correctly recorded`() {
-        assertFalse(Events.defaultBrowserChanged.testHasValue())
-        gleanService.track(Event.ChangedToDefaultBrowser)
-        assertTrue(Events.defaultBrowserChanged.testHasValue())
-
-        assertFalse(Events.defaultBrowserNotifTapped.testHasValue())
-        gleanService.track(Event.DefaultBrowserNotifTapped)
-        assertTrue(Events.defaultBrowserNotifTapped.testHasValue())
-    }
-
     @Test
     fun `Home screen recent bookmarks events are correctly recorded`() {
         assertFalse(RecentBookmarks.shown.testHasValue())

diff --git a/app/src/test/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessorTest.kt b/app/src/test/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessorTest.kt
@@ -10,22 +10,29 @@ import io.mockk.Called
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
+import mozilla.components.service.glean.testing.GleanTestRule
 import mozilla.components.support.test.robolectric.testContext
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 
 @RunWith(FenixRobolectricTestRunner::class)
 class DefaultBrowserIntentProcessorTest {
 
+    @get:Rule
+    val gleanTestRule = GleanTestRule(testContext)
+
     @Test
     fun `do not process blank intents`() {
         val navController: NavController = mockk()
         val out: Intent = mockk()
-        val result = DefaultBrowserIntentProcessor(mockk(), mockk())
+        val result = DefaultBrowserIntentProcessor(mockk())
             .process(Intent(), navController, out)
 
         assertFalse(result)
@@ -47,11 +54,14 @@ class DefaultBrowserIntentProcessorTest {
         every { activity.applicationContext } returns testContext
         every { metrics.track(any()) } returns Unit
 
-        val result = DefaultBrowserIntentProcessor(activity, metrics)
+        assertFalse(Events.defaultBrowserNotifTapped.testHasValue())
+
+        val result = DefaultBrowserIntentProcessor(activity)
             .process(intent, navController, out)
 
         assert(result)
-        verify { metrics.track(any()) }
+
+        assertTrue(Events.defaultBrowserNotifTapped.testHasValue())
         verify { navController wasNot Called }
         verify { out wasNot Called }
     }