From 0e6eb7ab3886cba736fc8cd4b5eabd8588fe2671 Mon Sep 17 00:00:00 2001 From: Arturo Mejia Date: Sat, 23 Sep 2023 12:35:12 -0400 Subject: [PATCH] Bug 1854739 - Migrate pref_key_search_widget_installed --- .../org/mozilla/fenix/FenixApplication.kt | 1 + .../java/org/mozilla/fenix/utils/Settings.kt | 21 ++++++++- app/src/main/res/values/preference_keys.xml | 2 +- .../org/mozilla/fenix/FenixApplicationTest.kt | 3 +- .../org/mozilla/fenix/utils/SettingsTest.kt | 45 +++++++++++++++++++ 5 files changed, 68 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt index 938af48c6c..3343a717b0 100644 --- a/app/src/main/java/org/mozilla/fenix/FenixApplication.kt +++ b/app/src/main/java/org/mozilla/fenix/FenixApplication.kt @@ -746,6 +746,7 @@ open class FenixApplication : LocaleAwareApplication(), Provider { adjustCreative.set(settings.adjustCreative) adjustNetwork.set(settings.adjustNetwork) + settings.migrateSearchWidgetInstalledPrefIfNeeded() searchWidgetInstalled.set(settings.searchWidgetInstalled) val openTabsCount = settings.openTabsCount diff --git a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt index 8843230255..70c0f6a093 100644 --- a/app/src/main/java/org/mozilla/fenix/utils/Settings.kt +++ b/app/src/main/java/org/mozilla/fenix/utils/Settings.kt @@ -1216,14 +1216,31 @@ class Settings(private val appContext: Context) : PreferencesHolder { * exists on home screen or if it has been removed completely. */ fun setSearchWidgetInstalled(installed: Boolean) { - val key = appContext.getPreferenceKey(R.string.pref_key_search_widget_installed) + val key = appContext.getPreferenceKey(R.string.pref_key_search_widget_installed_2) preferences.edit() .putBoolean(key, installed) .apply() } + /** + * In Bug 1853113, we changed the type of [searchWidgetInstalled] from int to boolean without + * changing the pref key, now we have to migrate users that were using the previous type int + * to the new one boolean. The migration will only happens if pref_key_search_widget_installed + * is detected. + */ + fun migrateSearchWidgetInstalledPrefIfNeeded() { + val oldKey = "pref_key_search_widget_installed" + val installedCount = preferences.getInt(oldKey, 0) + + if (installedCount > 0) { + setSearchWidgetInstalled(true) + preferences.edit() + .remove(oldKey).apply() + } + } + val searchWidgetInstalled by booleanPreference( - appContext.getPreferenceKey(R.string.pref_key_search_widget_installed), + appContext.getPreferenceKey(R.string.pref_key_search_widget_installed_2), default = false, ) diff --git a/app/src/main/res/values/preference_keys.xml b/app/src/main/res/values/preference_keys.xml index 26ea06ae5e..b0a9fcfb10 100644 --- a/app/src/main/res/values/preference_keys.xml +++ b/app/src/main/res/values/preference_keys.xml @@ -95,7 +95,7 @@ pref_key_sign_out pref_key_sync_sign_in project_id - pref_key_search_widget_installed + pref_key_search_widget_installed_2 pref_key_saved_logins_sorting_strategy pref_key_sync_credit_cards diff --git a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt index 87c0cff1ff..4e6ca6a5da 100644 --- a/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt +++ b/app/src/test/java/org/mozilla/fenix/FenixApplicationTest.kt @@ -105,7 +105,8 @@ class FenixApplicationTest { every { settings.adjustAdGroup } returns "group" every { settings.adjustCreative } returns "creative" every { settings.adjustNetwork } returns "network" - every { settings.searchWidgetInstalled } returns true + // Testing [settings.migrateSearchWidgetInstalledPrefIfNeeded] + settings.preferences.edit().putInt("pref_key_search_widget_installed", 5).apply() every { settings.openTabsCount } returns 1 every { settings.topSitesSize } returns 2 every { settings.installedAddonsCount } returns 3 diff --git a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt index 651c93f90c..268555130f 100644 --- a/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt +++ b/app/src/test/java/org/mozilla/fenix/utils/SettingsTest.kt @@ -18,6 +18,7 @@ import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue +import org.junit.Assert.fail import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -950,4 +951,48 @@ class SettingsTest { settings.openLinksInExternalApp = "pref_key_open_links_in_apps_never" assertEquals(settings.getOpenLinksInAppsString(), "Never") } + + @Test + fun `GIVEN a written integer value for pref_key_search_widget_installed WHEN reading searchWidgetInstalled THEN do not throw a ClassCastException`() { + val expectedInt = 5 + val oldPrefKey = "pref_key_search_widget_installed" + + settings.preferences.edit().putInt(oldPrefKey, expectedInt).apply() + + try { + assertEquals(expectedInt, settings.preferences.getInt(oldPrefKey, 0)) + assertFalse(settings.searchWidgetInstalled) + } catch (e: ClassCastException) { + fail("Unexpected ClassCastException") + } + } + + @Test + fun `GIVEN previously stored pref_key_search_widget_installed value WHEN calling migrateSearchWidgetInstalledIfNeeded THEN migrate the value`() { + val expectedInt = 5 + val oldPrefKey = "pref_key_search_widget_installed" + + settings.preferences.edit().putInt(oldPrefKey, expectedInt).apply() + + assertEquals(expectedInt, settings.preferences.getInt(oldPrefKey, 0)) + assertFalse(settings.searchWidgetInstalled) + + settings.migrateSearchWidgetInstalledPrefIfNeeded() + + assertTrue(settings.searchWidgetInstalled) + } + + @Test + fun `GIVEN none previously stored pref_key_search_widget_installed value WHEN calling migrateSearchWidgetInstalledIfNeeded THEN migration should not happen`() { + val oldPrefKey = "pref_key_search_widget_installed" + val expectedDefaultValue = 0 + val storedValue = settings.preferences.getInt(oldPrefKey, expectedDefaultValue) + + assertEquals(expectedDefaultValue, storedValue) + + settings.migrateSearchWidgetInstalledPrefIfNeeded() + + assertEquals(expectedDefaultValue, settings.preferences.getInt(oldPrefKey, expectedDefaultValue)) + assertFalse(settings.searchWidgetInstalled) + } }