From 3678b124e0a037871b1b7f0b36c18feb166fdd6a Mon Sep 17 00:00:00 2001 From: Alexandru2909 Date: Fri, 15 Apr 2022 10:28:16 +0300 Subject: [PATCH] For #24790 - Remove Event.wrapper for home screen related telemetry --- .../java/org/mozilla/fenix/HomeActivity.kt | 5 ++-- .../mozilla/fenix/components/metrics/Event.kt | 10 -------- .../components/metrics/GleanMetricsService.kt | 24 ------------------- .../org/mozilla/fenix/home/HomeFragment.kt | 17 +++++++------ .../SessionControlController.kt | 8 +++---- .../DefaultSessionControlControllerTest.kt | 7 +++--- .../browser/InactiveTabsControllerTest.kt | 2 -- detekt-baseline.xml | 1 - 8 files changed, 17 insertions(+), 57 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 98d33dcd1..84d9d7079 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -71,19 +71,18 @@ 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.GleanMetrics.StartOnHome import org.mozilla.fenix.addons.AddonDetailsFragmentDirections import org.mozilla.fenix.addons.AddonPermissionsDetailsFragmentDirections import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.browsingmode.BrowsingModeManager import org.mozilla.fenix.browser.browsingmode.DefaultBrowsingModeManager import org.mozilla.fenix.components.metrics.BreadcrumbsRecorder -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.databinding.ActivityHomeBinding import org.mozilla.fenix.exceptions.trackingprotection.TrackingProtectionExceptionsFragmentDirections import org.mozilla.fenix.ext.alreadyOnDestination import org.mozilla.fenix.ext.breadcrumb import org.mozilla.fenix.ext.components -import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.setNavigationIcon import org.mozilla.fenix.ext.settings @@ -248,7 +247,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { if (!shouldStartOnHome() && shouldNavigateToBrowserOnColdStart(savedInstanceState)) { navigateToBrowserOnColdStart() } else { - components.analytics.metrics.track(Event.StartOnHomeEnterHomeScreen) + StartOnHome.enterHomeScreen.record(NoExtras()) } Performance.processIntentIfPerformanceTest(intent, this) 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 04c97bc46..5d8708e81 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,16 +26,6 @@ sealed class Event { object SearchSuggestionClicked : Event() object OpenedTabSuggestionClicked : Event() - // Home menu interaction - object HomeMenuSettingsItemClicked : Event() - object HomeScreenDisplayed : Event() - object HomeScreenViewCount : Event() - object HomeScreenCustomizedHomeClicked : Event() - - // Start on Home - object StartOnHomeEnterHomeScreen : Event() - object StartOnHomeOpenTabsTray : Event() - // Interaction events with extras data class SearchWithAds(val providerName: String) : 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 6ef32acea..47ddf8629 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 @@ -10,11 +10,8 @@ import mozilla.components.service.glean.private.NoExtraKeys import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.GleanMetrics.Awesomebar import org.mozilla.fenix.GleanMetrics.BrowserSearch -import org.mozilla.fenix.GleanMetrics.HomeMenu -import org.mozilla.fenix.GleanMetrics.HomeScreen import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage -import org.mozilla.fenix.GleanMetrics.StartOnHome import org.mozilla.fenix.GleanMetrics.SyncedTabs import org.mozilla.fenix.GleanMetrics.Tabs import org.mozilla.fenix.GleanMetrics.Messaging @@ -105,27 +102,6 @@ private val Event.wrapper: EventWrapper<*>? { Awesomebar.openedTabSuggestionClicked.record(it) } ) - is Event.HomeMenuSettingsItemClicked -> EventWrapper( - { HomeMenu.settingsItemClicked.record(it) } - ) - - is Event.HomeScreenDisplayed -> EventWrapper( - { HomeScreen.homeScreenDisplayed.record(it) } - ) - is Event.HomeScreenViewCount -> EventWrapper( - { HomeScreen.homeScreenViewCount.add() } - ) - is Event.HomeScreenCustomizedHomeClicked -> EventWrapper( - { HomeScreen.customizeHomeClicked.record(it) } - ) - is Event.StartOnHomeEnterHomeScreen -> EventWrapper( - { StartOnHome.enterHomeScreen.record(it) } - ) - - is Event.StartOnHomeOpenTabsTray -> EventWrapper( - { StartOnHome.openTabsTray.record(it) } - ) - is Event.Messaging.MessageShown -> EventWrapper( { Messaging.messageShown.record( diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index cd8c212f1..d002835af 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -78,6 +78,8 @@ import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.Config import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.GleanMetrics.Events +import org.mozilla.fenix.GleanMetrics.HomeScreen +import org.mozilla.fenix.GleanMetrics.StartOnHome import org.mozilla.fenix.GleanMetrics.Wallpapers import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R @@ -95,7 +97,6 @@ import org.mozilla.fenix.components.toolbar.ToolbarPosition import org.mozilla.fenix.databinding.FragmentHomeBinding import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.hideToolbar -import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.runIfFragmentIsAttached @@ -133,6 +134,7 @@ import org.mozilla.fenix.wallpapers.WallpaperManager import org.mozilla.fenix.whatsnew.WhatsNew import java.lang.ref.WeakReference import kotlin.math.min +import org.mozilla.fenix.GleanMetrics.HomeMenu as HomeMenuMetrics @Suppress("TooManyFunctions", "LargeClass") class HomeFragment : Fragment() { @@ -335,7 +337,6 @@ class HomeFragment : Fragment() { activity = activity, settings = components.settings, engine = components.core.engine, - metrics = components.analytics.metrics, messageController = DefaultMessageController( appStore = components.appStore, messagingStorage = components.analytics.messagingStorage, @@ -497,10 +498,8 @@ class HomeFragment : Fragment() { val profilerStartTime = requireComponents.core.engine.profiler?.getProfilerTime() super.onViewCreated(view, savedInstanceState) - context?.metrics?.apply { - track(Event.HomeScreenDisplayed) - track(Event.HomeScreenViewCount) - } + HomeScreen.homeScreenDisplayed.record(NoExtras()) + HomeScreen.homeScreenViewCount.add() observeSearchEngineChanges() createHomeMenu(requireContext(), WeakReference(binding.menuButton)) @@ -530,7 +529,7 @@ class HomeFragment : Fragment() { } binding.tabButton.setOnClickListener { - requireComponents.analytics.metrics.track(Event.StartOnHomeOpenTabsTray) + StartOnHome.openTabsTray.record(NoExtras()) openTabsTray() } @@ -923,10 +922,10 @@ class HomeFragment : Fragment() { R.id.homeFragment, HomeFragmentDirections.actionGlobalSettingsFragment() ) - requireComponents.analytics.metrics.track(Event.HomeMenuSettingsItemClicked) + HomeMenuMetrics.settingsItemClicked.record(NoExtras()) } HomeMenu.Item.CustomizeHome -> { - context.metrics.track(Event.HomeScreenCustomizedHomeClicked) + HomeScreen.customizeHomeClicked.record(NoExtras()) nav( R.id.homeFragment, HomeFragmentDirections.actionGlobalHomeSettingsFragment() diff --git a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt index 9820c21d0..f7038bb14 100644 --- a/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/sessioncontrol/SessionControlController.kt @@ -33,6 +33,7 @@ import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.GleanMetrics.Collections import org.mozilla.fenix.GleanMetrics.Events +import org.mozilla.fenix.GleanMetrics.HomeScreen import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.Pocket import org.mozilla.fenix.GleanMetrics.RecentBookmarks @@ -47,8 +48,6 @@ import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.appstate.AppState -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.metrics.MetricsUtils import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.nav @@ -215,12 +214,11 @@ interface SessionControlController { fun handleReportSessionMetrics(state: AppState) } -@Suppress("TooManyFunctions", "LargeClass") +@Suppress("TooManyFunctions", "LargeClass", "LongParameterList") class DefaultSessionControlController( private val activity: HomeActivity, private val settings: Settings, private val engine: Engine, - private val metrics: MetricController, private val messageController: MessageController, private val store: BrowserStore, private val tabCollectionStorage: TabCollectionStorage, @@ -511,7 +509,7 @@ class DefaultSessionControlController( override fun handleCustomizeHomeTapped() { val directions = HomeFragmentDirections.actionGlobalHomeSettingsFragment() navController.nav(navController.currentDestination?.id, directions) - metrics.track(Event.HomeScreenCustomizedHomeClicked) + HomeScreen.customizeHomeClicked.record(NoExtras()) } override fun handleShowOnboardingDialog() { diff --git a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt index 0bb30c4ec..41b9524ec 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -50,6 +50,7 @@ import org.junit.runner.RunWith import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.GleanMetrics.Collections import org.mozilla.fenix.GleanMetrics.Events +import org.mozilla.fenix.GleanMetrics.HomeScreen import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.RecentTabs import org.mozilla.fenix.GleanMetrics.RecentBookmarks @@ -63,7 +64,6 @@ import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.TabCollectionStorage import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.appstate.AppState -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.settings @@ -186,9 +186,11 @@ class DefaultSessionControlControllerTest { @Test fun handleCustomizeHomeTapped() { + assertFalse(HomeScreen.customizeHomeClicked.testHasValue()) + createController().handleCustomizeHomeTapped() - verify { metrics.track(Event.HomeScreenCustomizedHomeClicked) } + assertTrue(HomeScreen.customizeHomeClicked.testHasValue()) verify { navController.navigate( match { @@ -1264,7 +1266,6 @@ class DefaultSessionControlControllerTest { activity = activity, settings = settings, engine = engine, - metrics = metrics, store = store, messageController = messageController, tabCollectionStorage = tabCollectionStorage, diff --git a/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt b/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt index 4696eed22..570ad2da4 100644 --- a/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/tabstray/browser/InactiveTabsControllerTest.kt @@ -26,7 +26,6 @@ import mozilla.components.browser.state.state.createTab as createTabState import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.components.AppStore -import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.tabstray.TabsTrayState import org.mozilla.fenix.tabstray.TabsTrayStore @@ -35,7 +34,6 @@ import org.mozilla.fenix.utils.Settings @RunWith(FenixRobolectricTestRunner::class) // for gleanTestRule class InactiveTabsControllerTest { - private val metrics: MetricController = mockk(relaxed = true) private val settings: Settings = mockk(relaxed = true) private val appStore = AppStore() diff --git a/detekt-baseline.xml b/detekt-baseline.xml index cb9ab36a8..b7859c338 100644 --- a/detekt-baseline.xml +++ b/detekt-baseline.xml @@ -18,7 +18,6 @@ LongParameterList:QuickSettingsController.kt$DefaultQuickSettingsController$( private val context: Context, private val quickSettingsStore: QuickSettingsFragmentStore, private val browserStore: BrowserStore, private val ioScope: CoroutineScope, private val navController: NavController, @VisibleForTesting internal val sessionId: String, @VisibleForTesting internal var sitePermissions: SitePermissions?, private val settings: Settings, private val permissionStorage: PermissionStorage, private val reload: ReloadUrlUseCase, private val requestRuntimePermissions: OnNeedToRequestPermissions = { }, private val displayPermissions: () -> Unit, private val engine: Engine = context.components.core.engine, ) LongParameterList:RecentVisitsController.kt$DefaultRecentVisitsController$( private val store: BrowserStore, private val appStore: AppStore, private val selectOrAddTabUseCase: SelectOrAddUseCase, private val navController: NavController, private val storage: HistoryMetadataStorage, private val scope: CoroutineScope, private val metrics: MetricController ) LongParameterList:RecentlyClosedController.kt$DefaultRecentlyClosedController$( private val navController: NavController, private val browserStore: BrowserStore, private val recentlyClosedStore: RecentlyClosedFragmentStore, private val recentlyClosedTabsStorage: RecentlyClosedTabsStorage, private val tabsUseCases: TabsUseCases, private val activity: HomeActivity, private val metrics: MetricController, private val lifecycleScope: CoroutineScope, private val openToBrowser: (url: String, mode: BrowsingMode?) -> Unit ) - LongParameterList:SessionControlController.kt$DefaultSessionControlController$( private val activity: HomeActivity, private val settings: Settings, private val engine: Engine, private val metrics: MetricController, private val messageController: MessageController, private val store: BrowserStore, private val tabCollectionStorage: TabCollectionStorage, private val addTabUseCase: TabsUseCases.AddNewTabUseCase, private val restoreUseCase: TabsUseCases.RestoreUseCase, private val reloadUrlUseCase: SessionUseCases.ReloadUrlUseCase, private val selectTabUseCase: TabsUseCases.SelectTabUseCase, private val appStore: AppStore, private val navController: NavController, private val viewLifecycleScope: CoroutineScope, private val hideOnboarding: () -> Unit, private val registerCollectionStorageObserver: () -> Unit, private val removeCollectionWithUndo: (tabCollection: TabCollection) -> Unit, private val showTabTray: () -> Unit ) LongParameterList:ShareController.kt$DefaultShareController$( private val context: Context, private val shareSubject: String?, private val shareData: List<ShareData>, private val sendTabUseCases: SendTabUseCases, private val snackbar: FenixSnackbar, private val navController: NavController, private val recentAppsStorage: RecentAppsStorage, private val viewLifecycleScope: CoroutineScope, private val dispatcher: CoroutineDispatcher = Dispatchers.IO, private val dismiss: (ShareController.Result) -> Unit ) LongParameterList:TabsTrayController.kt$DefaultTabsTrayController$( private val trayStore: TabsTrayStore, private val browserStore: BrowserStore, private val browsingModeManager: BrowsingModeManager, private val navController: NavController, private val navigateToHomeAndDeleteSession: (String) -> Unit, private val profiler: Profiler?, private val navigationInteractor: NavigationInteractor, private val metrics: MetricController, private val tabsUseCases: TabsUseCases, private val selectTabPosition: (Int, Boolean) -> Unit, private val dismissTray: () -> Unit, private val showUndoSnackbarForTab: (Boolean) -> Unit, @VisibleForTesting internal val showCancelledDownloadWarning: (downloadCount: Int, tabId: String?, source: String?) -> Unit, ) LongParameterList:ToolbarIntegration.kt$DefaultToolbarIntegration$( context: Context, toolbar: BrowserToolbar, toolbarMenu: ToolbarMenu, domainAutocompleteProvider: DomainAutocompleteProvider, historyStorage: HistoryStorage, lifecycleOwner: LifecycleOwner, sessionId: String? = null, isPrivate: Boolean, interactor: BrowserToolbarInteractor, engine: Engine )