From aff4c03d53dcd5c85de40afb065c85e9a61744bc Mon Sep 17 00:00:00 2001 From: Gabriel Luong Date: Sat, 16 Apr 2022 02:00:38 -0400 Subject: [PATCH] For #24856 - Toggle the "Manage addresses" and "Add address" preference label depending on whether there are saved addresses --- .../autofill/AutofillFragmentStore.kt | 22 +++++++-- .../autofill/AutofillSettingFragment.kt | 48 +++++++++++++++---- .../CreditCardsManagementFragment.kt | 2 +- app/src/main/res/values/strings.xml | 2 + .../autofill/AutofillFragmentStoreTest.kt | 14 +++++- .../autofill/AutofillSettingFragmentTest.kt | 45 ++++++++++++++++- .../CreditCardsManagementViewTest.kt | 2 +- 7 files changed, 118 insertions(+), 17 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillFragmentStore.kt index bce841cfdf..702d79ace3 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillFragmentStore.kt @@ -4,6 +4,7 @@ package org.mozilla.fenix.settings.autofill +import mozilla.components.concept.storage.Address import mozilla.components.concept.storage.CreditCard import mozilla.components.lib.state.Action import mozilla.components.lib.state.State @@ -18,14 +19,16 @@ class AutofillFragmentStore(initialState: AutofillFragmentState) : ) /** - * The state for [CreditCardsManagementFragment]. + * The state used for managing autofill data. * + * @property addresses The list of [Address]es to display in the address list. * @property creditCards The list of [CreditCard]s to display in the credit card list. - * @property isLoading True if the credit cards are still being loaded from storage, + * @property isLoading True if the addresses or credit cards are still being loaded from storage, * otherwise false. */ data class AutofillFragmentState( - val creditCards: List, + val addresses: List
= emptyList(), + val creditCards: List = emptyList(), val isLoading: Boolean = true ) : State @@ -34,6 +37,13 @@ data class AutofillFragmentState( * through the [autofillFragmentStateReducer]. */ sealed class AutofillAction : Action { + /** + * Updates the list of addresses with the provided [addresses]. + * + * @param addresses The list of [Address]es to display in the address list. + */ + data class UpdateAddresses(val addresses: List
) : AutofillAction() + /** * Updates the list of credit cards with the provided [creditCards]. * @@ -54,6 +64,12 @@ private fun autofillFragmentStateReducer( action: AutofillAction ): AutofillFragmentState { return when (action) { + is AutofillAction.UpdateAddresses -> { + state.copy( + addresses = action.addresses, + isLoading = false + ) + } is AutofillAction.UpdateCreditCards -> { state.copy( creditCards = action.creditCards, diff --git a/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragment.kt index e024705ff1..2782e5f1d7 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragment.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.launch import mozilla.components.lib.state.ext.consumeFrom import mozilla.components.service.fxa.SyncEngine +import mozilla.components.service.sync.autofill.AutofillCreditCardsAddressesStorage import org.mozilla.fenix.NavGraphDirections import org.mozilla.fenix.R import org.mozilla.fenix.components.StoreProvider @@ -46,7 +47,8 @@ import org.mozilla.fenix.settings.requirePreference class AutofillSettingFragment : BiometricPromptPreferenceFragment() { private lateinit var store: AutofillFragmentStore - private var isCreditCardsListLoaded: Boolean = false + + private var isAutofillStateLoaded: Boolean = false /** * List of preferences to be enabled or disabled during authentication. @@ -73,9 +75,9 @@ class AutofillSettingFragment : BiometricPromptPreferenceFragment() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) store = StoreProvider.get(this) { - AutofillFragmentStore(AutofillFragmentState(creditCards = emptyList())) + AutofillFragmentStore(AutofillFragmentState()) } - loadCreditCards() + loadAutofillState() } override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) { @@ -99,13 +101,17 @@ class AutofillSettingFragment : BiometricPromptPreferenceFragment() { container: ViewGroup?, savedInstanceState: Bundle? ): View? { - loadCreditCards() + loadAutofillState() return super.onCreateView(inflater, container, savedInstanceState) } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) + consumeFrom(store) { state -> + if (requireComponents.settings.addressFeature) { + updateAddressPreference(state.addresses.isNotEmpty()) + } updateCardManagementPreference(state.creditCards.isNotEmpty(), findNavController()) } @@ -114,7 +120,7 @@ class AutofillSettingFragment : BiometricPromptPreferenceFragment() { override fun onPause() { super.onPause() - isCreditCardsListLoaded = false + isAutofillStateLoaded = false } override fun onResume() { @@ -150,6 +156,25 @@ class AutofillSettingFragment : BiometricPromptPreferenceFragment() { togglePrefsEnabled(creditCardPreferences, true) } + /** + * Updates preferences visibility depending on addresses being already saved or not. + */ + @VisibleForTesting + internal fun updateAddressPreference(hasAddresses: Boolean) { + val manageAddressesPreference = + requirePreference(R.string.pref_key_addresses_manage_addresses) + + if (hasAddresses) { + manageAddressesPreference.icon = null + manageAddressesPreference.title = + getString(R.string.preferences_addresses_manage_addresses) + } else { + manageAddressesPreference.setIcon(R.drawable.ic_new) + manageAddressesPreference.title = + getString(R.string.preferences_addresses_add_address) + } + } + /** * Updates preferences visibility depending on credit cards being already saved or not. */ @@ -185,22 +210,25 @@ class AutofillSettingFragment : BiometricPromptPreferenceFragment() { } /** - * Fetches all the credit cards from autofillStorage and updates the [AutofillFragmentState] - * with the list of credit cards. + * Fetches all the addresses and credit cards from [AutofillCreditCardsAddressesStorage] and + * updates the [AutofillFragmentState]. */ - private fun loadCreditCards() { - if (isCreditCardsListLoaded) { + private fun loadAutofillState() { + if (isAutofillStateLoaded) { return } lifecycleScope.launch(Dispatchers.IO) { + val addresses = requireComponents.core.autofillStorage.getAllAddresses() val creditCards = requireComponents.core.autofillStorage.getAllCreditCards() + lifecycleScope.launch(Dispatchers.Main) { + store.dispatch(AutofillAction.UpdateAddresses(addresses)) store.dispatch(AutofillAction.UpdateCreditCards(creditCards)) } } - isCreditCardsListLoaded = true + isAutofillStateLoaded = true } /** diff --git a/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementFragment.kt index 8f56d7f54e..f2c0c80dc4 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementFragment.kt @@ -45,7 +45,7 @@ class CreditCardsManagementFragment : SecureFragment() { val view = inflater.inflate(CreditCardsManagementView.LAYOUT_ID, container, false) store = StoreProvider.get(this) { - AutofillFragmentStore(AutofillFragmentState(creditCards = emptyList())) + AutofillFragmentStore(AutofillFragmentState()) } interactor = DefaultCreditCardsManagementInteractor( diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 3c4004fbbb..f32f8b5c03 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -1499,6 +1499,8 @@ Add credit card Manage saved cards + + Add address Manage addresses diff --git a/app/src/test/java/org/mozilla/fenix/settings/autofill/AutofillFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/settings/autofill/AutofillFragmentStoreTest.kt index 4bfd3c24aa..5fe0a0d9dd 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/autofill/AutofillFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/autofill/AutofillFragmentStoreTest.kt @@ -6,6 +6,7 @@ package org.mozilla.fenix.settings.autofill import io.mockk.mockk import kotlinx.coroutines.runBlocking +import mozilla.components.concept.storage.Address import mozilla.components.concept.storage.CreditCard import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -20,7 +21,7 @@ class AutofillFragmentStoreTest { @Before fun setup() { - state = AutofillFragmentState(creditCards = emptyList()) + state = AutofillFragmentState() store = AutofillFragmentStore(state) } @@ -34,4 +35,15 @@ class AutofillFragmentStoreTest { assertEquals(creditCards, store.state.creditCards) assertFalse(store.state.isLoading) } + + @Test + fun `GIVEN a list of addresses WHEN update addresses action is dispatched THEN addresses state is updated`() = runBlocking { + assertTrue(store.state.isLoading) + + val addresses: List
= listOf(mockk(), mockk()) + store.dispatch(AutofillAction.UpdateAddresses(addresses)).join() + + assertEquals(addresses, store.state.addresses) + assertFalse(store.state.isLoading) + } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragmentTest.kt index 681c32ba7b..edf767793f 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragmentTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/autofill/AutofillSettingFragmentTest.kt @@ -11,6 +11,7 @@ import io.mockk.every import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.test.TestCoroutineDispatcher +import mozilla.components.concept.storage.Address import mozilla.components.concept.storage.CreditCard import mozilla.components.support.test.robolectric.testContext import org.junit.Assert.assertEquals @@ -35,6 +36,8 @@ class AutofillSettingFragmentTest { @Before fun setUp() { every { testContext.components.settings } returns mockk(relaxed = true) + every { testContext.components.settings.addressFeature } returns true + every { testContext.components.settings.shouldAutofillCreditCardDetails } returns true autofillSettingFragment = AutofillSettingFragment() @@ -80,7 +83,7 @@ class AutofillSettingFragmentTest { AutofillSettingFragmentDirections .actionAutofillSettingFragmentToCreditCardEditorFragment() - val state = AutofillFragmentState(creditCards = emptyList()) + val state = AutofillFragmentState() val store = AutofillFragmentStore(state) autofillSettingFragment.updateCardManagementPreference( @@ -95,4 +98,44 @@ class AutofillSettingFragmentTest { verify { navController.navigate(directions) } } + + @Test + fun `GIVEN the list of addresses is not empty WHEN fragment is displayed THEN the manage addresses preference label is 'Manage addresses'`() { + val preferenceTitle = + testContext.getString(R.string.preferences_addresses_manage_addresses) + val manageCardsPreference = autofillSettingFragment.findPreference( + autofillSettingFragment.getPreferenceKey(R.string.pref_key_addresses_manage_addresses) + ) + + val addresses: List
= listOf(mockk(), mockk()) + + val state = AutofillFragmentState(addresses = addresses) + val store = AutofillFragmentStore(state) + + autofillSettingFragment.updateAddressPreference( + store.state.addresses.isNotEmpty() + ) + + assertNull(manageCardsPreference?.icon) + assertEquals(preferenceTitle, manageCardsPreference?.title) + } + + @Test + fun `GIVEN the list of addresses is empty WHEN fragment is displayed THEN the manage addresses preference label is 'Add address'`() { + val preferenceTitle = + testContext.getString(R.string.preferences_addresses_add_address) + val manageCardsPreference = autofillSettingFragment.findPreference( + autofillSettingFragment.getPreferenceKey(R.string.pref_key_addresses_manage_addresses) + ) + + val state = AutofillFragmentState() + val store = AutofillFragmentStore(state) + + autofillSettingFragment.updateAddressPreference( + store.state.addresses.isNotEmpty() + ) + + assertNotNull(manageCardsPreference?.icon) + assertEquals(preferenceTitle, manageCardsPreference?.title) + } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementViewTest.kt b/app/src/test/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementViewTest.kt index b4bd9dcb62..cfb5139df3 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementViewTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/creditcards/CreditCardsManagementViewTest.kt @@ -42,7 +42,7 @@ class CreditCardsManagementViewTest { @Test fun testUpdate() { - creditCardsView.update(AutofillFragmentState(creditCards = emptyList())) + creditCardsView.update(AutofillFragmentState()) assertTrue(componentCreditCardsBinding.progressBar.isVisible) assertFalse(componentCreditCardsBinding.creditCardsList.isVisible)