[fenix] For https://github.com/mozilla-mobile/fenix/issues/24210: Remove wrapper from opened link event.

pull/600/head
mcarare 2 years ago committed by mergify[bot]
parent 40e36bb228
commit 8128c95710

@ -321,6 +321,7 @@ events:
The mode the link was opened in. Either 'PRIVATE' or 'NORMAL'. N.B.: The mode the link was opened in. Either 'PRIVATE' or 'NORMAL'. N.B.:
this probe may be incorrectly implemented: see this probe may be incorrectly implemented: see
https://github.com/mozilla-mobile/fenix/issues/14133 https://github.com/mozilla-mobile/fenix/issues/14133
type: string
bugs: bugs:
- https://github.com/mozilla-mobile/fenix/issues/5737 - https://github.com/mozilla-mobile/fenix/issues/5737
- https://github.com/mozilla-mobile/fenix/issues/19923 - https://github.com/mozilla-mobile/fenix/issues/19923

@ -15,10 +15,10 @@ import mozilla.components.feature.intent.ext.sanitize
import mozilla.components.feature.intent.processing.IntentProcessor import mozilla.components.feature.intent.processing.IntentProcessor
import mozilla.components.support.utils.EXTRA_ACTIVITY_REFERRER_CATEGORY import mozilla.components.support.utils.EXTRA_ACTIVITY_REFERRER_CATEGORY
import mozilla.components.support.utils.EXTRA_ACTIVITY_REFERRER_PACKAGE import mozilla.components.support.utils.EXTRA_ACTIVITY_REFERRER_PACKAGE
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.HomeActivity.Companion.PRIVATE_BROWSING_MODE import org.mozilla.fenix.HomeActivity.Companion.PRIVATE_BROWSING_MODE
import org.mozilla.fenix.components.IntentProcessorType import org.mozilla.fenix.components.IntentProcessorType
import org.mozilla.fenix.components.getType import org.mozilla.fenix.components.getType
import org.mozilla.fenix.components.metrics.Event
import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.components
import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.settings
import org.mozilla.fenix.perf.MarkersActivityLifecycleCallbacks import org.mozilla.fenix.perf.MarkersActivityLifecycleCallbacks
@ -58,9 +58,9 @@ class IntentReceiverActivity : Activity() {
val private = settings().openLinksInAPrivateTab val private = settings().openLinksInAPrivateTab
intent.putExtra(PRIVATE_BROWSING_MODE, private) intent.putExtra(PRIVATE_BROWSING_MODE, private)
if (private) { if (private) {
components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.PRIVATE)) Events.openedLink.record(Events.OpenedLinkExtra("PRIVATE"))
} else { } else {
components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.NORMAL)) Events.openedLink.record(Events.OpenedLinkExtra("NORMAL"))
} }
addReferrerInformation(intent) addReferrerInformation(intent)
@ -94,7 +94,7 @@ class IntentReceiverActivity : Activity() {
components.intentProcessors.privateIntentProcessor components.intentProcessors.privateIntentProcessor
) )
} else { } else {
components.analytics.metrics.track(Event.OpenedLink(Event.OpenedLink.Mode.NORMAL)) Events.openedLink.record(Events.OpenedLinkExtra("NORMAL"))
listOf( listOf(
components.intentProcessors.customTabIntentProcessor, components.intentProcessors.customTabIntentProcessor,
components.intentProcessors.intentProcessor components.intentProcessors.intentProcessor

@ -257,13 +257,6 @@ sealed class Event {
get() = hashMapOf(Addons.openAddonSettingKeys.addonId to addonId) get() = hashMapOf(Addons.openAddonSettingKeys.addonId to addonId)
} }
data class OpenedLink(val mode: Mode) : Event() {
enum class Mode { NORMAL, PRIVATE }
override val extras: Map<Events.openedLinkKeys, String>?
get() = hashMapOf(Events.openedLinkKeys.mode to mode.name)
}
data class SaveLoginsSettingChanged(val setting: Setting) : Event() { data class SaveLoginsSettingChanged(val setting: Setting) : Event() {
enum class Setting { NEVER_SAVE, ASK_TO_SAVE } enum class Setting { NEVER_SAVE, ASK_TO_SAVE }

@ -17,12 +17,17 @@ import io.mockk.unmockkStatic
import io.mockk.verify import io.mockk.verify
import kotlinx.coroutines.test.runBlockingTest import kotlinx.coroutines.test.runBlockingTest
import mozilla.components.feature.intent.processing.IntentProcessor import mozilla.components.feature.intent.processing.IntentProcessor
import mozilla.components.support.test.robolectric.testContext
import mozilla.telemetry.glean.testing.GleanTestRule
import org.junit.After import org.junit.After
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue import org.junit.Assert.assertTrue
import org.junit.Before import org.junit.Before
import org.junit.Rule
import org.junit.Test import org.junit.Test
import org.junit.runner.RunWith import org.junit.runner.RunWith
import org.mozilla.fenix.GleanMetrics.Events
import org.mozilla.fenix.components.IntentProcessors import org.mozilla.fenix.components.IntentProcessors
import org.mozilla.fenix.customtabs.ExternalAppBrowserActivity import org.mozilla.fenix.customtabs.ExternalAppBrowserActivity
import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.components
@ -40,6 +45,9 @@ class IntentReceiverActivityTest {
private lateinit var settings: Settings private lateinit var settings: Settings
private lateinit var intentProcessors: IntentProcessors private lateinit var intentProcessors: IntentProcessors
@get:Rule
val gleanTestRule = GleanTestRule(testContext)
@Before @Before
fun setup() { fun setup() {
mockkStatic("org.mozilla.fenix.ext.ContextKt") mockkStatic("org.mozilla.fenix.ext.ContextKt")
@ -67,6 +75,7 @@ class IntentReceiverActivityTest {
fun `process intent with flag launched from history`() = runBlockingTest { fun `process intent with flag launched from history`() = runBlockingTest {
val intent = Intent() val intent = Intent()
intent.flags = FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY intent.flags = FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY
assertFalse(Events.openedLink.testHasValue())
val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get() val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get()
attachMocks(activity) attachMocks(activity)
@ -75,6 +84,7 @@ class IntentReceiverActivityTest {
val shadow = shadowOf(activity) val shadow = shadowOf(activity)
val actualIntent = shadow.peekNextStartedActivity() val actualIntent = shadow.peekNextStartedActivity()
assertTrue(Events.openedLink.testHasValue())
assertEquals(HomeActivity::class.java.name, actualIntent.component?.className) assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
assertEquals(true, actualIntent.flags == FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY) assertEquals(true, actualIntent.flags == FLAG_ACTIVITY_LAUNCHED_FROM_HISTORY)
} }
@ -84,6 +94,7 @@ class IntentReceiverActivityTest {
runBlockingTest { runBlockingTest {
val uri = Uri.parse(BuildConfig.DEEP_LINK_SCHEME + "://settings_wallpapers") val uri = Uri.parse(BuildConfig.DEEP_LINK_SCHEME + "://settings_wallpapers")
val intent = Intent("", uri) val intent = Intent("", uri)
assertFalse(Events.openedLink.testHasValue())
coEvery { intentProcessors.intentProcessor.process(any()) } returns false coEvery { intentProcessors.intentProcessor.process(any()) } returns false
coEvery { intentProcessors.externalDeepLinkIntentProcessor.process(any()) } returns true coEvery { intentProcessors.externalDeepLinkIntentProcessor.process(any()) } returns true
@ -96,6 +107,7 @@ class IntentReceiverActivityTest {
val shadow = shadowOf(activity) val shadow = shadowOf(activity)
val actualIntent = shadow.peekNextStartedActivity() val actualIntent = shadow.peekNextStartedActivity()
assertTrue(Events.openedLink.testHasValue())
assertEquals(HomeActivity::class.java.name, actualIntent.component?.className) assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
} }
@ -103,6 +115,7 @@ class IntentReceiverActivityTest {
fun `process intent with action OPEN_PRIVATE_TAB`() = runBlockingTest { fun `process intent with action OPEN_PRIVATE_TAB`() = runBlockingTest {
val intent = Intent() val intent = Intent()
intent.action = NewTabShortcutIntentProcessor.ACTION_OPEN_PRIVATE_TAB intent.action = NewTabShortcutIntentProcessor.ACTION_OPEN_PRIVATE_TAB
assertFalse(Events.openedLink.testHasValue())
coEvery { intentProcessors.intentProcessor.process(intent) } returns false coEvery { intentProcessors.intentProcessor.process(intent) } returns false
coEvery { intentProcessors.customTabIntentProcessor.process(intent) } returns false coEvery { intentProcessors.customTabIntentProcessor.process(intent) } returns false
@ -113,6 +126,7 @@ class IntentReceiverActivityTest {
val shadow = shadowOf(activity) val shadow = shadowOf(activity)
val actualIntent = shadow.peekNextStartedActivity() val actualIntent = shadow.peekNextStartedActivity()
assertTrue(Events.openedLink.testHasValue())
assertEquals(HomeActivity::class.java.name, actualIntent.component?.className) assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
assertEquals(true, actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false)) assertEquals(true, actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false))
assertEquals(false, actualIntent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, true)) assertEquals(false, actualIntent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, true))
@ -120,6 +134,7 @@ class IntentReceiverActivityTest {
@Test @Test
fun `process intent with action OPEN_TAB`() = runBlockingTest { fun `process intent with action OPEN_TAB`() = runBlockingTest {
assertFalse(Events.openedLink.testHasValue())
val intent = Intent() val intent = Intent()
intent.action = NewTabShortcutIntentProcessor.ACTION_OPEN_TAB intent.action = NewTabShortcutIntentProcessor.ACTION_OPEN_TAB
@ -132,10 +147,12 @@ class IntentReceiverActivityTest {
assertEquals(HomeActivity::class.java.name, actualIntent.component?.className) assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
assertEquals(false, actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false)) assertEquals(false, actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false))
assertTrue(Events.openedLink.testHasValue())
} }
@Test @Test
fun `process intent starts Activity`() = runBlockingTest { fun `process intent starts Activity`() = runBlockingTest {
assertFalse(Events.openedLink.testHasValue())
val intent = Intent() val intent = Intent()
val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get() val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get()
attachMocks(activity) attachMocks(activity)
@ -146,10 +163,13 @@ class IntentReceiverActivityTest {
assertEquals(HomeActivity::class.java.name, actualIntent.component?.className) assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
assertEquals(true, actualIntent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, true)) assertEquals(true, actualIntent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, true))
assertTrue(Events.openedLink.testHasValue())
} }
@Test @Test
fun `process intent with launchLinksInPrivateTab set to true`() = runBlockingTest { fun `process intent with launchLinksInPrivateTab set to true`() = runBlockingTest {
assertFalse(Events.openedLink.testHasValue())
every { settings.openLinksInAPrivateTab } returns true every { settings.openLinksInAPrivateTab } returns true
coEvery { intentProcessors.intentProcessor.process(any()) } returns false coEvery { intentProcessors.intentProcessor.process(any()) } returns false
@ -168,10 +188,12 @@ class IntentReceiverActivityTest {
verify { intentProcessors.privateIntentProcessor.process(intent) } verify { intentProcessors.privateIntentProcessor.process(intent) }
assertEquals(HomeActivity::class.java.name, actualIntent.component?.className) assertEquals(HomeActivity::class.java.name, actualIntent.component?.className)
assertTrue(actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false)) assertTrue(actualIntent.getBooleanExtra(HomeActivity.PRIVATE_BROWSING_MODE, false))
assertTrue(Events.openedLink.testHasValue())
} }
@Test @Test
fun `process intent with launchLinksInPrivateTab set to false`() = runBlockingTest { fun `process intent with launchLinksInPrivateTab set to false`() = runBlockingTest {
assertFalse(Events.openedLink.testHasValue())
val intent = Intent() val intent = Intent()
val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get() val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get()
@ -180,13 +202,16 @@ class IntentReceiverActivityTest {
coVerify(exactly = 0) { intentProcessors.privateIntentProcessor.process(intent) } coVerify(exactly = 0) { intentProcessors.privateIntentProcessor.process(intent) }
coVerify { intentProcessors.intentProcessor.process(intent) } coVerify { intentProcessors.intentProcessor.process(intent) }
assertTrue(Events.openedLink.testHasValue())
} }
@Test @Test
fun `process custom tab intent`() = runBlockingTest { fun `process custom tab intent`() = runBlockingTest {
assertFalse(Events.openedLink.testHasValue())
val intent = Intent() val intent = Intent()
coEvery { intentProcessors.intentProcessor.process(intent) } returns false coEvery { intentProcessors.intentProcessor.process(intent) } returns false
coEvery { intentProcessors.customTabIntentProcessor.process(intent) } returns true coEvery { intentProcessors.customTabIntentProcessor.process(intent) } returns true
assertFalse(Events.openedLink.testHasValue())
val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get() val activity = Robolectric.buildActivity(IntentReceiverActivity::class.java, intent).get()
attachMocks(activity) attachMocks(activity)
@ -197,10 +222,12 @@ class IntentReceiverActivityTest {
assertEquals(ExternalAppBrowserActivity::class.java.name, intent.component!!.className) assertEquals(ExternalAppBrowserActivity::class.java.name, intent.component!!.className)
assertTrue(intent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, false)) assertTrue(intent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, false))
assertTrue(Events.openedLink.testHasValue())
} }
@Test @Test
fun `process private custom tab intent`() = runBlockingTest { fun `process private custom tab intent`() = runBlockingTest {
assertFalse(Events.openedLink.testHasValue())
every { settings.openLinksInAPrivateTab } returns true every { settings.openLinksInAPrivateTab } returns true
val intent = Intent() val intent = Intent()
@ -216,6 +243,7 @@ class IntentReceiverActivityTest {
assertEquals(ExternalAppBrowserActivity::class.java.name, intent.component!!.className) assertEquals(ExternalAppBrowserActivity::class.java.name, intent.component!!.className)
assertTrue(intent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, false)) assertTrue(intent.getBooleanExtra(HomeActivity.OPEN_TO_BROWSER, false))
assertTrue(Events.openedLink.testHasValue())
} }
private fun attachMocks(activity: Activity) { private fun attachMocks(activity: Activity) {

Loading…
Cancel
Save