From 8eaf1f7a82bceb47aeb6d36d0b54700a44300f37 Mon Sep 17 00:00:00 2001 From: Sebastian Kaspari Date: Fri, 22 Jan 2021 19:07:44 +0100 Subject: [PATCH] [fenix] Revert "For https://github.com/mozilla-mobile/fenix/issues/17044: Explicitly set a new default engine when default is deleted." This reverts commit 09f8e82e35f56b35865a67fd4dd4a9b0997b923d. --- .../search/RadioSearchEngineListPreference.kt | 52 ++-------- .../RadioSearchEngineListPreferenceTest.kt | 96 ------------------- 2 files changed, 10 insertions(+), 138 deletions(-) delete mode 100644 app/src/test/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreferenceTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt b/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt index b1c13bcc26..caea80564f 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreference.kt @@ -14,7 +14,6 @@ import android.view.ViewGroup import android.widget.CompoundButton import android.widget.LinearLayout import android.widget.RadioGroup -import androidx.annotation.VisibleForTesting import androidx.core.view.isVisible import androidx.navigation.Navigation import androidx.preference.Preference @@ -63,13 +62,12 @@ class RadioSearchEngineListPreference @JvmOverloads constructor( } @OptIn(ExperimentalCoroutinesApi::class) - private fun subscribeToSearchEngineUpdates(store: BrowserStore, view: View) = - view.toScope().launch { - store.flow() - .map { state -> state.search } - .ifChanged() - .collect { state -> refreshSearchEngineViews(view, state) } - } + private fun subscribeToSearchEngineUpdates(store: BrowserStore, view: View) = view.toScope().launch { + store.flow() + .map { state -> state.search } + .ifChanged() + .collect { state -> refreshSearchEngineViews(view, state) } + } private fun refreshSearchEngineViews(view: View, state: SearchState) { val searchEngineGroup = view.findViewById(R.id.search_engine_group) @@ -120,8 +118,7 @@ class RadioSearchEngineListPreference @JvmOverloads constructor( is SearchEngineMenu.Item.Edit -> editCustomSearchEngine(wrapper, engine) is SearchEngineMenu.Item.Delete -> deleteSearchEngine( context, - engine, - engine == context.components.core.store.state.search.selectedOrDefaultSearchEngine + engine ) } } @@ -152,50 +149,21 @@ class RadioSearchEngineListPreference @JvmOverloads constructor( Navigation.findNavController(view).navigate(directions) } - @VisibleForTesting - internal fun deleteSearchEngine( + private fun deleteSearchEngine( context: Context, - engine: SearchEngine, - isDefaultSearchEngine: Boolean + engine: SearchEngine ) { context.components.useCases.searchUseCases.removeSearchEngine(engine) - if (isDefaultSearchEngine) { - context.components.useCases.searchUseCases.selectSearchEngine( - context.components.core.store.state.search.searchEngines.first { - it.id != engine.id - } - ) - } - showUndoSnackbar(context, engine, isDefaultSearchEngine) - } - - @VisibleForTesting - internal fun showUndoSnackbar( - context: Context, - engine: SearchEngine, - isDefaultSearchEngine: Boolean - ) { MainScope().allowUndo( view = context.getRootView()!!, message = context .getString(R.string.search_delete_search_engine_success_message, engine.name), undoActionTitle = context.getString(R.string.snackbar_deleted_undo), onCancel = { - restoreSearchEngine(engine, isDefaultSearchEngine) + context.components.useCases.searchUseCases.addSearchEngine(engine) }, operation = {} ) } - - @VisibleForTesting - internal fun restoreSearchEngine( - engine: SearchEngine, - isDefaultSearchEngine: Boolean - ) { - context.components.useCases.searchUseCases.addSearchEngine(engine) - if (isDefaultSearchEngine) { - context.components.useCases.searchUseCases.selectSearchEngine(engine) - } - } } diff --git a/app/src/test/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreferenceTest.kt b/app/src/test/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreferenceTest.kt deleted file mode 100644 index a7e3eddf5f..0000000000 --- a/app/src/test/java/org/mozilla/fenix/settings/search/RadioSearchEngineListPreferenceTest.kt +++ /dev/null @@ -1,96 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.settings.search - -import android.content.Context -import io.mockk.Runs -import io.mockk.every -import io.mockk.just -import io.mockk.mockk -import io.mockk.spyk -import io.mockk.verify -import mozilla.components.browser.state.search.SearchEngine -import mozilla.components.browser.state.state.SearchState -import mozilla.components.feature.search.SearchUseCases -import org.junit.Before -import org.junit.Test -import org.junit.runner.RunWith -import org.mozilla.fenix.ext.components -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner - -@RunWith(FenixRobolectricTestRunner::class) -class RadioSearchEngineListPreferenceTest { - - private lateinit var radioSearchEngineListPreference: RadioSearchEngineListPreference - private lateinit var searchUseCases: SearchUseCases - private lateinit var searchState: SearchState - private lateinit var testContext: Context - private lateinit var defaultSearchEngine: SearchEngine - private lateinit var otherSearchEngine: SearchEngine - - @Before - fun before() { - testContext = mockk(relaxed = true) - searchUseCases = mockk(relaxed = true) - - defaultSearchEngine = mockk { - every { id } returns "default" - } - otherSearchEngine = mockk { - every { id } returns "other" - } - val engineList = listOf(defaultSearchEngine, otherSearchEngine) - - searchState = SearchState(customSearchEngines = engineList) - - every { testContext.components.useCases.searchUseCases } returns searchUseCases - every { testContext.components.core.store.state.search } returns searchState - - radioSearchEngineListPreference = spyk(RadioSearchEngineListPreference(testContext)) - } - - @Test - fun `when deleting default search engine a new one is selected`() { - every { radioSearchEngineListPreference.showUndoSnackbar(any(), any(), any()) } just Runs - - radioSearchEngineListPreference.deleteSearchEngine( - testContext, - defaultSearchEngine, - true - ) - - verify { searchUseCases.removeSearchEngine(defaultSearchEngine) } - verify { searchUseCases.selectSearchEngine(otherSearchEngine) } - verify { - radioSearchEngineListPreference.showUndoSnackbar( - testContext, - defaultSearchEngine, - true - ) - } - } - - @Test - fun `restoreSearchEngine ads engine and makes it default if it was the default before deletion`() { - radioSearchEngineListPreference.restoreSearchEngine( - defaultSearchEngine, - true - ) - - verify { searchUseCases.addSearchEngine(defaultSearchEngine) } - verify { searchUseCases.selectSearchEngine(defaultSearchEngine) } - } - - @Test - fun `restoreSearchEngine ads engine and it doe NOT make it default if it was NOT the default before deletion`() { - radioSearchEngineListPreference.restoreSearchEngine( - otherSearchEngine, - false - ) - - verify { searchUseCases.addSearchEngine(otherSearchEngine) } - verify(exactly = 0) { searchUseCases.selectSearchEngine(defaultSearchEngine) } - } -}