From dbbfca490324ae0c066e50d08b5d45b1286426dd Mon Sep 17 00:00:00 2001 From: mcarare Date: Wed, 30 Mar 2022 14:45:55 +0300 Subject: [PATCH] For #24210: Remove wrapper from "default browser" events. --- .../main/java/org/mozilla/fenix/HomeActivity.kt | 5 +++-- .../mozilla/fenix/components/metrics/Event.kt | 2 -- .../components/metrics/GleanMetricsService.kt | 7 +------ .../home/intent/DefaultBrowserIntentProcessor.kt | 7 +++---- .../metrics/GleanMetricsServiceTest.kt | 12 ------------ .../intent/DefaultBrowserIntentProcessorTest.kt | 16 +++++++++++++--- 6 files changed, 20 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 17c5610a9f8e..678a11b59176 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 c88291099430..8f105205cb25 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 @@ -177,8 +177,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 ad1a315748e0..94369b2de62f 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 @@ -129,12 +129,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.OpenedBookmark -> EventWrapper( { BookmarksManagement.open.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 bd395515d01f..8952a240ec27 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 be6ca9ee9742..86190cb837f6 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 @@ -17,7 +17,6 @@ import org.mozilla.fenix.GleanMetrics.Addons import org.mozilla.fenix.GleanMetrics.Awesomebar import org.mozilla.fenix.GleanMetrics.BookmarksManagement 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 @@ -203,17 +202,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 8df30c6cd33d..a8bd978f35b8 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 } }