Skip to content

Commit

Permalink
For mozilla-mobile#24711: Remove wrapper from recent tabs metrics.
Browse files Browse the repository at this point in the history
  • Loading branch information
mcarare authored and mergify[bot] committed Apr 14, 2022
1 parent 5d72e42 commit b8a59b2
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -160,26 +159,6 @@ private val Event.wrapper: EventWrapper<*>?
{ StartOnHome.openTabsTray.record(it) }
)

is Event.OpenRecentTab -> EventWrapper<NoExtraKeys>(
{ RecentTabs.recentTabOpened.record(it) }
)

is Event.OpenInProgressMediaTab -> EventWrapper<NoExtraKeys>(
{ RecentTabs.inProgressMediaTabOpened.record(it) }
)

is Event.ShowAllRecentTabs -> EventWrapper<NoExtraKeys>(
{ RecentTabs.showAllClicked.record(it) }
)

is Event.RecentTabsSectionIsVisible -> EventWrapper<NoExtraKeys>(
{ RecentTabs.sectionVisible.set(true) }
)

is Event.RecentTabsSectionIsNotVisible -> EventWrapper<NoExtraKeys>(
{ RecentTabs.sectionVisible.set(false) }
)

is Event.BookmarkClicked -> EventWrapper<NoExtraKeys>(
{ RecentBookmarks.bookmarkClicked.add() }
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -72,7 +74,7 @@ class DefaultRecentTabsController(

override fun handleRecentTabShowAllClicked() {
dismissSearchDialogIfDisplayed()
metrics.track(Event.ShowAllRecentTabs)
RecentTabs.showAllClicked.record(NoExtras())
navController.navigate(HomeFragmentDirections.actionGlobalTabsTrayFragment())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -93,15 +137,18 @@ class RecentTabControllerTest {
navController.navigate(
match<NavDirections> { 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
}
Expand All @@ -114,7 +161,8 @@ class RecentTabControllerTest {
navController.navigate(
match<NavDirections> { it.actionId == R.id.action_global_tabsTrayFragment }
)
metrics.track(Event.ShowAllRecentTabs)
}

assertTrue(RecentTabs.showAllClicked.testHasValue())
}
}

0 comments on commit b8a59b2

Please sign in to comment.