From 14875c61294bdcbba236cce6273f20816b1725d0 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Wed, 21 Dec 2022 18:01:45 +0200 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/28273 - Configure search with the selected search engine before user searching --- .../fenix/search/SearchDialogFragment.kt | 25 ++++++- .../fenix/search/SearchDialogFragmentTest.kt | 75 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt index 02accb89bd..658496155c 100644 --- a/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/search/SearchDialogFragment.kt @@ -105,7 +105,7 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { private var _binding: FragmentSearchDialogBinding? = null private val binding get() = _binding!! - private lateinit var interactor: SearchDialogInteractor + @VisibleForTesting internal lateinit var interactor: SearchDialogInteractor private lateinit var store: SearchDialogFragmentStore private lateinit var toolbarView: ToolbarView private lateinit var inlineAutocompleteEditText: InlineAutocompleteEditText @@ -266,6 +266,9 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { requireComponents.core.engine.speculativeCreateSession(isPrivate) + // Handle the scenario in which the user selects another search engine before starting a search. + maybeSelectShortcutEngine(args.searchEngine) + when (getPreviousDestination()?.destination?.id) { R.id.homeFragment -> { // When displayed above home, dispatches the touch events to scrim area to the HomeFragment @@ -477,6 +480,26 @@ class SearchDialogFragment : AppCompatDialogFragment(), UserInteractionHandler { } } + /** + * Check whether the search engine identified by [selectedSearchEngineId] is the default search engine + * and if not update the search state to reflect that a different search engine is currently selected. + * + * @param selectedSearchEngineId Id of the search engine currently selected for next searches. + */ + @VisibleForTesting + internal fun maybeSelectShortcutEngine(selectedSearchEngineId: String?) { + if (selectedSearchEngineId == null) return + + val searchState = requireComponents.core.store.state.search + searchState.searchEngines.firstOrNull { + it.id == selectedSearchEngineId + }?.let { selectedSearchEngine -> + if (selectedSearchEngine != searchState.selectedOrDefaultSearchEngine) { + interactor.onSearchShortcutEngineSelected(selectedSearchEngine) + } + } + } + private fun isTouchingPrivateButton(x: Float, y: Float): Boolean { val view = parentFragmentManager.primaryNavigationFragment?.view?.findViewInHierarchy { it.id == R.id.privateBrowsingButton diff --git a/app/src/test/java/org/mozilla/fenix/search/SearchDialogFragmentTest.kt b/app/src/test/java/org/mozilla/fenix/search/SearchDialogFragmentTest.kt index da26a21028..e2cff7b995 100644 --- a/app/src/test/java/org/mozilla/fenix/search/SearchDialogFragmentTest.kt +++ b/app/src/test/java/org/mozilla/fenix/search/SearchDialogFragmentTest.kt @@ -8,15 +8,25 @@ import androidx.fragment.app.Fragment import androidx.navigation.NavBackStackEntry import androidx.navigation.NavController import androidx.navigation.fragment.findNavController +import io.mockk.Called +import io.mockk.Runs import io.mockk.every +import io.mockk.just import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.unmockkStatic +import io.mockk.verify +import mozilla.components.browser.state.search.SearchEngine +import mozilla.components.browser.state.state.SearchState +import mozilla.components.browser.state.state.searchEngines +import mozilla.components.browser.state.state.selectedOrDefaultSearchEngine import org.junit.After import org.junit.Assert.assertNull import org.junit.Assert.assertSame import org.junit.Before import org.junit.Test +import org.mozilla.fenix.components.Components +import org.mozilla.fenix.ext.requireComponents internal class SearchDialogFragmentTest { private val navController: NavController = mockk() @@ -78,6 +88,71 @@ internal class SearchDialogFragmentTest { assertSame(fragmentADestination, fragment.getPreviousDestination()) } + + @Test + fun `GIVEN the default search engine is currently selected WHEN checking the need to update the current search engine THEN don't to anything`() { + fragment.interactor = mockk() + val defaultSearchEngine: SearchEngine = mockk { + every { id } returns "default" + } + val otherSearchEngine: SearchEngine = mockk { + every { id } returns "other" + } + val components: Components = mockk { + every { core.store.state.search } returns mockk(relaxed = true) { + every { searchEngines } returns listOf(otherSearchEngine, defaultSearchEngine) + } + } + mockkStatic( + "org.mozilla.fenix.ext.FragmentKt", + "mozilla.components.browser.state.state.SearchStateKt", + ) { + every { any().requireComponents } returns components + every { any().selectedOrDefaultSearchEngine } returns defaultSearchEngine + + fragment.maybeSelectShortcutEngine(defaultSearchEngine.id) + + verify { fragment.interactor wasNot Called } + } + } + + @Test + fun `GIVEN the default search engine is not currently selected WHEN checking the need to update the current search engine THEN update it to the current engine`() { + fragment.interactor = mockk { + every { onSearchShortcutEngineSelected(any()) } just Runs + } + val defaultSearchEngine: SearchEngine = mockk { + every { id } returns "default" + } + val otherSearchEngine: SearchEngine = mockk { + every { id } returns "other" + } + val components: Components = mockk { + every { core.store.state.search } returns mockk(relaxed = true) { + every { searchEngines } returns listOf(otherSearchEngine, defaultSearchEngine) + } + } + mockkStatic( + "org.mozilla.fenix.ext.FragmentKt", + "mozilla.components.browser.state.state.SearchStateKt", + ) { + every { any().requireComponents } returns components + every { any().selectedOrDefaultSearchEngine } returns defaultSearchEngine + + fragment.maybeSelectShortcutEngine(otherSearchEngine.id) + + verify { fragment.interactor.onSearchShortcutEngineSelected(otherSearchEngine) } + } + } + + @Test + fun `GIVEN the currently selected search engine is unknown WHEN checking the need to update the current search engine THEN don't do anything`() { + fragment.interactor = mockk() + + fragment.maybeSelectShortcutEngine(null) + + verify { fragment.interactor wasNot Called } + } } private val fragmentName = SearchDialogFragment::class.java.canonicalName?.substringAfterLast('.')!!