Revert "For #24224: Send telemetry events related to the messaging framework"

This reverts commit bfeceb56

app/metrics.yaml

@@ -7330,96 +7330,6 @@ search_terms:
       - android-probes@mozilla.com
     expires: 108
 
-messaging:
-  message_shown:
-    type: event
-    description: |
-      A message was shown to the user.
-    extra_keys:
-      message_key:
-        description: The id of the message
-        type: string
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    notification_emails:
-      - android-probes@mozilla.com
-    data_sensitivity:
-      - interaction
-    expires: 114
-  message_dismissed:
-    type: event
-    description: |
-      A message was dismissed by the user.
-    extra_keys:
-      message_key:
-        description: The id of the message
-        type: string
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    notification_emails:
-      - android-probes@mozilla.com
-    data_sensitivity:
-      - interaction
-    expires: 114
-  message_clicked:
-    type: event
-    description: |
-      A message was clicked by the user.
-    extra_keys:
-      message_key:
-        description: The id of the message
-        type: string
-      action_uuid:
-        description: The uuid of the action
-        type: string
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    notification_emails:
-      - android-probes@mozilla.com
-    data_sensitivity:
-      - interaction
-    expires: 114
-  message_expired:
-    type: event
-    description: |
-      A message maxDisplayCount has been surpassed.
-    extra_keys:
-      message_key:
-        description: The id of the message
-        type: string
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    notification_emails:
-      - android-probes@mozilla.com
-    data_sensitivity:
-      - interaction
-    expires: 114
-  malformed:
-    type: event
-    description: |
-      A message was malformed.
-    extra_keys:
-      message_key:
-        description: The id of the message
-        type: string
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/issues/24224
-    notification_emails:
-      - android-probes@mozilla.com
-    data_sensitivity:
-      - interaction
-    expires: 114
-
 wallpapers:
   wallpaper_settings_opened:
     type: event

app/src/main/java/org/mozilla/fenix/components/Analytics.kt

@@ -21,7 +21,6 @@ import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.ReleaseChannel
 import org.mozilla.fenix.components.metrics.AdjustMetricsService
-import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.components.metrics.GleanMetricsService
 import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.experiments.createNimbus
@@ -133,9 +132,6 @@ class Analytics(
             context = context,
             metadataStorage = KeyPairMessageMetadataStorage(),
             gleanPlumb = experiments,
-            reportMalformedMessage = {
-                metrics.track(Event.Messaging.MessageMalformed(it))
-            },
             messagingFeature = FxNimbus.features.messaging,
         )
     }

app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt

@@ -464,15 +464,6 @@ sealed class Event {
     data class WallpaperSwitched(val wallpaper: org.mozilla.fenix.wallpapers.Wallpaper) : Event()
     data class ChangeWallpaperWithLogoToggled(val checked: Boolean) : Event()
 
-    sealed class Messaging(open val messageId: String) : Event() {
-        data class MessageShown(override val messageId: String) : Messaging(messageId)
-        data class MessageDismissed(override val messageId: String) : Messaging(messageId)
-        data class MessageClicked(override val messageId: String, val uuid: String?) :
-            Messaging(messageId)
-        data class MessageMalformed(override val messageId: String) : Messaging(messageId)
-        data class MessageExpired(override val messageId: String) : Messaging(messageId)
-    }
-
     internal open val extras: Map<*, String>?
         get() = null
 }

app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt

@@ -43,7 +43,6 @@ import org.mozilla.fenix.GleanMetrics.Tabs
 import org.mozilla.fenix.GleanMetrics.TopSites
 import org.mozilla.fenix.GleanMetrics.VoiceSearch
 import org.mozilla.fenix.GleanMetrics.Wallpapers
