From f41a3ffbc3169d37f5c84c1c0d191889895161a8 Mon Sep 17 00:00:00 2001 From: Alexandru2909 Date: Thu, 24 Mar 2022 14:15:08 +0200 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/24215 - Remove Event.wrapper for CustomTab metrics --- .../mozilla/fenix/components/metrics/Event.kt | 3 -- .../components/metrics/GleanMetricsService.kt | 11 ------ .../components/metrics/MetricController.kt | 14 +++++-- .../metrics/MetricControllerTest.kt | 39 ++++++++++++++++++- 4 files changed, 47 insertions(+), 20 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 bacc1d158f..d3958555a2 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 @@ -26,9 +26,6 @@ sealed class Event { object DismissedOnboarding : Event() object ClearedPrivateData : Event() object AddBookmark : Event() - object CustomTabsClosed : Event() - object CustomTabsActionTapped : Event() - object CustomTabsMenuOpened : Event() object HistoryHighlightOpened : Event() object HistorySearchGroupOpened : Event() object TabMediaPlay : 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 4b09558095..2512b0a3aa 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 @@ -17,7 +17,6 @@ import org.mozilla.fenix.GleanMetrics.BrowserSearch import org.mozilla.fenix.GleanMetrics.ContextMenu import org.mozilla.fenix.GleanMetrics.ContextualMenu import org.mozilla.fenix.GleanMetrics.CreditCards -import org.mozilla.fenix.GleanMetrics.CustomTab import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.ExperimentsDefaultBrowser import org.mozilla.fenix.GleanMetrics.HomeMenu @@ -116,16 +115,6 @@ private val Event.wrapper: EventWrapper<*>? is Event.SetDefaultBrowserToolbarMenuClicked -> EventWrapper( { ExperimentsDefaultBrowser.toolbarMenuClicked.record(it) } ) - - is Event.CustomTabsMenuOpened -> EventWrapper( - { CustomTab.menu.record(it) } - ) - is Event.CustomTabsActionTapped -> EventWrapper( - { CustomTab.actionButton.record(it) } - ) - is Event.CustomTabsClosed -> EventWrapper( - { CustomTab.closed.record(it) } - ) is Event.TabMediaPlay -> EventWrapper( { Tab.mediaPlay.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 b18399612f..55dc16fe8d 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 @@ -26,7 +26,6 @@ import mozilla.components.feature.search.telemetry.ads.AdsTelemetry import mozilla.components.feature.search.telemetry.incontent.InContentTelemetry import mozilla.components.feature.syncedtabs.facts.SyncedTabsFacts import mozilla.components.feature.top.sites.facts.TopSitesFacts -import mozilla.components.service.glean.private.NoExtras import mozilla.components.support.base.Component import mozilla.components.support.base.facts.Action import mozilla.components.support.base.facts.Fact @@ -34,10 +33,12 @@ import mozilla.components.support.base.facts.FactProcessor import mozilla.components.support.base.facts.Facts 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.Events import org.mozilla.fenix.GleanMetrics.LoginDialog import org.mozilla.fenix.GleanMetrics.MediaNotification +import org.mozilla.fenix.GleanMetrics.CustomTab import org.mozilla.fenix.GleanMetrics.PerfAwesomebar import org.mozilla.fenix.search.awesomebar.ShortcutsSuggestionProvider import org.mozilla.fenix.utils.Settings @@ -124,7 +125,14 @@ internal class ReleaseMetricController( } } Component.BROWSER_TOOLBAR to ToolbarFacts.Items.MENU -> { - Events.toolbarMenuVisible.record(NoExtras()) + metadata?.get("customTab")?.let { CustomTab.menu.record(NoExtras()) } + ?: Events.toolbarMenuVisible.record(NoExtras()) + } + Component.FEATURE_CUSTOMTABS to CustomTabsFacts.Items.ACTION_BUTTON -> { + CustomTab.actionButton.record(NoExtras()) + } + Component.FEATURE_CUSTOMTABS to CustomTabsFacts.Items.CLOSE -> { + CustomTab.closed.record(NoExtras()) } else -> { this.toEvent()?.also { @@ -219,8 +227,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_CUSTOMTABS == component && CustomTabsFacts.Items.CLOSE == item -> Event.CustomTabsClosed - Component.FEATURE_CUSTOMTABS == component && CustomTabsFacts.Items.ACTION_BUTTON == item -> Event.CustomTabsActionTapped Component.FEATURE_MEDIA == component && MediaFacts.Items.STATE == item -> { when (action) { 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 27a830827a..ed29c112fc 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 @@ -10,6 +10,7 @@ import io.mockk.impl.annotations.MockK import io.mockk.mockk import io.mockk.verify import io.mockk.verifyAll +import mozilla.components.browser.toolbar.facts.ToolbarFacts import mozilla.components.feature.awesomebar.facts.AwesomeBarFacts import mozilla.components.feature.customtabs.CustomTabsFacts import mozilla.components.feature.media.facts.MediaFacts @@ -30,6 +31,7 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.fenix.GleanMetrics.CustomTab import org.mozilla.fenix.GleanMetrics.LoginDialog import org.mozilla.fenix.GleanMetrics.MediaNotification import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @@ -411,8 +413,6 @@ class MetricControllerTest { val simpleMappings = listOf( // CreditCardAutofillDialogFacts.Items is already tested. - Triple(Component.FEATURE_CUSTOMTABS, CustomTabsFacts.Items.CLOSE, Event.CustomTabsClosed), - Triple(Component.FEATURE_CUSTOMTABS, CustomTabsFacts.Items.ACTION_BUTTON, Event.CustomTabsActionTapped), 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), @@ -474,6 +474,41 @@ class MetricControllerTest { } } + @Test + fun `WHEN processing a CustomTab fact THEN the right metric is recorded`() { + val controller = ReleaseMetricController(emptyList(), { true }, { true }, mockk()) + val action = mockk(relaxed = true) + var fact: Fact + + with(controller) { + fact = Fact( + Component.BROWSER_TOOLBAR, + action, + ToolbarFacts.Items.MENU, + metadata = mapOf("customTab" to true) + ) + fact.process() + + assertEquals(true, CustomTab.menu.testHasValue()) + assertEquals(1, CustomTab.menu.testGetValue().size) + assertEquals(null, CustomTab.menu.testGetValue().single().extra) + + fact = Fact(Component.FEATURE_CUSTOMTABS, action, CustomTabsFacts.Items.ACTION_BUTTON) + fact.process() + + assertEquals(true, CustomTab.actionButton.testHasValue()) + assertEquals(1, CustomTab.actionButton.testGetValue().size) + assertEquals(null, CustomTab.actionButton.testGetValue().single().extra) + + fact = Fact(Component.FEATURE_CUSTOMTABS, action, CustomTabsFacts.Items.CLOSE) + fact.process() + + assertEquals(true, CustomTab.closed.testHasValue()) + assertEquals(1, CustomTab.closed.testGetValue().size) + assertEquals(null, CustomTab.closed.testGetValue().single().extra) + } + } + @Test fun `search term group events should be sent to enabled service`() { val controller = ReleaseMetricController(