From 4abb8521900f31a128bcb0bd9763d50095f1e133 Mon Sep 17 00:00:00 2001 From: Roger Yang Date: Fri, 19 Mar 2021 14:13:24 -0400 Subject: [PATCH] [fenix] Closes https://github.com/mozilla-mobile/fenix/issues/17531: Use shared preference to store top sites count for telemetry (https://github.com/mozilla-mobile/fenix/pull/18557) --- .../java/org/mozilla/fenix/HomeActivity.kt | 4 -- .../mozilla/fenix/components/metrics/Event.kt | 3 -- .../components/metrics/GleanMetricsService.kt | 7 --- .../components/metrics/MetricController.kt | 6 +-- .../metrics/MetricControllerTest.kt | 50 +++---------------- 5 files changed, 9 insertions(+), 61 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt index 4961038ef9..7520d8a04d 100644 --- a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt +++ b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt @@ -362,10 +362,6 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity { window.addFlags(FLAG_SECURE) } - // We will remove this when AC code lands to emit a fact on getTopSites in DefaultTopSitesStorage - // https://github.com/mozilla-mobile/android-components/issues/8679 - settings().topSitesSize = components.core.topSitesStorage.cachedTopSites.size - lifecycleScope.launch(IO) { components.core.bookmarksStorage.getTree(BookmarkRoot.Root.id, true)?.let { val desktopRootNode = DesktopFolders( 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 b0f0f32d7c..c6d17dfed0 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 @@ -216,9 +216,6 @@ sealed class Event { object ContextMenuSelectAllTapped : Event() object ContextMenuShareTapped : Event() - object HaveTopSites : Event() - object HaveNoTopSites : Event() - object SyncedTabSuggestionClicked : Event() object BookmarkSuggestionClicked : Event() object ClipboardSuggestionClicked : 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 1ac712a42d..6ae3601b3f 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 @@ -751,13 +751,6 @@ private val Event.wrapper: EventWrapper<*>? Event.HaveNoOpenTabs -> EventWrapper( { Metrics.hasOpenTabs.set(false) } ) - Event.HaveTopSites -> EventWrapper( - { Metrics.hasTopSites.set(true) } - ) - Event.HaveNoTopSites -> EventWrapper( - { Metrics.hasTopSites.set(false) } - ) - is Event.BannerOpenInAppDisplayed -> EventWrapper( { BannerOpenInApp.displayed.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt index 41a144c69f..181ff4ea2f 100644 --- a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt +++ b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt @@ -266,11 +266,7 @@ internal class ReleaseMetricController( // Do nothing } - return if (count > 0) { - Event.HaveTopSites - } else { - Event.HaveNoTopSites - } + settings.topSitesSize = count } null } diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt index 52ecfcdcd6..2a824564f5 100644 --- a/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt @@ -16,7 +16,6 @@ import mozilla.components.support.base.facts.Action import mozilla.components.support.base.facts.Fact import mozilla.components.support.base.log.logger.Logger import mozilla.components.support.webextensions.facts.WebExtensionFacts -import org.junit.Assert.assertEquals import org.junit.Before import org.junit.Test import org.mozilla.fenix.utils.Settings @@ -182,59 +181,26 @@ class MetricControllerTest { } @Test - fun `topsites fact should convert to the right events`() { - var enabled = true + fun `topsites fact should set value in SharedPreference`() { + val enabled = true + val settings: Settings = mockk(relaxed = true) val controller = ReleaseMetricController( services = listOf(dataService1), isDataTelemetryEnabled = { enabled }, isMarketingDataTelemetryEnabled = { enabled }, - mockk() + settings ) - var fact = Fact( + val fact = Fact( Component.FEATURE_TOP_SITES, Action.INTERACTION, TopSitesFacts.Items.COUNT, "1" ) - assertEquals(controller.factToEvent(fact), Event.HaveTopSites) - - fact = Fact( - Component.FEATURE_TOP_SITES, - Action.INTERACTION, - TopSitesFacts.Items.COUNT, - "0" - ) - - assertEquals(controller.factToEvent(fact), Event.HaveNoTopSites) - - fact = Fact( - Component.FEATURE_TOP_SITES, - Action.INTERACTION, - TopSitesFacts.Items.COUNT, - "10" - ) - - assertEquals(controller.factToEvent(fact), Event.HaveTopSites) - - fact = Fact( - Component.FEATURE_TOP_SITES, - Action.INTERACTION, - TopSitesFacts.Items.COUNT, - "-4" - ) - - assertEquals(controller.factToEvent(fact), Event.HaveNoTopSites) - - fact = Fact( - Component.FEATURE_TOP_SITES, - Action.INTERACTION, - TopSitesFacts.Items.COUNT, - "test" - ) - - assertEquals(controller.factToEvent(fact), Event.HaveNoTopSites) + verify(exactly = 0) { settings.topSitesSize = any() } + controller.factToEvent(fact) + verify(exactly = 1) { settings.topSitesSize = any() } } @Test