-import org.mozilla.fenix.GleanMetrics.Messaging
 import org.mozilla.fenix.ext.components
 
 private class EventWrapper<T : Enum<T>>(
@@ -599,52 +598,6 @@ private val Event.wrapper: EventWrapper<*>?
         is Event.JumpBackInGroupTapped -> EventWrapper<NoExtraKeys>(
             { SearchTerms.jumpBackInGroupTapped.record(it) }
         )
-        is Event.Messaging.MessageShown -> EventWrapper<NoExtraKeys>(
-            {
-                Messaging.messageShown.record(
-                    Messaging.MessageShownExtra(
-                        messageKey = this.messageId
-                    )
-                )
-            }
-        )
-        is Event.Messaging.MessageClicked -> EventWrapper<NoExtraKeys>(
-            {
-                Messaging.messageClicked.record(
-                    Messaging.MessageClickedExtra(
-                        messageKey = this.messageId,
-                        actionUuid = this.uuid
-                    )
-                )
-            }
-        )
-        is Event.Messaging.MessageDismissed -> EventWrapper<NoExtraKeys>(
-            {
-                Messaging.messageDismissed.record(
-                    Messaging.MessageDismissedExtra(
-                        messageKey = this.messageId
-                    )
-                )
-            }
-        )
-        is Event.Messaging.MessageMalformed -> EventWrapper<NoExtraKeys>(
-            {
-                Messaging.malformed.record(
-                    Messaging.MalformedExtra(
-                        messageKey = this.messageId
-                    )
-                )
-            }
-        )
-        is Event.Messaging.MessageExpired -> EventWrapper<NoExtraKeys>(
-            {
-                Messaging.messageExpired.record(
-                    Messaging.MessageExpiredExtra(
-                        messageKey = this.messageId
-                    )
-                )
-            }
-        )
         is Event.WallpaperSettingsOpened -> EventWrapper<NoExtraKeys>(
             { Wallpapers.wallpaperSettingsOpened.record() }
         )

app/src/main/java/org/mozilla/fenix/gleanplumb/DefaultMessageController.kt

@@ -14,38 +14,33 @@ import org.mozilla.fenix.components.AppStore
 import org.mozilla.fenix.components.appstate.AppAction.MessagingAction.MessageClicked
 import org.mozilla.fenix.components.appstate.AppAction.MessagingAction.MessageDismissed
 import org.mozilla.fenix.components.appstate.AppAction.MessagingAction.MessageDisplayed
-import org.mozilla.fenix.components.metrics.Event
-import org.mozilla.fenix.components.metrics.MetricController
 
 /**
  * Handles default interactions with the ui of GleanPlumb messages.
  */
 class DefaultMessageController(
     private val appStore: AppStore,
-    private val metrics: MetricController,
     private val messagingStorage: NimbusMessagingStorage,
     private val homeActivity: HomeActivity
 ) : MessageController {
 
     override fun onMessagePressed(message: Message) {
-        val result = messagingStorage.getMessageAction(message)
-        val uuid = result.first
-        val action = result.second
-        metrics.track(Event.Messaging.MessageClicked(message.id, uuid))
+        // Report telemetry event
+        // This will be covered on https://github.com/mozilla-mobile/fenix/issues/24224
+        val action = messagingStorage.getMessageAction(message)
         handleAction(action)
         appStore.dispatch(MessageClicked(message))
     }
 
     override fun onMessageDismissed(message: Message) {
-        metrics.track(Event.Messaging.MessageDismissed(message.id))
+        // Report telemetry event
+        // This will be covered on https://github.com/mozilla-mobile/fenix/issues/24224
         appStore.dispatch(MessageDismissed(message))
     }
 
     override fun onMessageDisplayed(message: Message) {
-        if (message.maxDisplayCount <= message.metadata.displayCount + 1) {
-            metrics.track(Event.Messaging.MessageExpired(message.id))
-        }
-        metrics.track(Event.Messaging.MessageShown(message.id))
+        // Report telemetry event
+        // This will be covered on https://github.com/mozilla-mobile/fenix/issues/24224
         appStore.dispatch(MessageDisplayed(message))
     }
 

app/src/main/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorage.kt

@@ -12,6 +12,7 @@ import org.mozilla.experiments.nimbus.GleanPlumbInterface
 import org.mozilla.experiments.nimbus.GleanPlumbMessageHelper
 import org.mozilla.experiments.nimbus.internal.FeatureHolder
 import org.mozilla.experiments.nimbus.internal.NimbusException
+
 import org.mozilla.fenix.nimbus.Messaging
 import org.mozilla.fenix.nimbus.StyleData
 
@@ -21,7 +22,6 @@ import org.mozilla.fenix.nimbus.StyleData
 class NimbusMessagingStorage(
     private val context: Context,
     private val metadataStorage: MessageMetadataStorage,
-    private val reportMalformedMessage: (String) -> Unit,
     private val gleanPlumb: GleanPlumbInterface,
     private val messagingFeature: FeatureHolder<Messaging>
 ) {
@@ -45,15 +45,14 @@ class NimbusMessagingStorage(
         }
 
         return nimbusMessages.mapNotNull { (key, value) ->
-            val action = sanitizeAction(key, value.action, nimbusActions) ?: return@mapNotNull null
+            val action = sanitizeAction(value.action, nimbusActions) ?: return@mapNotNull null
             Message(
                 id = key,
                 data = value,
                 action = action,
                 style = nimbusStyles[value.style] ?: defaultStyle,
                 metadata = storageMetadata[key] ?: addMetadata(key),
-                triggers = sanitizeTriggers(key, value.trigger, nimbusTriggers)
-                    ?: return@mapNotNull null
+                triggers = sanitizeTriggers(value.trigger, nimbusTriggers) ?: return@mapNotNull null
             )
         }.filter {
             it.data.maxDisplayCount >= it.metadata.displayCount &&
@@ -86,13 +85,13 @@ class NimbusMessagingStorage(
     }
 
     /**
-     * Returns a pair of uuid and valid action for the provided [message].
+     * Returns a valid action for the provided [message].
      */
-    fun getMessageAction(message: Message): Pair<String?, String> {
+    fun getMessageAction(message: Message): String {
         val helper = gleanPlumb.createMessageHelper(customAttributes)
         val uuid = helper.getUuid(message.action)
 
-        return Pair(uuid, helper.stringFormat(message.action, uuid))
+        return helper.stringFormat(message.action, uuid)
     }
 
     /**
@@ -104,7 +103,6 @@ class NimbusMessagingStorage(
 
     @VisibleForTesting
     internal fun sanitizeAction(
-        messageId: String,
         unsafeAction: String,
         nimbusActions: Map<String, String>
     ): String? {
@@ -113,7 +111,6 @@ class NimbusMessagingStorage(
         } else {
             val safeAction = nimbusActions[unsafeAction]
             if (safeAction.isNullOrBlank() || safeAction.isEmpty()) {
-                reportMalformedMessage(messageId)
                 return null
             }
             safeAction
@@ -122,14 +119,12 @@ class NimbusMessagingStorage(
 
     @VisibleForTesting
     internal fun sanitizeTriggers(
-        messageId: String,
         unsafeTriggers: List<String>,
         nimbusTriggers: Map<String, String>
     ): List<String>? {
         return unsafeTriggers.map {
             val safeTrigger = nimbusTriggers[it]
             if (safeTrigger.isNullOrBlank() || safeTrigger.isEmpty()) {
-                reportMalformedMessage(messageId)
                 return null
             }
             safeTrigger
@@ -160,7 +155,8 @@ class NimbusMessagingStorage(
             try {
                 helper.evalJexl(condition)
             } catch (e: NimbusException.EvaluationException) {
-                reportMalformedMessage(message.id)
+                // Report to glean as malformed message
+                // Will be addressed on https://github.com/mozilla-mobile/fenix/issues/24224
                 logger.info("Unable to evaluate $condition")
                 false
             }

app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt

@@ -314,8 +314,7 @@ class HomeFragment : Fragment() {
                 messageController = DefaultMessageController(
                     appStore = components.appStore,
                     messagingStorage = components.analytics.messagingStorage,
-                    homeActivity = activity,
-                    metrics = components.analytics.metrics
+                    homeActivity = activity
                 ),
                 store = store,
                 tabCollectionStorage = components.core.tabCollectionStorage,

app/src/test/java/org/mozilla/fenix/gleanplumb/DefaultMessageControllerTest.kt

@@ -20,8 +20,6 @@ import org.mozilla.fenix.components.AppStore
 import org.mozilla.fenix.components.appstate.AppAction
 import org.mozilla.fenix.components.appstate.AppAction.MessagingAction.MessageClicked
 import org.mozilla.fenix.components.appstate.AppAction.MessagingAction.MessageDisplayed
-import org.mozilla.fenix.components.metrics.Event
-import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.nimbus.MessageData
 
@@ -31,15 +29,12 @@ class DefaultMessageControllerTest {
     private val activity: HomeActivity = mockk(relaxed = true)
     private val storageNimbus: NimbusMessagingStorage = mockk(relaxed = true)
     private lateinit var controller: DefaultMessageController
-    private lateinit var metrics: MetricController
     private val store: AppStore = mockk(relaxed = true)
 
     @Before
     fun setup() {
-        metrics = mockk(relaxed = true)
         controller = DefaultMessageController(
             messagingStorage = storageNimbus,
-            metrics = metrics,
             appStore = store,
             homeActivity = activity
         )
@@ -48,13 +43,12 @@ class DefaultMessageControllerTest {
     @Test
     fun `WHEN calling onMessagePressed THEN update the store and handle the action`() {
         val customController = spyk(controller)
-        val message = mockMessage()
         every { customController.handleAction(any()) } returns mockk()
-        every { storageNimbus.getMessageAction(message) } returns Pair("uuid", message.id)
+
+        val message = mockMessage()
 
         customController.onMessagePressed(message)
 
-        verify { metrics.track(Event.Messaging.MessageClicked(message.id, "uuid")) }
         verify { customController.handleAction(any()) }
         verify { store.dispatch(MessageClicked(message)) }
     }
@@ -87,26 +81,22 @@ class DefaultMessageControllerTest {
 
         controller.onMessageDismissed(message)
 
-        verify { metrics.track(Event.Messaging.MessageDismissed(message.id)) }
         verify { store.dispatch(AppAction.MessagingAction.MessageDismissed(message)) }
     }
 
     @Test
     fun `WHEN calling onMessageDisplayed THEN report to the messageManager`() {
-        val data = MessageData(_context = testContext)
-        val message = mockMessage(data)
+        val message = mockMessage()
 
         controller.onMessageDisplayed(message)
 
-        verify { metrics.track(Event.Messaging.MessageExpired(message.id)) }
-        verify { metrics.track(Event.Messaging.MessageShown(message.id)) }
         verify { store.dispatch(MessageDisplayed(message)) }
     }
 
-    private fun mockMessage(data: MessageData = MessageData(_context = testContext)) = Message(
+    private fun mockMessage() = Message(
         id = "id",
-        data = data,
-        style = mockk(relaxed = true),
+        data = MessageData(_context = testContext),
+        style = mockk(),
         action = "action",
         triggers = emptyList(),
         metadata = Message.Metadata(

app/src/test/java/org/mozilla/fenix/gleanplumb/MessagingFeatureTest.kt

@@ -10,6 +10,8 @@ import io.mockk.spyk
 import io.mockk.unmockkObject
 import io.mockk.verify
 import kotlinx.coroutines.ExperimentalCoroutinesApi
+import mozilla.components.support.test.libstate.ext.waitUntilIdle
+import mozilla.components.support.test.mock
 import mozilla.components.support.test.rule.MainCoroutineRule
 import org.junit.Rule
 import org.junit.Test
@@ -17,6 +19,7 @@ import org.mozilla.fenix.Config
 import org.mozilla.fenix.FeatureFlags
 import org.mozilla.fenix.components.AppStore
 import org.mozilla.fenix.components.appstate.AppAction.MessagingAction
+import org.mozilla.fenix.components.appstate.AppAction.MessagingAction.UpdateMessageToShow
 
 class MessagingFeatureTest {
     @OptIn(ExperimentalCoroutinesApi::class)
@@ -33,6 +36,9 @@ class MessagingFeatureTest {
 
         binding.start()
 
+        store.dispatch(UpdateMessageToShow(mock()))
+        store.waitUntilIdle()
+
         verify { store.dispatch(MessagingAction.Evaluate) }
 
         unmockkObject(Config)

app/src/test/java/org/mozilla/fenix/gleanplumb/NimbusMessagingStorageTest.kt

@@ -37,16 +37,12 @@ class NimbusMessagingStorageTest {
     private lateinit var gleanPlumb: GleanPlumbInterface
     private lateinit var messagingFeature: FeatureHolder<Messaging>
     private lateinit var messaging: Messaging
-    private var malformedWasReported = false
-    private val reportMalformedMessage: (String) -> Unit = {
-        malformedWasReported = true
-    }
 
     @Before
     fun setup() {
         gleanPlumb = mockk(relaxed = true)
         metadataStorage = mockk(relaxed = true)
-        malformedWasReported = false
+
         messagingFeature = createMessagingFeature()
 
         every { metadataStorage.getMetadata() } returns listOf(Message.Metadata(id = "message-1"))
@@ -54,7 +50,6 @@ class NimbusMessagingStorageTest {
         storage = NimbusMessagingStorage(
             testContext,
             metadataStorage,
-            reportMalformedMessage,
             gleanPlumb,
             messagingFeature
         )
@@ -91,7 +86,6 @@ class NimbusMessagingStorageTest {
         val storage = NimbusMessagingStorage(
             testContext,
             metadataStorage,
-            reportMalformedMessage,
             gleanPlumb,
             messagingFeature
         )
@@ -127,7 +121,6 @@ class NimbusMessagingStorageTest {
         val storage = NimbusMessagingStorage(
             testContext,
             metadataStorage,
-            reportMalformedMessage,
             gleanPlumb,
             messagingFeature
         )
@@ -162,7 +155,6 @@ class NimbusMessagingStorageTest {
         val storage = NimbusMessagingStorage(
             testContext,
             metadataStorage,
-            reportMalformedMessage,
             gleanPlumb,
             messagingFeature
         )
@@ -200,7 +192,6 @@ class NimbusMessagingStorageTest {
         val storage = NimbusMessagingStorage(
             testContext,
             metadataStorage,
-            reportMalformedMessage,
             gleanPlumb,
             messagingFeature
         )
@@ -219,21 +210,19 @@ class NimbusMessagingStorageTest {
         assertEquals("message-1", firstMessage.id)
         assertEquals("message-1", firstMessage.metadata.id)
         assertTrue(messages.size == 1)
-        assertTrue(malformedWasReported)
     }
 
     @Test
     fun `GIVEN a malformed action WHEN calling sanitizeAction THEN return null`() {
         val actionsMap = mapOf("action-1" to "action-1-url")
 
-        val notFoundAction = storage.sanitizeAction("messageId", "no-found-action", actionsMap)
-        val emptyAction = storage.sanitizeAction("messageId", "", actionsMap)
-        val blankAction = storage.sanitizeAction("messageId", " ", actionsMap)
+        val notFoundAction = storage.sanitizeAction("no-found-action", actionsMap)
+        val emptyAction = storage.sanitizeAction("", actionsMap)
+        val blankAction = storage.sanitizeAction(" ", actionsMap)
 
         assertNull(notFoundAction)
         assertNull(emptyAction)
         assertNull(blankAction)
-        assertTrue(malformedWasReported)
     }
 
     @Test
@@ -248,7 +237,7 @@ class NimbusMessagingStorageTest {
     fun `GIVEN a valid action WHEN calling sanitizeAction THEN return the action`() {
         val actionsMap = mapOf("action-1" to "action-1-url")
 
-        val validAction = storage.sanitizeAction("messageId", "action-1", actionsMap)
+        val validAction = storage.sanitizeAction("action-1", actionsMap)
 
         assertEquals("action-1-url", validAction)
     }
@@ -257,22 +246,20 @@ class NimbusMessagingStorageTest {
     fun `GIVEN a trigger action WHEN calling sanitizeTriggers THEN return null`() {
         val triggersMap = mapOf("trigger-1" to "trigger-1-expression")
 
-        val notFoundTrigger =
-            storage.sanitizeTriggers("messageId", listOf("no-found-trigger"), triggersMap)
-        val emptyTrigger = storage.sanitizeTriggers("messageId", listOf(""), triggersMap)
-        val blankTrigger = storage.sanitizeTriggers("messageId", listOf(" "), triggersMap)
+        val notFoundTrigger = storage.sanitizeTriggers(listOf("no-found-trigger"), triggersMap)
+        val emptyTrigger = storage.sanitizeTriggers(listOf(""), triggersMap)
+        val blankTrigger = storage.sanitizeTriggers(listOf(" "), triggersMap)
 
         assertNull(notFoundTrigger)
         assertNull(emptyTrigger)
         assertNull(blankTrigger)
-        assertTrue(malformedWasReported)
     }
 
     @Test
     fun `GIVEN a valid trigger WHEN calling sanitizeAction THEN return the trigger`() {
         val triggersMap = mapOf("trigger-1" to "trigger-1-expression")
 
-        val validTrigger = storage.sanitizeTriggers("messageId", listOf("trigger-1"), triggersMap)
+        val validTrigger = storage.sanitizeTriggers(listOf("trigger-1"), triggersMap)
 
         assertEquals(listOf("trigger-1-expression"), validTrigger)
     }