From 80084ff0fb9bdf7e4d25ee7ad30809adcbf31e8e Mon Sep 17 00:00:00 2001 From: Oleg Semenenko <146032743+oleg-livefront@users.noreply.github.com> Date: Mon, 15 Jan 2024 10:26:43 -0600 Subject: [PATCH] Add persistence for isImageLoadingEnabled appearance setting (#612) --- .../datasource/disk/SettingsDiskSource.kt | 6 ++++ .../datasource/disk/SettingsDiskSourceImpl.kt | 10 +++++++ .../platform/repository/SettingsRepository.kt | 5 ++++ .../repository/SettingsRepositoryImpl.kt | 6 ++++ .../appearance/AppearanceViewModel.kt | 10 +++++-- .../datasource/disk/SettingsDiskSourceTest.kt | 30 +++++++++++++++++++ .../disk/util/FakeSettingsDiskSource.kt | 2 ++ .../repository/SettingsRepositoryTest.kt | 13 ++++++++ .../appearance/AppearanceViewModelTest.kt | 17 ++++++++--- 9 files changed, 92 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt index 548861b622..c958535e24 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSource.kt @@ -8,6 +8,12 @@ import kotlinx.coroutines.flow.Flow * Primary access point for general settings-related disk information. */ interface SettingsDiskSource { + + /** + * The currently persisted setting for getting login item icons (or `null` if not set). + */ + var isIconLoadingDisabled: Boolean? + /** * The currently persisted app language (or `null` if not set). */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt index ac659bf56c..d6edc33c81 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceImpl.kt @@ -13,6 +13,7 @@ private const val APP_LANGUAGE_KEY = "$BASE_KEY:appLocale" private const val PULL_TO_REFRESH_KEY = "$BASE_KEY:syncOnRefresh" private const val VAULT_TIMEOUT_ACTION_KEY = "$BASE_KEY:vaultTimeoutAction" private const val VAULT_TIME_IN_MINUTES_KEY = "$BASE_KEY:vaultTimeout" +private const val DISABLE_ICON_LOADING_KEY = "$BASE_KEY:disableFavicon" /** * Primary implementation of [SettingsDiskSource]. @@ -43,6 +44,15 @@ class SettingsDiskSourceImpl( ) } + override var isIconLoadingDisabled: Boolean? + get() = getBoolean(key = DISABLE_ICON_LOADING_KEY) + set(value) { + putBoolean( + key = DISABLE_ICON_LOADING_KEY, + value = value, + ) + } + override fun clearData(userId: String) { storeVaultTimeoutInMinutes(userId = userId, vaultTimeoutInMinutes = null) storeVaultTimeoutAction(userId = userId, vaultTimeoutAction = null) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt index d4c971210e..d99be678f4 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt @@ -14,6 +14,11 @@ interface SettingsRepository { */ var appLanguage: AppLanguage + /** + * Is icon loading for login items disabled. + */ + var isIconLoadingDisabled: Boolean + /** * The [VaultTimeout] for the current user. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt index c8d7ddcc9a..f82c872e9c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt @@ -31,6 +31,12 @@ class SettingsRepositoryImpl( settingsDiskSource.appLanguage = value } + override var isIconLoadingDisabled: Boolean + get() = settingsDiskSource.isIconLoadingDisabled ?: false + set(value) { + settingsDiskSource.isIconLoadingDisabled = value + } + override var vaultTimeout: VaultTimeout get() = activeUserId ?.let { diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/appearance/AppearanceViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/appearance/AppearanceViewModel.kt index 9480cc7c66..bb6709979f 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/appearance/AppearanceViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/appearance/AppearanceViewModel.kt @@ -28,7 +28,7 @@ class AppearanceViewModel @Inject constructor( initialState = savedStateHandle[KEY_STATE] ?: AppearanceState( language = settingsRepository.appLanguage, - showWebsiteIcons = false, + showWebsiteIcons = !settingsRepository.isIconLoadingDisabled, theme = AppearanceState.Theme.DEFAULT, ), ) { @@ -54,8 +54,12 @@ class AppearanceViewModel @Inject constructor( } private fun handleShowWebsiteIconsToggled(action: AppearanceAction.ShowWebsiteIconsToggle) { - // TODO: BIT-541 add website icon support - mutableStateFlow.update { it.copy(showWebsiteIcons = action.showWebsiteIcons) } + mutableStateFlow.update { + it.copy(showWebsiteIcons = action.showWebsiteIcons) + } + + // Negate the boolean to properly update the settings repository + settingsRepository.isIconLoadingDisabled = !action.showWebsiteIcons } private fun handleThemeChanged(action: AppearanceAction.ThemeChange) { diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt index fb3b38ac27..ff0c8b4202 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/SettingsDiskSourceTest.kt @@ -76,6 +76,36 @@ class SettingsDiskSourceTest { assertNull(settingsDiskSource.getVaultTimeoutAction(userId = userId)) } + @Test + fun `isIconLoadingDisabled should pull from and update SharedPreferences`() { + val isIconLoadingDisabled = "bwPreferencesStorage:disableFavicon" + val expected = false + + // Assert that the default value in disk source is null + assertNull(settingsDiskSource.isIconLoadingDisabled) + + // Updating the shared preferences should update disk source. + fakeSharedPreferences + .edit() + .putBoolean( + isIconLoadingDisabled, + expected, + ) + .apply() + assertEquals( + expected, + settingsDiskSource.isIconLoadingDisabled, + ) + + // Updating the disk source updates the shared preferences + settingsDiskSource.isIconLoadingDisabled = true + assertTrue( + fakeSharedPreferences.getBoolean( + isIconLoadingDisabled, false, + ), + ) + } + @Test fun `getVaultTimeoutInMinutes when values are present should pull from SharedPreferences`() { val vaultTimeoutBaseKey = "bwPreferencesStorage:vaultTimeout" diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt index e3e8bf943b..d4a3a153b7 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/disk/util/FakeSettingsDiskSource.kt @@ -29,6 +29,8 @@ class FakeSettingsDiskSource : SettingsDiskSource { override var appLanguage: AppLanguage? = null + override var isIconLoadingDisabled: Boolean? = null + override fun clearData(userId: String) { storedVaultTimeoutActions.remove(userId) storedVaultTimeoutInMinutes.remove(userId) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt index 858341e2c9..4fc336cf51 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt @@ -112,6 +112,19 @@ class SettingsRepositoryTest { ) } + @Test + fun `isIconLoadingDisabled should pull from and update SettingsDiskSource`() { + assertFalse(settingsRepository.isIconLoadingDisabled) + + // Updates to the disk source change the repository value. + fakeSettingsDiskSource.isIconLoadingDisabled = true + assertTrue(settingsRepository.isIconLoadingDisabled) + + // Updates to the repository change the disk source value + settingsRepository.isIconLoadingDisabled = false + assertFalse(fakeSettingsDiskSource.isIconLoadingDisabled!!) + } + @Test fun `vaultTimeout should pull from and update SettingsDiskSource for the current user`() { every { authDiskSource.userState?.activeUserId } returns null diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/appearance/AppearanceViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/appearance/AppearanceViewModelTest.kt index f5990ac679..ff5078c45f 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/appearance/AppearanceViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/appearance/AppearanceViewModelTest.kt @@ -23,6 +23,8 @@ class AppearanceViewModelTest : BaseViewModelTest() { private val mockSettingsRepository = mockk { every { appLanguage } returns AppLanguage.DEFAULT every { appLanguage = AppLanguage.ENGLISH } just runs + every { isIconLoadingDisabled } returns false + every { isIconLoadingDisabled = true } just runs } @BeforeEach @@ -85,18 +87,25 @@ class AppearanceViewModelTest : BaseViewModelTest() { } @Test - fun `on ShowWebsiteIconsToggle should update value in state`() = runTest { + fun `on ShowWebsiteIconsToggle should update state and store the value`() = runTest { val viewModel = createViewModel() + viewModel.stateFlow.test { assertEquals( DEFAULT_STATE, awaitItem(), ) - viewModel.trySendAction(AppearanceAction.ShowWebsiteIconsToggle(true)) + + viewModel.trySendAction(AppearanceAction.ShowWebsiteIconsToggle(false)) assertEquals( - DEFAULT_STATE.copy(showWebsiteIcons = true), + DEFAULT_STATE.copy(showWebsiteIcons = false), awaitItem(), ) + + // Since we negate the boolean in the ViewModel it should be true + verify { + mockSettingsRepository.isIconLoadingDisabled = true + } } } @@ -129,7 +138,7 @@ class AppearanceViewModelTest : BaseViewModelTest() { companion object { private val DEFAULT_STATE = AppearanceState( language = AppLanguage.DEFAULT, - showWebsiteIcons = false, + showWebsiteIcons = true, theme = AppearanceState.Theme.DEFAULT, ) }