update based on PR feedback and data review

app/metrics.yaml

@@ -6958,6 +6958,7 @@ search_terms:
 wallpapers:
   discovered_wallpaper_feature:
     type: boolean
+    lifetime: application
     description: |
       Whether or not the user has discovered the wallpaper feature.
     bugs:
@@ -6970,29 +6971,16 @@ wallpapers:
       - interaction
     expires: "2023-05-01"
   new_wallpaper_applied:
-    type: labeled_counter
-    description: |
-      How many and which type of wallpapers were applied.
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/23381
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/23382
-    notification_emails:
-      - android-probes@mozilla.com
-    data_sensitivity:
-      - interaction
-    expires: "2023-05-01"
-  selected_wallpaper:
     type: event
     description: |
-      Records the wallpaper that is active at session start.
+      An event that is fired when a new wallpaper is applied to the home screen.
     extra_keys:
       name:
-        description: The name of the selected wallpaper
+        description: The name of the applied wallpaper
+        type: string
+      theme_collection:
+        description: The theme collection the applied wallpaper belongs to.
         type: string
-      is_themed:
-        description: Whether the selected wallpaper is part of a theme.
-        type: boolean
     bugs:
       - https://github.com/mozilla-mobile/fenix/issues/23381
     data_reviews:
@@ -7002,18 +6990,3 @@ wallpapers:
     data_sensitivity:
       - interaction
     expires: "2023-05-01"
-  wallpaper_reset_to_default:
-    type: event
-    description: |
-      The user has reset their wallpaper back to the default background.
-      This means the default is active on startup, and has changed since the
-      previous session.
-    bugs:
-      - https://github.com/mozilla-mobile/fenix/issues/23381
-    data_reviews:
-      - https://github.com/mozilla-mobile/fenix/pull/23382
-    data_sensitivity:
-      - interaction
-    notification_emails:
-      - android-probes@mozilla.com
-    expires: "2023-05-01"

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

