Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For #24210: Remove wrapper for metrics in the "events" category #24483

Merged
merged 15 commits into from
Apr 6, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
For #24210: Remove wrapper from "default browser" events.
mcarare committed Apr 6, 2022
commit 020755df5fc00c9ef22fbe8744999c8bcd948fc9
5 changes: 3 additions & 2 deletions app/src/main/java/org/mozilla/fenix/HomeActivity.kt
Original file line number Diff line number Diff line change
@@ -68,6 +68,7 @@ import mozilla.components.support.locale.LocaleAwareAppCompatActivity
import mozilla.components.support.utils.SafeIntent
import mozilla.components.support.utils.toSafeIntent
import mozilla.components.support.webextensions.WebExtensionPopupFeature
import mozilla.telemetry.glean.private.NoExtras
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.GleanMetrics.Metrics
import org.mozilla.fenix.addons.AddonDetailsFragmentDirections
@@ -172,7 +173,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
StartSearchIntentProcessor(components.analytics.metrics),
OpenBrowserIntentProcessor(this, ::getIntentSessionId),
OpenSpecificTabIntentProcessor(this),
DefaultBrowserIntentProcessor(this, components.analytics.metrics)
DefaultBrowserIntentProcessor(this)
)
}

@@ -346,7 +347,7 @@ open class HomeActivity : LocaleAwareAppCompatActivity(), NavHostActivity {
}

if (settings().checkIfFenixIsDefaultBrowserOnAppResume()) {
metrics.track(Event.ChangedToDefaultBrowser)
Events.defaultBrowserChanged.record(NoExtras())
}

