From 9d06ac1f7c728da2f863748930284701bb4efc50 Mon Sep 17 00:00:00 2001 From: mcarare Date: Mon, 28 Mar 2022 18:36:08 +0300 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/24210: Remove wrapper from "app opened" event. --- mobile/android/fenix/app/metrics.yaml | 1 + .../main/java/org/mozilla/fenix/HomeActivity.kt | 9 ++++---- .../org/mozilla/fenix/components/metrics/Event.kt | 7 ------- .../components/metrics/GleanMetricsService.kt | 4 ---- .../fenix/customtabs/ExternalAppBrowserActivity.kt | 3 +-- .../java/org/mozilla/fenix/HomeActivityTest.kt | 5 ++--- .../components/metrics/GleanMetricsServiceTest.kt | 24 ---------------------- .../customtabs/ExternalAppBrowserActivityTest.kt | 7 +++---- 8 files changed, 12 insertions(+), 48 deletions(-) diff --git a/mobile/android/fenix/app/metrics.yaml b/mobile/android/fenix/app/metrics.yaml index cb18c9e3e9eb..d0119f158246 100644 --- a/mobile/android/fenix/app/metrics.yaml +++ b/mobile/android/fenix/app/metrics.yaml @@ -17,6 +17,7 @@ events: description: | The method used to open Fenix. Possible values are: `app_icon`, `custom_tab` or `link` + type: string bugs: - https://github.com/mozilla-mobile/fenix/issues/968 - https://github.com/mozilla-mobile/fenix/issues/10616 diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 2272041039cf..17c5610a9f8e 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/mobile/android/fenix/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 org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.Metrics import org.mozilla.fenix.addons.AddonDetailsFragmentDirections import org.mozilla.fenix.addons.AddonPermissionsDetailsFragmentDirections @@ -258,7 +259,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { val safeIntent = intent?.toSafeIntent() safeIntent ?.let(::getIntentSource) - ?.also { components.analytics.metrics.track(Event.OpenedApp(it)) } + ?.also { Events.appOpened.record(Events.AppOpenedExtra(it)) } } supportActionBar?.hide() @@ -667,10 +668,10 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { } @VisibleForTesting(otherwise = PROTECTED) - internal open fun getIntentSource(intent: SafeIntent): Event.OpenedApp.Source? { + internal open fun getIntentSource(intent: SafeIntent): String? { return when { - intent.isLauncherIntent -> Event.OpenedApp.Source.APP_ICON - intent.action == Intent.ACTION_VIEW -> Event.OpenedApp.Source.LINK + intent.isLauncherIntent -> "APP_ICON" + intent.action == Intent.ACTION_VIEW -> "LINK" else -> null } } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt index e008adb7f9b3..f1c4edc4f4bd 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt @@ -284,13 +284,6 @@ sealed class Event { get() = hashMapOf(Logins.saveLoginsSettingChangedKeys.setting to setting.name) } - data class OpenedApp(val source: Source) : Event() { - enum class Source { APP_ICON, LINK, CUSTOM_TAB } - - override val extras: Map? - get() = hashMapOf(Events.appOpenedKeys.source to source.name) - } - data class SearchBarTapped(val source: Source) : Event() { enum class Source { HOME, BROWSER } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index 4b5eebd6c75c..1f708e754296 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -89,10 +89,6 @@ private class EventWrapper>( // FIXME(#19967): Migrate to non-deprecated API. private val Event.wrapper: EventWrapper<*>? get() = when (this) { - is Event.OpenedApp -> EventWrapper( - { Events.appOpened.record(it) }, - { Events.appOpenedKeys.valueOf(it) } - ) is Event.SearchBarTapped -> EventWrapper( { Events.searchBarTapped.record(it) }, { Events.searchBarTappedKeys.valueOf(it) } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivity.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivity.kt index 566f00d7a6b8..896fe12969fe 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivity.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivity.kt @@ -17,7 +17,6 @@ import mozilla.components.support.utils.SafeIntent import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.NavGraphDirections -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import java.security.InvalidParameterException @@ -45,7 +44,7 @@ open class ExternalAppBrowserActivity : HomeActivity() { return "Changing to fragment $fragmentName, isCustomTab: true" } - final override fun getIntentSource(intent: SafeIntent) = Event.OpenedApp.Source.CUSTOM_TAB + final override fun getIntentSource(intent: SafeIntent) = "CUSTOM_TAB" final override fun getIntentSessionId(intent: SafeIntent) = intent.getSessionId() diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt index 31e3b18da878..e6133aca2840 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt +++ b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/HomeActivityTest.kt @@ -26,7 +26,6 @@ import org.junit.runner.RunWith import org.mozilla.fenix.HomeActivity.Companion.PRIVATE_BROWSING_MODE import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.settings import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @@ -50,10 +49,10 @@ class HomeActivityTest { val launcherIntent = Intent(Intent.ACTION_MAIN).apply { addCategory(Intent.CATEGORY_LAUNCHER) }.toSafeIntent() - assertEquals(Event.OpenedApp.Source.APP_ICON, activity.getIntentSource(launcherIntent)) + assertEquals("APP_ICON", activity.getIntentSource(launcherIntent)) val viewIntent = Intent(Intent.ACTION_VIEW).toSafeIntent() - assertEquals(Event.OpenedApp.Source.LINK, activity.getIntentSource(viewIntent)) + assertEquals("LINK", activity.getIntentSource(viewIntent)) val otherIntent = Intent().toSafeIntent() assertNull(activity.getIntentSource(otherIntent)) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt index c15717f9babd..b33dab1da0f0 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt +++ b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt @@ -37,30 +37,6 @@ class GleanMetricsServiceTest { } @Test - fun `the app_opened event is correctly recorded`() { - // Build the event wrapper used by Fenix. - val event = Event.OpenedApp(Event.OpenedApp.Source.APP_ICON) - - // Feed the wrapped event in the Glean service. - gleanService.track(event) - - // Use the testing API to verify that it's correctly recorded. - assertTrue(Events.appOpened.testHasValue()) - - // Get all the recorded events. We only expect 1 to be recorded. - val events = Events.appOpened.testGetValue() - assertEquals(1, events.size) - - // Verify that we get the expected content out. - assertEquals("events", events[0].category) - assertEquals("app_opened", events[0].name) - - // We only expect 1 extra key. - assertEquals(1, events[0].extra!!.size) - assertEquals("APP_ICON", events[0].extra!!["source"]) - } - - @Test fun `synced tab event is correctly recorded`() { assertFalse(SyncedTabs.syncedTabsSuggestionClicked.testHasValue()) gleanService.track(Event.SyncedTabSuggestionClicked) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt index 90545c701474..4d5bbd8d4119 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt +++ b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/customtabs/ExternalAppBrowserActivityTest.kt @@ -28,7 +28,6 @@ import org.junit.runner.RunWith import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.utils.Settings @@ -43,13 +42,13 @@ class ExternalAppBrowserActivityTest { val launcherIntent = Intent(Intent.ACTION_MAIN).apply { addCategory(Intent.CATEGORY_LAUNCHER) }.toSafeIntent() - assertEquals(Event.OpenedApp.Source.CUSTOM_TAB, activity.getIntentSource(launcherIntent)) + assertEquals("CUSTOM_TAB", activity.getIntentSource(launcherIntent)) val viewIntent = Intent(Intent.ACTION_VIEW).toSafeIntent() - assertEquals(Event.OpenedApp.Source.CUSTOM_TAB, activity.getIntentSource(viewIntent)) + assertEquals("CUSTOM_TAB", activity.getIntentSource(viewIntent)) val otherIntent = Intent().toSafeIntent() - assertEquals(Event.OpenedApp.Source.CUSTOM_TAB, activity.getIntentSource(otherIntent)) + assertEquals("CUSTOM_TAB", activity.getIntentSource(otherIntent)) } @Test -- 2.11.4.GIT