From 480ab3dca72f94da9951a29bd3a26492b27b633d Mon Sep 17 00:00:00 2001 From: Alexandru2909 Date: Fri, 25 Mar 2022 15:16:45 +0200 Subject: [PATCH] For #24454 - Remove Event.wrapper for SyncAccount metrics --- .../mozilla/fenix/components/metrics/Event.kt | 4 ---- .../components/metrics/GleanMetricsService.kt | 13 ------------- .../account/AccountSettingsFragment.kt | 7 ++++--- .../mozilla/fenix/share/ShareController.kt | 8 ++++---- .../fenix/share/ShareControllerTest.kt | 19 +++++++++++++++---- 5 files changed, 23 insertions(+), 28 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 57a3396bb..e008adb7f 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 @@ -33,10 +33,6 @@ sealed class Event { object CustomTabsActionTapped : Event() object CustomTabsMenuOpened : Event() object NormalAndPrivateUriOpened : Event() - object SyncAccountOpened : Event() - object SyncAccountSyncNow : Event() - object SendTab : Event() - object SignInToSendTab : Event() object HistoryOpened : Event() object HistoryItemShared : Event() object HistoryItemOpened : 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 a5a3cff00..4b5eebd6c 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 @@ -37,7 +37,6 @@ import org.mozilla.fenix.GleanMetrics.RecentTabs import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage import org.mozilla.fenix.GleanMetrics.SearchTerms import org.mozilla.fenix.GleanMetrics.StartOnHome -import org.mozilla.fenix.GleanMetrics.SyncAccount import org.mozilla.fenix.GleanMetrics.SyncedTabs import org.mozilla.fenix.GleanMetrics.Tab import org.mozilla.fenix.GleanMetrics.Tabs @@ -156,18 +155,6 @@ private val Event.wrapper: EventWrapper<*>? is Event.NormalAndPrivateUriOpened -> EventWrapper( { Events.normalAndPrivateUriCount.add(1) } ) - is Event.SyncAccountOpened -> EventWrapper( - { SyncAccount.opened.record(it) } - ) - is Event.SyncAccountSyncNow -> EventWrapper( - { SyncAccount.syncNow.record(it) } - ) - is Event.SignInToSendTab -> EventWrapper( - { SyncAccount.signInToSendTab.record(it) } - ) - is Event.SendTab -> EventWrapper( - { SyncAccount.sendTab.record(it) } - ) is Event.PreferenceToggled -> EventWrapper( { Events.preferenceToggled.record(it) }, { Events.preferenceToggledKeys.valueOf(it) } diff --git a/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsFragment.kt index 6c3e75b3d..e097a805a 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/account/AccountSettingsFragment.kt @@ -34,11 +34,12 @@ import mozilla.components.service.fxa.sync.SyncReason import mozilla.components.service.fxa.sync.SyncStatusObserver import mozilla.components.service.fxa.sync.getLastSynced import mozilla.components.support.ktx.android.content.getColorFromAttr +import mozilla.telemetry.glean.private.NoExtras import org.mozilla.fenix.FeatureFlags +import org.mozilla.fenix.GleanMetrics.SyncAccount import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.StoreProvider -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getPreferenceKey import org.mozilla.fenix.ext.requireComponents @@ -81,7 +82,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - requireComponents.analytics.metrics.track(Event.SyncAccountOpened) + SyncAccount.opened.record(NoExtras()) } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { @@ -326,7 +327,7 @@ class AccountSettingsFragment : PreferenceFragmentCompat() { */ private fun syncNow() { viewLifecycleOwner.lifecycleScope.launch { - requireComponents.analytics.metrics.track(Event.SyncAccountSyncNow) + SyncAccount.syncNow.record(NoExtras()) // Trigger a sync. requireComponents.backgroundServices.accountManager.syncNow(SyncReason.User) // Poll for device events & update devices. diff --git a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt index 80cca4471..864abaa38 100644 --- a/app/src/main/java/org/mozilla/fenix/share/ShareController.kt +++ b/app/src/main/java/org/mozilla/fenix/share/ShareController.kt @@ -30,11 +30,11 @@ import mozilla.components.concept.sync.Device import mozilla.components.concept.sync.TabData import mozilla.components.feature.accounts.push.SendTabUseCases import mozilla.components.feature.share.RecentAppsStorage +import mozilla.components.service.glean.private.NoExtras import mozilla.components.support.ktx.kotlin.isExtensionUrl +import org.mozilla.fenix.GleanMetrics.SyncAccount import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar -import org.mozilla.fenix.components.metrics.Event -import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav import org.mozilla.fenix.share.listadapters.AppShareOption @@ -136,7 +136,7 @@ class DefaultShareController( } override fun handleShareToDevice(device: Device) { - context.metrics.track(Event.SendTab) + SyncAccount.sendTab.record(NoExtras()) shareToDevicesWithRetry { sendTabUseCases.sendToDeviceAsync(device.id, shareData.toTabData()) } } @@ -145,7 +145,7 @@ class DefaultShareController( } override fun handleSignIn() { - context.metrics.track(Event.SignInToSendTab) + SyncAccount.signInToSendTab.record(NoExtras()) val directions = ShareFragmentDirections.actionGlobalTurnOnSync(padSnackbar = true) navController.nav(R.id.shareFragment, directions) diff --git a/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt b/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt index 1f4fe807a..143d4ef63 100644 --- a/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt @@ -26,19 +26,21 @@ import mozilla.components.concept.sync.DeviceType import mozilla.components.concept.sync.TabData import mozilla.components.feature.accounts.push.SendTabUseCases import mozilla.components.feature.share.RecentAppsStorage +import mozilla.components.service.glean.testing.GleanTestRule import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.fenix.GleanMetrics.SyncAccount import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.metrics import org.mozilla.fenix.ext.nav @@ -68,6 +70,9 @@ class ShareControllerTest { private val dismiss = mockk<(ShareController.Result) -> Unit>(relaxed = true) private val recentAppStorage = mockk(relaxed = true) + @get:Rule + val gleanTestRule = GleanTestRule(testContext) + @get:Rule val coroutinesTestRule = MainCoroutineRule() private val testDispatcher = coroutinesTestRule.testDispatcher @@ -265,9 +270,12 @@ class ShareControllerTest { controller.handleShareToDevice(deviceToShareTo) + assertTrue(SyncAccount.sendTab.testHasValue()) + assertEquals(1, SyncAccount.sendTab.testGetValue().size) + assertNull(SyncAccount.sendTab.testGetValue().single().extra) + // Verify all the needed methods are called. - verifyOrder { - metrics.track(Event.SendTab) + verify { sendTabUseCases.sendToDeviceAsync(capture(deviceId), capture(tabsShared)) // dismiss() is also to be called, but at the moment cannot test it in a coroutine. } @@ -321,8 +329,11 @@ class ShareControllerTest { fun `handleSignIn should navigate to the Sync Fragment and dismiss this one`() { controller.handleSignIn() + assertTrue(SyncAccount.signInToSendTab.testHasValue()) + assertEquals(1, SyncAccount.signInToSendTab.testGetValue().size) + assertNull(SyncAccount.signInToSendTab.testGetValue().single().extra) + verifyOrder { - metrics.track(Event.SignInToSendTab) navController.nav( R.id.shareFragment, ShareFragmentDirections.actionGlobalTurnOnSync()