From 3bc3ca9610070e3e0231f2b632a1bf79f0b0c1fc Mon Sep 17 00:00:00 2001 From: MatthewTighe Date: Fri, 22 Apr 2022 11:09:55 -0700 Subject: [PATCH] [fenix] for https://github.com/mozilla-mobile/fenix/issues/24929: remove locale restriction for remote firefox wallpapers --- .../java/org/mozilla/fenix/FeatureFlags.kt | 13 ---- .../org/mozilla/fenix/FenixApplication.kt | 2 +- .../mozilla/fenix/components/Components.kt | 8 ++- .../org/mozilla/fenix/home/HomeFragment.kt | 3 +- .../org/mozilla/fenix/wallpapers/Wallpaper.kt | 17 ++++- .../fenix/wallpapers/WallpaperManager.kt | 12 +++- .../fenix/wallpapers/WallpaperManagerTest.kt | 62 +++++++++++++++++-- 7 files changed, 94 insertions(+), 23 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt index 2c92932a93..82ddc89247 100644 --- a/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt +++ b/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt @@ -5,10 +5,8 @@ package org.mozilla.fenix import android.content.Context -import android.os.StrictMode import mozilla.components.support.locale.LocaleManager import mozilla.components.support.locale.LocaleManager.getSystemDefault -import org.mozilla.fenix.ext.components /** * A single source for setting feature flags that are mostly based on build type. @@ -89,17 +87,6 @@ object FeatureFlags { */ const val historyImprovementFeatures = true - /** - * Enables themed wallpapers feature. - */ - fun isThemedWallpapersFeatureEnabled(context: Context): Boolean { - return context.components.strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { - val langTag = LocaleManager.getCurrentLocale(context) - ?.toLanguageTag() ?: getSystemDefault().toLanguageTag() - listOf("en-US", "es-US").contains(langTag) - } - } - /** * Enables the Task Continuity enhancements. */ diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 0ef5975320..eb9c364bb1 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -796,7 +796,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider { @OptIn(DelicateCoroutinesApi::class) open fun downloadWallpapers() { - if (FeatureFlags.showWallpapers && FeatureFlags.isThemedWallpapersFeatureEnabled(this)) { + if (FeatureFlags.showWallpapers) { GlobalScope.launch { components.wallpaperManager.downloadAllRemoteWallpapers() } diff --git a/app/src/main/java/org/mozilla/fenix/components/Components.kt b/app/src/main/java/org/mozilla/fenix/components/Components.kt index e1da153fb1..1a422eb996 100644 --- a/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -19,6 +19,7 @@ import mozilla.components.feature.addons.update.DefaultAddonUpdater import mozilla.components.feature.autofill.AutofillConfiguration import mozilla.components.lib.publicsuffixlist.PublicSuffixList import mozilla.components.support.base.worker.Frequency +import mozilla.components.support.locale.LocaleManager import org.mozilla.fenix.BuildConfig import org.mozilla.fenix.Config import org.mozilla.fenix.HomeActivity @@ -163,11 +164,16 @@ class Components(private val context: Context) { val strictMode by lazyMonitored { StrictModeManager(Config, this) } val wallpaperManager by lazyMonitored { + val currentLocale = strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { + LocaleManager.getCurrentLocale(context)?.toLanguageTag() + ?: LocaleManager.getSystemDefault().toLanguageTag() + } strictMode.resetAfter(StrictMode.allowThreadDiskReads()) { WallpaperManager( settings, WallpaperDownloader(context, core.client), - WallpaperFileManager(context.filesDir) + WallpaperFileManager(context.filesDir), + currentLocale ) } } diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt index d602e3514a..103c0d0c3d 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeFragment.kt @@ -1209,8 +1209,7 @@ class HomeFragment : Fragment() { return shouldEnableWallpaper() && settings.shouldAnimateFirefoxLogo && onboarding.userHasBeenOnboarded() && - settings.numberOfAppLaunches >= 3 && - FeatureFlags.isThemedWallpapersFeatureEnabled(localContext) + settings.numberOfAppLaunches >= 3 } private fun shouldEnableWallpaper() = diff --git a/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt b/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt index 28a77b73f1..54934ce51a 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/Wallpaper.kt @@ -49,14 +49,19 @@ sealed class Wallpaper { abstract val expirationDate: Date? abstract val remoteParentDirName: String @Suppress("MagicNumber") + /** + * A promotional partnered wallpaper. + */ data class House( override val name: String, override val expirationDate: Date? = Calendar.getInstance().run { set(2022, Calendar.APRIL, 30) time } - ) : Remote() { + ) : Remote(), Promotional { override val remoteParentDirName: String = "house" + override fun isAvailableInLocale(locale: String): Boolean = + listOf("en-US", "es-US").contains(locale) } /** @@ -70,6 +75,16 @@ sealed class Wallpaper { } } + /** + * Designates whether a wallpaper is part of a promotion that is locale-restricted. + */ + interface Promotional { + /** + * Returns whether the wallpaper is available in [locale] or not. + */ + fun isAvailableInLocale(locale: String): Boolean + } + companion object { /** * Defines the standard path at which a wallpaper resource is kept on disk. diff --git a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperManager.kt b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperManager.kt index 718db876ba..50ba7a8e6d 100644 --- a/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperManager.kt +++ b/app/src/main/java/org/mozilla/fenix/wallpapers/WallpaperManager.kt @@ -32,11 +32,14 @@ class WallpaperManager( private val settings: Settings, private val downloader: WallpaperDownloader, private val fileManager: WallpaperFileManager, + private val currentLocale: String, allWallpapers: List = availableWallpapers ) { val logger = Logger("WallpaperManager") - val wallpapers = allWallpapers.filter(::filterExpiredRemoteWallpapers) + val wallpapers = allWallpapers + .filter(::filterExpiredRemoteWallpapers) + .filter(::filterPromotionalWallpapers) var currentWallpaper: Wallpaper = getCurrentWallpaperFromSettings() set(value) { @@ -104,6 +107,13 @@ class WallpaperManager( else -> true } + private fun filterPromotionalWallpapers(wallpaper: Wallpaper): Boolean = + if (wallpaper is Wallpaper.Promotional) { + wallpaper.isAvailableInLocale(currentLocale) + } else { + true + } + private fun getCurrentWallpaperFromSettings(): Wallpaper { val currentWallpaper = settings.currentWallpaper return if (currentWallpaper.isEmpty()) { diff --git a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperManagerTest.kt b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperManagerTest.kt index f75eb7f799..5c09017532 100644 --- a/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperManagerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/wallpapers/WallpaperManagerTest.kt @@ -42,14 +42,14 @@ class WallpaperManagerTest { val updatedName = "new name" val updatedWallpaper = Wallpaper.Local.Firefox(updatedName, drawableId = 0) - val wallpaperManager = WallpaperManager(mockSettings, mockk(), mockFileManager, listOf()) + val wallpaperManager = WallpaperManager(mockSettings, mockk(), mockFileManager, "en-US", listOf()) wallpaperManager.currentWallpaper = updatedWallpaper assertEquals(updatedWallpaper.name, currentCaptureSlot.captured) } @Test - fun `GIVEN no remote wallpapers expired WHEN downloading remote wallpapers THEN all downloaded`() = runBlockingTest { + fun `GIVEN no remote wallpapers expired and locale in promo WHEN downloading remote wallpapers THEN all downloaded`() = runBlockingTest { every { mockSettings.currentWallpaper } returns "" val fakeRemoteWallpapers = listOf("first", "second", "third").map { name -> makeFakeRemoteWallpaper(TimeRelation.LATER, name) @@ -58,6 +58,7 @@ class WallpaperManagerTest { mockSettings, mockDownloader, mockFileManager, + "en-US", allWallpapers = fakeRemoteWallpapers ) wallpaperManager.downloadAllRemoteWallpapers() @@ -67,6 +68,51 @@ class WallpaperManagerTest { } } + @Test + fun `GIVEN no remote wallpapers expired and locale not in promo WHEN downloading remote wallpapers THEN none downloaded`() = runBlockingTest { + every { mockSettings.currentWallpaper } returns "" + val fakeRemoteWallpapers = listOf("first", "second", "third").map { name -> + makeFakeRemoteWallpaper(TimeRelation.LATER, name) + } + val wallpaperManager = WallpaperManager( + mockSettings, + mockDownloader, + mockFileManager, + "en-CA", + allWallpapers = fakeRemoteWallpapers + ) + wallpaperManager.downloadAllRemoteWallpapers() + + for (fakeRemoteWallpaper in fakeRemoteWallpapers) { + coVerify(exactly = 0) { mockDownloader.downloadWallpaper(fakeRemoteWallpaper) } + } + } + + @Test + fun `GIVEN no remote wallpapers expired and locale not in promo WHEN downloading remote wallpapers THEN non-promo wallpapers downloaded`() = runBlockingTest { + every { mockSettings.currentWallpaper } returns "" + val fakePromoWallpapers = listOf("first", "second", "third").map { name -> + makeFakeRemoteWallpaper(TimeRelation.LATER, name) + } + val fakeNonPromoWallpapers = listOf(makeFakeRemoteWallpaper(TimeRelation.LATER, "fourth", false)) + val fakeRemoteWallpapers = fakePromoWallpapers + fakeNonPromoWallpapers + val wallpaperManager = WallpaperManager( + mockSettings, + mockDownloader, + mockFileManager, + "en-CA", + allWallpapers = fakeRemoteWallpapers + ) + wallpaperManager.downloadAllRemoteWallpapers() + + for (wallpaper in fakePromoWallpapers) { + coVerify(exactly = 0) { mockDownloader.downloadWallpaper(wallpaper) } + } + for (wallpaper in fakeNonPromoWallpapers) { + coVerify { mockDownloader.downloadWallpaper(wallpaper) } + } + } + @Test fun `GIVEN some expired wallpapers WHEN initialized THEN wallpapers are not available`() { every { mockSettings.currentWallpaper } returns "" @@ -76,6 +122,7 @@ class WallpaperManagerTest { mockSettings, mockDownloader, mockFileManager, + "en-US", allWallpapers = listOf(expiredRemoteWallpaper, activeRemoteWallpaper) ) @@ -96,6 +143,7 @@ class WallpaperManagerTest { mockSettings, mockDownloader, mockFileManager, + "en-US", allWallpapers = listOf(expiredRemoteWallpaper) ) @@ -115,6 +163,7 @@ class WallpaperManagerTest { mockSettings, mockDownloader, mockFileManager, + "en-US", allWallpapers = listOf() ) @@ -132,7 +181,8 @@ class WallpaperManagerTest { */ private fun makeFakeRemoteWallpaper( timeRelation: TimeRelation, - name: String = "name" + name: String = "name", + isInPromo: Boolean = true ): Wallpaper.Remote { fakeCalendar.time = baseFakeDate when (timeRelation) { @@ -141,6 +191,10 @@ class WallpaperManagerTest { TimeRelation.LATER -> fakeCalendar.add(Calendar.DATE, 5) } val relativeTime = fakeCalendar.time - return Wallpaper.Remote.House(name = name, expirationDate = relativeTime) + return if (isInPromo) { + Wallpaper.Remote.House(name = name, expirationDate = relativeTime) + } else { + Wallpaper.Remote.Firefox(name = name) + } } }