[fenix] For https://github.com/mozilla-mobile/fenix/issues/24710 - Remove Event.wrapper for RecentBookmarks telemetry

pull/600/head
Alexandru2909 3 years ago committed by mergify[bot]
parent 38db7db00c
commit ce2c88512e

@ -48,12 +48,6 @@ sealed class Event {
object StartOnHomeEnterHomeScreen : Event() object StartOnHomeEnterHomeScreen : Event()
object StartOnHomeOpenTabsTray : Event() object StartOnHomeOpenTabsTray : Event()
// Recent bookmarks
object BookmarkClicked : Event()
object ShowAllBookmarks : Event()
object RecentBookmarksShown : Event()
data class RecentBookmarkCount(val count: Int) : Event()
// Recently visited/Recent searches // Recently visited/Recent searches
object RecentSearchesGroupDeleted : Event() object RecentSearchesGroupDeleted : Event()

@ -16,7 +16,6 @@ import org.mozilla.fenix.GleanMetrics.HomeMenu
import org.mozilla.fenix.GleanMetrics.HomeScreen import org.mozilla.fenix.GleanMetrics.HomeScreen
import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.Pings
import org.mozilla.fenix.GleanMetrics.ProgressiveWebApp import org.mozilla.fenix.GleanMetrics.ProgressiveWebApp
import org.mozilla.fenix.GleanMetrics.RecentBookmarks
import org.mozilla.fenix.GleanMetrics.RecentSearches import org.mozilla.fenix.GleanMetrics.RecentSearches
import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage
import org.mozilla.fenix.GleanMetrics.SearchTerms import org.mozilla.fenix.GleanMetrics.SearchTerms
@ -158,25 +157,10 @@ private val Event.wrapper: EventWrapper<*>?
{ StartOnHome.openTabsTray.record(it) } { StartOnHome.openTabsTray.record(it) }
) )
is Event.BookmarkClicked -> EventWrapper<NoExtraKeys>(
{ RecentBookmarks.bookmarkClicked.add() }
)
is Event.ShowAllBookmarks -> EventWrapper<NoExtraKeys>(
{ RecentBookmarks.showAllBookmarks.add() }
)
is Event.RecentSearchesGroupDeleted -> EventWrapper<NoExtraKeys>( is Event.RecentSearchesGroupDeleted -> EventWrapper<NoExtraKeys>(
{ RecentSearches.groupDeleted.record(it) } { RecentSearches.groupDeleted.record(it) }
) )
is Event.RecentBookmarksShown -> EventWrapper<NoExtraKeys>(
{ RecentBookmarks.shown.record(it) }
)
is Event.RecentBookmarkCount -> EventWrapper<NoExtraKeys>(
{ RecentBookmarks.recentBookmarksCount.set(this.count.toLong()) },
)
is Event.SearchTermGroupCount -> EventWrapper( is Event.SearchTermGroupCount -> EventWrapper(
{ SearchTerms.numberOfSearchTermGroup.record(it) }, { SearchTerms.numberOfSearchTermGroup.record(it) },
{ SearchTerms.numberOfSearchTermGroupKeys.valueOf(it) } { SearchTerms.numberOfSearchTermGroupKeys.valueOf(it) }

@ -11,12 +11,11 @@ import mozilla.appservices.places.BookmarkRoot
import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL
import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.GleanMetrics.RecentBookmarks
import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.components.AppStore import org.mozilla.fenix.components.AppStore
import org.mozilla.fenix.components.appstate.AppAction import org.mozilla.fenix.components.appstate.AppAction
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components
import org.mozilla.fenix.home.HomeFragmentDirections import org.mozilla.fenix.home.HomeFragmentDirections
import org.mozilla.fenix.home.recentbookmarks.RecentBookmark import org.mozilla.fenix.home.recentbookmarks.RecentBookmark
import org.mozilla.fenix.home.recentbookmarks.interactor.RecentBookmarksInteractor import org.mozilla.fenix.home.recentbookmarks.interactor.RecentBookmarksInteractor
@ -60,11 +59,11 @@ class DefaultRecentBookmarksController(
from = BrowserDirection.FromHome, from = BrowserDirection.FromHome,
flags = EngineSession.LoadUrlFlags.select(ALLOW_JAVASCRIPT_URL) flags = EngineSession.LoadUrlFlags.select(ALLOW_JAVASCRIPT_URL)
) )
activity.components.core.metrics.track(Event.BookmarkClicked) RecentBookmarks.bookmarkClicked.add()
} }
override fun handleShowAllBookmarksClicked() { override fun handleShowAllBookmarksClicked() {
activity.components.core.metrics.track(Event.ShowAllBookmarks) RecentBookmarks.showAllBookmarks.add()
dismissSearchDialogIfDisplayed() dismissSearchDialogIfDisplayed()
navController.navigate( navController.navigate(
HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id) HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id)

@ -10,22 +10,21 @@ import androidx.compose.ui.platform.ComposeView
import androidx.compose.ui.res.stringResource import androidx.compose.ui.res.stringResource
import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.LifecycleOwner
import mozilla.components.lib.state.ext.observeAsComposableState import mozilla.components.lib.state.ext.observeAsComposableState
import mozilla.components.service.glean.private.NoExtras
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.components.components import org.mozilla.fenix.components.components
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.compose.ComposeViewHolder import org.mozilla.fenix.compose.ComposeViewHolder
import org.mozilla.fenix.home.recentbookmarks.interactor.RecentBookmarksInteractor import org.mozilla.fenix.home.recentbookmarks.interactor.RecentBookmarksInteractor
import org.mozilla.fenix.GleanMetrics.RecentBookmarks as RecentBookmarksMetrics
class RecentBookmarksViewHolder( class RecentBookmarksViewHolder(
composeView: ComposeView, composeView: ComposeView,
viewLifecycleOwner: LifecycleOwner, viewLifecycleOwner: LifecycleOwner,
val interactor: RecentBookmarksInteractor, val interactor: RecentBookmarksInteractor,
val metrics: MetricController
) : ComposeViewHolder(composeView, viewLifecycleOwner) { ) : ComposeViewHolder(composeView, viewLifecycleOwner) {
init { init {
metrics.track(Event.RecentBookmarksShown) RecentBookmarksMetrics.shown.record(NoExtras())
} }
companion object { companion object {

@ -241,7 +241,6 @@ class SessionControlAdapter(
composeView = ComposeView(parent.context), composeView = ComposeView(parent.context),
viewLifecycleOwner = viewLifecycleOwner, viewLifecycleOwner = viewLifecycleOwner,
interactor = interactor, interactor = interactor,
metrics = components.analytics.metrics
) )
RecentTabViewHolder.LAYOUT_ID -> return RecentTabViewHolder( RecentTabViewHolder.LAYOUT_ID -> return RecentTabViewHolder(
composeView = ComposeView(parent.context), composeView = ComposeView(parent.context),

@ -35,6 +35,7 @@ import org.mozilla.fenix.GleanMetrics.Collections
import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.Pings
import org.mozilla.fenix.GleanMetrics.Pocket import org.mozilla.fenix.GleanMetrics.Pocket
import org.mozilla.fenix.GleanMetrics.RecentBookmarks
import org.mozilla.fenix.GleanMetrics.RecentTabs import org.mozilla.fenix.GleanMetrics.RecentTabs
import org.mozilla.fenix.GleanMetrics.TopSites import org.mozilla.fenix.GleanMetrics.TopSites
import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.HomeActivity
@ -654,6 +655,6 @@ class DefaultSessionControlController(
RecentTabs.sectionVisible.set(true) RecentTabs.sectionVisible.set(true)
} }
metrics.track(Event.RecentBookmarkCount(state.recentBookmarks.size)) RecentBookmarks.recentBookmarksCount.set(state.recentBookmarks.size.toLong())
} }
} }

@ -13,7 +13,6 @@ import org.junit.Rule
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.mozilla.fenix.GleanMetrics.Awesomebar import org.mozilla.fenix.GleanMetrics.Awesomebar
import org.mozilla.fenix.GleanMetrics.RecentBookmarks
import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage
import org.mozilla.fenix.GleanMetrics.SyncedTabs import org.mozilla.fenix.GleanMetrics.SyncedTabs
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@ -64,21 +63,6 @@ class GleanMetricsServiceTest {
assertTrue(Awesomebar.openedTabSuggestionClicked.testHasValue()) assertTrue(Awesomebar.openedTabSuggestionClicked.testHasValue())
} }
@Test
fun `Home screen recent bookmarks events are correctly recorded`() {
assertFalse(RecentBookmarks.shown.testHasValue())
gleanService.track(Event.RecentBookmarksShown)
assertTrue(RecentBookmarks.shown.testHasValue())
assertFalse(RecentBookmarks.bookmarkClicked.testHasValue())
gleanService.track(Event.BookmarkClicked)
assertTrue(RecentBookmarks.bookmarkClicked.testHasValue())
assertFalse(RecentBookmarks.showAllBookmarks.testHasValue())
gleanService.track(Event.ShowAllBookmarks)
assertTrue(RecentBookmarks.showAllBookmarks.testHasValue())
}
@Test @Test
fun `Home screen recently visited events are correctly recorded`() { fun `Home screen recently visited events are correctly recorded`() {
assertFalse(RecentlyVisitedHomepage.historyHighlightOpened.testHasValue()) assertFalse(RecentlyVisitedHomepage.historyHighlightOpened.testHasValue())

@ -52,6 +52,7 @@ import org.mozilla.fenix.GleanMetrics.Collections
import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.Pings
import org.mozilla.fenix.GleanMetrics.RecentTabs import org.mozilla.fenix.GleanMetrics.RecentTabs
import org.mozilla.fenix.GleanMetrics.RecentBookmarks
import org.mozilla.fenix.GleanMetrics.TopSites import org.mozilla.fenix.GleanMetrics.TopSites
import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R import org.mozilla.fenix.R
@ -1127,10 +1128,12 @@ class DefaultSessionControlControllerTest {
fun `WHEN handleReportSessionMetrics is called AND there are zero recent bookmarks THEN report Event#RecentBookmarkCount(0)`() { fun `WHEN handleReportSessionMetrics is called AND there are zero recent bookmarks THEN report Event#RecentBookmarkCount(0)`() {
every { appState.recentBookmarks } returns emptyList() every { appState.recentBookmarks } returns emptyList()
every { appState.recentTabs } returns emptyList() every { appState.recentTabs } returns emptyList()
assertFalse(RecentBookmarks.recentBookmarksCount.testHasValue())
createController().handleReportSessionMetrics(appState) createController().handleReportSessionMetrics(appState)
verify {
metrics.track(Event.RecentBookmarkCount(0)) assertTrue(RecentBookmarks.recentBookmarksCount.testHasValue())
} assertEquals(0, RecentBookmarks.recentBookmarksCount.testGetValue())
} }
@Test @Test
@ -1138,10 +1141,12 @@ class DefaultSessionControlControllerTest {
val recentBookmark: RecentBookmark = mockk(relaxed = true) val recentBookmark: RecentBookmark = mockk(relaxed = true)
every { appState.recentBookmarks } returns listOf(recentBookmark) every { appState.recentBookmarks } returns listOf(recentBookmark)
every { appState.recentTabs } returns emptyList() every { appState.recentTabs } returns emptyList()
assertFalse(RecentBookmarks.recentBookmarksCount.testHasValue())
createController().handleReportSessionMetrics(appState) createController().handleReportSessionMetrics(appState)
verify {
metrics.track(Event.RecentBookmarkCount(1)) assertTrue(RecentBookmarks.recentBookmarksCount.testHasValue())
} assertEquals(1, RecentBookmarks.recentBookmarksCount.testGetValue())
} }
@Test @Test

@ -16,22 +16,31 @@ import kotlinx.coroutines.test.runBlockingTest
import mozilla.appservices.places.BookmarkRoot import mozilla.appservices.places.BookmarkRoot
import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.EngineSession
import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL import mozilla.components.concept.engine.EngineSession.LoadUrlFlags.Companion.ALLOW_JAVASCRIPT_URL
import mozilla.components.support.test.robolectric.testContext
import mozilla.components.support.test.rule.MainCoroutineRule 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.Before
import org.junit.Rule import org.junit.Rule
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.GleanMetrics.RecentBookmarks
import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R import org.mozilla.fenix.R
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.components
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
import org.mozilla.fenix.home.HomeFragmentDirections import org.mozilla.fenix.home.HomeFragmentDirections
import org.mozilla.fenix.home.recentbookmarks.controller.DefaultRecentBookmarksController import org.mozilla.fenix.home.recentbookmarks.controller.DefaultRecentBookmarksController
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
@RunWith(FenixRobolectricTestRunner::class)
class DefaultRecentBookmarksControllerTest { class DefaultRecentBookmarksControllerTest {
@get:Rule
val gleanTestRule = GleanTestRule(testContext)
@get:Rule @get:Rule
val coroutinesTestRule = MainCoroutineRule() val coroutinesTestRule = MainCoroutineRule()
@ -65,6 +74,7 @@ class DefaultRecentBookmarksControllerTest {
every { navController.currentDestination } returns mockk { every { navController.currentDestination } returns mockk {
every { id } returns R.id.homeFragment every { id } returns R.id.homeFragment
} }
assertFalse(RecentBookmarks.bookmarkClicked.testHasValue())
val bookmark = RecentBookmark( val bookmark = RecentBookmark(
title = null, title = null,
@ -82,7 +92,7 @@ class DefaultRecentBookmarksControllerTest {
from = BrowserDirection.FromHome from = BrowserDirection.FromHome
) )
} }
verify { metrics.track(Event.BookmarkClicked) } assertTrue(RecentBookmarks.bookmarkClicked.testHasValue())
verify(exactly = 0) { verify(exactly = 0) {
navController.navigateUp() navController.navigateUp()
} }
@ -93,14 +103,15 @@ class DefaultRecentBookmarksControllerTest {
every { navController.currentDestination } returns mockk { every { navController.currentDestination } returns mockk {
every { id } returns R.id.homeFragment every { id } returns R.id.homeFragment
} }
assertFalse(RecentBookmarks.showAllBookmarks.testHasValue())
controller.handleShowAllBookmarksClicked() controller.handleShowAllBookmarksClicked()
val directions = HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id) val directions = HomeFragmentDirections.actionGlobalBookmarkFragment(BookmarkRoot.Mobile.id)
verify { verify {
navController.navigate(directions) navController.navigate(directions)
metrics.track(Event.ShowAllBookmarks)
} }
assertTrue(RecentBookmarks.showAllBookmarks.testHasValue())
verify(exactly = 0) { verify(exactly = 0) {
navController.navigateUp() navController.navigateUp()
} }
@ -111,6 +122,7 @@ class DefaultRecentBookmarksControllerTest {
every { navController.currentDestination } returns mockk { every { navController.currentDestination } returns mockk {
every { id } returns R.id.searchDialogFragment every { id } returns R.id.searchDialogFragment
} }
assertFalse(RecentBookmarks.showAllBookmarks.testHasValue())
controller.handleShowAllBookmarksClicked() controller.handleShowAllBookmarksClicked()
@ -120,7 +132,7 @@ class DefaultRecentBookmarksControllerTest {
controller.dismissSearchDialogIfDisplayed() controller.dismissSearchDialogIfDisplayed()
navController.navigateUp() navController.navigateUp()
navController.navigate(directions) navController.navigate(directions)
metrics.track(Event.ShowAllBookmarks)
} }
assertTrue(RecentBookmarks.showAllBookmarks.testHasValue())
} }
} }

Loading…
Cancel
Save