From 2f2945d523b2b9282a55c397b2c14a0e8ff58d5b Mon Sep 17 00:00:00 2001 From: Roger Yang Date: Wed, 27 Jan 2021 10:51:03 -0500 Subject: [PATCH] [fenix] Closes https://github.com/mozilla-mobile/fenix/issues/17530: Update has open tabs metrics when tabs are opened or closed (https://github.com/mozilla-mobile/fenix/pull/17557) --- .../org/mozilla/fenix/TelemetryMiddleware.kt | 5 +++++ .../mozilla/fenix/components/metrics/Event.kt | 2 ++ .../components/metrics/GleanMetricsService.kt | 6 ++++++ .../mozilla/fenix/TelemetryMiddlewareTest.kt | 21 ++++++++++++++++++- 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/mozilla/fenix/TelemetryMiddleware.kt b/app/src/main/java/org/mozilla/fenix/TelemetryMiddleware.kt index c5c64df0a9..c39b97e5f7 100644 --- a/app/src/main/java/org/mozilla/fenix/TelemetryMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/TelemetryMiddleware.kt @@ -104,6 +104,11 @@ class TelemetryMiddleware( is TabListAction.RestoreAction -> { // Update/Persist tabs count whenever it changes settings.openTabsCount = context.state.normalTabs.count() + if (context.state.normalTabs.count() > 0) { + metrics.track(Event.HaveOpenTabs) + } else { + metrics.track(Event.HaveNoOpenTabs) + } } } } 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 f946077b27..3fa6b65897 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 @@ -197,6 +197,8 @@ sealed class Event { object SyncedTabOpened : Event() object RecentlyClosedTabsOpened : Event() + object HaveOpenTabs : Event() + object HaveNoOpenTabs : Event() object ContextMenuCopyTapped : Event() object ContextMenuSearchTapped : 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 035605dc09..00fc6bc338 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 @@ -720,6 +720,12 @@ private val Event.wrapper: EventWrapper<*>? is Event.ContextMenuShareTapped -> EventWrapper( { ContextualMenu.shareTapped.record(it) } ) + Event.HaveOpenTabs -> EventWrapper( + { Metrics.hasOpenTabs.set(true) } + ) + Event.HaveNoOpenTabs -> EventWrapper( + { Metrics.hasOpenTabs.set(false) } + ) // Don't record other events in Glean: is Event.AddBookmark -> null diff --git a/app/src/test/java/org/mozilla/fenix/TelemetryMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/TelemetryMiddlewareTest.kt index 37fb3c1aff..355c2bd10e 100644 --- a/app/src/test/java/org/mozilla/fenix/TelemetryMiddlewareTest.kt +++ b/app/src/test/java/org/mozilla/fenix/TelemetryMiddlewareTest.kt @@ -6,6 +6,8 @@ package org.mozilla.fenix import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestCoroutineDispatcher import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.DownloadAction import mozilla.components.browser.state.action.TabListAction @@ -15,10 +17,12 @@ import mozilla.components.browser.state.store.BrowserStore import mozilla.components.support.test.ext.joinBlocking import mozilla.components.support.test.mock import mozilla.components.support.test.robolectric.testContext +import mozilla.components.support.test.rule.MainCoroutineRule import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Before +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.components.metrics.Event @@ -28,6 +32,7 @@ import org.mozilla.fenix.search.telemetry.ads.AdsTelemetry import org.mozilla.fenix.utils.Settings @RunWith(FenixRobolectricTestRunner::class) +@ExperimentalCoroutinesApi class TelemetryMiddlewareTest { private lateinit var store: BrowserStore @@ -35,11 +40,15 @@ class TelemetryMiddlewareTest { private lateinit var telemetryMiddleware: TelemetryMiddleware private lateinit var metrics: MetricController private lateinit var adsTelemetry: AdsTelemetry + private val testDispatcher = TestCoroutineDispatcher() + + @get:Rule + val coroutinesTestRule = MainCoroutineRule(testDispatcher) @Before fun setUp() { settings = Settings(testContext) - metrics = mockk() + metrics = mockk(relaxed = true) adsTelemetry = mockk() telemetryMiddleware = TelemetryMiddleware( settings, @@ -55,6 +64,7 @@ class TelemetryMiddlewareTest { store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))).joinBlocking() assertEquals(1, settings.openTabsCount) + verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } } @Test @@ -63,6 +73,7 @@ class TelemetryMiddlewareTest { store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org", private = true))).joinBlocking() assertEquals(0, settings.openTabsCount) + verify(exactly = 1) { metrics.track(Event.HaveNoOpenTabs) } } @Test @@ -76,6 +87,7 @@ class TelemetryMiddlewareTest { ).joinBlocking() assertEquals(2, settings.openTabsCount) + verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } } @Test @@ -87,9 +99,11 @@ class TelemetryMiddlewareTest { ) ).joinBlocking() assertEquals(2, settings.openTabsCount) + verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } store.dispatch(TabListAction.RemoveTabAction("1")).joinBlocking() assertEquals(1, settings.openTabsCount) + verify(exactly = 2) { metrics.track(Event.HaveOpenTabs) } } @Test @@ -101,9 +115,11 @@ class TelemetryMiddlewareTest { ) ).joinBlocking() assertEquals(2, settings.openTabsCount) + verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } store.dispatch(TabListAction.RemoveAllTabsAction).joinBlocking() assertEquals(0, settings.openTabsCount) + verify(exactly = 1) { metrics.track(Event.HaveNoOpenTabs) } } @Test @@ -116,9 +132,11 @@ class TelemetryMiddlewareTest { ) ).joinBlocking() assertEquals(2, settings.openTabsCount) + verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } store.dispatch(TabListAction.RemoveAllNormalTabsAction).joinBlocking() assertEquals(0, settings.openTabsCount) + verify(exactly = 1) { metrics.track(Event.HaveNoOpenTabs) } } @Test @@ -131,6 +149,7 @@ class TelemetryMiddlewareTest { store.dispatch(TabListAction.RestoreAction(tabsToRestore)).joinBlocking() assertEquals(2, settings.openTabsCount) + verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } } @Test