For #24210: Remove wrapper from opened link event.

mozilla-mobile · Apr 6, 2022 · ac1eeec · ac1eeec
1 parent 09e5394
commit ac1eeec
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 11 deletions.
diff --git a/app/metrics.yaml b/app/metrics.yaml
@@ -321,6 +321,7 @@ events:
           The mode the link was opened in. Either 'PRIVATE' or 'NORMAL'. N.B.:
           this probe may be incorrectly implemented: see
           https://github.com/mozilla-mobile/fenix/issues/14133
+        type: string
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/5737
       - https://github.com/mozilla-mobile/fenix/issues/19923

diff --git a/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt b/app/src/main/java/org/mozilla/fenix/IntentReceiverActivity.kt
@@ -15,10 +15,10 @@ import mozilla.components.feature.intent.ext.sanitize
 import mozilla.components.feature.intent.processing.IntentProcessor
 import mozilla.components.support.utils.EXTRA_ACTIVITY_REFERRER_CATEGORY
 import mozilla.components.support.utils.EXTRA_ACTIVITY_REFERRER_PACKAGE
+import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.HomeActivity.Companion.PRIVATE_BROWSING_MODE
 import org.mozilla.fenix.components.IntentProcessorType
 import org.mozilla.fenix.components.getType
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.perf.MarkersActivityLifecycleCallbacks
@@ -58,9 +58,9 @@ class IntentReceiverActivity : Activity() {
         val private = settings().openLinksInAPrivateTab
         intent.putExtra(PRIVATE_BROWSING_MODE, private)
         if (private) {
-            components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.PRIVATE))
+            Events.openedLink.record(Events.OpenedLinkExtra("PRIVATE"))
         } else {
-            components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.NORMAL))
+            Events.openedLink.record(Events.OpenedLinkExtra("NORMAL"))
         }
 
         addReferrerInformation(intent)
