[fenix] For https://github.com/mozilla-mobile/fenix/issues/24210: Remove wrapper from "url entered" event.

pull/600/head
mcarare 2 years ago committed by mergify[bot]
parent b2935a866a
commit d5016f5e9b

@ -75,6 +75,7 @@ events:
description: |
A boolean that tells us whether the URL was autofilled by an
Autocomplete suggestion
type: boolean
bugs:
- https://github.com/mozilla-mobile/fenix/issues/959
- https://github.com/mozilla-mobile/fenix/issues/19923

@ -284,11 +284,6 @@ sealed class Event {
get() = hashMapOf(Logins.saveLoginsSettingChangedKeys.setting to setting.name)
}
data class EnteredUrl(val autoCompleted: Boolean) : Event() {
override val extras: Map<Events.enteredUrlKeys, String>?
get() = mapOf(Events.enteredUrlKeys.autocomplete to autoCompleted.toString())
}
data class PerformedSearch(val eventSource: EventSource) : Event() {
sealed class EngineSource {
abstract val engine: SearchEngine

@ -89,10 +89,6 @@ private class EventWrapper<T : Enum<T>>(
// FIXME(#19967): Migrate to non-deprecated API.
private val Event.wrapper: EventWrapper<*>?
get() = when (this) {
is Event.EnteredUrl -> EventWrapper(
{ Events.enteredUrl.record(it) },
{ Events.enteredUrlKeys.valueOf(it) }
)
is Event.PerformedSearch -> EventWrapper(
{
Metrics.searchCount[this.eventSource.countLabel].add(1)

@ -31,6 +31,7 @@ import mozilla.telemetry.glean.private.NoExtras
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.FeatureFlags
import org.mozilla.fenix.GleanMetrics.Collections
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.GleanMetrics.Pings
import org.mozilla.fenix.GleanMetrics.TopSites
import org.mozilla.fenix.HomeActivity
@ -592,20 +593,19 @@ class DefaultSessionControlController(
engine = searchEngine
)
val event = if (clipboardText.isUrl() || searchEngine == null) {
Event.EnteredUrl(false)
if (clipboardText.isUrl() || searchEngine == null) {
Events.enteredUrl.record(Events.EnteredUrlExtra(autocomplete = false))
} else {
val searchAccessPoint = Event.PerformedSearch.SearchAccessPoint.ACTION
searchAccessPoint.let { sap ->
val event = searchAccessPoint.let { sap ->
MetricsUtils.createSearchEvent(
searchEngine,
store,
sap
)
}
event?.let { activity.metrics.track(it) }
}
event?.let { activity.metrics.track(it) }
}
override fun handlePaste(clipboardText: String) {

@ -18,6 +18,7 @@ import mozilla.components.concept.engine.EngineSession.LoadUrlFlags
import mozilla.components.feature.tabs.TabsUseCases
import mozilla.components.support.ktx.kotlin.isUrl
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.GleanMetrics.SearchShortcuts
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.R
@ -101,24 +102,25 @@ class SearchDialogController(
requestDesktopMode = fromHomeScreen && activity.settings().openNextTabInDesktopMode
)
val event = if (url.isUrl() || searchEngine == null) {
Event.EnteredUrl(false)
if (url.isUrl() || searchEngine == null) {
Events.enteredUrl.record(Events.EnteredUrlExtra(autocomplete = false))
} else {
val searchAccessPoint = when (fragmentStore.state.searchAccessPoint) {
Event.PerformedSearch.SearchAccessPoint.NONE -> Event.PerformedSearch.SearchAccessPoint.ACTION
else -> fragmentStore.state.searchAccessPoint
}
searchAccessPoint?.let { sap ->
val event = searchAccessPoint?.let { sap ->
MetricsUtils.createSearchEvent(
searchEngine,
store,
sap
)
}
event?.let {
metrics.track(it)
}
}
event?.let { metrics.track(it) }
}
override fun handleEditingCancelled() {
@ -157,7 +159,7 @@ class SearchDialogController(
flags = flags
)
metrics.track(Event.EnteredUrl(false))
Events.enteredUrl.record(Events.EnteredUrlExtra(autocomplete = false))
}
override fun handleSearchTermsTapped(searchTerms: String) {

@ -39,6 +39,7 @@ import mozilla.components.support.test.robolectric.testContext
import mozilla.components.support.test.rule.MainCoroutineRule
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Ignore
@ -47,6 +48,7 @@ import org.junit.Test
import org.junit.runner.RunWith
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.TopSites
import org.mozilla.fenix.HomeActivity
@ -841,6 +843,8 @@ class DefaultSessionControlControllerTest {
@Test
fun handlePasteAndGo() {
assertFalse(Events.enteredUrl.testHasValue())
createController().handlePasteAndGo("text")
verify {
@ -861,8 +865,8 @@ class DefaultSessionControlControllerTest {
from = BrowserDirection.FromHome,
engine = searchEngine
)
metrics.track(any<Event.EnteredUrl>())
}
assertTrue(Events.enteredUrl.testHasValue())
}
@Test

@ -29,6 +29,7 @@ import mozilla.components.support.test.middleware.CaptureActionsMiddleware
import mozilla.components.support.test.robolectric.testContext
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertTrue
import org.junit.Before
@ -36,10 +37,10 @@ import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.GleanMetrics.SearchShortcuts
import org.mozilla.fenix.HomeActivity
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.MetricsUtils
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
@ -48,12 +49,9 @@ import org.mozilla.fenix.search.SearchDialogFragmentDirections.Companion.actionG
import org.mozilla.fenix.settings.SupportUtils
import org.mozilla.fenix.utils.Settings
@RunWith(FenixRobolectricTestRunner::class) // For gleanTestRule
@RunWith(FenixRobolectricTestRunner::class) // for gleanTestRule
class SearchDialogControllerTest {
@get:Rule
val gleanTestRule = GleanTestRule(testContext)
@MockK(relaxed = true) private lateinit var activity: HomeActivity
@MockK(relaxed = true) private lateinit var store: SearchDialogFragmentStore
@MockK(relaxed = true) private lateinit var navController: NavController
@ -64,6 +62,9 @@ class SearchDialogControllerTest {
private lateinit var middleware: CaptureActionsMiddleware<BrowserState, BrowserAction>
private lateinit var browserStore: BrowserStore
@get:Rule
val gleanTestRule = GleanTestRule(testContext)
@Before
fun setUp() {
MockKAnnotations.init(this)
@ -88,6 +89,7 @@ class SearchDialogControllerTest {
@Test
fun handleUrlCommitted() {
val url = "https://www.google.com/"
assertFalse(Events.enteredUrl.testHasValue())
createController().handleUrlCommitted(url)
@ -99,7 +101,11 @@ class SearchDialogControllerTest {
engine = searchEngine
)
}
verify { metrics.track(Event.EnteredUrl(false)) }
assertTrue(Events.enteredUrl.testHasValue())
val snapshot = Events.enteredUrl.testGetValue()
assertEquals(1, snapshot.size)
assertEquals("false", snapshot.single().extra?.getValue("autocomplete"))
}
@Test
@ -157,6 +163,7 @@ class SearchDialogControllerTest {
@Test
fun handleMozillaUrlCommitted() {
val url = "moz://a"
assertFalse(Events.enteredUrl.testHasValue())
createController().handleUrlCommitted(url)
@ -168,7 +175,11 @@ class SearchDialogControllerTest {
engine = searchEngine
)
}
verify { metrics.track(Event.EnteredUrl(false)) }
assertTrue(Events.enteredUrl.testHasValue())
val snapshot = Events.enteredUrl.testGetValue()
assertEquals(1, snapshot.size)
assertEquals("false", snapshot.single().extra?.getValue("autocomplete"))
}
@Test
@ -257,6 +268,7 @@ class SearchDialogControllerTest {
fun handleUrlTapped() {
val url = "https://www.google.com/"
val flags = EngineSession.LoadUrlFlags.all()
assertFalse(Events.enteredUrl.testHasValue())
createController().handleUrlTapped(url, flags)
createController().handleUrlTapped(url)
@ -269,7 +281,12 @@ class SearchDialogControllerTest {
flags = flags
)
}
verify { metrics.track(Event.EnteredUrl(false)) }
assertTrue(Events.enteredUrl.testHasValue())
val snapshot = Events.enteredUrl.testGetValue()
assertEquals(2, snapshot.size)
assertEquals("false", snapshot.first().extra?.getValue("autocomplete"))
assertEquals("false", snapshot[1].extra?.getValue("autocomplete"))
}
@Test

Loading…
Cancel
Save