From fd021b28193d17a03596241570c97e2312e5d95f Mon Sep 17 00:00:00 2001 From: Alexandru2909 Date: Thu, 7 Apr 2022 16:26:43 +0300 Subject: [PATCH] For #24642 - Remove Event.wrapper for TopSites metrics --- app/metrics.yaml | 2 + .../mozilla/fenix/components/metrics/Event.kt | 35 --- .../components/metrics/GleanMetricsService.kt | 71 ------ .../SessionControlController.kt | 50 ++-- .../home/topsites/TopSiteItemViewHolder.kt | 10 +- .../home/topsites/TopSitePagerViewHolder.kt | 9 +- .../metrics/GleanMetricsServiceTest.kt | 42 ---- .../DefaultSessionControlControllerTest.kt | 218 ++++++++++++++---- .../topsites/TopSiteItemViewHolderTest.kt | 48 ++-- 9 files changed, 235 insertions(+), 250 deletions(-) diff --git a/app/metrics.yaml b/app/metrics.yaml index 20d5113af..3c365378b 100644 --- a/app/metrics.yaml +++ b/app/metrics.yaml @@ -4953,6 +4953,7 @@ top_sites: A user swiped to change the page of the top sites carousel extra_keys: page: + type: string description: | The page number the carousel is now on bugs: @@ -4978,6 +4979,7 @@ top_sites: A user long pressed on a top site extra_keys: type: + type: string description: | The type of top site. Options are: "FRECENCY", "DEFAULT", "PINNED", or "PROVIDED" 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 1b2a95337..10eee9866 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 @@ -5,15 +5,12 @@ package org.mozilla.fenix.components.metrics import mozilla.components.browser.state.search.SearchEngine -import mozilla.components.feature.top.sites.TopSite import org.mozilla.fenix.GleanMetrics.Addons import org.mozilla.fenix.GleanMetrics.Autoplay import org.mozilla.fenix.GleanMetrics.ContextMenu import org.mozilla.fenix.GleanMetrics.Events import org.mozilla.fenix.GleanMetrics.Pocket import org.mozilla.fenix.GleanMetrics.SearchTerms -import org.mozilla.fenix.GleanMetrics.TopSites -import org.mozilla.fenix.ext.name import java.util.Locale sealed class Event { @@ -31,20 +28,6 @@ sealed class Event { object MediaStopState : Event() object MediaFullscreenState : Event() object MediaPictureInPictureState : Event() - object TopSiteOpenDefault : Event() - object TopSiteOpenGoogle : Event() - object TopSiteOpenBaidu : Event() - object TopSiteOpenFrecent : Event() - object TopSiteOpenPinned : Event() - object TopSiteOpenProvided : Event() - object TopSiteOpenInNewTab : Event() - object TopSiteOpenInPrivateTab : Event() - object TopSiteOpenContileInPrivateTab : Event() - object TopSiteRemoved : Event() - object TopSiteContileSettings : Event() - object TopSiteContilePrivacy : Event() - object GoogleTopSiteRemoved : Event() - object BaiduTopSiteRemoved : Event() object PocketTopSiteClicked : Event() object PocketTopSiteRemoved : Event() object PocketHomeRecsShown : Event() @@ -153,24 +136,6 @@ sealed class Event { // Interaction events with extras - data class TopSiteSwipeCarousel(val page: Int) : Event() { - override val extras: Map? - get() = hashMapOf(TopSites.swipeCarouselKeys.page to page.toString()) - } - - data class TopSiteLongPress(val topSite: TopSite) : Event() { - override val extras: Map? - get() = hashMapOf(TopSites.longPressKeys.type to topSite.name()) - } - - data class TopSiteContileImpression(val position: Int, val source: Source) : Event() { - enum class Source { NEWTAB } - } - - data class TopSiteContileClick(val position: Int, val source: Source) : Event() { - enum class Source { NEWTAB } - } - data class AddonsOpenInToolbarMenu(val addonId: String) : Event() { override val extras: Map? get() = hashMapOf(Addons.openAddonInToolbarMenuKeys.addonId to addonId) 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 ed9aa43a8..02d16e93b 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 @@ -33,7 +33,6 @@ import org.mozilla.fenix.GleanMetrics.SearchTerms import org.mozilla.fenix.GleanMetrics.StartOnHome import org.mozilla.fenix.GleanMetrics.SyncedTabs import org.mozilla.fenix.GleanMetrics.Tabs -import org.mozilla.fenix.GleanMetrics.TopSites import org.mozilla.fenix.GleanMetrics.Wallpapers import org.mozilla.fenix.GleanMetrics.Messaging import org.mozilla.fenix.ext.components @@ -126,76 +125,6 @@ private val Event.wrapper: EventWrapper<*>? is Event.MediaPictureInPictureState -> EventWrapper( { MediaState.pictureInPicture.record(it) } ) - is Event.TopSiteOpenDefault -> EventWrapper( - { TopSites.openDefault.record(it) } - ) - is Event.TopSiteOpenGoogle -> EventWrapper( - { TopSites.openGoogleSearchAttribution.record(it) } - ) - is Event.TopSiteOpenBaidu -> EventWrapper( - { TopSites.openBaiduSearchAttribution.record(it) } - ) - is Event.TopSiteOpenFrecent -> EventWrapper( - { TopSites.openFrecency.record(it) } - ) - is Event.TopSiteOpenPinned -> EventWrapper( - { TopSites.openPinned.record(it) } - ) - is Event.TopSiteOpenProvided -> EventWrapper( - { TopSites.openContileTopSite.record(it) } - ) - is Event.TopSiteOpenInNewTab -> EventWrapper( - { TopSites.openInNewTab.record(it) } - ) - is Event.TopSiteOpenInPrivateTab -> EventWrapper( - { TopSites.openInPrivateTab.record(it) } - ) - is Event.TopSiteOpenContileInPrivateTab -> EventWrapper( - { TopSites.openContileInPrivateTab.record(it) } - ) - is Event.TopSiteRemoved -> EventWrapper( - { TopSites.remove.record(it) } - ) - is Event.TopSiteContileSettings -> EventWrapper( - { TopSites.contileSettings.record(it) } - ) - is Event.TopSiteContilePrivacy -> EventWrapper( - { TopSites.contileSponsorsAndPrivacy.record(it) } - ) - is Event.GoogleTopSiteRemoved -> EventWrapper( - { TopSites.googleTopSiteRemoved.record(it) } - ) - is Event.BaiduTopSiteRemoved -> EventWrapper( - { TopSites.baiduTopSiteRemoved.record(it) } - ) - is Event.TopSiteLongPress -> EventWrapper( - { TopSites.longPress.record(it) }, - { TopSites.longPressKeys.valueOf(it) } - ) - is Event.TopSiteSwipeCarousel -> EventWrapper( - { TopSites.swipeCarousel.record(it) }, - { TopSites.swipeCarouselKeys.valueOf(it) } - ) - is Event.TopSiteContileImpression -> EventWrapper( - { - TopSites.contileImpression.record( - TopSites.ContileImpressionExtra( - position = this.position, - source = this.source.name.lowercase() - ) - ) - } - ) - is Event.TopSiteContileClick -> EventWrapper( - { - TopSites.contileClick.record( - TopSites.ContileClickExtra( - position = this.position, - source = this.source.name.lowercase() - ) - ) - } - ) is Event.PocketTopSiteClicked -> EventWrapper( { Pocket.pocketTopSiteClicked.record(it) } ) 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 01e0671fe..986bc00d9 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 @@ -315,13 +315,11 @@ class DefaultSessionControlController( } override fun handleOpenInPrivateTabClicked(topSite: TopSite) { - metrics.track( - if (topSite is TopSite.Provided) { - Event.TopSiteOpenContileInPrivateTab - } else { - Event.TopSiteOpenInPrivateTab - } - ) + if (topSite is TopSite.Provided) { + TopSites.openContileInPrivateTab.record(NoExtras()) + } else { + TopSites.openInPrivateTab.record(NoExtras()) + } with(activity) { browsingModeManager.mode = BrowsingMode.Private openToBrowserAndLoad( @@ -376,11 +374,11 @@ class DefaultSessionControlController( } override fun handleRemoveTopSiteClicked(topSite: TopSite) { - metrics.track(Event.TopSiteRemoved) + TopSites.remove.record(NoExtras()) when (topSite.url) { SupportUtils.POCKET_TRENDING_URL -> metrics.track(Event.PocketTopSiteRemoved) - SupportUtils.GOOGLE_URL -> metrics.track(Event.GoogleTopSiteRemoved) - SupportUtils.BAIDU_URL -> metrics.track(Event.BaiduTopSiteRemoved) + SupportUtils.GOOGLE_URL -> TopSites.googleTopSiteRemoved.record(NoExtras()) + SupportUtils.BAIDU_URL -> TopSites.baiduTopSiteRemoved.record(NoExtras()) } viewLifecycleScope.launch(Dispatchers.IO) { @@ -401,22 +399,20 @@ class DefaultSessionControlController( override fun handleSelectTopSite(topSite: TopSite, position: Int) { dismissSearchDialogIfDisplayed() - metrics.track(Event.TopSiteOpenInNewTab) + TopSites.openInNewTab.record(NoExtras()) - metrics.track( - when (topSite) { - is TopSite.Default -> Event.TopSiteOpenDefault - is TopSite.Frecent -> Event.TopSiteOpenFrecent - is TopSite.Pinned -> Event.TopSiteOpenPinned - is TopSite.Provided -> Event.TopSiteOpenProvided.also { - submitTopSitesImpressionPing(topSite, position) - } + when (topSite) { + is TopSite.Default -> TopSites.openDefault.record(NoExtras()) + is TopSite.Frecent -> TopSites.openFrecency.record(NoExtras()) + is TopSite.Pinned -> TopSites.openPinned.record(NoExtras()) + is TopSite.Provided -> TopSites.openContileTopSite.record(NoExtras()).also { + submitTopSitesImpressionPing(topSite, position) } - ) + } when (topSite.url) { - SupportUtils.GOOGLE_URL -> metrics.track(Event.TopSiteOpenGoogle) - SupportUtils.BAIDU_URL -> metrics.track(Event.TopSiteOpenBaidu) + SupportUtils.GOOGLE_URL -> TopSites.openGoogleSearchAttribution.record(NoExtras()) + SupportUtils.BAIDU_URL -> TopSites.openBaiduSearchAttribution.record(NoExtras()) SupportUtils.POCKET_TRENDING_URL -> metrics.track(Event.PocketTopSiteClicked) } @@ -446,10 +442,10 @@ class DefaultSessionControlController( @VisibleForTesting internal fun submitTopSitesImpressionPing(topSite: TopSite.Provided, position: Int) { - metrics.track( - Event.TopSiteContileClick( + TopSites.contileClick.record( + TopSites.ContileClickExtra( position = position + 1, - source = Event.TopSiteContileClick.Source.NEWTAB + source = "newtab" ) ) @@ -460,7 +456,7 @@ class DefaultSessionControlController( } override fun handleTopSiteSettingsClicked() { - metrics.track(Event.TopSiteContileSettings) + TopSites.contileSettings.record(NoExtras()) navController.nav( R.id.homeFragment, HomeFragmentDirections.actionGlobalHomeSettingsFragment() @@ -468,7 +464,7 @@ class DefaultSessionControlController( } override fun handleSponsorPrivacyClicked() { - metrics.track(Event.TopSiteContilePrivacy) + TopSites.contileSponsorsAndPrivacy.record(NoExtras()) activity.openToBrowserAndLoad( searchTermOrURL = SupportUtils.getGenericSumoURLForTopic(SupportUtils.SumoTopic.SPONSOR_PRIVACY), newTab = true, diff --git a/app/src/main/java/org/mozilla/fenix/home/topsites/TopSiteItemViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/topsites/TopSiteItemViewHolder.kt index 15f3a34a4..08c3f8d79 100644 --- a/app/src/main/java/org/mozilla/fenix/home/topsites/TopSiteItemViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/topsites/TopSiteItemViewHolder.kt @@ -21,11 +21,11 @@ import mozilla.components.feature.top.sites.TopSite import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.TopSites import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.databinding.TopSiteItemBinding import org.mozilla.fenix.ext.bitmapForUrl import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.loadIntoView +import org.mozilla.fenix.ext.name import org.mozilla.fenix.home.sessioncontrol.TopSiteInteractor import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.utils.view.ViewHolder @@ -41,7 +41,7 @@ class TopSiteItemViewHolder( init { binding.topSiteItem.setOnLongClickListener { interactor.onTopSiteMenuOpened() - it.context.components.analytics.metrics.track(Event.TopSiteLongPress(topSite)) + TopSites.longPress.record(TopSites.LongPressExtra(topSite.name())) val topSiteMenu = TopSiteItemMenu( context = view.context, @@ -127,10 +127,10 @@ class TopSiteItemViewHolder( @VisibleForTesting internal fun submitTopSitesImpressionPing(topSite: TopSite.Provided, position: Int) { - itemView.context.components.analytics.metrics.track( - Event.TopSiteContileImpression( + TopSites.contileImpression.record( + TopSites.ContileImpressionExtra( position = position + 1, - source = Event.TopSiteContileImpression.Source.NEWTAB + source = "newtab" ) ) diff --git a/app/src/main/java/org/mozilla/fenix/home/topsites/TopSitePagerViewHolder.kt b/app/src/main/java/org/mozilla/fenix/home/topsites/TopSitePagerViewHolder.kt index 6941d5cc8..138c04ec9 100644 --- a/app/src/main/java/org/mozilla/fenix/home/topsites/TopSitePagerViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/home/topsites/TopSitePagerViewHolder.kt @@ -10,10 +10,9 @@ import androidx.lifecycle.LifecycleOwner import androidx.recyclerview.widget.RecyclerView import androidx.viewpager2.widget.ViewPager2 import mozilla.components.feature.top.sites.TopSite +import org.mozilla.fenix.GleanMetrics.TopSites import org.mozilla.fenix.R -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.databinding.ComponentTopSitesPagerBinding -import org.mozilla.fenix.ext.components import org.mozilla.fenix.home.sessioncontrol.AdapterItem import org.mozilla.fenix.home.sessioncontrol.TopSiteInteractor @@ -31,9 +30,9 @@ class TopSitePagerViewHolder( private val topSitesPageChangeCallback = object : ViewPager2.OnPageChangeCallback() { override fun onPageSelected(position: Int) { if (currentPage != position) { - pageIndicator.context.components.analytics.metrics.track( - Event.TopSiteSwipeCarousel( - position + TopSites.swipeCarousel.record( + TopSites.SwipeCarouselExtra( + position.toString() ) ) } diff --git a/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt b/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt index 33f839eb9..fab7a0074 100644 --- a/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt @@ -19,7 +19,6 @@ import org.mozilla.fenix.GleanMetrics.CreditCards import org.mozilla.fenix.GleanMetrics.RecentBookmarks import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage import org.mozilla.fenix.GleanMetrics.SyncedTabs -import org.mozilla.fenix.GleanMetrics.TopSites import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @RunWith(FenixRobolectricTestRunner::class) @@ -163,45 +162,4 @@ class GleanMetricsServiceTest { gleanService.track(Event.CreditCardManagementCardTapped) assertTrue(CreditCards.managementCardTapped.testHasValue()) } - - @Test - fun `GIVEN contile top site events WHEN the event is track THEN verify the event is correctly recorded`() { - assertFalse(TopSites.contileImpression.testHasValue()) - - gleanService.track( - Event.TopSiteContileImpression( - position = 1, - source = Event.TopSiteContileImpression.Source.NEWTAB - ) - ) - - assertTrue(TopSites.contileImpression.testHasValue()) - - var event = TopSites.contileImpression.testGetValue() - - assertEquals(1, event.size) - assertEquals("top_sites", event[0].category) - assertEquals("contile_impression", event[0].name) - assertEquals("1", event[0].extra!!["position"]) - assertEquals("newtab", event[0].extra!!["source"]) - - assertFalse(TopSites.contileClick.testHasValue()) - - gleanService.track( - Event.TopSiteContileClick( - position = 2, - source = Event.TopSiteContileClick.Source.NEWTAB - ) - ) - - assertTrue(TopSites.contileClick.testHasValue()) - - event = TopSites.contileClick.testGetValue() - - assertEquals(1, event.size) - assertEquals("top_sites", event[0].category) - assertEquals("contile_click", event[0].name) - assertEquals("2", event[0].extra!!["position"]) - assertEquals("newtab", event[0].extra!!["source"]) - } } 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 38fdbce4a..d5da96e5e 100644 --- a/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/DefaultSessionControlControllerTest.kt @@ -37,9 +37,11 @@ import mozilla.components.service.glean.testing.GleanTestRule 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.private.NoExtras import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Ignore @@ -457,8 +459,14 @@ class DefaultSessionControlControllerTest { controller.handleSelectTopSite(topSite, position = 0) - verify { metrics.track(Event.TopSiteOpenInNewTab) } - verify { metrics.track(Event.TopSiteOpenDefault) } + assertTrue(TopSites.openInNewTab.testHasValue()) + assertEquals(1, TopSites.openInNewTab.testGetValue().size) + assertNull(TopSites.openInNewTab.testGetValue().single().extra) + + assertTrue(TopSites.openDefault.testHasValue()) + assertEquals(1, TopSites.openDefault.testGetValue().size) + assertNull(TopSites.openDefault.testGetValue().single().extra) + verify { tabsUseCases.addTab.invoke( url = topSite.url, @@ -483,7 +491,10 @@ class DefaultSessionControlControllerTest { controller.handleSelectTopSite(topSite, position = 0) - verify { metrics.track(Event.TopSiteOpenInNewTab) } + assertTrue(TopSites.openInNewTab.testHasValue()) + assertEquals(1, TopSites.openInNewTab.testGetValue().size) + assertNull(TopSites.openInNewTab.testGetValue().single().extra) + verify { tabsUseCases.addTab.invoke( url = topSite.url, @@ -510,9 +521,18 @@ class DefaultSessionControlControllerTest { controller.handleSelectTopSite(topSite, position = 0) - verify { metrics.track(Event.TopSiteOpenInNewTab) } - verify { metrics.track(Event.TopSiteOpenDefault) } - verify { metrics.track(Event.TopSiteOpenGoogle) } + assertTrue(TopSites.openInNewTab.testHasValue()) + assertEquals(1, TopSites.openInNewTab.testGetValue().size) + assertNull(TopSites.openInNewTab.testGetValue().single().extra) + + assertTrue(TopSites.openDefault.testHasValue()) + assertEquals(1, TopSites.openDefault.testGetValue().size) + assertNull(TopSites.openDefault.testGetValue().single().extra) + + assertTrue(TopSites.openGoogleSearchAttribution.testHasValue()) + assertEquals(1, TopSites.openGoogleSearchAttribution.testGetValue().size) + assertNull(TopSites.openGoogleSearchAttribution.testGetValue().single().extra) + verify { tabsUseCases.addTab.invoke( url = SupportUtils.GOOGLE_US_URL, @@ -539,9 +559,18 @@ class DefaultSessionControlControllerTest { controller.handleSelectTopSite(topSite, position = 0) - verify { metrics.track(Event.TopSiteOpenInNewTab) } - verify { metrics.track(Event.TopSiteOpenDefault) } - verify { metrics.track(Event.TopSiteOpenGoogle) } + assertTrue(TopSites.openInNewTab.testHasValue()) + assertEquals(1, TopSites.openInNewTab.testGetValue().size) + assertNull(TopSites.openInNewTab.testGetValue().single().extra) + + assertTrue(TopSites.openDefault.testHasValue()) + assertEquals(1, TopSites.openDefault.testGetValue().size) + assertNull(TopSites.openDefault.testGetValue().single().extra) + + assertTrue(TopSites.openGoogleSearchAttribution.testHasValue()) + assertEquals(1, TopSites.openGoogleSearchAttribution.testGetValue().size) + assertNull(TopSites.openGoogleSearchAttribution.testGetValue().single().extra) + verify { tabsUseCases.addTab.invoke( SupportUtils.GOOGLE_XX_URL, @@ -580,9 +609,15 @@ class DefaultSessionControlControllerTest { ) ) ) - metrics.track(Event.TopSiteOpenGoogle) - metrics.track(Event.TopSiteOpenDefault) } + + assertTrue(TopSites.openDefault.testHasValue()) + assertEquals(1, TopSites.openDefault.testGetValue().size) + assertNull(TopSites.openDefault.testGetValue().single().extra) + + assertTrue(TopSites.openGoogleSearchAttribution.testHasValue()) + assertEquals(1, TopSites.openGoogleSearchAttribution.testGetValue().size) + assertNull(TopSites.openGoogleSearchAttribution.testGetValue().single().extra) } finally { unmockkStatic("mozilla.components.browser.state.state.SearchStateKt") } @@ -616,9 +651,8 @@ class DefaultSessionControlControllerTest { ) ) ) - - metrics.track(Event.TopSiteOpenPinned) } + TopSites.openPinned.record(NoExtras()) } finally { unmockkStatic("mozilla.components.browser.state.state.SearchStateKt") } @@ -640,9 +674,18 @@ class DefaultSessionControlControllerTest { controller.handleSelectTopSite(topSite, position = 0) - verify { metrics.track(Event.TopSiteOpenInNewTab) } - verify { metrics.track(Event.TopSiteOpenPinned) } - verify { metrics.track(Event.TopSiteOpenGoogle) } + assertTrue(TopSites.openInNewTab.testHasValue()) + assertEquals(1, TopSites.openInNewTab.testGetValue().size) + assertNull(TopSites.openInNewTab.testGetValue().single().extra) + + assertTrue(TopSites.openPinned.testHasValue()) + assertEquals(1, TopSites.openPinned.testGetValue().size) + assertNull(TopSites.openPinned.testGetValue().single().extra) + + assertTrue(TopSites.openGoogleSearchAttribution.testHasValue()) + assertEquals(1, TopSites.openGoogleSearchAttribution.testGetValue().size) + assertNull(TopSites.openGoogleSearchAttribution.testGetValue().single().extra) + verify { tabsUseCases.addTab.invoke( SupportUtils.GOOGLE_US_URL, @@ -669,9 +712,18 @@ class DefaultSessionControlControllerTest { controller.handleSelectTopSite(topSite, position = 0) - verify { metrics.track(Event.TopSiteOpenInNewTab) } - verify { metrics.track(Event.TopSiteOpenPinned) } - verify { metrics.track(Event.TopSiteOpenGoogle) } + assertTrue(TopSites.openInNewTab.testHasValue()) + assertEquals(1, TopSites.openInNewTab.testGetValue().size) + assertNull(TopSites.openInNewTab.testGetValue().single().extra) + + assertTrue(TopSites.openPinned.testHasValue()) + assertEquals(1, TopSites.openPinned.testGetValue().size) + assertNull(TopSites.openPinned.testGetValue().single().extra) + + assertTrue(TopSites.openGoogleSearchAttribution.testHasValue()) + assertEquals(1, TopSites.openGoogleSearchAttribution.testGetValue().size) + assertNull(TopSites.openGoogleSearchAttribution.testGetValue().single().extra) + verify { tabsUseCases.addTab.invoke( SupportUtils.GOOGLE_XX_URL, @@ -698,9 +750,18 @@ class DefaultSessionControlControllerTest { controller.handleSelectTopSite(topSite, position = 0) - verify { metrics.track(Event.TopSiteOpenInNewTab) } - verify { metrics.track(Event.TopSiteOpenFrecent) } - verify { metrics.track(Event.TopSiteOpenGoogle) } + assertTrue(TopSites.openInNewTab.testHasValue()) + assertEquals(1, TopSites.openInNewTab.testGetValue().size) + assertNull(TopSites.openInNewTab.testGetValue().single().extra) + + assertTrue(TopSites.openFrecency.testHasValue()) + assertEquals(1, TopSites.openFrecency.testGetValue().size) + assertNull(TopSites.openFrecency.testGetValue().single().extra) + + assertTrue(TopSites.openGoogleSearchAttribution.testHasValue()) + assertEquals(1, TopSites.openGoogleSearchAttribution.testGetValue().size) + assertNull(TopSites.openGoogleSearchAttribution.testGetValue().single().extra) + verify { tabsUseCases.addTab.invoke( SupportUtils.GOOGLE_US_URL, @@ -727,9 +788,18 @@ class DefaultSessionControlControllerTest { controller.handleSelectTopSite(topSite, position = 0) - verify { metrics.track(Event.TopSiteOpenInNewTab) } - verify { metrics.track(Event.TopSiteOpenFrecent) } - verify { metrics.track(Event.TopSiteOpenGoogle) } + assertTrue(TopSites.openInNewTab.testHasValue()) + assertEquals(1, TopSites.openInNewTab.testGetValue().size) + assertNull(TopSites.openInNewTab.testGetValue().single().extra) + + assertTrue(TopSites.openFrecency.testHasValue()) + assertEquals(1, TopSites.openFrecency.testGetValue().size) + assertNull(TopSites.openFrecency.testGetValue().single().extra) + + assertTrue(TopSites.openGoogleSearchAttribution.testHasValue()) + assertEquals(1, TopSites.openGoogleSearchAttribution.testGetValue().size) + assertNull(TopSites.openGoogleSearchAttribution.testGetValue().single().extra) + verify { tabsUseCases.addTab.invoke( SupportUtils.GOOGLE_XX_URL, @@ -758,8 +828,14 @@ class DefaultSessionControlControllerTest { controller.handleSelectTopSite(topSite, position) - verify { metrics.track(Event.TopSiteOpenInNewTab) } - verify { metrics.track(Event.TopSiteOpenProvided) } + assertTrue(TopSites.openInNewTab.testHasValue()) + assertEquals(1, TopSites.openInNewTab.testGetValue().size) + assertNull(TopSites.openInNewTab.testGetValue().single().extra) + + assertTrue(TopSites.openContileTopSite.testHasValue()) + assertEquals(1, TopSites.openContileTopSite.testGetValue().size) + assertNull(TopSites.openContileTopSite.testGetValue().single().extra) + verify { tabsUseCases.addTab.invoke( url = topSite.url, @@ -784,22 +860,58 @@ class DefaultSessionControlControllerTest { createdAt = 3 ) val position = 0 + assertFalse(TopSites.contileImpression.testHasValue()) - controller.submitTopSitesImpressionPing(topSite, position) + var topSiteImpressionPinged = false + Pings.topsitesImpression.testBeforeNextSubmit { + assertTrue(TopSites.contileTileId.testHasValue()) + assertEquals(3, TopSites.contileTileId.testGetValue()) - verify { - metrics.track( - Event.TopSiteContileClick( - position = position + 1, - source = Event.TopSiteContileClick.Source.NEWTAB - ) - ) + assertTrue(TopSites.contileAdvertiser.testHasValue()) + assertEquals("mozilla", TopSites.contileAdvertiser.testGetValue()) + + assertTrue(TopSites.contileReportingUrl.testHasValue()) + assertEquals(topSite.clickUrl, TopSites.contileReportingUrl.testGetValue()) - TopSites.contileTileId.set(3) - TopSites.contileAdvertiser.set("mozilla") - TopSites.contileReportingUrl.set(topSite.clickUrl) - Pings.topsitesImpression.submit() + topSiteImpressionPinged = true } + + controller.submitTopSitesImpressionPing(topSite, position) + + assertTrue(TopSites.contileClick.testHasValue()) + + val event = TopSites.contileClick.testGetValue() + + assertEquals(1, event.size) + assertEquals("top_sites", event[0].category) + assertEquals("contile_click", event[0].name) + assertEquals("1", event[0].extra!!["position"]) + assertEquals("newtab", event[0].extra!!["source"]) + + assertTrue(topSiteImpressionPinged) + } + + @Test + fun `WHEN the default Google top site is removed THEN the correct metric is recorded`() { + val controller = spyk(createController()) + val topSite = TopSite.Default( + id = 1L, + title = "Google", + url = SupportUtils.GOOGLE_URL, + createdAt = 0 + ) + assertFalse(TopSites.remove.testHasValue()) + assertFalse(TopSites.googleTopSiteRemoved.testHasValue()) + + controller.handleRemoveTopSiteClicked(topSite) + + assertTrue(TopSites.googleTopSiteRemoved.testHasValue()) + assertEquals(1, TopSites.googleTopSiteRemoved.testGetValue().size) + assertNull(TopSites.googleTopSiteRemoved.testGetValue().single().extra) + + assertTrue(TopSites.remove.testHasValue()) + assertEquals(1, TopSites.remove.testGetValue().size) + assertNull(TopSites.remove.testGetValue().single().extra) } @Test @@ -1053,9 +1165,9 @@ class DefaultSessionControlControllerTest { fun `WHEN handleTopSiteSettingsClicked is called THEN navigate to the HomeSettingsFragment AND report the interaction`() { createController().handleTopSiteSettingsClicked() - verify { - metrics.track(Event.TopSiteContileSettings) - } + assertTrue(TopSites.contileSettings.testHasValue()) + assertEquals(1, TopSites.contileSettings.testGetValue().size) + assertNull(TopSites.contileSettings.testGetValue().single().extra) verify { navController.navigate( match { @@ -1070,9 +1182,9 @@ class DefaultSessionControlControllerTest { fun `WHEN handleSponsorPrivacyClicked is called THEN navigate to the privacy webpage AND report the interaction`() { createController().handleSponsorPrivacyClicked() - verify { - metrics.track(Event.TopSiteContilePrivacy) - } + assertTrue(TopSites.contileSponsorsAndPrivacy.testHasValue()) + assertEquals(1, TopSites.contileSponsorsAndPrivacy.testGetValue().size) + assertNull(TopSites.contileSponsorsAndPrivacy.testGetValue().single().extra) verify { activity.openToBrowserAndLoad( searchTermOrURL = SupportUtils.getGenericSumoURLForTopic(SupportUtils.SumoTopic.SPONSOR_PRIVACY), @@ -1095,13 +1207,13 @@ class DefaultSessionControlControllerTest { ) createController().handleOpenInPrivateTabClicked(topSite) - verify { - metrics.track(Event.TopSiteOpenContileInPrivateTab) - } + assertTrue(TopSites.openContileInPrivateTab.testHasValue()) + assertEquals(1, TopSites.openContileInPrivateTab.testGetValue().size) + assertNull(TopSites.openContileInPrivateTab.testGetValue().single().extra) } @Test - fun `WHEN handleOpenInPrivateTabClicked is called with a Default, Pinned, or Frecent top site THEN TopSiteOpenInPrivateTab event is reported`() { + fun `WHEN handleOpenInPrivateTabClicked is called with a Default, Pinned, or Frecent top site THEN openInPrivateTab event is recorded`() { val controller = createController() val topSite1 = TopSite.Default( id = 1L, @@ -1121,12 +1233,16 @@ class DefaultSessionControlControllerTest { url = "mozilla.org", createdAt = 0 ) + assertFalse(TopSites.openInPrivateTab.testHasValue()) + controller.handleOpenInPrivateTabClicked(topSite1) controller.handleOpenInPrivateTabClicked(topSite2) controller.handleOpenInPrivateTabClicked(topSite3) - verify(exactly = 3) { - metrics.track(Event.TopSiteOpenInPrivateTab) + assertTrue(TopSites.openInPrivateTab.testHasValue()) + assertEquals(3, TopSites.openInPrivateTab.testGetValue().size) + for (event in TopSites.openInPrivateTab.testGetValue()) { + assertNull(event.extra) } } diff --git a/app/src/test/java/org/mozilla/fenix/home/topsites/TopSiteItemViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/home/topsites/TopSiteItemViewHolderTest.kt index d00d93156..02d37453d 100644 --- a/app/src/test/java/org/mozilla/fenix/home/topsites/TopSiteItemViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/home/topsites/TopSiteItemViewHolderTest.kt @@ -12,14 +12,18 @@ import io.mockk.verify import mozilla.components.browser.icons.BrowserIcons import mozilla.components.feature.top.sites.TopSite import mozilla.components.support.test.robolectric.testContext +import mozilla.telemetry.glean.testing.GleanTestRule +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull +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.Pings import org.mozilla.fenix.GleanMetrics.TopSites -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.databinding.TopSiteItemBinding import org.mozilla.fenix.ext.components @@ -29,6 +33,9 @@ import org.mozilla.fenix.home.sessioncontrol.TopSiteInteractor @RunWith(FenixRobolectricTestRunner::class) class TopSiteItemViewHolderTest { + @get:Rule + val gleanTestRule = GleanTestRule(testContext) + private lateinit var binding: TopSiteItemBinding private lateinit var interactor: TopSiteInteractor private lateinit var lifecycleOwner: LifecycleOwner @@ -126,21 +133,34 @@ class TopSiteItemViewHolderTest { createdAt = 3 ) val position = 0 + assertFalse(TopSites.contileImpression.testHasValue()) - TopSiteItemViewHolder(binding.root, lifecycleOwner, interactor).submitTopSitesImpressionPing(topSite, position) + var topSiteImpressionSubmitted = false + Pings.topsitesImpression.testBeforeNextSubmit { + assertTrue(TopSites.contileTileId.testHasValue()) + assertEquals(3, TopSites.contileTileId.testGetValue()) + + assertTrue(TopSites.contileAdvertiser.testHasValue()) + assertEquals("mozilla", TopSites.contileAdvertiser.testGetValue()) - verify { - metrics.track( - Event.TopSiteContileImpression( - position = position + 1, - source = Event.TopSiteContileImpression.Source.NEWTAB - ) - ) - - TopSites.contileTileId.set(3) - TopSites.contileAdvertiser.set("mozilla") - TopSites.contileReportingUrl.set(topSite.impressionUrl) - Pings.topsitesImpression.submit() + assertTrue(TopSites.contileReportingUrl.testHasValue()) + assertEquals(topSite.impressionUrl, TopSites.contileReportingUrl.testGetValue()) + + topSiteImpressionSubmitted = true } + + TopSiteItemViewHolder(binding.root, lifecycleOwner, interactor).submitTopSitesImpressionPing(topSite, position) + + assertTrue(TopSites.contileImpression.testHasValue()) + + val event = TopSites.contileImpression.testGetValue() + + assertEquals(1, event.size) + assertEquals("top_sites", event[0].category) + assertEquals("contile_impression", event[0].name) + assertEquals("1", event[0].extra!!["position"]) + assertEquals("newtab", event[0].extra!!["source"]) + + assertTrue(topSiteImpressionSubmitted) } }