For mozilla-mobile#24715: Remove wrapper from addons metrics.

mcarare · Apr 11, 2022 · 34dc838 · 34dc838
1 parent 08ce8fb
commit 34dc838
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 64 deletions.
diff --git a/app/metrics.yaml b/app/metrics.yaml
@@ -5662,6 +5662,7 @@ addons:
       addon_id:
         description: |
           The id of the add-on that was interacted with in the toolbar menu
+        type: string
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/6174
       - https://github.com/mozilla-mobile/fenix/issues/19923
@@ -5687,6 +5688,7 @@ addons:
       addon_id:
         description: |
           The id of the add-on that was interacted with
+        type: string
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/17644
     data_reviews:

diff --git a/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt b/app/src/main/java/org/mozilla/fenix/addons/InstalledAddonDetailsFragment.kt
@@ -20,9 +20,9 @@ import kotlinx.coroutines.launch
 import mozilla.components.feature.addons.Addon
 import mozilla.components.feature.addons.AddonManagerException
 import mozilla.components.feature.addons.ui.translateName
+import org.mozilla.fenix.GleanMetrics.Addons
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.databinding.FragmentInstalledAddOnDetailsBinding
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.runIfFragmentIsAttached
@@ -212,9 +212,8 @@ class InstalledAddonDetailsFragment : Fragment() {
         binding.settings.apply {
             isVisible = shouldSettingsBeVisible()
             setOnClickListener {
-                requireContext().components.analytics.metrics.track(
-                    Event.AddonOpenSetting(addon.id)
-                )
+                Addons.openAddonSetting.record(Addons.OpenAddonSettingExtra(addon.id))
+
                 val settingUrl = addon.installedState?.optionsPageUrl ?: return@setOnClickListener
                 val directions = if (addon.installedState?.openOptionsPageInTab == true) {
                     val components = it.context.components

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
@@ -6,7 +6,6 @@ package org.mozilla.fenix.components.metrics
 
 import mozilla.components.browser.state.search.SearchEngine
 import mozilla.components.feature.top.sites.TopSite
-import org.mozilla.fenix.GleanMetrics.Addons
 import org.mozilla.fenix.GleanMetrics.AppTheme
 import org.mozilla.fenix.GleanMetrics.Autoplay
 import org.mozilla.fenix.GleanMetrics.ContextMenu
@@ -77,7 +76,6 @@ sealed class Event {
                 }
             )
     }
-    object AddonsOpenInSettings : Event()
     object SearchWidgetInstalled : Event()
 
     object ProgressiveWebAppOpenFromHomescreenTap : Event()
@@ -172,16 +170,6 @@ sealed class Event {
         enum class Source { NEWTAB }
     }
 
-    data class AddonsOpenInToolbarMenu(val addonId: String) : Event() {
-        override val extras: Map<Addons.openAddonInToolbarMenuKeys, String>?
-            get() = hashMapOf(Addons.openAddonInToolbarMenuKeys.addonId to addonId)
-    }
-
-    data class AddonOpenSetting(val addonId: String) : Event() {
-        override val extras: Map<Addons.openAddonSettingKeys, String>?
-            get() = hashMapOf(Addons.openAddonSettingKeys.addonId to addonId)
-    }
-
     data class PerformedSearch(val eventSource: EventSource) : Event() {
         sealed class EngineSource {
             abstract val engine: SearchEngine

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
@@ -8,7 +8,6 @@ import android.content.Context
 import mozilla.components.service.glean.Glean
 import mozilla.components.service.glean.private.NoExtraKeys
 import mozilla.components.support.base.log.logger.Logger
-import org.mozilla.fenix.GleanMetrics.Addons
 import org.mozilla.fenix.GleanMetrics.AndroidAutofill
 import org.mozilla.fenix.GleanMetrics.AppTheme
 import org.mozilla.fenix.GleanMetrics.Autoplay
@@ -224,17 +223,7 @@ private val Event.wrapper: EventWrapper<*>?
             { AppTheme.darkThemeSelected.record(it) },
             { AppTheme.darkThemeSelectedKeys.valueOf(it) }
         )
-        is Event.AddonsOpenInSettings -> EventWrapper<NoExtraKeys>(
-            { Addons.openAddonsInSettings.record(it) }
-        )
-        is Event.AddonsOpenInToolbarMenu -> EventWrapper(
-            { Addons.openAddonInToolbarMenu.record(it) },
-            { Addons.openAddonInToolbarMenuKeys.valueOf(it) }
-        )
-        is Event.AddonOpenSetting -> EventWrapper(
-            { Addons.openAddonSetting.record(it) },
-            { Addons.openAddonSettingKeys.valueOf(it) }
-        )
+
         is Event.AutoPlaySettingVisited -> EventWrapper<NoExtraKeys>(
             { Autoplay.visitedSetting.record(it) }
         )

diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt
@@ -35,6 +35,7 @@ import mozilla.components.support.base.log.logger.Logger
 import mozilla.components.support.webextensions.facts.WebExtensionFacts
 import mozilla.telemetry.glean.private.NoExtras
 import org.mozilla.fenix.BuildConfig
+import org.mozilla.fenix.GleanMetrics.Addons
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.LoginDialog
 import org.mozilla.fenix.GleanMetrics.MediaNotification
@@ -134,6 +135,12 @@ internal class ReleaseMetricController(
         Component.FEATURE_CUSTOMTABS to CustomTabsFacts.Items.CLOSE -> {
             CustomTab.closed.record(NoExtras())
         }
+
+        Component.BROWSER_MENU to BrowserMenuFacts.Items.WEB_EXTENSION_MENU_ITEM -> {
+            metadata?.get("id")?.let { Addons.openAddonInToolbarMenu.record(Addons.OpenAddonInToolbarMenuExtra(it.toString())) }
+            Unit
+        }
+
         else -> {
             this.toEvent()?.also {
                 track(it)
@@ -224,10 +231,6 @@ internal class ReleaseMetricController(
             }
         }
 
-        Component.BROWSER_MENU == component && BrowserMenuFacts.Items.WEB_EXTENSION_MENU_ITEM == item -> {
-            metadata?.get("id")?.let { Event.AddonsOpenInToolbarMenu(it.toString()) }
-        }
-
         Component.FEATURE_MEDIA == component && MediaFacts.Items.STATE == item -> {
             when (action) {
                 Action.PLAY -> Event.MediaPlayState

diff --git a/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt
@@ -33,20 +33,19 @@ import mozilla.components.concept.sync.AccountObserver
 import mozilla.components.concept.sync.AuthType
 import mozilla.components.concept.sync.OAuthAccount
 import mozilla.components.concept.sync.Profile
+import mozilla.components.service.glean.private.NoExtras
 import mozilla.components.support.ktx.android.view.showKeyboard
-import mozilla.telemetry.glean.private.NoExtras
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.Config
+import org.mozilla.fenix.GleanMetrics.Addons
 import org.mozilla.fenix.GleanMetrics.Events
 import org.mozilla.fenix.GleanMetrics.TrackingProtection
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.databinding.AmoCollectionOverrideDialogBinding
 import org.mozilla.fenix.ext.application
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.getPreferenceKey
-import org.mozilla.fenix.ext.metrics
 import org.mozilla.fenix.ext.navigateToNotificationsSettings
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.ext.settings
@@ -280,7 +279,7 @@ class SettingsFragment : PreferenceFragmentCompat() {
                 SettingsFragmentDirections.actionSettingsFragmentToLocaleSettingsFragment()
             }
             resources.getString(R.string.pref_key_addons) -> {
-                requireContext().metrics.track(Event.AddonsOpenInSettings)
+                Addons.openAddonsInSettings.record(mozilla.components.service.glean.private.NoExtras())
                 SettingsFragmentDirections.actionSettingsFragmentToAddonsFragment()
             }
             resources.getString(R.string.pref_key_data_choices) -> {

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.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.mozilla.fenix.GleanMetrics.Addons
 import org.mozilla.fenix.GleanMetrics.Awesomebar
 import org.mozilla.fenix.GleanMetrics.CreditCards
 import org.mozilla.fenix.GleanMetrics.RecentBookmarks
@@ -68,33 +67,6 @@ class GleanMetricsServiceTest {
         assertTrue(Awesomebar.openedTabSuggestionClicked.testHasValue())
     }
 
-    @Test
-    fun `Addon events are correctly recorded`() {
-        assertFalse(Addons.openAddonsInSettings.testHasValue())
-        gleanService.track(Event.AddonsOpenInSettings)
-        assertTrue(Addons.openAddonsInSettings.testHasValue())
-
-        assertFalse(Addons.openAddonInToolbarMenu.testHasValue())
-        gleanService.track(Event.AddonsOpenInToolbarMenu("123"))
-        assertTrue(Addons.openAddonInToolbarMenu.testHasValue())
-        var events = Addons.openAddonInToolbarMenu.testGetValue()
-        assertEquals(1, events.size)
-        assertEquals("addons", events[0].category)
-        assertEquals("open_addon_in_toolbar_menu", events[0].name)
-        assertEquals(1, events[0].extra!!.size)
-        assertEquals("123", events[0].extra!!["addon_id"])
-
-        assertFalse(Addons.openAddonSetting.testHasValue())
-        gleanService.track(Event.AddonOpenSetting("123"))
-        assertTrue(Addons.openAddonSetting.testHasValue())
-        events = Addons.openAddonSetting.testGetValue()
-        assertEquals(1, events.size)
-        assertEquals("addons", events[0].category)
-        assertEquals("open_addon_setting", events[0].name)
-        assertEquals(1, events[0].extra!!.size)
-        assertEquals("123", events[0].extra!!["addon_id"])
-    }
-
     @Test
     fun `Home screen recent bookmarks events are correctly recorded`() {
         assertFalse(RecentBookmarks.shown.testHasValue())