[fenix] Revert "For https://github.com/mozilla-mobile/fenix/issues/18711 - Refactor credit card telemetry under the Metrics ping (https://github.com/mozilla-mobile/fenix/pull/19733)"

This reverts commit f6dd0834df4e2a15de684292a8422279487c5a23.
3 years ago · cffad4f424
parent 60da5771b7
commit cffad4f424
13 changed files with 98 additions and 185 deletions
--- a/app/metrics.yaml
+++ b/app/metrics.yaml
@ -1020,62 +1020,8 @@ metrics:
      - technical
      - interaction
    notification_emails:
-      - android-probes@mozilla.com
-    expires: "2022-02-01"
-  credit_cards_saved_count:
-    type: counter
-    lifetime: application
-    description: |
-      A counter that indicates the number of credit cards that have been
-      saved manually by the user.
-    send_in_pings:
-      - metrics
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/18711
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/19548#issuecomment-848811030
-      - https://github.com/mozilla-mobile/fenix/pull/19924#issuecomment-861423789
-    data_sensitivity:
-      - interaction
-    notification_emails:
-      - android-probes@mozilla.com
-    expires: "2021-10-01"
-  credit_cards_deleted_count:
-    type: counter
-    lifetime: application
-    description: |
-      A counter that indicates the number of credit cards that have been
-      deleted by the user.
-    send_in_pings:
-      - metrics
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/18711
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/19548#issuecomment-848811030
-      - https://github.com/mozilla-mobile/fenix/pull/19924#issuecomment-861423789
-    data_sensitivity:
-      - interaction
-    notification_emails:
-      - android-probes@mozilla.com
-    expires: "2021-10-01"
-  credit_cards_autofill_count:
-    type: counter
-    lifetime: application
-    description: |
-      A counter that indicates the number of times the user has autofilled
-      a credit card.
-    send_in_pings:
-      - metrics
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/18711
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/19548#issuecomment-848811030
-      - https://github.com/mozilla-mobile/fenix/pull/19924#issuecomment-861423789
-    data_sensitivity:
-      - interaction
-    notification_emails:
-      - android-probes@mozilla.com
-    expires: "2021-10-01"
+      - fenix-core@mozilla.com
+    expires: "2021-08-01"
  mozilla_products:
    type: string_list
    lifetime: application
@ -6073,3 +6019,52 @@ recent_tabs:
    notification_emails:
      - android-probes@mozilla.com
    expires: "2022-06-23"
+credit_cards:
+  manual_save:
+    type: counter
+    description: |
+      A counter of the number of credit cards that have been saved
+      manually by the user.
+    send_in_pings:
+      - metrics
+    bugs:
+      - https://github.com/mozilla-mobile/fenix/issues/18711
+    data_reviews:
+      - https://github.com/mozilla-mobile/fenix/pull/19548#issuecomment-848811030
+    data_sensitivity:
+      - interaction
+    notification_emails:
+      - fenix-core@mozilla.com
+    expires: "2021-10-01"
+  delete_card:
+    type: counter
+    description: |
+      A counter of the number of credit cards that have been deleted by
+      the user.
+    send_in_pings:
+      - metrics
+    bugs:
+      - https://github.com/mozilla-mobile/fenix/issues/18711
+    data_reviews:
+      - https://github.com/mozilla-mobile/fenix/pull/19548#issuecomment-848811030
+    data_sensitivity:
+      - interaction
+    notification_emails:
+      - fenix-core@mozilla.com
+    expires: "2021-10-01"
+  autofill_card:
+    type: counter
+    description: |
+      A counter of the number of times the user has autofilled
+      a credit card.
+    send_in_pings:
+      - metrics
+    bugs:
+      - https://github.com/mozilla-mobile/fenix/issues/18711
+    data_reviews:
+      - https://github.com/mozilla-mobile/fenix/pull/19548#issuecomment-848811030
+    data_sensitivity:
+      - interaction
+    notification_emails:
+      - fenix-core@mozilla.com
+    expires: "2021-10-01"
--- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt
+++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt
@ -573,18 +573,6 @@ open class FenixApplication : LocaleAwareApplication(), Provider {
                topSitesCount.add(topSitesSize)
            }

