For #6556: Adds Telemetry counts for Search Access Points

Added two new sources to be counted ('widget' and 'shortcut') besides 'action' and 'suggestion'. Also modified/fixed some tests.
mozilla-mobile · Jan 15, 2020 · 32439e3 · 32439e3
1 parent f93437c
commit 32439e3
Show file tree

Hide file tree

Showing 16 changed files with 247 additions and 57 deletions.
diff --git a/app/metrics.yaml b/app/metrics.yaml
@@ -325,15 +325,18 @@ metrics:
       If the search engine is bundled with Fenix `search-engine-name` will be the name of the search engine. If it's a
       custom search engine (defined: https://github.com/mozilla-mobile/fenix/issues/1607) the value will be `custom`.
 
-      `source` will either be `action` or `suggestion`
+      `source` will be: `action`, `suggestion`, `widget` or `shortcut` (depending on the source from which the search started).
+      Also added the `other` option for the source but it should never enter on this case.
     send_in_pings:
     - metrics
     - baseline
     bugs:
     - https://github.com/mozilla-mobile/fenix/issues/1158
+    - https://github.com/mozilla-mobile/fenix/issues/6556
     data_reviews:
     - https://github.com/mozilla-mobile/fenix/pull/1677
     - https://github.com/mozilla-mobile/fenix/pull/5216
+    - https://github.com/mozilla-mobile/fenix/pull/7310
     notification_emails:
     - fenix-core@mozilla.com
     expires: "2020-03-01"

diff --git a/app/src/main/java/org/mozilla/fenix/HomeActivity.kt b/app/src/main/java/org/mozilla/fenix/HomeActivity.kt
@@ -78,7 +78,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity() {
 
     private val externalSourceIntentProcessors by lazy {
         listOf(
-            SpeechProcessingIntentProcessor(this),
+            SpeechProcessingIntentProcessor(this, components.analytics.metrics),
             StartSearchIntentProcessor(components.analytics.metrics),
             DeepLinkIntentProcessor(this),
             OpenBrowserIntentProcessor(this, ::getIntentSessionId)

diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/Metrics.kt
@@ -271,27 +271,31 @@ sealed class Event {
                 }
         }
 
-        sealed class EventSource {
-            data class Suggestion(val engineSource: EngineSource) : EventSource()
-            data class Action(val engineSource: EngineSource) : EventSource()
-
-            private val source: EngineSource
-                get() = when (this) {
-                    is Suggestion -> engineSource
-                    is Action -> engineSource
-                }
+        sealed class EventSource(open val engineSource: EngineSource) {
+            data class Suggestion(override val engineSource: EngineSource) : EventSource(engineSource)
+            data class Action(override val engineSource: EngineSource) : EventSource(engineSource)
+            data class Widget(override val engineSource: EngineSource) : EventSource(engineSource)
+            data class Shortcut(override val engineSource: EngineSource) : EventSource(engineSource)
+            data class Other(override val engineSource: EngineSource) : EventSource(engineSource)
 
             private val label: String
                 get() = when (this) {
                     is Suggestion -> "suggestion"
                     is Action -> "action"
+                    is Widget -> "widget"
+                    is Shortcut -> "shortcut"
+                    is Other -> "other"
                 }
 
             val countLabel: String
-                get() = "${source.identifier.toLowerCase(Locale.getDefault())}.$label"
+                get() = "${engineSource.identifier.toLowerCase(Locale.getDefault())}.$label"
 
             val sourceLabel: String
-                get() = "${source.descriptor}.$label"
+                get() = "${engineSource.descriptor}.$label"
+        }
+
+        enum class SearchAccessPoint {
+            SUGGESTION, ACTION, WIDGET, SHORTCUT, NONE
         }
 
         override val extras: Map<Events.performedSearchKeys, String>?

diff --git a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricsUtils.kt
@@ -0,0 +1,54 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.components.metrics
+
+import android.content.Context
+import mozilla.components.browser.search.SearchEngine
+import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint
+import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore
+import org.mozilla.fenix.ext.searchEngineManager
+
+object MetricsUtils {
+    fun createSearchEvent(
+        engine: SearchEngine,
+        context: Context,
+        searchAccessPoint: SearchAccessPoint
+    ): Event.PerformedSearch? {
+        val isShortcut = engine != context.searchEngineManager.defaultSearchEngine
+        val isCustom = CustomSearchEngineStore.isCustomSearchEngine(context, engine.identifier)
+
+        val engineSource =
+            if (isShortcut) Event.PerformedSearch.EngineSource.Shortcut(engine, isCustom)
+            else Event.PerformedSearch.EngineSource.Default(engine, isCustom)
+
+        return when (searchAccessPoint) {
+            SearchAccessPoint.SUGGESTION -> Event.PerformedSearch(
+                Event.PerformedSearch.EventSource.Suggestion(
+                    engineSource
+                )
+            )
+            SearchAccessPoint.ACTION -> Event.PerformedSearch(
+                Event.PerformedSearch.EventSource.Action(
+                    engineSource
+                )
+            )
+            SearchAccessPoint.WIDGET -> Event.PerformedSearch(
+                Event.PerformedSearch.EventSource.Widget(
+                    engineSource
+                )
+            )
+            SearchAccessPoint.SHORTCUT -> Event.PerformedSearch(
+                Event.PerformedSearch.EventSource.Shortcut(
+                    engineSource
+                )
+            )
+            SearchAccessPoint.NONE -> Event.PerformedSearch(
+                Event.PerformedSearch.EventSource.Other(
+                    engineSource
+                )
+            )
+        }
+    }
+}
diff --git a/app/src/main/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessor.kt b/app/src/main/java/org/mozilla/fenix/home/intent/SpeechProcessingIntentProcessor.kt
@@ -8,19 +8,32 @@ import android.content.Intent
 import androidx.navigation.NavController
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.components.metrics.MetricController
+import org.mozilla.fenix.components.metrics.MetricsUtils
+import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.widget.VoiceSearchActivity.Companion.SPEECH_PROCESSING
 
 /**
  * The search widget has a microphone button to let users search with their voice.
  * Once the search is complete then a new search should be started.
  */
 class SpeechProcessingIntentProcessor(
-    private val activity: HomeActivity
+    private val activity: HomeActivity,
+    private val metrics: MetricController
 ) : HomeIntentProcessor {
 
     override fun process(intent: Intent, navController: NavController, out: Intent): Boolean {
         return if (intent.extras?.getBoolean(HomeActivity.OPEN_TO_BROWSER_AND_LOAD) == true) {
             out.putExtra(HomeActivity.OPEN_TO_BROWSER_AND_LOAD, false)
+
+            val searchEvent = MetricsUtils.createSearchEvent(
+                activity.components.search.provider.getDefaultEngine(activity),
+                activity,
+                Event.PerformedSearch.SearchAccessPoint.WIDGET
+            )
+            searchEvent?.let { metrics.track(it) }
+
             activity.openToBrowserAndLoad(
                 searchTermOrURL = intent.getStringExtra(SPEECH_PROCESSING).orEmpty(),
                 newTab = true,

diff --git a/app/src/main/java/org/mozilla/fenix/home/intent/StartSearchIntentProcessor.kt b/app/src/main/java/org/mozilla/fenix/home/intent/StartSearchIntentProcessor.kt
@@ -22,20 +22,36 @@ class StartSearchIntentProcessor(
 
     override fun process(intent: Intent, navController: NavController, out: Intent): Boolean {
         val event = intent.extras?.getString(HomeActivity.OPEN_TO_SEARCH)
+        var source: Event.PerformedSearch.SearchAccessPoint? = null
         return if (event != null) {
             when (event) {
-                SEARCH_WIDGET -> metrics.track(Event.SearchWidgetNewTabPressed)
-                STATIC_SHORTCUT_NEW_TAB -> metrics.track(Event.PrivateBrowsingStaticShortcutTab)
-                STATIC_SHORTCUT_NEW_PRIVATE_TAB -> metrics.track(Event.PrivateBrowsingStaticShortcutPrivateTab)
-                PRIVATE_BROWSING_PINNED_SHORTCUT -> metrics.track(Event.PrivateBrowsingPinnedShortcutPrivateTab)
+                SEARCH_WIDGET -> {
+                    metrics.track(Event.SearchWidgetNewTabPressed)
+                    source = Event.PerformedSearch.SearchAccessPoint.WIDGET
+                }
+                STATIC_SHORTCUT_NEW_TAB -> {
+                    metrics.track(Event.PrivateBrowsingStaticShortcutTab)
+                    source = Event.PerformedSearch.SearchAccessPoint.SHORTCUT
+                }
+                STATIC_SHORTCUT_NEW_PRIVATE_TAB -> {
+                    metrics.track(Event.PrivateBrowsingStaticShortcutPrivateTab)
+                    source = Event.PerformedSearch.SearchAccessPoint.SHORTCUT
+                }
+                PRIVATE_BROWSING_PINNED_SHORTCUT -> {
+                    metrics.track(Event.PrivateBrowsingPinnedShortcutPrivateTab)
+                    source = Event.PerformedSearch.SearchAccessPoint.SHORTCUT
+                }
             }
 
             out.removeExtra(HomeActivity.OPEN_TO_SEARCH)
 
-            val directions = NavGraphDirections.actionGlobalSearch(
-                sessionId = null
-            )
-            navController.nav(null, directions)
+            val directions = source?.let {
+                NavGraphDirections.actionGlobalSearch(
+                    sessionId = null,
+                    searchAccessPoint = it
+                )
+            }
+            directions?.let { navController.nav(null, it) }
             true
         } else {
             false

diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchController.kt b/app/src/main/java/org/mozilla/fenix/search/SearchController.kt
@@ -14,12 +14,15 @@ import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint.ACTION
+import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint.NONE
+import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint.SUGGESTION
+import org.mozilla.fenix.components.metrics.MetricsUtils
 import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore
+import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.metrics
 import org.mozilla.fenix.ext.nav
 import org.mozilla.fenix.ext.settings
-import org.mozilla.fenix.ext.components
-import org.mozilla.fenix.ext.searchEngineManager
 
 /**
  * An interface that handles the view manipulation of the Search, triggered by the Interactor
@@ -54,10 +57,21 @@ class DefaultSearchController(
             val event = if (url.isUrl()) {
                 Event.EnteredUrl(false)
             } else {
-                createSearchEvent(store.state.searchEngineSource.searchEngine, false)
+                val searchAccessPoint = when (store.state.searchAccessPoint) {
+                    NONE -> ACTION
+                    else -> store.state.searchAccessPoint
+                }
+
+                searchAccessPoint?.let { sap ->
+                    MetricsUtils.createSearchEvent(
+                        store.state.searchEngineSource.searchEngine,
+                        context,
+                        sap
+                    )
+                }
             }
 
-            context.metrics.track(event)
+            event?.let { context.metrics.track(it) }
             if (CustomSearchEngineStore.isCustomSearchEngine(
                     context,
                     store.state.searchEngineSource.searchEngine.identifier
@@ -111,8 +125,20 @@ class DefaultSearchController(
             forceSearch = true
         )
 
-        val event = createSearchEvent(store.state.searchEngineSource.searchEngine, true)
-        context.metrics.track(event)
+        val searchAccessPoint = when (store.state.searchAccessPoint) {
+            NONE -> SUGGESTION
+            else -> store.state.searchAccessPoint
+        }
+
+        val event = searchAccessPoint?.let { sap ->
+            MetricsUtils.createSearchEvent(
+                store.state.searchEngineSource.searchEngine,
+                context,
+                sap
+            )
+        }
+        event?.let { context.metrics.track(it) }
+
         if (CustomSearchEngineStore.isCustomSearchEngine(
                 context,
                 store.state.searchEngineSource.searchEngine.identifier
@@ -142,22 +168,4 @@ class DefaultSearchController(
         navController.nav(R.id.searchFragment, directions)
         context.components.core.sessionManager.select(session)
     }
-
-    private fun createSearchEvent(
-        engine: SearchEngine,
-        isSuggestion: Boolean
-    ): Event.PerformedSearch {
-        val isShortcut = engine != context.searchEngineManager.defaultSearchEngine
-        val isCustom = CustomSearchEngineStore.isCustomSearchEngine(context, engine.identifier)
-
-        val engineSource =
-            if (isShortcut) Event.PerformedSearch.EngineSource.Shortcut(engine, isCustom)
-            else Event.PerformedSearch.EngineSource.Default(engine, isCustom)
-
-        val source =
-            if (isSuggestion) Event.PerformedSearch.EventSource.Suggestion(engineSource)
-            else Event.PerformedSearch.EventSource.Action(engineSource)
-
-        return Event.PerformedSearch(source)
-    }
 }
diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt
@@ -80,6 +80,9 @@ class SearchFragment : Fragment(), UserInteractionHandler {
             ?.let(SearchFragmentArgs.Companion::fromBundle)
             ?.let { it.pastedText }
 
+        val searchAccessPoint = arguments
+                ?.let(SearchFragmentArgs.Companion::fromBundle)?.searchAccessPoint
+
         val view = inflater.inflate(R.layout.fragment_search, container, false)
         val url = session?.url.orEmpty()
         val currentSearchEngine = SearchEngineSource.Default(
@@ -107,7 +110,8 @@ class SearchFragment : Fragment(), UserInteractionHandler {
                     showHistorySuggestions = requireContext().settings().shouldShowHistorySuggestions,
                     showBookmarkSuggestions = requireContext().settings().shouldShowBookmarkSuggestions,
                     session = session,
-                    pastedText = pastedText
+                    pastedText = pastedText,
+                    searchAccessPoint = searchAccessPoint
                 )
             )
         }

diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/search/SearchFragmentStore.kt
@@ -9,6 +9,7 @@ import mozilla.components.browser.session.Session
 import mozilla.components.lib.state.Action
 import mozilla.components.lib.state.State
 import mozilla.components.lib.state.Store
+import org.mozilla.fenix.components.metrics.Event
 
 /**
  * The [Store] for holding the [SearchFragmentState] and applying [SearchFragmentAction]s.
@@ -55,7 +56,8 @@ data class SearchFragmentState(
     val showHistorySuggestions: Boolean,
     val showBookmarkSuggestions: Boolean,
     val session: Session?,
-    val pastedText: String? = null
+    val pastedText: String? = null,
+    val searchAccessPoint: Event.PerformedSearch.SearchAccessPoint?
 ) : State
 
 /**

diff --git a/app/src/main/res/navigation/nav_graph.xml b/app/src/main/res/navigation/nav_graph.xml
@@ -112,6 +112,10 @@
             android:defaultValue="@null"
             app:argType="string"
             app:nullable="true" />
+        <argument
+            android:name="search_access_point"
+            app:argType="org.mozilla.fenix.components.metrics.Event$PerformedSearch$SearchAccessPoint"
+            android:defaultValue="NONE" />
     </fragment>
 
     <fragment