DefaultBrowserNotificationWorker.setDefaultBrowserNotificationIfNeeded(applicationContext)
Original file line number Diff line number Diff line change
@@ -122,8 +122,6 @@ sealed class Event {
object AddonsOpenInSettings : Event()
object VoiceSearchTapped : Event()
object SearchWidgetInstalled : Event()
object ChangedToDefaultBrowser : Event()
object DefaultBrowserNotifTapped : Event()

object ProgressiveWebAppOpenFromHomescreenTap : Event()
object ProgressiveWebAppInstallAsShortcut : Event()
Original file line number Diff line number Diff line change
@@ -122,12 +122,7 @@ private val Event.wrapper: EventWrapper<*>?
is Event.ToolbarMenuShown -> EventWrapper<NoExtraKeys>(
{ Events.toolbarMenuVisible.record(it) }
)
is Event.ChangedToDefaultBrowser -> EventWrapper<NoExtraKeys>(
{ Events.defaultBrowserChanged.record(it) }
)
is Event.DefaultBrowserNotifTapped -> EventWrapper<NoExtraKeys>(
{ Events.defaultBrowserNotifTapped.record(it) }
)

is Event.CustomTabsMenuOpened -> EventWrapper<NoExtraKeys>(
{ CustomTab.menu.record(it) }
)
@@ -140,10 +135,6 @@ private val Event.wrapper: EventWrapper<*>?
is Event.NormalAndPrivateUriOpened -> EventWrapper<NoExtraKeys>(
{ Events.normalAndPrivateUriCount.add(1) }
)
is Event.PreferenceToggled -> EventWrapper(
{ Events.preferenceToggled.record(it) },
{ Events.preferenceToggledKeys.valueOf(it) }
)
is Event.HistoryOpened -> EventWrapper<NoExtraKeys>(
{ History.opened.record(it) }
)
@@ -235,11 +226,6 @@ private val Event.wrapper: EventWrapper<*>?
is Event.NotificationMediaPause -> EventWrapper<NoExtraKeys>(
{ MediaNotification.pause.record(it) }
)

is Event.OpenedLink -> EventWrapper(
{ Events.openedLink.record(it) },
{ Events.openedLinkKeys.valueOf(it) }
)
is Event.OpenLogins -> EventWrapper<NoExtraKeys>(
{ Logins.openLogins.record(it) }
)
@@ -459,11 +445,6 @@ private val Event.wrapper: EventWrapper<*>?
is Event.HomeScreenCustomizedHomeClicked -> EventWrapper<NoExtraKeys>(
{ HomeScreen.customizeHomeClicked.record(it) }
)
is Event.TabViewSettingChanged -> EventWrapper(
{ Events.tabViewChanged.record(it) },
{ Events.tabViewChangedKeys.valueOf(it) }
)

is Event.BrowserToolbarHomeButtonClicked -> EventWrapper<NoExtraKeys>(
{ Events.browserToolbarHomeTapped.record(it) }
)
Original file line number Diff line number Diff line change
@@ -6,9 +6,9 @@ package org.mozilla.fenix.home.intent

import android.content.Intent
import androidx.navigation.NavController
import mozilla.telemetry.glean.private.NoExtras
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.ext.openSetDefaultBrowserOption
import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.onboarding.DefaultBrowserNotificationWorker.Companion.isDefaultBrowserNotificationIntent
@@ -21,13 +21,12 @@ import org.mozilla.fenix.onboarding.DefaultBrowserNotificationWorker.Companion.i
*/
class DefaultBrowserIntentProcessor(
private val activity: HomeActivity,
private val metrics: MetricController
) : HomeIntentProcessor {

override fun process(intent: Intent, navController: NavController, out: Intent): Boolean {
return if (isDefaultBrowserNotificationIntent(intent)) {
activity.openSetDefaultBrowserOption()
metrics.track(Event.DefaultBrowserNotifTapped)
Events.defaultBrowserNotifTapped.record(NoExtras())
true
} else {
false
Original file line number Diff line number Diff line change
@@ -16,7 +16,6 @@ import org.junit.runner.RunWith
import org.mozilla.fenix.GleanMetrics.Addons
import org.mozilla.fenix.GleanMetrics.Awesomebar
import org.mozilla.fenix.GleanMetrics.CreditCards
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.GleanMetrics.History
import org.mozilla.fenix.GleanMetrics.RecentBookmarks
import org.mozilla.fenix.GleanMetrics.RecentlyVisitedHomepage
@@ -147,17 +146,6 @@ class GleanMetricsServiceTest {
assertEquals("123", events[0].extra!!["addon_id"])
}

@Test
fun `default browser events are correctly recorded`() {
assertFalse(Events.defaultBrowserChanged.testHasValue())
gleanService.track(Event.ChangedToDefaultBrowser)
assertTrue(Events.defaultBrowserChanged.testHasValue())

assertFalse(Events.defaultBrowserNotifTapped.testHasValue())
gleanService.track(Event.DefaultBrowserNotifTapped)
assertTrue(Events.defaultBrowserNotifTapped.testHasValue())
}

@Test
fun `Home screen recent bookmarks events are correctly recorded`() {
assertFalse(RecentBookmarks.shown.testHasValue())
Original file line number Diff line number Diff line change
@@ -10,22 +10,29 @@ import io.mockk.Called
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import mozilla.components.service.glean.testing.GleanTestRule
import mozilla.components.support.test.robolectric.testContext
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.HomeActivity
import org.mozilla.fenix.components.metrics.MetricController
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner

@RunWith(FenixRobolectricTestRunner::class)
class DefaultBrowserIntentProcessorTest {

@get:Rule
val gleanTestRule = GleanTestRule(testContext)

@Test
fun `do not process blank intents`() {
val navController: NavController = mockk()
val out: Intent = mockk()
val result = DefaultBrowserIntentProcessor(mockk(), mockk())
val result = DefaultBrowserIntentProcessor(mockk())
.process(Intent(), navController, out)

assertFalse(result)
@@ -47,11 +54,14 @@ class DefaultBrowserIntentProcessorTest {
every { activity.applicationContext } returns testContext
every { metrics.track(any()) } returns Unit

val result = DefaultBrowserIntentProcessor(activity, metrics)
assertFalse(Events.defaultBrowserNotifTapped.testHasValue())

val result = DefaultBrowserIntentProcessor(activity)
.process(intent, navController, out)

assert(result)
verify { metrics.track(any()) }

assertTrue(Events.defaultBrowserNotifTapped.testHasValue())
verify { navController wasNot Called }
verify { out wasNot Called }
}