For mozilla-mobile#24789: Remove wrapper from PWA metrics.

mcarare · Apr 15, 2022 · 6185ed0 · 6185ed0
1 parent 62b89da
commit 6185ed0
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 21 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
@@ -16,9 +16,6 @@ sealed class Event {
     object HistorySearchGroupOpened : Event()
     object SearchWidgetInstalled : Event()
 
-    object ProgressiveWebAppOpenFromHomescreenTap : Event()
-    object ProgressiveWebAppInstallAsShortcut : Event()
-
     object TabSettingsOpened : Event()
 
     object SyncedTabSuggestionClicked : 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
@@ -13,7 +13,6 @@ import org.mozilla.fenix.GleanMetrics.BrowserSearch
 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.RecentlyVisitedHomepage
 import org.mozilla.fenix.GleanMetrics.StartOnHome
 import org.mozilla.fenix.GleanMetrics.SyncedTabs
@@ -80,13 +79,6 @@ private val Event.wrapper: EventWrapper<*>?
             }
         )
 
-        is Event.ProgressiveWebAppOpenFromHomescreenTap -> EventWrapper<NoExtraKeys>(
-            { ProgressiveWebApp.homescreenTap.record(it) }
-        )
-        is Event.ProgressiveWebAppInstallAsShortcut -> EventWrapper<NoExtraKeys>(
-            { ProgressiveWebApp.installTap.record(it) }
-        )
-
         is Event.TabSettingsOpened -> EventWrapper<NoExtraKeys>(
             { Tabs.settingOpened.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
@@ -46,6 +46,7 @@ import org.mozilla.fenix.GleanMetrics.LoginDialog
 import org.mozilla.fenix.GleanMetrics.MediaNotification
 import org.mozilla.fenix.GleanMetrics.MediaState
 import org.mozilla.fenix.GleanMetrics.PerfAwesomebar
+import org.mozilla.fenix.GleanMetrics.ProgressiveWebApp
 import org.mozilla.fenix.search.awesomebar.ShortcutsSuggestionProvider
 import org.mozilla.fenix.utils.Settings
 
@@ -216,6 +217,13 @@ internal class ReleaseMetricController(
             }
         }
 
+        Component.FEATURE_PWA to ProgressiveWebAppFacts.Items.HOMESCREEN_ICON_TAP -> {
+            ProgressiveWebApp.homescreenTap.record(NoExtras())
+        }
+        Component.FEATURE_PWA to ProgressiveWebAppFacts.Items.INSTALL_SHORTCUT -> {
+            ProgressiveWebApp.installTap.record(NoExtras())
+        }
+
         else -> {
             this.toEvent()?.also {
                 track(it)
@@ -318,12 +326,7 @@ internal class ReleaseMetricController(
             }
             null
         }
-        Component.FEATURE_PWA == component && ProgressiveWebAppFacts.Items.HOMESCREEN_ICON_TAP == item -> {
-            Event.ProgressiveWebAppOpenFromHomescreenTap
-        }
-        Component.FEATURE_PWA == component && ProgressiveWebAppFacts.Items.INSTALL_SHORTCUT == item -> {
-            Event.ProgressiveWebAppInstallAsShortcut
-        }
+
         Component.FEATURE_TOP_SITES == component && TopSitesFacts.Items.COUNT == item -> {
             value?.let {
                 var count = 0

diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt
@@ -41,6 +41,7 @@ import org.mozilla.fenix.GleanMetrics.CreditCards
 import org.mozilla.fenix.GleanMetrics.CustomTab
 import org.mozilla.fenix.GleanMetrics.LoginDialog
 import org.mozilla.fenix.GleanMetrics.MediaNotification
+import org.mozilla.fenix.GleanMetrics.ProgressiveWebApp
 import org.mozilla.fenix.components.metrics.ReleaseMetricController.Companion
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.utils.Settings
@@ -285,8 +286,6 @@ class MetricControllerTest {
 
         val simpleMappings = listOf(
             // CreditCardAutofillDialogFacts.Items is already tested.
-            Triple(Component.FEATURE_PWA, ProgressiveWebAppFacts.Items.HOMESCREEN_ICON_TAP, Event.ProgressiveWebAppOpenFromHomescreenTap),
-            Triple(Component.FEATURE_PWA, ProgressiveWebAppFacts.Items.INSTALL_SHORTCUT, Event.ProgressiveWebAppInstallAsShortcut),
             Triple(Component.FEATURE_SYNCEDTABS, SyncedTabsFacts.Items.SYNCED_TABS_SUGGESTION_CLICKED, Event.SyncedTabSuggestionClicked),
             Triple(Component.FEATURE_AWESOMEBAR, AwesomeBarFacts.Items.BOOKMARK_SUGGESTION_CLICKED, Event.BookmarkSuggestionClicked),
             Triple(Component.FEATURE_AWESOMEBAR, AwesomeBarFacts.Items.CLIPBOARD_SUGGESTION_CLICKED, Event.ClipboardSuggestionClicked),
@@ -500,4 +499,38 @@ class MetricControllerTest {
             assertEquals(null, event.testGetValue().single().extra)
         }
     }
+
+    @Test
+    fun `GIVEN pwa facts WHEN they are processed THEN the right metric is recorded`() {
+        val controller = ReleaseMetricController(emptyList(), { true }, { true }, mockk())
+        val action = mockk<Action>(relaxed = true)
+
+        // a PWA shortcut from homescreen was opened
+        val openPWA = Fact(
+            Component.FEATURE_PWA,
+            action,
+            ProgressiveWebAppFacts.Items.HOMESCREEN_ICON_TAP,
+        )
+
+        assertFalse(ProgressiveWebApp.homescreenTap.testHasValue())
+        controller.run {
+            openPWA.process()
+        }
+        assertTrue(ProgressiveWebApp.homescreenTap.testHasValue())
+
+        // a PWA shortcut was installed
+        val installPWA = Fact(
+            Component.FEATURE_PWA,
+            action,
+            ProgressiveWebAppFacts.Items.INSTALL_SHORTCUT,
+        )
+
+        assertFalse(ProgressiveWebApp.installTap.testHasValue())
+
+        controller.run {
+            installPWA.process()
+        }
+
+        assertTrue(ProgressiveWebApp.installTap.testHasValue())
+    }
 }
diff --git a/detekt-baseline.xml b/detekt-baseline.xml
@@ -324,8 +324,6 @@
     <ID>UndocumentedPublicClass:Event.kt$Event$PocketTopSiteClicked : Event</ID>
     <ID>UndocumentedPublicClass:Event.kt$Event$PocketTopSiteRemoved : Event</ID>
     <ID>UndocumentedPublicClass:Event.kt$Event$PreferenceToggled : Event</ID>
-    <ID>UndocumentedPublicClass:Event.kt$Event$ProgressiveWebAppInstallAsShortcut : Event</ID>
-    <ID>UndocumentedPublicClass:Event.kt$Event$ProgressiveWebAppOpenFromHomescreenTap : Event</ID>
     <ID>UndocumentedPublicClass:Event.kt$Event$ReaderModeAppearanceOpened : Event</ID>
     <ID>UndocumentedPublicClass:Event.kt$Event$ReaderModeAvailable : Event</ID>
     <ID>UndocumentedPublicClass:Event.kt$Event$ReaderModeClosed : Event</ID>