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 b0b9a953d..864df2fa6 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 @@ -28,9 +28,6 @@ sealed class Event { object TabSettingsOpened : Event() - object HaveOpenTabs : Event() - object HaveNoOpenTabs : Event() - object ContextMenuCopyTapped : Event() object ContextMenuSearchTapped : Event() object ContextMenuSelectAllTapped : 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 9e7a39b0f..aacf6d5e3 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 @@ -131,12 +131,6 @@ 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) } - ) is Event.SyncedTabSuggestionClicked -> EventWrapper( { SyncedTabs.syncedTabsSuggestionClicked.record(it) } ) diff --git a/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt b/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt index 1bd92278c..908f07d78 100644 --- a/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt +++ b/app/src/main/java/org/mozilla/fenix/telemetry/TelemetryMiddleware.kt @@ -21,7 +21,7 @@ import mozilla.components.lib.state.MiddlewareContext import mozilla.components.support.base.android.Clock import mozilla.components.support.base.log.logger.Logger import org.mozilla.fenix.GleanMetrics.Events -import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.GleanMetrics.Metrics import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.utils.Settings import org.mozilla.fenix.GleanMetrics.EngineTab as EngineMetrics @@ -75,10 +75,10 @@ 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) + if (context.state.normalTabs.isNotEmpty()) { + Metrics.hasOpenTabs.set(true) } else { - metrics.track(Event.HaveNoOpenTabs) + Metrics.hasOpenTabs.set(false) } } } diff --git a/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt index a28448e2b..571e4963a 100644 --- a/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt +++ b/app/src/test/java/org/mozilla/fenix/telemetry/TelemetryMiddlewareTest.kt @@ -6,7 +6,6 @@ package org.mozilla.fenix.telemetry import androidx.test.core.app.ApplicationProvider import io.mockk.mockk -import io.mockk.verify import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.EngineAction import mozilla.components.browser.state.action.TabListAction @@ -30,7 +29,7 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mozilla.fenix.GleanMetrics.Events -import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.GleanMetrics.Metrics import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.utils.Settings @@ -76,24 +75,32 @@ class TelemetryMiddlewareTest { @Test fun `WHEN a tab is added THEN the open tab count is updated`() { assertEquals(0, settings.openTabsCount) + assertFalse(Metrics.hasOpenTabs.testHasValue()) store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org"))).joinBlocking() assertEquals(1, settings.openTabsCount) - verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } + + assertTrue(Metrics.hasOpenTabs.testHasValue()) + assertTrue(Metrics.hasOpenTabs.testGetValue()) } @Test fun `WHEN a private tab is added THEN the open tab count is not updated`() { assertEquals(0, settings.openTabsCount) + assertFalse(Metrics.hasOpenTabs.testHasValue()) store.dispatch(TabListAction.AddTabAction(createTab("https://mozilla.org", private = true))).joinBlocking() assertEquals(0, settings.openTabsCount) - verify(exactly = 1) { metrics.track(Event.HaveNoOpenTabs) } + + assertTrue(Metrics.hasOpenTabs.testHasValue()) + assertFalse(Metrics.hasOpenTabs.testGetValue()) } @Test fun `WHEN multiple tabs are added THEN the open tab count is updated`() { assertEquals(0, settings.openTabsCount) + assertFalse(Metrics.hasOpenTabs.testHasValue()) + store.dispatch( TabListAction.AddMultipleTabsAction( listOf( @@ -104,11 +111,15 @@ class TelemetryMiddlewareTest { ).joinBlocking() assertEquals(2, settings.openTabsCount) - verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } + + assertTrue(Metrics.hasOpenTabs.testHasValue()) + assertTrue(Metrics.hasOpenTabs.testGetValue()) } @Test fun `WHEN a tab is removed THEN the open tab count is updated`() { + assertFalse(Metrics.hasOpenTabs.testHasValue()) + store.dispatch( TabListAction.AddMultipleTabsAction( listOf( @@ -118,15 +129,18 @@ 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) } + + assertTrue(Metrics.hasOpenTabs.testHasValue()) + assertTrue(Metrics.hasOpenTabs.testGetValue()) } @Test fun `WHEN all tabs are removed THEN the open tab count is updated`() { + assertFalse(Metrics.hasOpenTabs.testHasValue()) + store.dispatch( TabListAction.AddMultipleTabsAction( listOf( @@ -136,15 +150,21 @@ class TelemetryMiddlewareTest { ) ).joinBlocking() assertEquals(2, settings.openTabsCount) - verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } + + assertTrue(Metrics.hasOpenTabs.testHasValue()) + assertTrue(Metrics.hasOpenTabs.testGetValue()) store.dispatch(TabListAction.RemoveAllTabsAction()).joinBlocking() assertEquals(0, settings.openTabsCount) - verify(exactly = 1) { metrics.track(Event.HaveNoOpenTabs) } + + assertTrue(Metrics.hasOpenTabs.testHasValue()) + assertFalse(Metrics.hasOpenTabs.testGetValue()) } @Test fun `WHEN all normal tabs are removed THEN the open tab count is updated`() { + assertFalse(Metrics.hasOpenTabs.testHasValue()) + store.dispatch( TabListAction.AddMultipleTabsAction( listOf( @@ -155,16 +175,20 @@ class TelemetryMiddlewareTest { ) ).joinBlocking() assertEquals(2, settings.openTabsCount) - verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } + assertTrue(Metrics.hasOpenTabs.testHasValue()) + assertTrue(Metrics.hasOpenTabs.testGetValue()) store.dispatch(TabListAction.RemoveAllNormalTabsAction).joinBlocking() assertEquals(0, settings.openTabsCount) - verify(exactly = 1) { metrics.track(Event.HaveNoOpenTabs) } + assertTrue(Metrics.hasOpenTabs.testHasValue()) + assertFalse(Metrics.hasOpenTabs.testGetValue()) } @Test fun `WHEN tabs are restored THEN the open tab count is updated`() { assertEquals(0, settings.openTabsCount) + assertFalse(Metrics.hasOpenTabs.testHasValue()) + val tabsToRestore = listOf( RecoverableTab(null, TabState(url = "https://mozilla.org", id = "1")), RecoverableTab(null, TabState(url = "https://firefox.com", id = "2")) @@ -177,7 +201,9 @@ class TelemetryMiddlewareTest { ) ).joinBlocking() assertEquals(2, settings.openTabsCount) - verify(exactly = 1) { metrics.track(Event.HaveOpenTabs) } + + assertTrue(Metrics.hasOpenTabs.testHasValue()) + assertTrue(Metrics.hasOpenTabs.testGetValue()) } @Test