Commit fcdba20e authored by MozLando's avatar MozLando
Browse files

Merge #8149

8149: For mozilla-mobile/fenix#9056: Search with custom tab r=rocketsroger,jonalmeida a=NotWoods

We need to know if a search was started from a custom tab using the tabId, along with setting the tab ID for the search request.

Related Fenix PR: https://github.com/mozilla-mobile/fenix/pull/13903

Co-authored-by: default avatarTiger Oakes <toakes@mozilla.com>
parents aed89094 be0d8903
......@@ -5,7 +5,7 @@
package mozilla.components.feature.search
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.selector.findTabOrCustomTabOrSelectedTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.search.SearchRequest
......@@ -15,18 +15,27 @@ import mozilla.components.concept.engine.search.SearchRequest
* This class uses the [browserStore] to determine when private mode is active, and updates the
* [browserStore] whenever a new search has been requested.
*
* NOTE: this will add [SearchRequest]s to [browserStore.state] when [sendSearch] is called. Client
* NOTE: this will add [SearchRequest]s to [BrowserStore.state] when [sendSearch] is called. Client
* code is responsible for consuming these requests and displaying something to the user.
*
* NOTE: client code is responsible for sending [ContentAction.ConsumeSearchRequestAction]s
* after consuming events. See [SearchFeature] for a component that will handle this for you.
*
* @param browserStore The application's [BrowserStore].
* @param tabId ID of the tab that requests the search, or null to use the selected tab.
*/
class BrowserStoreSearchAdapter(private val browserStore: BrowserStore) : SearchAdapter {
class BrowserStoreSearchAdapter(
private val browserStore: BrowserStore,
private val tabId: String? = null
) : SearchAdapter {
override fun sendSearch(isPrivate: Boolean, text: String) {
val selectedTabId = browserStore.state.selectedTabId ?: return
browserStore.dispatch(ContentAction.UpdateSearchRequestAction(selectedTabId, SearchRequest(isPrivate, text)))
val selectedTabId = tabId ?: browserStore.state.selectedTabId ?: return
browserStore.dispatch(
ContentAction.UpdateSearchRequestAction(selectedTabId, SearchRequest(isPrivate, text))
)
}
override fun isPrivateSession(): Boolean = browserStore.state.selectedTab?.content?.private ?: false
override fun isPrivateSession(): Boolean =
browserStore.state.findTabOrCustomTabOrSelectedTab(tabId)?.content?.private ?: false
}
......@@ -11,7 +11,7 @@ import kotlinx.coroutines.flow.distinctUntilChangedBy
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.mapNotNull
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.state.selector.findTabOrCustomTabOrSelectedTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.search.SearchRequest
import mozilla.components.lib.state.ext.flowScoped
......@@ -23,25 +23,31 @@ import mozilla.components.support.utils.ext.toNullablePair
* then consumes them.
*
* NOTE: that this only consumes SearchRequests, and will not hook up the [store] to a source of
* SearchRequests. See [SelectionActionDelegate] for use in generating SearchRequests.
* SearchRequests. See [mozilla.components.concept.engine.selection.SelectionActionDelegate] for use
* in generating SearchRequests.
*/
class SearchFeature(
private val store: BrowserStore,
private val performSearch: (SearchRequest) -> Unit
private val tabId: String? = null,
private val performSearch: (SearchRequest, tabId: String) -> Unit
) : LifecycleAwareFeature {
private var scope: CoroutineScope? = null
override fun start() {
scope = store.flowScoped { flow ->
flow
.map { it.selectedTab?.content?.searchRequest to it.selectedTabId }
.map { state ->
val tab = state.findTabOrCustomTabOrSelectedTab(tabId)
tab?.content?.searchRequest to tab?.id
}
// Do nothing if searchRequest or sessionId is null
.mapNotNull { pair -> pair.toNullablePair() }
// We may see repeat values if other state changes before we handle the request.
// Filter these out.
.distinctUntilChangedBy { (searchRequest, _) -> searchRequest }
.collect { (searchRequest, sessionId) ->
performSearch(searchRequest)
performSearch(searchRequest, sessionId)
store.dispatch(ContentAction.ConsumeSearchRequestAction(sessionId))
}
}
......
/* 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 mozilla.components.feature.search
import mozilla.components.browser.state.action.ContentAction
import mozilla.components.browser.state.state.BrowserState
import mozilla.components.browser.state.state.createCustomTab
import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.search.SearchRequest
import mozilla.components.support.test.any
import mozilla.components.support.test.mock
import mozilla.components.support.test.whenever
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
private const val SELECTED_TAB_ID = "1"
private const val CUSTOM_TAB_ID = "2"
class BrowserStoreSearchAdapterTest {
private lateinit var browserStore: BrowserStore
private val state = BrowserState(
tabs = listOf(createTab(id = SELECTED_TAB_ID, url = "https://mozilla.org", private = true)),
customTabs = listOf(createCustomTab(id = CUSTOM_TAB_ID, url = "https://firefox.com")),
selectedTabId = SELECTED_TAB_ID
)
@Before
fun setup() {
browserStore = mock()
whenever(browserStore.state).thenReturn(state)
}
@Test
fun `adapter does nothing with null tab`() {
whenever(browserStore.state).thenReturn(BrowserState())
val searchAdapter = BrowserStoreSearchAdapter(browserStore)
searchAdapter.sendSearch(isPrivate = false, text = "normal search")
searchAdapter.sendSearch(isPrivate = true, text = "private search")
verify(browserStore, never()).dispatch(any())
assertFalse(searchAdapter.isPrivateSession())
}
@Test
fun `sendSearch with selected tab`() {
val searchAdapter = BrowserStoreSearchAdapter(browserStore)
searchAdapter.sendSearch(isPrivate = false, text = "normal search")
verify(browserStore).dispatch(
ContentAction.UpdateSearchRequestAction(
SELECTED_TAB_ID,
SearchRequest(isPrivate = false, query = "normal search")
)
)
searchAdapter.sendSearch(isPrivate = true, text = "private search")
verify(browserStore).dispatch(
ContentAction.UpdateSearchRequestAction(
SELECTED_TAB_ID,
SearchRequest(isPrivate = true, query = "private search")
)
)
assertTrue(searchAdapter.isPrivateSession())
}
@Test
fun `sendSearch with custom tab`() {
val searchAdapter = BrowserStoreSearchAdapter(browserStore, CUSTOM_TAB_ID)
searchAdapter.sendSearch(isPrivate = false, text = "normal search")
verify(browserStore).dispatch(
ContentAction.UpdateSearchRequestAction(
CUSTOM_TAB_ID,
SearchRequest(isPrivate = false, query = "normal search")
)
)
searchAdapter.sendSearch(isPrivate = true, text = "private search")
verify(browserStore).dispatch(
ContentAction.UpdateSearchRequestAction(
CUSTOM_TAB_ID,
SearchRequest(isPrivate = true, query = "private search")
)
)
assertFalse(searchAdapter.isPrivateSession())
}
}
......@@ -9,7 +9,9 @@ import mozilla.components.browser.state.state.createTab
import mozilla.components.browser.state.store.BrowserStore
import mozilla.components.concept.engine.search.SearchRequest
import mozilla.components.support.test.any
import mozilla.components.support.test.eq
import mozilla.components.support.test.ext.joinBlocking
import mozilla.components.support.test.libstate.ext.waitUntilIdle
import mozilla.components.support.test.mock
import mozilla.components.support.test.rule.MainCoroutineRule
import org.junit.After
......@@ -20,8 +22,6 @@ import org.junit.Test
import org.mockito.Mockito.times
import org.mockito.Mockito.verify
import mozilla.components.support.test.libstate.ext.waitUntilIdle
private const val SELECTED_TAB_ID = "1"
class SearchFeatureTest {
......@@ -31,7 +31,7 @@ class SearchFeatureTest {
@get:Rule
val coroutinesTestRule = MainCoroutineRule(testDispatcher)
private lateinit var performSearch: (SearchRequest) -> Unit
private lateinit var performSearch: (SearchRequest, String) -> Unit
private lateinit var store: BrowserStore
private lateinit var searchFeature: SearchFeature
......@@ -41,7 +41,7 @@ class SearchFeatureTest {
mockBrowserState()
)
performSearch = mock()
searchFeature = SearchFeature(store, performSearch).apply {
searchFeature = SearchFeature(store, null, performSearch).apply {
start()
}
}
......@@ -64,38 +64,38 @@ class SearchFeatureTest {
@Test
fun `GIVEN a tab is selected WHEN a search request is sent THEN a search should be performed`() {
verify(performSearch, times(0)).invoke(any())
verify(performSearch, times(0)).invoke(any(), eq(SELECTED_TAB_ID))
val normalSearchRequest = SearchRequest(isPrivate = false, query = "query")
store.dispatch(ContentAction.UpdateSearchRequestAction(SELECTED_TAB_ID, normalSearchRequest)).joinBlocking()
verify(performSearch, times(1)).invoke(any())
verify(performSearch, times(1)).invoke(normalSearchRequest)
verify(performSearch, times(1)).invoke(any(), eq(SELECTED_TAB_ID))
verify(performSearch, times(1)).invoke(normalSearchRequest, SELECTED_TAB_ID)
val privateSearchRequest = SearchRequest(isPrivate = true, query = "query")
store.dispatch(ContentAction.UpdateSearchRequestAction(SELECTED_TAB_ID, privateSearchRequest)).joinBlocking()
verify(performSearch, times(2)).invoke(any())
verify(performSearch, times(1)).invoke(privateSearchRequest)
verify(performSearch, times(2)).invoke(any(), eq(SELECTED_TAB_ID))
verify(performSearch, times(1)).invoke(privateSearchRequest, SELECTED_TAB_ID)
}
@Test
fun `GIVEN no tab is selected WHEN a search request is sent THEN no search should be performed`() {
store.dispatch(TabListAction.RemoveTabAction(tabId = SELECTED_TAB_ID, selectParentIfExists = false))
verify(performSearch, times(0)).invoke(any())
verify(performSearch, times(0)).invoke(any(), eq(SELECTED_TAB_ID))
val normalSearchRequest = SearchRequest(isPrivate = false, query = "query")
store.dispatch(ContentAction.UpdateSearchRequestAction(SELECTED_TAB_ID, normalSearchRequest)).joinBlocking()
verify(performSearch, times(0)).invoke(any())
verify(performSearch, times(0)).invoke(normalSearchRequest)
verify(performSearch, times(0)).invoke(any(), eq(SELECTED_TAB_ID))
verify(performSearch, times(0)).invoke(normalSearchRequest, SELECTED_TAB_ID)
val privateSearchRequest = SearchRequest(isPrivate = true, query = "query")
store.dispatch(ContentAction.UpdateSearchRequestAction(SELECTED_TAB_ID, privateSearchRequest)).joinBlocking()
verify(performSearch, times(0)).invoke(any())
verify(performSearch, times(0)).invoke(privateSearchRequest)
verify(performSearch, times(0)).invoke(any(), eq(SELECTED_TAB_ID))
verify(performSearch, times(0)).invoke(privateSearchRequest, SELECTED_TAB_ID)
}
@Test
......
......@@ -12,6 +12,10 @@ permalink: /changelog/
* [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt)
* [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Config.kt)
* **feature-search**
* ⚠️ **This is a breaking change**: `SearchFeature.performSearch` now takes a second parameter.
* `BrowserStoreSearchAdapter` and `SearchFeature` can now take a `tabId` parameter.
# 56.0.0
* [Commits](https://github.com/mozilla-mobile/android-components/compare/v55.0.0...v56.0.0)
......
......@@ -142,10 +142,11 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
)
searchFeature.set(
feature = SearchFeature(components.store) {
when (it.isPrivate) {
true -> components.searchUseCases.newPrivateTabSearch.invoke(it.query)
false -> components.searchUseCases.newTabSearch.invoke(it.query)
feature = SearchFeature(components.store) { request, _ ->
if (request.isPrivate) {
components.searchUseCases.newPrivateTabSearch.invoke(request.query)
} else {
components.searchUseCases.newTabSearch.invoke(request.query)
}
},
owner = this,
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment