diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsFragmentStore.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsFragmentStore.kt index dcb399bfc..9e10508fd 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsFragmentStore.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsFragmentStore.kt @@ -54,6 +54,7 @@ sealed class LoginsAction : Action { data class UpdateLoginsList(val list: List) : LoginsAction() data class UpdateCurrentLogin(val item: SavedLogin) : LoginsAction() data class SortLogins(val sortingStrategy: SortingStrategy) : LoginsAction() + data class LoginSelected(val item: SavedLogin) : LoginsAction() } /** @@ -110,6 +111,13 @@ private fun savedLoginsStateReducer( state ) } + is LoginsAction.LoginSelected -> { + state.copy( + isLoading = true, + loginList = emptyList(), + filteredItems = emptyList() + ) + } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsListViewHolder.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsListViewHolder.kt index 62183ae71..f7bd68faa 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsListViewHolder.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/LoginsListViewHolder.kt @@ -34,7 +34,7 @@ class LoginsListViewHolder( updateFavIcon(item.origin) view.setOnClickListener { - interactor.itemClicked(item) + interactor.onItemClicked(item) } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsFragment.kt index e76800c6f..cec4c1c29 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsFragment.kt @@ -37,12 +37,10 @@ import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.HomeActivity import org.mozilla.fenix.R import org.mozilla.fenix.components.StoreProvider -import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.redirectToReAuth import org.mozilla.fenix.ext.settings import org.mozilla.fenix.ext.showToolbar -import org.mozilla.fenix.settings.SupportUtils @SuppressWarnings("TooManyFunctions") class SavedLoginsFragment : Fragment() { @@ -88,9 +86,14 @@ class SavedLoginsFragment : Fragment() { ) } val savedLoginsController: SavedLoginsController = - SavedLoginsController(savedLoginsStore, requireContext().settings()) - savedLoginsInteractor = - SavedLoginsInteractor(savedLoginsController, ::itemClicked, ::openLearnMore) + SavedLoginsController( + store = savedLoginsStore, + navController = findNavController(), + browserNavigator = ::openToBrowserAndLoad, + settings = requireContext().settings(), + metrics = requireContext().components.analytics.metrics + ) + savedLoginsInteractor = SavedLoginsInteractor(savedLoginsController) savedLoginsView = SavedLoginsView(view.savedLoginsLayout, savedLoginsInteractor) loadAndMapLogins() return view @@ -138,20 +141,8 @@ class SavedLoginsFragment : Fragment() { super.onPause() } - private fun itemClicked(item: SavedLogin) { - context?.components?.analytics?.metrics?.track(Event.OpenOneLogin) - val directions = - SavedLoginsFragmentDirections.actionSavedLoginsFragmentToLoginDetailFragment(item.guid) - findNavController().navigate(directions) - } - - private fun openLearnMore() { - (activity as HomeActivity).openToBrowserAndLoad( - searchTermOrURL = SupportUtils.getGenericSumoURLForTopic - (SupportUtils.SumoTopic.SYNC_SETUP), - newTab = true, - from = BrowserDirection.FromSavedLoginsFragment - ) + private fun openToBrowserAndLoad(searchTermOrURL: String, newTab: Boolean, from: BrowserDirection) { + (activity as HomeActivity).openToBrowserAndLoad(searchTermOrURL, newTab, from) } private fun loadAndMapLogins() { @@ -222,11 +213,15 @@ class SavedLoginsFragment : Fragment() { sortingStrategyMenu = SavedLoginsSortingStrategyMenu(requireContext(), itemToHighlight) { when (it) { SavedLoginsSortingStrategyMenu.Item.AlphabeticallySort -> { - savedLoginsInteractor.sort(SortingStrategy.Alphabetically(requireContext().applicationContext)) + savedLoginsInteractor.onSortingStrategyChanged( + SortingStrategy.Alphabetically(requireContext().applicationContext) + ) } SavedLoginsSortingStrategyMenu.Item.LastUsedSort -> { - savedLoginsInteractor.sort(SortingStrategy.LastUsed(requireContext().applicationContext)) + savedLoginsInteractor.onSortingStrategyChanged( + SortingStrategy.LastUsed(requireContext().applicationContext) + ) } } } diff --git a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsView.kt b/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsView.kt index 3f83360f4..dd1564df3 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsView.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/logins/SavedLoginsView.kt @@ -9,11 +9,16 @@ import android.view.LayoutInflater import android.view.ViewGroup import android.widget.FrameLayout import androidx.core.view.isVisible +import androidx.navigation.NavController import androidx.recyclerview.widget.LinearLayoutManager import kotlinx.android.extensions.LayoutContainer import kotlinx.android.synthetic.main.component_saved_logins.view.* +import org.mozilla.fenix.BrowserDirection import org.mozilla.fenix.R +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.ext.addUnderline +import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.utils.Settings /** @@ -40,7 +45,7 @@ class SavedLoginsView( with(view.saved_passwords_empty_learn_more) { movementMethod = LinkMovementMethod.getInstance() addUnderline() - setOnClickListener { interactor.onLearnMore() } + setOnClickListener { interactor.onLearnMoreClicked() } } with(view.saved_passwords_empty_message) { @@ -54,6 +59,7 @@ class SavedLoginsView( } fun update(state: LoginsListState) { + // todo MVI views should not have logic. Needs refactoring. if (state.isLoading) { view.progress_bar.isVisible = true } else { @@ -67,29 +73,63 @@ class SavedLoginsView( /** * Interactor for the saved logins screen + * + * @param savedLoginsController [SavedLoginsController] which will be delegated for all users interactions. */ class SavedLoginsInteractor( - private val savedLoginsController: SavedLoginsController, - private val itemClicked: (SavedLogin) -> Unit, - private val learnMore: () -> Unit + private val savedLoginsController: SavedLoginsController ) { - fun itemClicked(item: SavedLogin) { - itemClicked.invoke(item) + fun onItemClicked(item: SavedLogin) { + savedLoginsController.handleItemClicked(item) } - fun onLearnMore() { - learnMore.invoke() + + fun onLearnMoreClicked() { + savedLoginsController.handleLearnMoreClicked() } - fun sort(sortingStrategy: SortingStrategy) { + + fun onSortingStrategyChanged(sortingStrategy: SortingStrategy) { savedLoginsController.handleSort(sortingStrategy) } } /** * Controller for the saved logins screen + * + * @param store Store used to hold in-memory collection state. + * @param navController NavController manages app navigation within a NavHost. + * @param browserNavigator Controller allowing browser navigation to any Uri. + * @param settings SharedPreferences wrapper for easier usage. + * @param metrics Controller that handles telemetry events. */ -class SavedLoginsController(val store: LoginsFragmentStore, val settings: Settings) { +class SavedLoginsController( + private val store: LoginsFragmentStore, + private val navController: NavController, + private val browserNavigator: ( + searchTermOrURL: String, + newTab: Boolean, + from: BrowserDirection + ) -> Unit, + private val settings: Settings, + private val metrics: MetricController +) { fun handleSort(sortingStrategy: SortingStrategy) { store.dispatch(LoginsAction.SortLogins(sortingStrategy)) settings.savedLoginsSortingStrategy = sortingStrategy } + + fun handleItemClicked(item: SavedLogin) { + store.dispatch(LoginsAction.LoginSelected(item)) + metrics.track(Event.OpenOneLogin) + navController.navigate( + SavedLoginsFragmentDirections.actionSavedLoginsFragmentToLoginDetailFragment(item.guid) + ) + } + + fun handleLearnMoreClicked() { + browserNavigator.invoke( + SupportUtils.getGenericSumoURLForTopic(SupportUtils.SumoTopic.SYNC_SETUP), + true, + BrowserDirection.FromSavedLoginsFragment + ) + } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsFragmentStoreTest.kt b/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsFragmentStoreTest.kt index 236023dfd..56fc68d6c 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsFragmentStoreTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsFragmentStoreTest.kt @@ -10,6 +10,7 @@ import mozilla.components.support.test.ext.joinBlocking import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Test class LoginsFragmentStoreTest { @@ -123,4 +124,19 @@ class LoginsFragmentStoreTest { assertEquals("example", store.state.searchedForText) assertEquals(listOf(exampleLogin), store.state.filteredItems) } + + @Test + fun `LoginSelected action`() { + val store = LoginsFragmentStore(baseState.copy( + isLoading = false, + loginList = listOf(mockk()), + filteredItems = listOf(mockk()) + )) + + store.dispatch(LoginsAction.LoginSelected(mockk())).joinBlocking() + + assertTrue(store.state.isLoading) + assertTrue(store.state.loginList.isEmpty()) + assertTrue(store.state.filteredItems.isEmpty()) + } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsListViewHolderTest.kt b/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsListViewHolderTest.kt index 7509bb10a..3f15ad9fe 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsListViewHolderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/logins/LoginsListViewHolderTest.kt @@ -52,6 +52,6 @@ class LoginsListViewHolderTest { holder.bind(baseLogin) view.performClick() - verify { interactor.itemClicked(baseLogin) } + verify { interactor.onItemClicked(baseLogin) } } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsControllerTest.kt b/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsControllerTest.kt index 62f27e507..ea0e0f418 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsControllerTest.kt @@ -4,20 +4,30 @@ package org.mozilla.fenix.settings.logins +import androidx.navigation.NavController import io.mockk.mockk import io.mockk.verify +import io.mockk.verifyAll import mozilla.components.support.test.robolectric.testContext import org.junit.Test import org.junit.runner.RunWith +import org.mozilla.fenix.BrowserDirection +import org.mozilla.fenix.components.metrics.Event +import org.mozilla.fenix.components.metrics.MetricController import org.mozilla.fenix.helpers.FenixRobolectricTestRunner +import org.mozilla.fenix.settings.SupportUtils import org.mozilla.fenix.utils.Settings @RunWith(FenixRobolectricTestRunner::class) class SavedLoginsControllerTest { private val store: LoginsFragmentStore = mockk(relaxed = true) + private val navController: NavController = mockk(relaxed = true) + private val browserNavigator: (String, Boolean, BrowserDirection) -> Unit = mockk(relaxed = true) + private val settings: Settings = mockk(relaxed = true) + private val metrics: MetricController = mockk(relaxed = true) private val sortingStrategy: SortingStrategy = SortingStrategy.Alphabetically(testContext) - private val controller = SavedLoginsController(store, settings) + private val controller = SavedLoginsController(store, navController, browserNavigator, settings, metrics) @Test fun `GIVEN a sorting strategy, WHEN handleSort is called on the controller, THEN the correct action should be dispatched and the strategy saved in sharedPref`() { @@ -34,4 +44,32 @@ class SavedLoginsControllerTest { settings.savedLoginsSortingStrategy = sortingStrategy } } + + @Test + fun `GIVEN a SavedLogin, WHEN handleItemClicked is called for it, THEN LoginsAction$LoginSelected should be emitted`() { + val login: SavedLogin = mockk(relaxed = true) + + controller.handleItemClicked(login) + + verifyAll { + store.dispatch(LoginsAction.LoginSelected(login)) + metrics.track(Event.OpenOneLogin) + navController.navigate( + SavedLoginsFragmentDirections.actionSavedLoginsFragmentToLoginDetailFragment(login.guid) + ) + } + } + + @Test + fun `GIVEN the learn more option, WHEN handleLearnMoreClicked is called for it, then we should open the right support webpage`() { + controller.handleLearnMoreClicked() + + verify { + browserNavigator.invoke( + SupportUtils.getGenericSumoURLForTopic(SupportUtils.SumoTopic.SYNC_SETUP), + true, + BrowserDirection.FromSavedLoginsFragment + ) + } + } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsInteractorTest.kt b/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsInteractorTest.kt index b9db6f70b..7e56dd379 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsInteractorTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/logins/SavedLoginsInteractorTest.kt @@ -5,7 +5,7 @@ package org.mozilla.fenix.settings.logins import io.mockk.mockk -import io.mockk.verify +import io.mockk.verifyAll import mozilla.components.support.test.robolectric.testContext import org.junit.Test import org.junit.runner.RunWith @@ -15,32 +15,35 @@ import kotlin.random.Random @RunWith(FenixRobolectricTestRunner::class) class SavedLoginsInteractorTest { private val controller: SavedLoginsController = mockk(relaxed = true) - private val savedLoginClicked: (SavedLogin) -> Unit = mockk(relaxed = true) - private val learnMore: () -> Unit = mockk(relaxed = true) - private val interactor = SavedLoginsInteractor( - controller, - savedLoginClicked, - learnMore - ) + private val interactor = SavedLoginsInteractor(controller) @Test - fun itemClicked() { + fun `GIVEN a SavedLogin being clicked, WHEN the interactor is called for it, THEN it should just delegate the controller`() { val item = SavedLogin("mozilla.org", "username", "password", "id", Random.nextLong()) - interactor.itemClicked(item) + interactor.onItemClicked(item) - verify { - savedLoginClicked.invoke(item) + verifyAll { + controller.handleItemClicked(item) } } @Test - fun `GIVEN a sorting strategy, WHEN sort method is called on the interactor, THEN controller should call handleSort with the same parameter`() { + fun `GIVEN a change in sorting strategy, WHEN the interactor is called for it, THEN it should just delegate the controller`() { val sortingStrategy: SortingStrategy = SortingStrategy.Alphabetically(testContext) - interactor.sort(sortingStrategy) + interactor.onSortingStrategyChanged(sortingStrategy) - verify { + verifyAll { controller.handleSort(sortingStrategy) } } + + @Test + fun `GIVEN the learn more option is clicked, WHEN the interactor is called for it, THEN it should just delegate the controller`() { + interactor.onLearnMoreClicked() + + verifyAll { + controller.handleLearnMoreClicked() + } + } }