For mozilla-mobile#24210: Remove wrapper from "app opened" event.

mcarare · Mar 30, 2022 · 5ce2c65 · 5ce2c65
1 parent a1243a8
commit 5ce2c65
Show file tree

Hide file tree

Showing 8 changed files with 12 additions and 48 deletions.
diff --git a/app/metrics.yaml b/app/metrics.yaml
@@ -17,6 +17,7 @@ events:
         description: |
           The method used to open Fenix. Possible values are: `app_icon`,
           `custom_tab` or `link`
+        type: string
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/968
       - https://github.com/mozilla-mobile/fenix/issues/10616

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 org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.Metrics
 import org.mozilla.fenix.addons.AddonDetailsFragmentDirections
 import org.mozilla.fenix.addons.AddonPermissionsDetailsFragmentDirections
@@ -258,7 +259,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
             val safeIntent = intent?.toSafeIntent()
             safeIntent
                 ?.let(::getIntentSource)
-                ?.also { components.analytics.metrics.track(Event.OpenedApp(it)) }
+                ?.also { Events.appOpened.record(Events.AppOpenedExtra(it)) }
         }
         supportActionBar?.hide()
 
@@ -667,10 +668,10 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
     }
 
     @VisibleForTesting(otherwise = PROTECTED)
-    internal open fun getIntentSource(intent: SafeIntent): Event.OpenedApp.Source? {
+    internal open fun getIntentSource(intent: SafeIntent): String? {
         return when {
-            intent.isLauncherIntent -> Event.OpenedApp.Source.APP_ICON
-            intent.action == Intent.ACTION_VIEW -> Event.OpenedApp.Source.LINK
+            intent.isLauncherIntent -> "APP_ICON"
+            intent.action == Intent.ACTION_VIEW -> "LINK"
             else -> null
         }
     }

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
@@ -365,13 +365,6 @@ sealed class Event {
             get() = hashMapOf(Logins.saveLoginsSettingChangedKeys.setting to setting.name)
     }
 
-    data class OpenedApp(val source: Source) : Event() {
-        enum class Source { APP_ICON, LINK, CUSTOM_TAB }
-
-        override val extras: Map<Events.appOpenedKeys, String>?
-            get() = hashMapOf(Events.appOpenedKeys.source to source.name)
-    }
-
     data class CollectionSaveButtonPressed(val fromScreen: String) : Event() {
         override val extras: Map<Collections.saveButtonKeys, String>?
             get() = mapOf(Collections.saveButtonKeys.fromScreen to fromScreen)

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
@@ -96,10 +96,6 @@ private class EventWrapper<T : Enum<T>>(
 // FIXME(#19967): Migrate to non-deprecated API.
 private val Event.wrapper: EventWrapper<*>?
     get() = when (this) {
-        is Event.OpenedApp -> EventWrapper(
-            { Events.appOpened.record(it) },
-            { Events.appOpenedKeys.valueOf(it) }
-        )
         is Event.SearchBarTapped -> EventWrapper(
             { Events.searchBarTapped.record(it) },
             { Events.searchBarTappedKeys.valueOf(it) }

diff --git a/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivity.kt b/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivity.kt
@@ -17,7 +17,6 @@ import mozilla.components.support.utils.SafeIntent
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.NavGraphDirections
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.components
 import java.security.InvalidParameterException
 
@@ -45,7 +44,7 @@ open class ExternalAppBrowserActivity : HomeActivity() {
         return "Changing to fragment $fragmentName, isCustomTab: true"
     }
 
-    final override fun getIntentSource(intent: SafeIntent) = Event.OpenedApp.Source.CUSTOM_TAB
+    final override fun getIntentSource(intent: SafeIntent) = "CUSTOM_TAB"
 
     final override fun getIntentSessionId(intent: SafeIntent) = intent.getSessionId()
 

diff --git a/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt b/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt
@@ -26,7 +26,6 @@ import org.junit.runner.RunWith
 import org.mozilla.fenix.HomeActivity.Companion.PRIVATE_BROWSING_MODE
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
 import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@@ -50,10 +49,10 @@ class HomeActivityTest {
         val launcherIntent = Intent(Intent.ACTION_MAIN).apply {
             addCategory(Intent.CATEGORY_LAUNCHER)
         }.toSafeIntent()
-        assertEquals(Event.OpenedApp.Source.APP_ICON, activity.getIntentSource(launcherIntent))
+        assertEquals("APP_ICON", activity.getIntentSource(launcherIntent))
 
         val viewIntent = Intent(Intent.ACTION_VIEW).toSafeIntent()
-        assertEquals(Event.OpenedApp.Source.LINK, activity.getIntentSource(viewIntent))
+        assertEquals("LINK", activity.getIntentSource(viewIntent))
 
         val otherIntent = Intent().toSafeIntent()
         assertNull(activity.getIntentSource(otherIntent))

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
@@ -37,30 +37,6 @@ class GleanMetricsServiceTest {
         gleanService = GleanMetricsService(testContext)
     }
 
-    @Test
-    fun `the app_opened event is correctly recorded`() {
-        // Build the event wrapper used by Fenix.
-        val event = Event.OpenedApp(Event.OpenedApp.Source.APP_ICON)
-
-        // Feed the wrapped event in the Glean service.
-        gleanService.track(event)
-
-        // Use the testing API to verify that it's correctly recorded.
-        assertTrue(Events.appOpened.testHasValue())
-
-        // Get all the recorded events. We only expect 1 to be recorded.
-        val events = Events.appOpened.testGetValue()
-        assertEquals(1, events.size)
-
-        // Verify that we get the expected content out.
-        assertEquals("events", events[0].category)
-        assertEquals("app_opened", events[0].name)
-
-        // We only expect 1 extra key.
-        assertEquals(1, events[0].extra!!.size)
-        assertEquals("APP_ICON", events[0].extra!!["source"])
-    }
-
     @Test
     fun `synced tab event is correctly recorded`() {
         assertFalse(SyncedTabs.syncedTabsSuggestionClicked.testHasValue())

diff --git a/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt b/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt
@@ -28,7 +28,6 @@ import org.junit.runner.RunWith
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
 import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.utils.Settings
@@ -43,13 +42,13 @@ class ExternalAppBrowserActivityTest {
         val launcherIntent = Intent(Intent.ACTION_MAIN).apply {
             addCategory(Intent.CATEGORY_LAUNCHER)
         }.toSafeIntent()
-        assertEquals(Event.OpenedApp.Source.CUSTOM_TAB, activity.getIntentSource(launcherIntent))
+        assertEquals("CUSTOM_TAB", activity.getIntentSource(launcherIntent))
 
         val viewIntent = Intent(Intent.ACTION_VIEW).toSafeIntent()
-        assertEquals(Event.OpenedApp.Source.CUSTOM_TAB, activity.getIntentSource(viewIntent))
+        assertEquals("CUSTOM_TAB", activity.getIntentSource(viewIntent))
 
         val otherIntent = Intent().toSafeIntent()
-        assertEquals(Event.OpenedApp.Source.CUSTOM_TAB, activity.getIntentSource(otherIntent))
+        assertEquals("CUSTOM_TAB", activity.getIntentSource(otherIntent))
     }
 
     @Test