From 14082b117ccea4b3da86e984afe91215f18ef119 Mon Sep 17 00:00:00 2001 From: mcarare Date: Fri, 15 Apr 2022 11:43:48 +0300 Subject: [PATCH] For #24789: Remove wrapper from PWA metrics. --- .../mozilla/fenix/components/metrics/Event.kt | 3 -- .../components/metrics/GleanMetricsService.kt | 8 ---- .../components/metrics/MetricController.kt | 15 +++++--- .../metrics/MetricControllerTest.kt | 37 ++++++++++++++++++- detekt-baseline.xml | 2 - 5 files changed, 44 insertions(+), 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 index 88a718468..04c97bc46 100644 --- 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 index 7d8fb108b..6ef32acea 100644 --- 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( - { ProgressiveWebApp.homescreenTap.record(it) } - ) - is Event.ProgressiveWebAppInstallAsShortcut -> EventWrapper( - { ProgressiveWebApp.installTap.record(it) } - ) - is Event.TabSettingsOpened -> EventWrapper( { 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 index 62d85890f..b6e78fcac 100644 --- 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 index baaad94ba..4413cc8f1 100644 --- 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(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 index 718343327..cb9ab36a8 100644 --- a/detekt-baseline.xml +++ b/detekt-baseline.xml @@ -324,8 +324,6 @@ UndocumentedPublicClass:Event.kt$Event$PocketTopSiteClicked : Event UndocumentedPublicClass:Event.kt$Event$PocketTopSiteRemoved : Event UndocumentedPublicClass:Event.kt$Event$PreferenceToggled : Event - UndocumentedPublicClass:Event.kt$Event$ProgressiveWebAppInstallAsShortcut : Event - UndocumentedPublicClass:Event.kt$Event$ProgressiveWebAppOpenFromHomescreenTap : Event UndocumentedPublicClass:Event.kt$Event$ReaderModeAppearanceOpened : Event UndocumentedPublicClass:Event.kt$Event$ReaderModeAvailable : Event UndocumentedPublicClass:Event.kt$Event$ReaderModeClosed : Event