From bd599caa833f8cec74448611f4a45f31f28b38e7 Mon Sep 17 00:00:00 2001 From: mcarare Date: Thu, 7 Apr 2022 21:06:36 +0300 Subject: [PATCH] For #24205: Remove wrapper from set default browser toolbar menu. This metric is recorded in the context of the default browser experiment. --- .../mozilla/fenix/components/metrics/Event.kt | 3 --- .../components/metrics/GleanMetricsService.kt | 4 ---- .../toolbar/BrowserToolbarMenuController.kt | 4 ++-- ...DefaultBrowserToolbarMenuControllerTest.kt | 19 +++++++++++++++++++ detekt-baseline.xml | 1 - 5 files changed, 21 insertions(+), 10 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 b89e7d4f0..d21aa9766 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 @@ -76,9 +76,6 @@ sealed class Event { object SearchSuggestionClicked : Event() object OpenedTabSuggestionClicked : Event() - // Set default browser experiment metrics - object SetDefaultBrowserToolbarMenuClicked : Event() - // Home menu interaction object HomeMenuSettingsItemClicked : Event() object HomeScreenDisplayed : 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 c759da73f..3739ce59e 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 @@ -16,7 +16,6 @@ import org.mozilla.fenix.GleanMetrics.ContextMenu import org.mozilla.fenix.GleanMetrics.ContextualMenu import org.mozilla.fenix.GleanMetrics.CreditCards import org.mozilla.fenix.GleanMetrics.Events -import org.mozilla.fenix.GleanMetrics.ExperimentsDefaultBrowser import org.mozilla.fenix.GleanMetrics.HomeMenu import org.mozilla.fenix.GleanMetrics.HomeScreen import org.mozilla.fenix.GleanMetrics.Metrics @@ -105,9 +104,6 @@ private val Event.wrapper: EventWrapper<*>? { ContextMenu.itemTappedKeys.valueOf(it) } ) - is Event.SetDefaultBrowserToolbarMenuClicked -> EventWrapper( - { ExperimentsDefaultBrowser.toolbarMenuClicked.record(it) } - ) is Event.PocketTopSiteClicked -> EventWrapper( { Pocket.pocketTopSiteClicked.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt index 0cc037b0b..4b12e7675 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/BrowserToolbarMenuController.kt @@ -30,6 +30,7 @@ import mozilla.components.service.glean.private.NoExtras import mozilla.components.support.base.feature.ViewBoundFeatureWrapper import org.mozilla.fenix.GleanMetrics.Collections import org.mozilla.fenix.GleanMetrics.Events +import org.mozilla.fenix.GleanMetrics.ExperimentsDefaultBrowser import org.mozilla.fenix.GleanMetrics.ReaderMode import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.NavGraphDirections @@ -41,7 +42,6 @@ import org.mozilla.fenix.collections.SaveCollectionStep import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.accounts.AccountState -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView @@ -361,7 +361,7 @@ class DefaultBrowserToolbarMenuController( ) } is ToolbarMenu.Item.SetDefaultBrowser -> { - metrics.track(Event.SetDefaultBrowserToolbarMenuClicked) + ExperimentsDefaultBrowser.toolbarMenuClicked.record(NoExtras()) activity.openSetDefaultBrowserOption() } is ToolbarMenu.Item.RemoveFromTopSites -> { diff --git a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt index 42eda214f..b5a1d2387 100644 --- a/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/toolbar/DefaultBrowserToolbarMenuControllerTest.kt @@ -60,6 +60,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.GleanMetrics.Collections import org.mozilla.fenix.GleanMetrics.Events +import org.mozilla.fenix.GleanMetrics.ExperimentsDefaultBrowser import org.mozilla.fenix.GleanMetrics.ReaderMode import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.NavGraphDirections @@ -783,6 +784,24 @@ class DefaultBrowserToolbarMenuControllerTest { verify { navController.navigate(turnOnSyncDirections, null) } } + @Test + fun `GIVEN the default browser experiment WHEN SetDefaultBrowser menu item is pressed THEN proper metrics are recorded`() = runBlockingTest { + val item = ToolbarMenu.Item.SetDefaultBrowser + + val store: BrowserStore = mockk() + + val controller = createController( + scope = this, store = store, + bookmarkTapped = { _, _ -> } + ) + + assertFalse(ExperimentsDefaultBrowser.toolbarMenuClicked.testHasValue()) + + controller.handleToolbarItemInteraction(item) + + assertTrue(ExperimentsDefaultBrowser.toolbarMenuClicked.testHasValue()) + } + @Suppress("LongParameterList") private fun createController( scope: CoroutineScope, diff --git a/detekt-baseline.xml b/detekt-baseline.xml index 05e520a51..2ac68074a 100644 --- a/detekt-baseline.xml +++ b/detekt-baseline.xml @@ -368,7 +368,6 @@ UndocumentedPublicClass:Event.kt$Event$SearchWidgetVoiceSearchPressed : Event UndocumentedPublicClass:Event.kt$Event$SearchWithAds : Event UndocumentedPublicClass:Event.kt$Event$SendTab : Event - UndocumentedPublicClass:Event.kt$Event$SetDefaultBrowserToolbarMenuClicked : Event UndocumentedPublicClass:Event.kt$Event$ShareBookmark : Event UndocumentedPublicClass:Event.kt$Event$ShowAllBookmarks : Event UndocumentedPublicClass:Event.kt$Event$ShowAllRecentTabs : Event