From cf9b6193ced8ddb4b299455a08f50361b3b54a6f Mon Sep 17 00:00:00 2001 From: mcarare Date: Wed, 30 Mar 2022 14:45:55 +0300 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/24210: Remove wrapper from "default browser" events. --- .../java/org/mozilla/fenix/HomeActivity.kt | 5 +++-- .../mozilla/fenix/components/metrics/Event.kt | 2 -- .../components/metrics/GleanMetricsService.kt | 21 +------------------ .../intent/DefaultBrowserIntentProcessor.kt | 7 +++---- .../metrics/GleanMetricsServiceTest.kt | 12 ----------- .../DefaultBrowserIntentProcessorTest.kt | 16 +++++++++++--- 6 files changed, 20 insertions(+), 43 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 17c5610a9f..678a11b591 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -68,6 +68,7 @@ import mozilla.components.support.locale.LocaleAwareAppCompatActivity import mozilla.components.support.utils.SafeIntent import mozilla.components.support.utils.toSafeIntent import mozilla.components.support.webextensions.WebExtensionPopupFeature +import mozilla.telemetry.glean.private.NoExtras import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.Metrics import org.mozilla.fenix.addons.AddonDetailsFragmentDirections @@ -172,7 +173,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { StartSearchIntentProcessor(components.analytics.metrics), OpenBrowserIntentProcessor(this, ::getIntentSessionId), OpenSpecificTabIntentProcessor(this), - DefaultBrowserIntentProcessor(this, components.analytics.metrics) + DefaultBrowserIntentProcessor(this) ) } @@ -346,7 +347,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { } if (settings().checkIfFenixIsDefaultBrowserOnAppResume()) { - metrics.track(Event.ChangedToDefaultBrowser) + Events.defaultBrowserChanged.record(NoExtras()) } DefaultBrowserNotificationWorker.setDefaultBrowserNotificationIfNeeded(applicationContext) 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 ef8fbc1678..52ff0ef7ae 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 @@ -122,8 +122,6 @@ sealed class Event { object AddonsOpenInSettings : Event() object VoiceSearchTapped : Event() object SearchWidgetInstalled : Event() - object ChangedToDefaultBrowser : Event() - object DefaultBrowserNotifTapped : Event() object ProgressiveWebAppOpenFromHomescreenTap : Event() object ProgressiveWebAppInstallAsShortcut : 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 8c5228c2fa..ecdafad7d6 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 @@ -122,12 +122,7 @@ private val Event.wrapper: EventWrapper<*>? is Event.ToolbarMenuShown -> EventWrapper( { Events.toolbarMenuVisible.record(it) } ) - is Event.ChangedToDefaultBrowser -> EventWrapper( - { Events.defaultBrowserChanged.record(it) } - ) - is Event.DefaultBrowserNotifTapped -> EventWrapper( - { Events.defaultBrowserNotifTapped.record(it) } - ) + is Event.CustomTabsMenuOpened -> EventWrapper( { CustomTab.menu.record(it) } ) @@ -140,10 +135,6 @@ private val Event.wrapper: EventWrapper<*>? is Event.NormalAndPrivateUriOpened -> EventWrapper( { Events.normalAndPrivateUriCount.add(1) } ) - is Event.PreferenceToggled -> EventWrapper( - { Events.preferenceToggled.record(it) }, - { Events.preferenceToggledKeys.valueOf(it) } - ) is Event.HistoryOpened -> EventWrapper( { History.opened.record(it) } ) @@ -235,11 +226,6 @@ private val Event.wrapper: EventWrapper<*>? is Event.NotificationMediaPause -> EventWrapper( { MediaNotification.pause.record(it) } ) - - is Event.OpenedLink -> EventWrapper( - { Events.openedLink.record(it) }, - { Events.openedLinkKeys.valueOf(it) } - ) is Event.OpenLogins -> EventWrapper( { Logins.openLogins.record(it) } ) @@ -459,11 +445,6 @@ private val Event.wrapper: EventWrapper<*>? is Event.HomeScreenCustomizedHomeClicked -> EventWrapper( { HomeScreen.customizeHomeClicked.record(it) } ) - is Event.TabViewSettingChanged -> EventWrapper( - { Events.tabViewChanged.record(it) }, - { Events.tabViewChangedKeys.valueOf(it) } - ) - is Event.BrowserToolbarHomeButtonClicked -> EventWrapper( { Events.browserToolbarHomeTapped.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessor.kt b/app/src/main/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessor.kt index bd395515d0..8952a240ec 100644 --- a/app/src/main/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessor.kt +++ b/app/src/main/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessor.kt @@ -6,9 +6,9 @@ package org.mozilla.fenix.home.intent import android.content.Intent import androidx.navigation.NavController +import mozilla.telemetry.glean.private.NoExtras +import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.HomeActivity -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.openSetDefaultBrowserOption import org.mozilla.fenix.ext.settings import org.mozilla.fenix.onboarding.DefaultBrowserNotificationWorker.Companion.isDefaultBrowserNotificationIntent @@ -21,13 +21,12 @@ import org.mozilla.fenix.onboarding.DefaultBrowserNotificationWorker.Companion.i */ class DefaultBrowserIntentProcessor( private val activity: HomeActivity, - private val metrics: MetricController ) : HomeIntentProcessor { override fun process(intent: Intent, navController: NavController, out: Intent): Boolean { return if (isDefaultBrowserNotificationIntent(intent)) { activity.openSetDefaultBrowserOption() - metrics.track(Event.DefaultBrowserNotifTapped) + Events.defaultBrowserNotifTapped.record(NoExtras()) true } else { false 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 index b33dab1da0..6bd280bf92 100644 --- a/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt @@ -16,7 +16,6 @@ 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.Events import org.mozilla.fenix.GleanMetrics.History import org.mozilla.fenix.GleanMetrics.RecentBookmarks import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage @@ -147,17 +146,6 @@ class GleanMetricsServiceTest { assertEquals("123", events[0].extra!!["addon_id"]) } - @Test - fun `default browser events are correctly recorded`() { - assertFalse(Events.defaultBrowserChanged.testHasValue()) - gleanService.track(Event.ChangedToDefaultBrowser) - assertTrue(Events.defaultBrowserChanged.testHasValue()) - - assertFalse(Events.defaultBrowserNotifTapped.testHasValue()) - gleanService.track(Event.DefaultBrowserNotifTapped) - assertTrue(Events.defaultBrowserNotifTapped.testHasValue()) - } - @Test fun `Home screen recent bookmarks events are correctly recorded`() { assertFalse(RecentBookmarks.shown.testHasValue()) diff --git a/app/src/test/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessorTest.kt b/app/src/test/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessorTest.kt index 8df30c6cd3..a8bd978f35 100644 --- a/app/src/test/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/intent/DefaultBrowserIntentProcessorTest.kt @@ -10,10 +10,14 @@ import io.mockk.Called import io.mockk.every import io.mockk.mockk import io.mockk.verify +import mozilla.components.service.glean.testing.GleanTestRule import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @@ -21,11 +25,14 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @RunWith(FenixRobolectricTestRunner::class) class DefaultBrowserIntentProcessorTest { + @get:Rule + val gleanTestRule = GleanTestRule(testContext) + @Test fun `do not process blank intents`() { val navController: NavController = mockk() val out: Intent = mockk() - val result = DefaultBrowserIntentProcessor(mockk(), mockk()) + val result = DefaultBrowserIntentProcessor(mockk()) .process(Intent(), navController, out) assertFalse(result) @@ -47,11 +54,14 @@ class DefaultBrowserIntentProcessorTest { every { activity.applicationContext } returns testContext every { metrics.track(any()) } returns Unit - val result = DefaultBrowserIntentProcessor(activity, metrics) + assertFalse(Events.defaultBrowserNotifTapped.testHasValue()) + + val result = DefaultBrowserIntentProcessor(activity) .process(intent, navController, out) assert(result) - verify { metrics.track(any()) } + + assertTrue(Events.defaultBrowserNotifTapped.testHasValue()) verify { navController wasNot Called } verify { out wasNot Called } }