@@ -94,7 +94,7 @@ class IntentReceiverActivity : Activity() {
                 components.intentProcessors.privateIntentProcessor
             )
         } else {
-            components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.NORMAL))
+            Events.openedLink.record(Events.OpenedLinkExtra("NORMAL"))
             listOf(
                 components.intentProcessors.customTabIntentProcessor,
                 components.intentProcessors.intentProcessor

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
@@ -257,13 +257,6 @@ sealed class Event {
             get() = hashMapOf(Addons.openAddonSettingKeys.addonId to addonId)
     }
 
-    data class OpenedLink(val mode: Mode) : Event() {
-        enum class Mode { NORMAL, PRIVATE }
-
-        override val extras: Map<Events.openedLinkKeys, String>?
-            get() = hashMapOf(Events.openedLinkKeys.mode to mode.name)
-    }
-
     data class SaveLoginsSettingChanged(val setting: Setting) : Event() {
         enum class Setting { NEVER_SAVE, ASK_TO_SAVE }
 

diff --git a/app/src/test/java/org/mozilla/fenix/IntentReceiverActivityTest.kt b/app/src/test/java/org/mozilla/fenix/IntentReceiverActivityTest.kt
@@ -17,12 +17,17 @@ import io.mockk.unmockkStatic
 import io.mockk.verify
 import kotlinx.coroutines.test.runBlockingTest
 import mozilla.components.feature.intent.processing.IntentProcessor
+import mozilla.components.support.test.robolectric.testContext
+import mozilla.telemetry.glean.testing.GleanTestRule
 import org.junit.After
 import org.junit.Assert.assertEquals
+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.GleanMetrics.Events
 import org.mozilla.fenix.components.IntentProcessors
 import org.mozilla.fenix.customtabs.ExternalAppBrowserActivity
 import org.mozilla.fenix.ext.components
@@ -40,6 +45,9 @@ class IntentReceiverActivityTest {
     private lateinit var settings: Settings
     private lateinit var intentProcessors: IntentProcessors
 
+    @get:Rule
+    val gleanTestRule = GleanTestRule(testContext)
+
     @Before
     fun setup() {
         mockkStatic("org.mozilla.fenix.ext.ContextKt")
@@ -67,6 +75,7 @@ class IntentReceiverActivityTest {
     fun `process intent with flag launched from history`() = runBlockingTest {
         val intent = Intent()
         intent.flags = FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY
+        assertFalse(Events.openedLink.testHasValue())
 
         val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get()
         attachMocks(activity)
@@ -75,6 +84,7 @@ class IntentReceiverActivityTest {
         val shadow = shadowOf(activity)
         val actualIntent = shadow.peekNextStartedActivity()
 
+        assertTrue(Events.openedLink.testHasValue())
         assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
         assertEquals(true, actualIntent.flags == FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY)
     }
@@ -84,6 +94,7 @@ class IntentReceiverActivityTest {
         runBlockingTest {
             val uri = Uri.parse(BuildConfig.DEEP_LINK_SCHEME + "://settings_wallpapers")
             val intent = Intent("", uri)
+            assertFalse(Events.openedLink.testHasValue())
 
             coEvery { intentProcessors.intentProcessor.process(any()) } returns false
             coEvery { intentProcessors.externalDeepLinkIntentProcessor.process(any()) } returns true
@@ -96,13 +107,15 @@ class IntentReceiverActivityTest {
             val shadow = shadowOf(activity)
             val actualIntent = shadow.peekNextStartedActivity()
 
+            assertTrue(Events.openedLink.testHasValue())
             assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
         }
 
     @Test
     fun `process intent with action OPEN_PRIVATE_TAB`() = runBlockingTest {
         val intent = Intent()
         intent.action = NewTabShortcutIntentProcessor.ACTION_OPEN_PRIVATE_TAB
+        assertFalse(Events.openedLink.testHasValue())
 
         coEvery { intentProcessors.intentProcessor.process(intent) } returns false
         coEvery { intentProcessors.customTabIntentProcessor.process(intent) } returns false
@@ -113,13 +126,15 @@ class IntentReceiverActivityTest {
         val shadow = shadowOf(activity)
         val actualIntent = shadow.peekNextStartedActivity()
 
+        assertTrue(Events.openedLink.testHasValue())
         assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
         assertEquals(true, actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false))
         assertEquals(false, actualIntent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, true))
     }
 
     @Test
     fun `process intent with action OPEN_TAB`() = runBlockingTest {
+        assertFalse(Events.openedLink.testHasValue())
         val intent = Intent()
         intent.action = NewTabShortcutIntentProcessor.ACTION_OPEN_TAB
 
@@ -132,10 +147,12 @@ class IntentReceiverActivityTest {
 
         assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
         assertEquals(false, actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false))
+        assertTrue(Events.openedLink.testHasValue())
     }
 
     @Test
     fun `process intent starts Activity`() = runBlockingTest {
+        assertFalse(Events.openedLink.testHasValue())
         val intent = Intent()
         val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get()
         attachMocks(activity)
@@ -146,10 +163,13 @@ class IntentReceiverActivityTest {
 
         assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
         assertEquals(true, actualIntent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, true))
+        assertTrue(Events.openedLink.testHasValue())
     }
 
     @Test
     fun `process intent with launchLinksInPrivateTab set to true`() = runBlockingTest {
+        assertFalse(Events.openedLink.testHasValue())
+
         every { settings.openLinksInAPrivateTab } returns true
 
         coEvery { intentProcessors.intentProcessor.process(any()) } returns false
@@ -168,10 +188,12 @@ class IntentReceiverActivityTest {
         verify { intentProcessors.privateIntentProcessor.process(intent) }
         assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
         assertTrue(actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false))
+        assertTrue(Events.openedLink.testHasValue())
     }
 
     @Test
     fun `process intent with launchLinksInPrivateTab set to false`() = runBlockingTest {
+        assertFalse(Events.openedLink.testHasValue())
         val intent = Intent()
 
         val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get()
@@ -180,13 +202,16 @@ class IntentReceiverActivityTest {
 
         coVerify(exactly = 0) { intentProcessors.privateIntentProcessor.process(intent) }
         coVerify { intentProcessors.intentProcessor.process(intent) }
+        assertTrue(Events.openedLink.testHasValue())
     }
 
     @Test
     fun `process custom tab intent`() = runBlockingTest {
+        assertFalse(Events.openedLink.testHasValue())
         val intent = Intent()
         coEvery { intentProcessors.intentProcessor.process(intent) } returns false
         coEvery { intentProcessors.customTabIntentProcessor.process(intent) } returns true
+        assertFalse(Events.openedLink.testHasValue())
 
         val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get()
         attachMocks(activity)
@@ -197,10 +222,12 @@ class IntentReceiverActivityTest {
 
         assertEquals(ExternalAppBrowserActivity::class.java.name, intent.component!!.className)
         assertTrue(intent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, false))
+        assertTrue(Events.openedLink.testHasValue())
     }
 
     @Test
     fun `process private custom tab intent`() = runBlockingTest {
+        assertFalse(Events.openedLink.testHasValue())
         every { settings.openLinksInAPrivateTab } returns true
 
         val intent = Intent()
@@ -216,6 +243,7 @@ class IntentReceiverActivityTest {
 
         assertEquals(ExternalAppBrowserActivity::class.java.name, intent.component!!.className)
         assertTrue(intent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, false))
+        assertTrue(Events.openedLink.testHasValue())
     }
 
     private fun attachMocks(activity: Activity) {