-            if (settings.creditCardsSavedCount > 0) {
-                creditCardsSavedCount.add(settings.creditCardsSavedCount)
-            }
-
-            if (settings.creditCardsDeletedCount > 0) {
-                creditCardsDeletedCount.add(settings.creditCardsDeletedCount)
-            }
-
-            if (settings.creditCardsAutofilledCount > 0) {
-                creditCardsAutofillCount.add(settings.creditCardsAutofilledCount)
-            }
-
            val installedAddonSize = settings.installedAddonsCount
            Addons.hasInstalledAddons.set(installedAddonSize > 0)
            if (installedAddonSize > 0) {
--- a/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/metrics/Event.kt
@ -257,6 +257,11 @@ sealed class Event {
    object AndroidAutofillRequestWithLogins : Event()
    object AndroidAutofillRequestWithoutLogins : Event()

+    // Credit cards
+    object CreditCardManualSave : Event()
+    object CreditCardDelete : Event()
+    object CreditCardAutofill : Event()
+
    // Interaction events with extras

    data class TopSiteSwipeCarousel(val page: Int) : Event() {
--- a/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/metrics/GleanMetricsService.kt
@ -23,6 +23,7 @@ import org.mozilla.fenix.GleanMetrics.ContextMenu
 import org.mozilla.fenix.GleanMetrics.ContextualHintTrackingProtection
 import org.mozilla.fenix.GleanMetrics.ContextualMenu
 import org.mozilla.fenix.GleanMetrics.CrashReporter
+import org.mozilla.fenix.GleanMetrics.CreditCards
 import org.mozilla.fenix.GleanMetrics.CustomTab
 import org.mozilla.fenix.GleanMetrics.DownloadNotification
 import org.mozilla.fenix.GleanMetrics.DownloadsMisc
@ -875,6 +876,16 @@ private val Event.wrapper: EventWrapper<*>?
        )
        is Event.AndroidAutofillConfirmationSuccessful -> EventWrapper<NoExtraKeys>(
            { AndroidAutofill.confirmSuccessful.record(it) }
+
+        // Credit cards
+        is Event.CreditCardManualSave -> EventWrapper<NoExtraKeys>(
+            { CreditCards.manualSave.add(1) }
+        )
+        is Event.CreditCardDelete -> EventWrapper<NoExtraKeys>(
+            { CreditCards.deleteCard.add(1) }
+        )
+        is Event.CreditCardAutofill -> EventWrapper<NoExtraKeys>(
+            { CreditCards.autofillCard.add(1) }
        )

        // Don't record other events in Glean:
--- a/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt
+++ b/app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt
@ -165,10 +165,8 @@ internal class ReleaseMetricController(
        Component.FEATURE_PROMPTS to LoginDialogFacts.Items.CANCEL -> Event.LoginDialogPromptCancelled
        Component.FEATURE_PROMPTS to LoginDialogFacts.Items.NEVER_SAVE -> Event.LoginDialogPromptNeverSave
        Component.FEATURE_PROMPTS to LoginDialogFacts.Items.SAVE -> Event.LoginDialogPromptSave
-        Component.FEATURE_PROMPTS to CreditCardAutofillDialogFacts.Items.AUTOFILL_CREDIT_CARD_SUCCESS -> {
-            settings.creditCardsAutofilledCount += 1
-            null
-        }
+        Component.FEATURE_PROMPTS to CreditCardAutofillDialogFacts.Items.AUTOFILL_CREDIT_CARD_SUCCESS ->
+            Event.CreditCardAutofill

        Component.FEATURE_FINDINPAGE to FindInPageFacts.Items.CLOSE -> Event.FindInPageClosed
        Component.FEATURE_FINDINPAGE to FindInPageFacts.Items.INPUT -> Event.FindInPageSearchCommitted
--- a/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardEditorFragment.kt
+++ b/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardEditorFragment.kt
@ -17,7 +17,6 @@ import org.mozilla.fenix.SecureFragment
 import org.mozilla.fenix.databinding.FragmentCreditCardEditorBinding
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.redirectToReAuth
-import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.ext.showToolbar
 import org.mozilla.fenix.settings.creditcards.controller.DefaultCreditCardEditorController
 import org.mozilla.fenix.settings.creditcards.interactor.CreditCardEditorInteractor
@ -57,10 +56,10 @@ class CreditCardEditorFragment : SecureFragment(R.layout.fragment_credit_card_ed
        val storage = requireContext().components.core.autofillStorage
        interactor = DefaultCreditCardEditorInteractor(
            controller = DefaultCreditCardEditorController(
+                context = requireContext(),
                storage = storage,
                lifecycleScope = lifecycleScope,
-                navController = findNavController(),
-                settings = requireContext().settings()
+                navController = findNavController()
            )
        )

--- a/app/src/main/java/org/mozilla/fenix/settings/creditcards/controller/CreditCardEditorController.kt
+++ b/app/src/main/java/org/mozilla/fenix/settings/creditcards/controller/CreditCardEditorController.kt
@ -4,6 +4,7 @@

 package org.mozilla.fenix.settings.creditcards.controller

+import android.content.Context
 import androidx.navigation.NavController
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
@ -12,9 +13,10 @@ import kotlinx.coroutines.launch
 import mozilla.components.concept.storage.NewCreditCardFields
 import mozilla.components.concept.storage.UpdatableCreditCardFields
 import mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage
+import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.settings.creditcards.CreditCardEditorFragment
 import org.mozilla.fenix.settings.creditcards.interactor.CreditCardEditorInteractor
-import org.mozilla.fenix.utils.Settings
+import org.mozilla.fenix.ext.components

 /**
 * [CreditCardEditorFragment] controller. An interface that handles the view manipulation of the
@ -50,26 +52,26 @@ interface CreditCardEditorController {
 * credit cards.
 * @param lifecycleScope [CoroutineScope] scope to launch coroutines.
 * @param navController [NavController] used for navigation.
- * @param settings [Settings] application settings.
 * @param ioDispatcher [CoroutineDispatcher] used for executing async tasks. Defaults to [Dispatchers.IO].
 */
 class DefaultCreditCardEditorController(
+    context: Context,
    private val storage: AutofillCreditCardsAddressesStorage,
    private val lifecycleScope: CoroutineScope,
    private val navController: NavController,
-    private val settings: Settings,
    private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
 ) : CreditCardEditorController {

+    private val metrics = context.components.analytics.metrics
+
    override fun handleCancelButtonClicked() {
        navController.popBackStack()
    }

    override fun handleDeleteCreditCard(guid: String) {
-        settings.creditCardsDeletedCount += 1
-
        lifecycleScope.launch(ioDispatcher) {
            storage.deleteCreditCard(guid)
+            metrics.track(Event.CreditCardDelete)

            lifecycleScope.launch(Dispatchers.Main) {
                navController.popBackStack()
@ -78,10 +80,9 @@ class DefaultCreditCardEditorController(
    }

    override fun handleSaveCreditCard(creditCardFields: NewCreditCardFields) {
-        settings.creditCardsSavedCount += 1
-
        lifecycleScope.launch(ioDispatcher) {
            storage.addCreditCard(creditCardFields)
+            metrics.track(Event.CreditCardManualSave)

            lifecycleScope.launch(Dispatchers.Main) {
                navController.popBackStack()
--- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt
+++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt
@ -1038,7 +1038,7 @@ class Settings(private val appContext: Context) : PreferencesHolder {
    )

    /**
-     * Stores the number of installed add-ons for telemetry purposes
+     * Storing number of installed add-ons for telemetry purposes
     */
    var installedAddonsCount by intPreference(
        appContext.getPreferenceKey(R.string.pref_key_installed_addons_count),
@ -1046,7 +1046,7 @@ class Settings(private val appContext: Context) : PreferencesHolder {
    )

    /**
-     * Stores the list of installed add-ons for telemetry purposes
+     * Storing the list of installed add-ons for telemetry purposes
     */
    var installedAddonsList by stringPreference(
        appContext.getPreferenceKey(R.string.pref_key_installed_addons_list),
@ -1054,7 +1054,7 @@ class Settings(private val appContext: Context) : PreferencesHolder {
    )

    /**
-     * Stores the number of enabled add-ons for telemetry purposes
+     * Storing number of enabled add-ons for telemetry purposes
     */
    var enabledAddonsCount by intPreference(
        appContext.getPreferenceKey(R.string.pref_key_enabled_addons_count),
@ -1062,37 +1062,13 @@ class Settings(private val appContext: Context) : PreferencesHolder {
    )

    /**
-     * Stores the list of enabled add-ons for telemetry purposes
+     * Storing the list of enabled add-ons for telemetry purposes
     */
    var enabledAddonsList by stringPreference(
        appContext.getPreferenceKey(R.string.pref_key_enabled_addons_list),
        default = ""
    )

-    /**
-     * Stores the number of credit cards that have been saved manually by the user.
-     */
-    var creditCardsSavedCount by intPreference(
-        appContext.getPreferenceKey(R.string.pref_key_credit_cards_saved_count),
-        0
-    )
-
-    /**
-     * Stores the number of credit cards that have been deleted by the user.
-     */
-    var creditCardsDeletedCount by intPreference(
-        appContext.getPreferenceKey(R.string.pref_key_credit_cards_deleted_count),
-        0
-    )
-
-    /**
-     * Stores the number of times that user has autofilled a credit card.
-     */
-    var creditCardsAutofilledCount by intPreference(
-        appContext.getPreferenceKey(R.string.pref_key_credit_cards_autofilled_count),
-        0
-    )
-
    private var savedLoginsSortingStrategyString by stringPreference(
        appContext.getPreferenceKey(R.string.pref_key_saved_logins_sorting_strategy),
        default = SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort.strategyString
--- a/app/src/main/res/values/preference_keys.xml
+++ b/app/src/main/res/values/preference_keys.xml
@ -185,12 +185,6 @@
    <string name="pref_key_credit_cards_sync_cards_across_devices" translatable="false">pref_key_credit_cards_sync_cards_across_devices</string>
    <!-- Key for the "Manage saved cards" preference in the "Credit cards" fragment -->
    <string name="pref_key_credit_cards_manage_cards" translatable="false">pref_key_credit_cards_manage_cards</string>
-    <!-- Key for the number of credit cards that have been saved manually by the user metric value -->
-    <string name="pref_key_credit_cards_saved_count" translatable="false">pref_key_credit_cards_saved_count</string>
-    <!-- Key for the number of credit cards that have been deleted by the user metric value -->
-    <string name="pref_key_credit_cards_deleted_count" translatable="false">pref_key_credit_cards_deleted_count</string>
-    <!-- Key for the number of times the user has autofilled a credit card metric value -->
-    <string name="pref_key_credit_cards_autofilled_count" translatable="false">pref_key_credit_cards_autofilled_count</string>

    <!-- Privacy Settings -->
    <string name="pref_key_open_links_in_a_private_tab" translatable="false">pref_key_open_links_in_a_private_tab</string>
--- a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt
@ -128,9 +128,6 @@ class FenixApplicationTest {
        every { settings.touchExplorationIsEnabled } returns true
        every { settings.shouldUseLightTheme } returns true
        every { settings.signedInFxaAccount } returns true
-        every { settings.creditCardsSavedCount } returns 1
-        every { settings.creditCardsDeletedCount } returns 2
-        every { settings.creditCardsAutofilledCount } returns 3

        application.setStartupMetrics(browserStore, settings, browsersCache, mozillaProductDetector)

@ -149,9 +146,6 @@ class FenixApplicationTest {
        assertEquals(true, Metrics.hasTopSites.testGetValue())
        assertEquals(2, Metrics.topSitesCount.testGetValue())
        assertEquals(true, Addons.hasInstalledAddons.testGetValue())
-        assertEquals(1, Metrics.creditCardsSavedCount.testGetValue())
-        assertEquals(2, Metrics.creditCardsDeletedCount.testGetValue())
-        assertEquals(3, Metrics.creditCardsAutofillCount.testGetValue())
        assertEquals(listOf("test1", "test2", "test3"), Addons.installedAddons.testGetValue())
        assertEquals(true, Addons.hasEnabledAddons.testGetValue())
        assertEquals(listOf("test1", "test2"), Addons.enabledAddons.testGetValue())
--- a/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/components/metrics/MetricControllerTest.kt
@ -10,7 +10,6 @@ import io.mockk.impl.annotations.MockK
 import io.mockk.mockk
 import io.mockk.verify
 import io.mockk.verifyAll
-import mozilla.components.feature.prompts.facts.CreditCardAutofillDialogFacts
 import mozilla.components.feature.top.sites.facts.TopSitesFacts
 import mozilla.components.support.base.Component
 import mozilla.components.support.base.facts.Action
@ -365,39 +364,10 @@ class MetricControllerTest {
        assertEquals(settings.installedAddonsList, "")
        assertEquals(settings.enabledAddonsCount, 0)
        assertEquals(settings.enabledAddonsList, "")
-
        controller.factToEvent(fact)
-
        assertEquals(settings.installedAddonsCount, 4)
        assertEquals(settings.installedAddonsList, "test1,test2,test3,test4")
        assertEquals(settings.enabledAddonsCount, 2)
        assertEquals(settings.enabledAddonsList, "test2,test4")
    }
-
-    @Test
-    fun `credit card autofill fact shold set value in SharedPreference`() {
-        val enabled = true
-        val settings = Settings(testContext)
-        val controller = ReleaseMetricController(
-            services = listOf(dataService1),
-            isDataTelemetryEnabled = { enabled },
-            isMarketingDataTelemetryEnabled = { enabled },
-            settings
-        )
-        val fact = Fact(
-            component = Component.FEATURE_PROMPTS,
-            action = Action.INTERACTION,
-            item = CreditCardAutofillDialogFacts.Items.AUTOFILL_CREDIT_CARD_SUCCESS
-        )
-
-        assertEquals(0, settings.creditCardsAutofilledCount)
-
-        controller.factToEvent(fact)
-
-        assertEquals(1, settings.creditCardsAutofilledCount)
-
-        controller.factToEvent(fact)
-
-        assertEquals(2, settings.creditCardsAutofilledCount)
-    }
 }
--- a/app/src/test/java/org/mozilla/fenix/settings/creditcards/DefaultCreditCardEditorControllerTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/settings/creditcards/DefaultCreditCardEditorControllerTest.kt
@ -4,8 +4,10 @@

 package org.mozilla.fenix.settings.creditcards

+import android.content.Context
 import androidx.navigation.NavController
 import io.mockk.coVerify
+import io.mockk.every
 import io.mockk.mockk
 import io.mockk.spyk
 import io.mockk.verify
@ -17,18 +19,18 @@ import mozilla.components.concept.storage.CreditCardNumber
 import mozilla.components.concept.storage.NewCreditCardFields
 import mozilla.components.concept.storage.UpdatableCreditCardFields
 import mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage
-import mozilla.components.support.test.robolectric.testContext
 import mozilla.components.support.test.rule.MainCoroutineRule
 import mozilla.components.support.utils.CreditCardNetworkType
 import org.junit.After
-import org.junit.Assert.assertEquals
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.components.metrics.MetricController
+import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.settings.creditcards.controller.DefaultCreditCardEditorController
-import org.mozilla.fenix.utils.Settings

@ExperimentalCoroutinesApi
@RunWith(FenixRobolectricTestRunner::class)
@ -41,20 +43,25 @@ class DefaultCreditCardEditorControllerTest {
    private val testDispatcher = TestCoroutineDispatcher()

    private lateinit var controller: DefaultCreditCardEditorController
-    private lateinit var settings: Settings
+    private lateinit var context: Context
+    private lateinit var metrics: MetricController

    @get:Rule
    val coroutinesTestRule = MainCoroutineRule(testDispatcher)

    @Before
    fun setup() {
-        settings = Settings(testContext)
+        metrics = mockk()
+        context = mockk()
+        every { metrics.track(any()) } returns Unit
+        every { context.components.analytics.metrics } returns metrics
+
        controller = spyk(
            DefaultCreditCardEditorController(
+                context = context,
                storage = storage,
                lifecycleScope = testCoroutineScope,
                navController = navController,
-                settings = settings,
                ioDispatcher = testDispatcher
            )
        )
@ -77,24 +84,19 @@ class DefaultCreditCardEditorControllerTest {

    @Test
    fun handleDeleteCreditCard() = testCoroutineScope.runBlockingTest {
-        assertEquals(0, settings.creditCardsDeletedCount)
-
        val creditCardId = "id"

        controller.handleDeleteCreditCard(creditCardId)

-        assertEquals(1, settings.creditCardsDeletedCount)
-
        coVerify {
            storage.deleteCreditCard(creditCardId)
            navController.popBackStack()
+            metrics.track(Event.CreditCardDelete)
        }
    }

    @Test
    fun handleSaveCreditCard() = testCoroutineScope.runBlockingTest {
-        assertEquals(0, settings.creditCardsSavedCount)
-
        val creditCardFields = NewCreditCardFields(
            billingName = "Banana Apple",
            plaintextCardNumber = CreditCardNumber.Plaintext("4111111111111112"),
@ -106,11 +108,10 @@ class DefaultCreditCardEditorControllerTest {

        controller.handleSaveCreditCard(creditCardFields)

-        assertEquals(1, settings.creditCardsSavedCount)
-
        coVerify {
            storage.addCreditCard(creditCardFields)
            navController.popBackStack()
+            metrics.track(Event.CreditCardManualSave)
        }
    }

--- a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt
+++ b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt
@ -675,25 +675,6 @@ class SettingsTest {
        assertTrue(settings.amoCollectionOverrideConfigured())
    }

-    @Test
-    fun creditCardsSavedCount() {
-        // When just created
-        // Then
-        assertEquals(0, settings.creditCardsSavedCount)
-
-        // When
-        settings.creditCardsSavedCount += 1
-
-        // Then
-        assertEquals(1, settings.creditCardsSavedCount)
-
-        // When
-        settings.creditCardsSavedCount += 1
-
-        // Then
-        assertEquals(2, settings.creditCardsSavedCount)
-    }
-
    @Test
    fun `GIVEN startOnHomeAlways is selected WHEN calling shouldStartOnHome THEN return true`() {
        settings.startOnHomeAlways = true