@@ -676,12 +676,8 @@ sealed class Event {
 
     sealed class Search
 
-    sealed class Wallpaper : Event() {
-        object DiscoveredFeature : Wallpaper()
-        data class NewWallpaperApplied(val wallpaper: org.mozilla.fenix.wallpapers.Wallpaper) : Wallpaper()
-        data class WallpaperSelected(val wallpaper: org.mozilla.fenix.wallpapers.Wallpaper) : Wallpaper()
-        object WallpaperResetToDefault : Wallpaper()
-    }
+    object WallpaperFeatureDiscovered : Event()
+    data class NewWallpaperApplied(val wallpaper: org.mozilla.fenix.wallpapers.Wallpaper) : Event()
 
     internal open val extras: Map<*, String>?
         get() = null

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

@@ -908,24 +908,18 @@ private val Event.wrapper: EventWrapper<*>?
         is Event.JumpBackInGroupTapped -> EventWrapper<NoExtraKeys>(
             { SearchTerms.jumpBackInGroupTapped.record(it) }
         )
-        is Event.Wallpaper.DiscoveredFeature -> EventWrapper<NoExtraKeys>(
+        is Event.WallpaperFeatureDiscovered -> EventWrapper<NoExtraKeys>(
             { Wallpapers.discoveredWallpaperFeature.set(true) }
         )
-        is Event.Wallpaper.NewWallpaperApplied -> EventWrapper<NoExtraKeys>(
-            { Wallpapers.newWallpaperApplied[this.wallpaper.name].add() }
-        )
-        is Event.Wallpaper.WallpaperSelected -> EventWrapper<NoExtraKeys>(
+        is Event.NewWallpaperApplied -> EventWrapper<NoExtraKeys>(
             {
-                Wallpapers.selectedWallpaper.record(
-                    Wallpapers.SelectedWallpaperExtra(
+                Wallpapers.newWallpaperApplied.record(
+                    Wallpapers.NewWallpaperAppliedExtra(
                         name = this.wallpaper.name,
-                        isThemed = this.wallpaper.isThemed,
+                        themeCollection = this.wallpaper.themeCollection::class.simpleName,
                     ),
                 )
-            },
-        )
-        Event.Wallpaper.WallpaperResetToDefault -> EventWrapper<NoExtraKeys>(
-            { Wallpapers.wallpaperResetToDefault.record() }
+            }
         )
 
         // Don't record other events in Glean:

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

@@ -767,7 +767,7 @@ class HomeFragment : Fragment() {
         if (shouldEnableWallpaper() && context.settings().wallpapersSwitchedByLogoTap) {
             binding.wordmark.setOnClickListener {
                 val manager = requireComponents.wallpaperManager
-                manager.recordDiscoveredMetric()
+                requireComponents.analytics.metrics.track(Event.WallpaperFeatureDiscovered)
                 manager.updateWallpaper(
                     wallpaperContainer = binding.homeLayout,
                     newWallpaper = manager.switchToNextWallpaper()

app/src/main/java/org/mozilla/fenix/settings/wallpaper/WallpaperSettingsFragment.kt

@@ -17,6 +17,7 @@ import androidx.compose.ui.platform.ViewCompositionStrategy
 import androidx.fragment.app.Fragment
 import androidx.navigation.fragment.findNavController
 import org.mozilla.fenix.R
+import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.theme.FirefoxTheme
 import org.mozilla.fenix.wallpapers.Wallpaper
@@ -31,12 +32,16 @@ class WallpaperSettingsFragment : Fragment() {
         requireComponents.settings
     }
 
+    private val metrics by lazy {
+        requireComponents.analytics.metrics
+    }
+
     override fun onCreateView(
         inflater: LayoutInflater,
         container: ViewGroup?,
         savedInstanceState: Bundle?
     ): View? {
-        wallpaperManager.recordDiscoveredMetric()
+        metrics.track(Event.WallpaperFeatureDiscovered)
         return ComposeView(requireContext()).apply {
             setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed)
             setContent {

app/src/main/java/org/mozilla/fenix/utils/Settings.kt

@@ -166,11 +166,6 @@ class Settings(private val appContext: Context) : PreferencesHolder {
         default = ""
     )
 
-    var previousWallpaper by stringPreference(
-        appContext.getPreferenceKey(R.string.pref_key_previous_wallpaper),
-        default = WallpaperManager.defaultWallpaper.name
-    )
-
     var currentWallpaper by stringPreference(
         appContext.getPreferenceKey(R.string.pref_key_current_wallpaper),
         default = WallpaperManager.defaultWallpaper.name
@@ -181,11 +176,6 @@ class Settings(private val appContext: Context) : PreferencesHolder {
         default = true
     )
 
-    var wallpapersDiscovered by booleanPreference(
-        appContext.getPreferenceKey(R.string.pref_key_wallpapers_discovered),
-        default = false
-    )
-
     var openLinksInAPrivateTab by booleanPreference(
         appContext.getPreferenceKey(R.string.pref_key_open_links_in_a_private_tab),
         default = false

app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt

@@ -10,12 +10,20 @@ package org.mozilla.fenix.wallpapers
  * @property portraitPath A file path for the portrait version of this wallpaper.
  * @property landscapePath A file path for the landscape version of this wallpaper.
  * @property isDark Indicates if the most predominant color on the wallpaper is dark.
- * @property isThemed Whether the wallpaper belongs to a themed collection.
+ * @property themeCollection The theme collection this wallpaper belongs to.
  */
 data class Wallpaper(
     val name: String,
     val portraitPath: String,
     val landscapePath: String,
     val isDark: Boolean,
-    val isThemed: Boolean = false,
+    val themeCollection: WallpaperThemeCollection,
 )
+
+/**
+ * A type hierarchy representing the different theme collections [Wallpaper]s belong to.
+ */
+sealed class WallpaperThemeCollection {
+    object None : WallpaperThemeCollection()
+    object Firefox : WallpaperThemeCollection()
+}

app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperManager.kt

@@ -34,7 +34,6 @@ class WallpaperManager(
 
     var currentWallpaper: Wallpaper = getCurrentWallpaperFromSettings()
         set(value) {
-            settings.previousWallpaper = currentWallpaper.name
             settings.currentWallpaper = value.name
             field = value
         }
@@ -53,9 +52,9 @@ class WallpaperManager(
             wallpaperContainer.background = BitmapDrawable(context.resources, bitmap)
         }
         currentWallpaper = newWallpaper
+        metrics.track(Event.NewWallpaperApplied(newWallpaper))
 
         adjustTheme(wallpaperContainer.context)
-        recordWallpaperAppliedMetric(newWallpaper)
     }
 
     private fun adjustTheme(context: Context) {
@@ -113,8 +112,6 @@ class WallpaperManager(
             defaultWallpaper
         } else {
             availableWallpapers.find { it.name == currentWallpaper } ?: defaultWallpaper
-        }.also {
-            recordSelectionMetrics(it)
         }
     }
 
@@ -142,35 +139,14 @@ class WallpaperManager(
         }
     }
 
-    fun recordDiscoveredMetric() {
-        val hasSentMetric = settings.wallpapersDiscovered
-        if (!hasSentMetric) {
-            metrics.track(Event.Wallpaper.DiscoveredFeature)
-            settings.wallpapersDiscovered = true
-        }
-    }
-
-    private fun recordWallpaperAppliedMetric(appliedWallpaper: Wallpaper) {
-        metrics.track(Event.Wallpaper.NewWallpaperApplied(appliedWallpaper))
-    }
-
-    private fun recordSelectionMetrics(currentWallpaper: Wallpaper) {
-        val previousWallpaperName = settings.previousWallpaper
-        if (previousWallpaperName != defaultWallpaper.name && currentWallpaper == defaultWallpaper) {
-            metrics.track(Event.Wallpaper.WallpaperResetToDefault)
-            // This metric will continue to be reported unless the previous wallpaper is reset
-            settings.previousWallpaper = currentWallpaper.name
-        }
-        metrics.track(Event.Wallpaper.WallpaperSelected(currentWallpaper))
-    }
-
     companion object {
         const val DEFAULT_RESOURCE = R.attr.homeBackground
         val defaultWallpaper = Wallpaper(
             name = "default_wallpaper",
             portraitPath = "",
             landscapePath = "",
-            isDark = false
+            isDark = false,
+            themeCollection = WallpaperThemeCollection.None
         )
     }
 }

app/src/main/java/org/mozilla/fenix/wallpapers/WallpapersAssetsStorage.kt

@@ -39,7 +39,13 @@ class WallpapersAssetsStorage(private val context: Context) : WallpaperStorage {
                 name = getString("name"),
                 portraitPath = getString("portrait"),
                 landscapePath = getString("landscape"),
-                isDark = getBoolean("isDark")
+                isDark = getBoolean("isDark"),
+                themeCollection = Result.runCatching {
+                    when (getString("themeCollection")) {
+                        "firefox" -> WallpaperThemeCollection.Firefox
+                        else -> WallpaperThemeCollection.None
+                    }
+                }.getOrDefault(WallpaperThemeCollection.None)
             )
         } catch (e: JSONException) {
             logger.error("unable to parse json for wallpaper $this", e)

app/src/main/res/values/preference_keys.xml

@@ -189,10 +189,8 @@
 
     <!-- Wallpaper Settings -->
     <string name="pref_key_wallpapers" translatable="false">pref_key_wallpapers</string>
-    <string name="pref_key_previous_wallpaper" translatable="false">pref_key_previous_wallpaper</string>
     <string name="pref_key_current_wallpaper" translatable="false">pref_key_current_wallpaper</string>
     <string name="pref_key_wallpapers_switched_by_logo_tap">pref_key_wallpapers_switched_by_logo_tap</string>
-    <string name="pref_key_wallpapers_discovered" translatable="false">pref_key_wallpapers_discovered</string>
 
     <string name="pref_key_encryption_key_generated" translatable="false">pref_key_encryption_key_generated</string>
 

app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperManagerTest.kt

@@ -14,93 +14,30 @@ import org.mozilla.fenix.utils.Settings
 
 class WallpaperManagerTest {
 
-    private val fakeWallpaper = Wallpaper(
-        name = "fake wallpaper",
-        portraitPath = "",
-        landscapePath = "",
-        isDark = false,
-        isThemed = false,
-    )
-    private val fakeThemedWallpaper = Wallpaper(
-        name = "fake themed wallpaper",
-        portraitPath = "",
-        landscapePath = "",
-        isDark = false,
-        isThemed = true,
-    )
-    private val fakeWallpapers = listOf(fakeWallpaper, fakeThemedWallpaper)
-
     private val mockSettings: Settings = mockk()
     private val mockStorage: WallpaperStorage = mockk {
-        every { loadAll() } returns fakeWallpapers
+        every { loadAll() } returns listOf()
     }
     private val mockMetrics: MetricController = mockk()
 
     @Test
     fun `WHEN wallpaper set THEN current wallpaper updated in settings`() {
         every { mockMetrics.track(any()) } just runs
+
         val currentCaptureSlot = slot<String>()
-        val updatedWallpaper = fakeWallpaper
-        every { mockSettings.currentWallpaper } returns WallpaperManager.defaultWallpaper.name
+        every { mockSettings.currentWallpaper } returns "a different name"
         every { mockSettings.currentWallpaper = capture(currentCaptureSlot) } just runs
-        every { mockSettings.previousWallpaper } returns WallpaperManager.defaultWallpaper.name
-        every { mockSettings.previousWallpaper = capture(slot()) } just runs
 
+        val updatedWallpaper = WallpaperManager.defaultWallpaper
         val wallpaperManager = WallpaperManager(mockSettings, mockStorage, mockMetrics)
         wallpaperManager.currentWallpaper = updatedWallpaper
 
         assertEquals(updatedWallpaper.name, currentCaptureSlot.captured)
     }
 
-    @Test
-    fun `WHEN wallpaper set THEN current wallpaper saved as previous in settings`() {
-        every { mockMetrics.track(any()) } just runs
-        val previousCaptureSlot = slot<String>()
-        val currentWallpaper = fakeWallpaper
-        every { mockSettings.currentWallpaper } returns currentWallpaper.name
-        every { mockSettings.currentWallpaper = capture(slot()) } just runs
-        every { mockSettings.previousWallpaper } returns WallpaperManager.defaultWallpaper.name
-        every { mockSettings.previousWallpaper = capture(previousCaptureSlot) } just runs
-
-        val wallpaperManager = WallpaperManager(mockSettings, mockStorage, mockMetrics)
-        wallpaperManager.currentWallpaper = WallpaperManager.defaultWallpaper
-
-        assertEquals(currentWallpaper.name, previousCaptureSlot.captured)
-    }
-
-    @Test
-    fun `WHEN wallpaper is initially fetched from settings THEN selection metric reported`() {
-        val defaultWallpaper = WallpaperManager.defaultWallpaper
-        every { mockSettings.previousWallpaper } returns defaultWallpaper.name
-        every { mockSettings.currentWallpaper } returns defaultWallpaper.name
-        every { mockMetrics.track(any()) } just runs
-
-        WallpaperManager(mockSettings, mockStorage, mockMetrics)
-
-        verify { mockMetrics.track(Event.Wallpaper.WallpaperSelected(defaultWallpaper)) }
-    }
-
-    @Test
-    fun `GIVEN previous wallpaper was not default WHEN current wallpaper is loaded as default THEN selection metric reported and previous reset`() {
-        val defaultWallpaper = WallpaperManager.defaultWallpaper
-        val previousSlot = slot<String>()
-        every { mockSettings.previousWallpaper } returns fakeWallpaper.name
-        every { mockSettings.previousWallpaper = capture(previousSlot) } just runs
-        every { mockSettings.currentWallpaper } returns defaultWallpaper.name
-        every { mockMetrics.track(any()) } just runs
-
-        WallpaperManager(mockSettings, mockStorage, mockMetrics)
-
-        assertEquals(defaultWallpaper.name, previousSlot.captured)
-        verify { mockMetrics.track(Event.Wallpaper.WallpaperResetToDefault) }
-        verify { mockMetrics.track(Event.Wallpaper.WallpaperSelected(defaultWallpaper)) }
-    }
-
     @Test
     fun `WHEN wallpaper updated THEN wallpaper applied metric recorded`() {
         val defaultWallpaper = WallpaperManager.defaultWallpaper
-        every { mockSettings.previousWallpaper } returns defaultWallpaper.name
-        every { mockSettings.previousWallpaper = capture(slot()) } just runs
         every { mockSettings.currentWallpaper } returns defaultWallpaper.name
         every { mockSettings.currentWallpaper = capture(slot()) } just runs
         every { mockMetrics.track(any()) } just runs
@@ -108,36 +45,6 @@ class WallpaperManagerTest {
         val manager = WallpaperManager(mockSettings, mockStorage, mockMetrics)
         manager.updateWallpaper(mockk(relaxed = true), defaultWallpaper)
 
-        verify { mockMetrics.track(Event.Wallpaper.NewWallpaperApplied(defaultWallpaper)) }
-    }
-
-    @Test
-    fun `WHEN wallpaper discovered metric not previously recorded WHEN metric record attempted THEN metric sent`() {
-        every { mockSettings.wallpapersDiscovered } returns false
-        every { mockSettings.previousWallpaper } returns WallpaperManager.defaultWallpaper.name
-        every { mockSettings.currentWallpaper } returns WallpaperManager.defaultWallpaper.name
-        every { mockSettings.wallpapersDiscovered = true } just runs
-        every { mockMetrics.track(any()) } just runs
-
-        val wallpaperManager = WallpaperManager(mockSettings, mockStorage, mockMetrics)
-
-        wallpaperManager.recordDiscoveredMetric()
-
-        verify { mockMetrics.track(Event.Wallpaper.DiscoveredFeature) }
-        verify { mockSettings.wallpapersDiscovered = true }
-    }
-
-    @Test
-    fun `WHEN wallpaper discovered metric previously recorded WHEN metric record attempted THEN metric not sent`() {
-        every { mockSettings.wallpapersDiscovered } returns true
-        every { mockSettings.previousWallpaper } returns WallpaperManager.defaultWallpaper.name
-        every { mockSettings.currentWallpaper } returns WallpaperManager.defaultWallpaper.name
-        every { mockMetrics.track(any()) } just runs
-
-        val wallpaperManager = WallpaperManager(mockSettings, mockStorage, mockMetrics)
-
-        wallpaperManager.recordDiscoveredMetric()
-
-        verify(exactly = 0) { mockMetrics.track(Event.Wallpaper.DiscoveredFeature) }
+        verify { mockMetrics.track(Event.NewWallpaperApplied(defaultWallpaper)) }
     }
 }