From 0602b96420a0c6ea6b6c700183e0aa24d579cebd Mon Sep 17 00:00:00 2001 From: Alexandru2909 Date: Thu, 14 Apr 2022 10:28:10 +0300 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/24760 - Remove Event.wrapper for suggestions telemetry --- .../org/mozilla/fenix/components/metrics/Event.kt | 8 -- .../components/metrics/GleanMetricsService.kt | 25 ---- .../fenix/components/metrics/MetricController.kt | 44 +++---- .../mozilla/fenix/search/SearchDialogFragment.kt | 3 +- .../components/metrics/GleanMetricsServiceTest.kt | 36 ------ .../components/metrics/MetricControllerTest.kt | 140 ++++++++++----------- 6 files changed, 92 insertions(+), 164 deletions(-) diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt index 0e4522ad9ac6..48895c48c793 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt @@ -16,14 +16,6 @@ sealed class Event { object HistorySearchGroupOpened : Event() object SearchWidgetInstalled : Event() - object SyncedTabSuggestionClicked : Event() - object BookmarkSuggestionClicked : Event() - object ClipboardSuggestionClicked : Event() - object HistorySuggestionClicked : Event() - object SearchActionClicked : Event() - object SearchSuggestionClicked : Event() - object OpenedTabSuggestionClicked : Event() - // Interaction events with extras data class SearchWithAds(val providerName: String) : Event() { diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt index 8aac5ac92308..ca0633a53374 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt @@ -8,11 +8,9 @@ import android.content.Context import mozilla.components.service.glean.Glean import mozilla.components.service.glean.private.NoExtraKeys import mozilla.components.support.base.log.logger.Logger -import org.mozilla.fenix.GleanMetrics.Awesomebar import org.mozilla.fenix.GleanMetrics.BrowserSearch import org.mozilla.fenix.GleanMetrics.Pings import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage -import org.mozilla.fenix.GleanMetrics.SyncedTabs import org.mozilla.fenix.GleanMetrics.Messaging import org.mozilla.fenix.ext.components @@ -75,29 +73,6 @@ private val Event.wrapper: EventWrapper<*>? } ) - is Event.SyncedTabSuggestionClicked -> EventWrapper( - { SyncedTabs.syncedTabsSuggestionClicked.record(it) } - ) - - is Event.BookmarkSuggestionClicked -> EventWrapper( - { Awesomebar.bookmarkSuggestionClicked.record(it) } - ) - is Event.ClipboardSuggestionClicked -> EventWrapper( - { Awesomebar.clipboardSuggestionClicked.record(it) } - ) - is Event.HistorySuggestionClicked -> EventWrapper( - { Awesomebar.historySuggestionClicked.record(it) } - ) - is Event.SearchActionClicked -> EventWrapper( - { Awesomebar.searchActionClicked.record(it) } - ) - is Event.SearchSuggestionClicked -> EventWrapper( - { Awesomebar.searchSuggestionClicked.record(it) } - ) - is Event.OpenedTabSuggestionClicked -> EventWrapper( - { Awesomebar.openedTabSuggestionClicked.record(it) } - ) - is Event.Messaging.MessageShown -> EventWrapper( { Messaging.messageShown.record( diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt index b65cfd72dabd..5e800ce145ca 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt @@ -38,6 +38,7 @@ import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.GleanMetrics.Addons import org.mozilla.fenix.GleanMetrics.ContextMenu import org.mozilla.fenix.GleanMetrics.AndroidAutofill +import org.mozilla.fenix.GleanMetrics.Awesomebar import org.mozilla.fenix.GleanMetrics.ContextualMenu import org.mozilla.fenix.GleanMetrics.CreditCards import org.mozilla.fenix.GleanMetrics.CustomTab @@ -47,6 +48,7 @@ import org.mozilla.fenix.GleanMetrics.MediaNotification import org.mozilla.fenix.GleanMetrics.MediaState import org.mozilla.fenix.GleanMetrics.PerfAwesomebar import org.mozilla.fenix.GleanMetrics.ProgressiveWebApp +import org.mozilla.fenix.GleanMetrics.SyncedTabs import org.mozilla.fenix.search.awesomebar.ShortcutsSuggestionProvider import org.mozilla.fenix.utils.Settings @@ -206,6 +208,27 @@ internal class ReleaseMetricController( AndroidAutofill.unlockCancelled.record(NoExtras()) } } + Component.FEATURE_SYNCEDTABS to SyncedTabsFacts.Items.SYNCED_TABS_SUGGESTION_CLICKED -> { + SyncedTabs.syncedTabsSuggestionClicked.record(NoExtras()) + } + Component.FEATURE_AWESOMEBAR to AwesomeBarFacts.Items.BOOKMARK_SUGGESTION_CLICKED -> { + Awesomebar.bookmarkSuggestionClicked.record(NoExtras()) + } + Component.FEATURE_AWESOMEBAR to AwesomeBarFacts.Items.CLIPBOARD_SUGGESTION_CLICKED -> { + Awesomebar.clipboardSuggestionClicked.record(NoExtras()) + } + Component.FEATURE_AWESOMEBAR to AwesomeBarFacts.Items.HISTORY_SUGGESTION_CLICKED -> { + Awesomebar.historySuggestionClicked.record(NoExtras()) + } + Component.FEATURE_AWESOMEBAR to AwesomeBarFacts.Items.SEARCH_ACTION_CLICKED -> { + Awesomebar.searchActionClicked.record(NoExtras()) + } + Component.FEATURE_AWESOMEBAR to AwesomeBarFacts.Items.SEARCH_SUGGESTION_CLICKED -> { + Awesomebar.searchSuggestionClicked.record(NoExtras()) + } + Component.FEATURE_AWESOMEBAR to AwesomeBarFacts.Items.OPENED_TAB_SUGGESTION_CLICKED -> { + Awesomebar.openedTabSuggestionClicked.record(NoExtras()) + } Component.FEATURE_CONTEXTMENU to ContextMenuFacts.Items.TEXT_SELECTION_OPTION -> { when (metadata?.get("textSelectionOption")?.toString()) { CONTEXT_MENU_COPY -> ContextualMenu.copyTapped.record(NoExtras()) @@ -340,27 +363,6 @@ internal class ReleaseMetricController( } null } - Component.FEATURE_SYNCEDTABS == component && SyncedTabsFacts.Items.SYNCED_TABS_SUGGESTION_CLICKED == item -> { - Event.SyncedTabSuggestionClicked - } - Component.FEATURE_AWESOMEBAR == component && AwesomeBarFacts.Items.BOOKMARK_SUGGESTION_CLICKED == item -> { - Event.BookmarkSuggestionClicked - } - Component.FEATURE_AWESOMEBAR == component && AwesomeBarFacts.Items.CLIPBOARD_SUGGESTION_CLICKED == item -> { - Event.ClipboardSuggestionClicked - } - Component.FEATURE_AWESOMEBAR == component && AwesomeBarFacts.Items.HISTORY_SUGGESTION_CLICKED == item -> { - Event.HistorySuggestionClicked - } - Component.FEATURE_AWESOMEBAR == component && AwesomeBarFacts.Items.SEARCH_ACTION_CLICKED == item -> { - Event.SearchActionClicked - } - Component.FEATURE_AWESOMEBAR == component && AwesomeBarFacts.Items.SEARCH_SUGGESTION_CLICKED == item -> { - Event.SearchSuggestionClicked - } - Component.FEATURE_AWESOMEBAR == component && AwesomeBarFacts.Items.OPENED_TAB_SUGGESTION_CLICKED == item -> { - Event.OpenedTabSuggestionClicked - } Component.FEATURE_SEARCH == component && AdsTelemetry.SERP_ADD_CLICKED == item -> { Event.SearchAdClicked(value!!) } diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt index cd2893f26484..b3b56a3e98eb 100644 --- a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt +++ b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt @@ -65,6 +65,7 @@ import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifAnyChanged import mozilla.components.support.ktx.kotlinx.coroutines.flow.ifChanged import mozilla.components.ui.autocomplete.InlineAutocompleteEditText import org.mozilla.fenix.BrowserDirection +import org.mozilla.fenix.GleanMetrics.Awesomebar import org.mozilla.fenix.GleanMetrics.VoiceSearch import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R @@ -319,7 +320,7 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { } binding.fillLinkFromClipboard.setOnClickListener { - requireComponents.analytics.metrics.track(Event.ClipboardSuggestionClicked) + Awesomebar.clipboardSuggestionClicked.record(NoExtras()) val clipboardUrl = requireContext().components.clipboardHandler.extractURL() ?: "" if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt index 8658a6c26e58..cddd9c4bf32e 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt +++ b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/GleanMetricsServiceTest.kt @@ -12,9 +12,7 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.mozilla.fenix.GleanMetrics.Awesomebar import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage -import org.mozilla.fenix.GleanMetrics.SyncedTabs import org.mozilla.fenix.helpers.FenixRobolectricTestRunner @RunWith(FenixRobolectricTestRunner::class) @@ -30,40 +28,6 @@ class GleanMetricsServiceTest { } @Test - fun `synced tab event is correctly recorded`() { - assertFalse(SyncedTabs.syncedTabsSuggestionClicked.testHasValue()) - gleanService.track(Event.SyncedTabSuggestionClicked) - assertTrue(SyncedTabs.syncedTabsSuggestionClicked.testHasValue()) - } - - @Test - fun `awesomebar events are correctly recorded`() { - assertFalse(Awesomebar.bookmarkSuggestionClicked.testHasValue()) - gleanService.track(Event.BookmarkSuggestionClicked) - assertTrue(Awesomebar.bookmarkSuggestionClicked.testHasValue()) - - assertFalse(Awesomebar.clipboardSuggestionClicked.testHasValue()) - gleanService.track(Event.ClipboardSuggestionClicked) - assertTrue(Awesomebar.clipboardSuggestionClicked.testHasValue()) - - assertFalse(Awesomebar.historySuggestionClicked.testHasValue()) - gleanService.track(Event.HistorySuggestionClicked) - assertTrue(Awesomebar.historySuggestionClicked.testHasValue()) - - assertFalse(Awesomebar.searchActionClicked.testHasValue()) - gleanService.track(Event.SearchActionClicked) - assertTrue(Awesomebar.searchActionClicked.testHasValue()) - - assertFalse(Awesomebar.searchSuggestionClicked.testHasValue()) - gleanService.track(Event.SearchSuggestionClicked) - assertTrue(Awesomebar.searchSuggestionClicked.testHasValue()) - - assertFalse(Awesomebar.openedTabSuggestionClicked.testHasValue()) - gleanService.track(Event.OpenedTabSuggestionClicked) - assertTrue(Awesomebar.openedTabSuggestionClicked.testHasValue()) - } - - @Test fun `Home screen recently visited events are correctly recorded`() { assertFalse(RecentlyVisitedHomepage.historyHighlightOpened.testHasValue()) gleanService.track(Event.HistoryHighlightOpened) diff --git a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt index 4413cc8f1cc1..e8f2227d0b8b 100644 --- a/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt +++ b/mobile/android/fenix/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt @@ -38,11 +38,13 @@ import org.junit.runner.RunWith import org.mozilla.fenix.GleanMetrics.AndroidAutofill import org.mozilla.fenix.GleanMetrics.ContextualMenu import org.mozilla.fenix.GleanMetrics.CreditCards +import org.mozilla.fenix.GleanMetrics.Awesomebar import org.mozilla.fenix.GleanMetrics.CustomTab import org.mozilla.fenix.GleanMetrics.LoginDialog import org.mozilla.fenix.GleanMetrics.MediaNotification import org.mozilla.fenix.GleanMetrics.ProgressiveWebApp import org.mozilla.fenix.components.metrics.ReleaseMetricController.Companion +import org.mozilla.fenix.GleanMetrics.SyncedTabs import org.mozilla.fenix.helpers.FenixRobolectricTestRunner import org.mozilla.fenix.utils.Settings @@ -199,56 +201,6 @@ class MetricControllerTest { } @Test - fun `tracking synced tab event should be sent to enabled service`() { - val controller = ReleaseMetricController( - listOf(marketingService1), - isDataTelemetryEnabled = { true }, - isMarketingDataTelemetryEnabled = { true }, - mockk() - ) - every { marketingService1.shouldTrack(Event.SyncedTabSuggestionClicked) } returns true - controller.start(MetricServiceType.Marketing) - - controller.track(Event.SyncedTabSuggestionClicked) - verify { marketingService1.track(Event.SyncedTabSuggestionClicked) } - } - - @Test - fun `tracking awesomebar events should be sent to enabled service`() { - val controller = ReleaseMetricController( - listOf(marketingService1), - isDataTelemetryEnabled = { true }, - isMarketingDataTelemetryEnabled = { true }, - mockk() - ) - every { marketingService1.shouldTrack(Event.BookmarkSuggestionClicked) } returns true - every { marketingService1.shouldTrack(Event.ClipboardSuggestionClicked) } returns true - every { marketingService1.shouldTrack(Event.HistorySuggestionClicked) } returns true - every { marketingService1.shouldTrack(Event.SearchActionClicked) } returns true - every { marketingService1.shouldTrack(Event.SearchSuggestionClicked) } returns true - every { marketingService1.shouldTrack(Event.OpenedTabSuggestionClicked) } returns true - controller.start(MetricServiceType.Marketing) - - controller.track(Event.BookmarkSuggestionClicked) - verify { marketingService1.track(Event.BookmarkSuggestionClicked) } - - controller.track(Event.ClipboardSuggestionClicked) - verify { marketingService1.track(Event.ClipboardSuggestionClicked) } - - controller.track(Event.HistorySuggestionClicked) - verify { marketingService1.track(Event.HistorySuggestionClicked) } - - controller.track(Event.SearchActionClicked) - verify { marketingService1.track(Event.SearchActionClicked) } - - controller.track(Event.SearchSuggestionClicked) - verify { marketingService1.track(Event.SearchSuggestionClicked) } - - controller.track(Event.OpenedTabSuggestionClicked) - verify { marketingService1.track(Event.OpenedTabSuggestionClicked) } - } - - @Test fun `web extension fact should set value in SharedPreference`() { val enabled = true val settings = Settings(testContext) @@ -280,29 +232,6 @@ class MetricControllerTest { } @Test - fun `WHEN changing Fact(component, item) without additional vals to events THEN it returns the right event`() { - // This naive test was added for a refactoring. It only covers the comparisons that were easy to add. - val controller = ReleaseMetricController(emptyList(), { true }, { true }, mockk()) - - val simpleMappings = listOf( - // CreditCardAutofillDialogFacts.Items is already tested. - Triple(Component.FEATURE_SYNCEDTABS, SyncedTabsFacts.Items.SYNCED_TABS_SUGGESTION_CLICKED, Event.SyncedTabSuggestionClicked), - Triple(Component.FEATURE_AWESOMEBAR, AwesomeBarFacts.Items.BOOKMARK_SUGGESTION_CLICKED, Event.BookmarkSuggestionClicked), - Triple(Component.FEATURE_AWESOMEBAR, AwesomeBarFacts.Items.CLIPBOARD_SUGGESTION_CLICKED, Event.ClipboardSuggestionClicked), - Triple(Component.FEATURE_AWESOMEBAR, AwesomeBarFacts.Items.HISTORY_SUGGESTION_CLICKED, Event.HistorySuggestionClicked), - Triple(Component.FEATURE_AWESOMEBAR, AwesomeBarFacts.Items.SEARCH_ACTION_CLICKED, Event.SearchActionClicked), - Triple(Component.FEATURE_AWESOMEBAR, AwesomeBarFacts.Items.SEARCH_SUGGESTION_CLICKED, Event.SearchSuggestionClicked), - Triple(Component.FEATURE_AWESOMEBAR, AwesomeBarFacts.Items.OPENED_TAB_SUGGESTION_CLICKED, Event.OpenedTabSuggestionClicked), - ) - - simpleMappings.forEach { (component, item, expectedEvent) -> - val fact = Fact(component, Action.CANCEL, item) - val message = "$expectedEvent $component $item" - assertEquals(message, expectedEvent, controller.factToEvent(fact)) - } - } - - @Test fun `WHEN processing a fact with FEATURE_PROMPTS component THEN the right metric is recorded with no extras`() { val controller = ReleaseMetricController(emptyList(), { true }, { true }, mockk()) val action = mockk(relaxed = true) @@ -533,4 +462,69 @@ class MetricControllerTest { assertTrue(ProgressiveWebApp.installTap.testHasValue()) } + + @Test + fun `WHEN processing a suggestion fact THEN the right metric is recorded`() { + val controller = ReleaseMetricController(emptyList(), { true }, { true }, mockk()) + + // Verify synced tabs suggestion clicked + assertFalse(SyncedTabs.syncedTabsSuggestionClicked.testHasValue()) + var fact = Fact(Component.FEATURE_SYNCEDTABS, Action.CANCEL, SyncedTabsFacts.Items.SYNCED_TABS_SUGGESTION_CLICKED) + + with(controller) { + fact.process() + } + + assertTrue(SyncedTabs.syncedTabsSuggestionClicked.testHasValue()) + + // Verify bookmark suggestion clicked + assertFalse(Awesomebar.bookmarkSuggestionClicked.testHasValue()) + fact = Fact(Component.FEATURE_AWESOMEBAR, Action.CANCEL, AwesomeBarFacts.Items.BOOKMARK_SUGGESTION_CLICKED) + + with(controller) { + fact.process() + } + + assertTrue(Awesomebar.bookmarkSuggestionClicked.testHasValue()) + + // Verify clipboard suggestion clicked + assertFalse(Awesomebar.clipboardSuggestionClicked.testHasValue()) + fact = Fact(Component.FEATURE_AWESOMEBAR, Action.CANCEL, AwesomeBarFacts.Items.CLIPBOARD_SUGGESTION_CLICKED) + + with(controller) { + fact.process() + } + + assertTrue(Awesomebar.clipboardSuggestionClicked.testHasValue()) + + // Verify history suggestion clicked + assertFalse(Awesomebar.historySuggestionClicked.testHasValue()) + fact = Fact(Component.FEATURE_AWESOMEBAR, Action.CANCEL, AwesomeBarFacts.Items.HISTORY_SUGGESTION_CLICKED) + + with(controller) { + fact.process() + } + + assertTrue(Awesomebar.historySuggestionClicked.testHasValue()) + + // Verify search action clicked + assertFalse(Awesomebar.searchActionClicked.testHasValue()) + fact = Fact(Component.FEATURE_AWESOMEBAR, Action.CANCEL, AwesomeBarFacts.Items.SEARCH_ACTION_CLICKED) + + with(controller) { + fact.process() + } + + assertTrue(Awesomebar.searchActionClicked.testHasValue()) + + // Verify bookmark opened tab suggestion clicked + assertFalse(Awesomebar.openedTabSuggestionClicked.testHasValue()) + fact = Fact(Component.FEATURE_AWESOMEBAR, Action.CANCEL, AwesomeBarFacts.Items.OPENED_TAB_SUGGESTION_CLICKED) + + with(controller) { + fact.process() + } + + assertTrue(Awesomebar.openedTabSuggestionClicked.testHasValue()) + } } -- 2.11.4.GIT