From b8a59b2602c32cf7ba1b425064b1a65624a99e70 Mon Sep 17 00:00:00 2001 From: mcarare Date: Wed, 13 Apr 2022 11:48:35 +0300 Subject: [PATCH] For #24711: Remove wrapper from recent tabs metrics. --- .../mozilla/fenix/components/metrics/Event.kt | 7 --- .../components/metrics/GleanMetricsService.kt | 21 ------- .../controller/RecentTabController.kt | 8 ++- .../SessionControlController.kt | 17 +++--- .../DefaultSessionControlControllerTest.kt | 22 ++++---- .../controller/RecentTabControllerTest.kt | 56 +++++++++++++++++-- 6 files changed, 74 insertions(+), 57 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 1e9bb3ba4f..220361bbb0 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 @@ -48,13 +48,6 @@ sealed class Event { object StartOnHomeEnterHomeScreen : Event() object StartOnHomeOpenTabsTray : Event() - // Recent tabs - object ShowAllRecentTabs : Event() - object OpenRecentTab : Event() - object OpenInProgressMediaTab : Event() - object RecentTabsSectionIsVisible : Event() - object RecentTabsSectionIsNotVisible : Event() - // Recent bookmarks object BookmarkClicked : Event() object ShowAllBookmarks : 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 121daf06cc..5265413c8e 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 @@ -19,7 +19,6 @@ import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.ProgressiveWebApp import org.mozilla.fenix.GleanMetrics.RecentBookmarks import org.mozilla.fenix.GleanMetrics.RecentSearches -import org.mozilla.fenix.GleanMetrics.RecentTabs import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage import org.mozilla.fenix.GleanMetrics.SearchTerms import org.mozilla.fenix.GleanMetrics.StartOnHome @@ -160,26 +159,6 @@ private val Event.wrapper: EventWrapper<*>? { StartOnHome.openTabsTray.record(it) } ) - is Event.OpenRecentTab -> EventWrapper( - { RecentTabs.recentTabOpened.record(it) } - ) - - is Event.OpenInProgressMediaTab -> EventWrapper( - { RecentTabs.inProgressMediaTabOpened.record(it) } - ) - - is Event.ShowAllRecentTabs -> EventWrapper( - { RecentTabs.showAllClicked.record(it) } - ) - - is Event.RecentTabsSectionIsVisible -> EventWrapper( - { RecentTabs.sectionVisible.set(true) } - ) - - is Event.RecentTabsSectionIsNotVisible -> EventWrapper( - { RecentTabs.sectionVisible.set(false) } - ) - is Event.BookmarkClicked -> EventWrapper( { RecentBookmarks.bookmarkClicked.add() } ) diff --git a/app/src/main/java/org/mozilla/fenix/home/recenttabs/controller/RecentTabController.kt b/app/src/main/java/org/mozilla/fenix/home/recenttabs/controller/RecentTabController.kt index 5b22ab65ef..b541c13312 100644 --- a/app/src/main/java/org/mozilla/fenix/home/recenttabs/controller/RecentTabController.kt +++ b/app/src/main/java/org/mozilla/fenix/home/recenttabs/controller/RecentTabController.kt @@ -9,6 +9,7 @@ import androidx.annotation.VisibleForTesting.PRIVATE import androidx.navigation.NavController import mozilla.components.browser.state.store.BrowserStore import mozilla.components.feature.tabs.TabsUseCases.SelectTabUseCase +import mozilla.components.service.glean.private.NoExtras import org.mozilla.fenix.R import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.appstate.AppAction @@ -18,6 +19,7 @@ import org.mozilla.fenix.ext.inProgressMediaTab import org.mozilla.fenix.home.HomeFragmentDirections import org.mozilla.fenix.home.recenttabs.RecentTab import org.mozilla.fenix.home.recenttabs.interactor.RecentTabInteractor +import org.mozilla.fenix.GleanMetrics.RecentTabs as RecentTabs /** * An interface that handles the view manipulation of the recent tabs in the Home screen. @@ -61,9 +63,9 @@ class DefaultRecentTabsController( override fun handleRecentTabClicked(tabId: String) { if (tabId == store.state.inProgressMediaTab?.id) { - metrics.track(Event.OpenInProgressMediaTab) + RecentTabs.inProgressMediaTabOpened.record(NoExtras()) } else { - metrics.track(Event.OpenRecentTab) + RecentTabs.recentTabOpened.record(NoExtras()) } selectTabUseCase.invoke(tabId) @@ -72,7 +74,7 @@ class DefaultRecentTabsController( override fun handleRecentTabShowAllClicked() { dismissSearchDialogIfDisplayed() - metrics.track(Event.ShowAllRecentTabs) + RecentTabs.showAllClicked.record(NoExtras()) navController.navigate(HomeFragmentDirections.actionGlobalTabsTrayFragment()) } 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 7be3a45a16..a4306c2866 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 @@ -35,6 +35,7 @@ import org.mozilla.fenix.GleanMetrics.Collections import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.Pocket +import org.mozilla.fenix.GleanMetrics.RecentTabs import org.mozilla.fenix.GleanMetrics.TopSites import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R @@ -647,16 +648,12 @@ class DefaultSessionControlController( } override fun handleReportSessionMetrics(state: AppState) { - with(metrics) { - track( - if (state.recentTabs.isEmpty()) { - Event.RecentTabsSectionIsNotVisible - } else { - Event.RecentTabsSectionIsVisible - } - ) - - track(Event.RecentBookmarkCount(state.recentBookmarks.size)) + if (state.recentTabs.isEmpty()) { + RecentTabs.sectionVisible.set(false) + } else { + RecentTabs.sectionVisible.set(true) } + + metrics.track(Event.RecentBookmarkCount(state.recentBookmarks.size)) } } 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 f4108f4cee..3078e76bef 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -51,6 +51,7 @@ import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.GleanMetrics.Collections import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.Pings +import org.mozilla.fenix.GleanMetrics.RecentTabs import org.mozilla.fenix.GleanMetrics.TopSites import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R @@ -1102,27 +1103,24 @@ class DefaultSessionControlControllerTest { @Test fun `WHEN handleReportSessionMetrics is called AND there are zero recent tabs THEN report Event#RecentTabsSectionIsNotVisible`() { + assertFalse(RecentTabs.sectionVisible.testHasValue()) + every { appState.recentTabs } returns emptyList() createController().handleReportSessionMetrics(appState) - verify(exactly = 0) { - metrics.track(Event.RecentTabsSectionIsVisible) - } - verify { - metrics.track(Event.RecentTabsSectionIsNotVisible) - } + assertTrue(RecentTabs.sectionVisible.testHasValue()) + assertFalse(RecentTabs.sectionVisible.testGetValue()) } @Test fun `WHEN handleReportSessionMetrics is called AND there is at least one recent tab THEN report Event#RecentTabsSectionIsVisible`() { + assertFalse(RecentTabs.sectionVisible.testHasValue()) + val recentTab: RecentTab = mockk(relaxed = true) every { appState.recentTabs } returns listOf(recentTab) createController().handleReportSessionMetrics(appState) - verify(exactly = 0) { - metrics.track(Event.RecentTabsSectionIsNotVisible) - } - verify { - metrics.track(Event.RecentTabsSectionIsVisible) - } + + assertTrue(RecentTabs.sectionVisible.testHasValue()) + assertTrue(RecentTabs.sectionVisible.testGetValue()) } @Test diff --git a/app/src/test/java/org/mozilla/fenix/home/recenttabs/controller/RecentTabControllerTest.kt b/app/src/test/java/org/mozilla/fenix/home/recenttabs/controller/RecentTabControllerTest.kt index d7fe13d17f..9008d4c5be 100644 --- a/app/src/test/java/org/mozilla/fenix/home/recenttabs/controller/RecentTabControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/recenttabs/controller/RecentTabControllerTest.kt @@ -13,25 +13,36 @@ import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import mozilla.components.browser.state.action.TabListAction import mozilla.components.browser.state.state.BrowserState +import mozilla.components.browser.state.state.LastMediaAccessState import mozilla.components.browser.state.state.createTab import mozilla.components.browser.state.store.BrowserStore import mozilla.components.feature.tabs.TabsUseCases import mozilla.components.support.test.ext.joinBlocking +import mozilla.components.support.test.robolectric.testContext import mozilla.components.support.test.rule.MainCoroutineRule +import mozilla.telemetry.glean.testing.GleanTestRule +import org.junit.Assert.assertFalse +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.RecentTabs import org.mozilla.fenix.R import org.mozilla.fenix.components.AppStore -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController +import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @OptIn(ExperimentalCoroutinesApi::class) +@RunWith(FenixRobolectricTestRunner::class) class RecentTabControllerTest { @get:Rule val coroutinesTestRule = MainCoroutineRule() + @get:Rule + val gleanTestRule = GleanTestRule(testContext) + private val navController: NavController = mockk(relaxed = true) private val selectTabUseCase: TabsUseCases = mockk(relaxed = true) private val metrics: MetricController = mockk(relaxed = true) @@ -60,6 +71,9 @@ class RecentTabControllerTest { @Test fun handleRecentTabClicked() { + assertFalse(RecentTabs.recentTabOpened.testHasValue()) + assertFalse(RecentTabs.inProgressMediaTabOpened.testHasValue()) + every { navController.currentDestination } returns mockk { every { id } returns R.id.homeFragment } @@ -76,12 +90,42 @@ class RecentTabControllerTest { verify { selectTabUseCase.selectTab.invoke(tab.id) navController.navigate(R.id.browserFragment) - metrics.track(Event.OpenRecentTab) } + assertTrue(RecentTabs.recentTabOpened.testHasValue()) + assertFalse(RecentTabs.inProgressMediaTabOpened.testHasValue()) + } + + @Test + fun handleRecentTabClickedForMediaTab() { + assertFalse(RecentTabs.recentTabOpened.testHasValue()) + assertFalse(RecentTabs.inProgressMediaTabOpened.testHasValue()) + + every { navController.currentDestination } returns mockk { + every { id } returns R.id.homeFragment + } + + val inProgressMediaTab = createTab( + url = "mediaUrl", id = "2", + lastMediaAccessState = LastMediaAccessState("https://mozilla.com", 123, true) + ) + + store.dispatch(TabListAction.AddTabAction(inProgressMediaTab)).joinBlocking() + store.dispatch(TabListAction.SelectTabAction(inProgressMediaTab.id)).joinBlocking() + + controller.handleRecentTabClicked(inProgressMediaTab.id) + + verify { + selectTabUseCase.selectTab.invoke(inProgressMediaTab.id) + navController.navigate(R.id.browserFragment) + } + assertFalse(RecentTabs.recentTabOpened.testHasValue()) + assertTrue(RecentTabs.inProgressMediaTabOpened.testHasValue()) } @Test fun handleRecentTabShowAllClickedFromHome() { + assertFalse(RecentTabs.showAllClicked.testHasValue()) + every { navController.currentDestination } returns mockk { every { id } returns R.id.homeFragment } @@ -93,15 +137,18 @@ class RecentTabControllerTest { navController.navigate( match { it.actionId == R.id.action_global_tabsTrayFragment } ) - metrics.track(Event.ShowAllRecentTabs) } verify(exactly = 0) { navController.navigateUp() } + + assertTrue(RecentTabs.showAllClicked.testHasValue()) } @Test fun handleRecentTabShowAllClickedFromSearchDialog() { + assertFalse(RecentTabs.showAllClicked.testHasValue()) + every { navController.currentDestination } returns mockk { every { id } returns R.id.searchDialogFragment } @@ -114,7 +161,8 @@ class RecentTabControllerTest { navController.navigate( match { it.actionId == R.id.action_global_tabsTrayFragment } ) - metrics.track(Event.ShowAllRecentTabs) } + + assertTrue(RecentTabs.showAllClicked.testHasValue()) } }