From 90c66beb6e7e6e33b4b309a7870b7595fb557e16 Mon Sep 17 00:00:00 2001 From: Marcos Holgado Date: Thu, 20 May 2021 12:32:09 +0100 Subject: [PATCH 1/5] Add nav parameter to autocomplete --- .../autocomplete/api/AutoCompleteApiTest.kt | 18 ++++---- .../app/browser/BrowserTabViewModelTest.kt | 2 +- .../app/browser/BrowserViewModelTest.kt | 2 +- .../app/browser/QueryUrlConverterTest.kt | 43 +++++++++++++++++++ .../app/autocomplete/api/AutoComplete.kt | 5 +-- .../autocomplete/api/AutoCompleteService.kt | 5 ++- .../app/browser/BrowserTabFragment.kt | 7 ++- .../app/browser/BrowserTabViewModel.kt | 5 ++- .../browser/omnibar/OmnibarEntryConverter.kt | 7 ++- .../app/browser/omnibar/QueryUrlConverter.kt | 9 +++- 10 files changed, 81 insertions(+), 22 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/autocomplete/api/AutoCompleteApiTest.kt b/app/src/androidTest/java/com/duckduckgo/app/autocomplete/api/AutoCompleteApiTest.kt index ba00a6312009..1f159d0deb11 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/autocomplete/api/AutoCompleteApiTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/autocomplete/api/AutoCompleteApiTest.kt @@ -105,10 +105,10 @@ class AutoCompleteApiTest { whenever(mockAutoCompleteService.autoComplete("title")).thenReturn( Observable.just( listOf( - AutoCompleteServiceRawResult("example.com"), - AutoCompleteServiceRawResult("foo.com"), - AutoCompleteServiceRawResult("bar.com"), - AutoCompleteServiceRawResult("baz.com") + AutoCompleteServiceRawResult("example.com", false), + AutoCompleteServiceRawResult("foo.com", true), + AutoCompleteServiceRawResult("bar.com", true), + AutoCompleteServiceRawResult("baz.com", true) ) ) ) @@ -143,10 +143,10 @@ class AutoCompleteApiTest { whenever(mockAutoCompleteService.autoComplete("title")).thenReturn( Observable.just( listOf( - AutoCompleteServiceRawResult("example.com"), - AutoCompleteServiceRawResult("foo.com"), - AutoCompleteServiceRawResult("bar.com"), - AutoCompleteServiceRawResult("baz.com") + AutoCompleteServiceRawResult("example.com", false), + AutoCompleteServiceRawResult("foo.com", true), + AutoCompleteServiceRawResult("bar.com", true), + AutoCompleteServiceRawResult("baz.com", true) ) ) ) @@ -167,7 +167,7 @@ class AutoCompleteApiTest { listOf( AutoComplete.AutoCompleteSuggestion.AutoCompleteBookmarkSuggestion(phrase = "foo.com?key=value", "title foo", "https://foo.com?key=value"), AutoComplete.AutoCompleteSuggestion.AutoCompleteBookmarkSuggestion(phrase = "foo.com", "title foo", "https://foo.com"), - AutoComplete.AutoCompleteSuggestion.AutoCompleteSearchSuggestion(phrase = "example.com", true), + AutoComplete.AutoCompleteSuggestion.AutoCompleteSearchSuggestion(phrase = "example.com", false), AutoComplete.AutoCompleteSuggestion.AutoCompleteSearchSuggestion(phrase = "bar.com", true), AutoComplete.AutoCompleteSuggestion.AutoCompleteSearchSuggestion(phrase = "baz.com", true) ), diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index e2e70abe3d69..3a299c43db50 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -329,7 +329,7 @@ class BrowserTabViewModelTest { val siteFactory = SiteFactory(mockPrivacyPractices, mockEntityLookup) - whenever(mockOmnibarConverter.convertQueryToUrl(any(), any())).thenReturn("duckduckgo.com") + whenever(mockOmnibarConverter.convertQueryToUrl(any(), any(), any())).thenReturn("duckduckgo.com") whenever(mockVariantManager.getVariant()).thenReturn(DEFAULT_VARIANT) whenever(mockTabRepository.liveSelectedTab).thenReturn(selectedTabLiveData) whenever(mockNavigationAwareLoginDetector.loginEventLiveData).thenReturn(loginEventLiveData) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt index f8cfd17b7244..1d4aa0d68446 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt @@ -111,7 +111,7 @@ class BrowserViewModelTest { runBlocking { whenever(mockTabRepository.add()).thenReturn(TAB_ID) - whenever(mockOmnibarEntryConverter.convertQueryToUrl(any(), any())).then { it.arguments.first() } + whenever(mockOmnibarEntryConverter.convertQueryToUrl(any(), any(), any())).then { it.arguments.first() } } } diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt index a02a62778b59..d99529024575 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt @@ -17,6 +17,7 @@ package com.duckduckgo.app.browser import android.net.Uri +import com.duckduckgo.app.browser.omnibar.QueryOrigin import com.duckduckgo.app.browser.omnibar.QueryUrlConverter import com.duckduckgo.app.referral.AppReferrerDataStore import com.duckduckgo.app.statistics.VariantManager @@ -80,6 +81,48 @@ class QueryUrlConverterTest { assertEquals(expected, result) } + @Test + fun whenQueryOriginIsFromUserAndIsQueryThenSearchQueryBuilt() { + val input = "foo" + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromUser) + assertDuckDuckGoSearchQuery("foo", result) + } + + @Test + fun whenQueryOriginIsFromUserAndIsUrlThenUrlReturned() { + val input = "http://example.com" + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromUser) + assertEquals(input, result) + } + + @Test + fun whenQueryOriginIsFromAutocompleteAndNavIsFalseThenSearchQueryBuilt() { + val input = "example.com" + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(nav = false)) + assertDuckDuckGoSearchQuery("example.com", result) + } + + @Test + fun whenQueryOriginIsFromAutocompleteAndNavIsTrueThenUrlReturned() { + val input = "http://example.com" + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(nav = true)) + assertEquals(input, result) + } + + @Test + fun whenQueryOriginIsFromAutocompleteAndNavIsNullAndIsUrlThenUrlReturned() { + val input = "http://example.com" + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(nav = null)) + assertEquals(input, result) + } + + @Test + fun whenQueryOriginIsFromAutocompleteAndNavIsNullAndIsNotUrlThenSearchQueryBuilt() { + val input = "foo" + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(nav = null)) + assertDuckDuckGoSearchQuery("foo", result) + } + private fun assertDuckDuckGoSearchQuery(query: String, url: String) { val uri = Uri.parse(url) assertEquals("duckduckgo.com", uri.host) diff --git a/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoComplete.kt b/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoComplete.kt index f38c7bcd0989..2c8cc5efc771 100644 --- a/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoComplete.kt +++ b/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoComplete.kt @@ -26,7 +26,6 @@ import com.duckduckgo.app.global.UriString import com.duckduckgo.app.global.baseHost import com.duckduckgo.app.global.toStringDropScheme import io.reactivex.Observable -import io.reactivex.functions.BiFunction import javax.inject.Inject interface AutoComplete { @@ -59,7 +58,7 @@ class AutoCompleteApi @Inject constructor( return getAutoCompleteBookmarkResults(query).zipWith( getAutoCompleteSearchResults(query), - BiFunction { bookmarksResults, searchResults -> + { bookmarksResults, searchResults -> AutoCompleteResult( query = query, suggestions = (bookmarksResults + searchResults).distinctBy { it.phrase } @@ -72,7 +71,7 @@ class AutoCompleteApi @Inject constructor( autoCompleteService.autoComplete(query) .flatMapIterable { it } .map { - AutoCompleteSearchSuggestion(phrase = it.phrase, isUrl = UriString.isWebUrl(it.phrase)) + AutoCompleteSearchSuggestion(phrase = it.phrase, isUrl = (it.nav ?: UriString.isWebUrl(it.phrase))) } .toList() .onErrorReturn { emptyList() } diff --git a/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoCompleteService.kt b/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoCompleteService.kt index 1ef7a4be2254..1de5925b783d 100644 --- a/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoCompleteService.kt +++ b/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoCompleteService.kt @@ -27,8 +27,9 @@ interface AutoCompleteService { @GET("${AppUrl.Url.API}/ac/") fun autoComplete( @Query("q") query: String, - @Query("kl") languageCode: String = Locale.getDefault().language + @Query("kl") languageCode: String = Locale.getDefault().language, + @Query("nav") nav: String = "1" ): Observable> } -data class AutoCompleteServiceRawResult(val phrase: String) +data class AutoCompleteServiceRawResult(val phrase: String, val nav: Boolean?) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 08395727f019..f7406b361a44 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -79,6 +79,7 @@ import com.duckduckgo.app.browser.model.BasicAuthenticationRequest import com.duckduckgo.app.browser.model.LongPressTarget import com.duckduckgo.app.browser.omnibar.KeyboardAwareEditText import com.duckduckgo.app.browser.omnibar.OmnibarScrolling +import com.duckduckgo.app.browser.omnibar.QueryOrigin import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.browser.shortcut.ShortcutBuilder import com.duckduckgo.app.browser.tabpreview.WebViewPreviewGenerator @@ -968,7 +969,11 @@ class BrowserTabFragment : GlobalScope.launch { viewModel.fireAutocompletePixel(suggestion) withContext(Dispatchers.Main) { - viewModel.onUserSubmittedQuery(suggestion.phrase) + val origin = when (suggestion) { + is AutoCompleteSuggestion.AutoCompleteBookmarkSuggestion -> QueryOrigin.FromAutocomplete(nav = null) + is AutoCompleteSuggestion.AutoCompleteSearchSuggestion -> QueryOrigin.FromAutocomplete(nav = suggestion.isUrl) + } + viewModel.onUserSubmittedQuery(suggestion.phrase, origin) } } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 974884bbe8af..bd3501ca4593 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -62,6 +62,7 @@ import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials import com.duckduckgo.app.browser.model.BasicAuthenticationRequest import com.duckduckgo.app.browser.model.LongPressTarget import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter +import com.duckduckgo.app.browser.omnibar.QueryOrigin import com.duckduckgo.app.browser.omnibar.QueryUrlConverter import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.browser.ui.HttpAuthenticationDialogFragment.HttpAuthenticationListener @@ -531,7 +532,7 @@ class BrowserTabViewModel( pixel.fire(pixelName, params) } - fun onUserSubmittedQuery(query: String) { + fun onUserSubmittedQuery(query: String, queryOrigin: QueryOrigin = QueryOrigin.FromUser) { navigationAwareLoginDetector.onEvent(NavigationEvent.UserAction.NewQuerySubmitted) if (query.isBlank()) { @@ -551,7 +552,7 @@ class BrowserTabViewModel( } val verticalParameter = extractVerticalParameter(url) - val urlToNavigate = queryUrlConverter.convertQueryToUrl(trimmedInput, verticalParameter) + val urlToNavigate = queryUrlConverter.convertQueryToUrl(trimmedInput, verticalParameter, queryOrigin) val type = specialUrlDetector.determineType(trimmedInput) if (type is IntentType) { diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarEntryConverter.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarEntryConverter.kt index 3ab5da30b41c..dc919831b4bf 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarEntryConverter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarEntryConverter.kt @@ -18,5 +18,10 @@ package com.duckduckgo.app.browser.omnibar interface OmnibarEntryConverter { - fun convertQueryToUrl(searchQuery: String, vertical: String? = null): String + fun convertQueryToUrl(searchQuery: String, vertical: String? = null, queryOrigin: QueryOrigin = QueryOrigin.FromUser): String +} + +sealed class QueryOrigin { + object FromUser : QueryOrigin() + data class FromAutocomplete(val nav: Boolean?) : QueryOrigin() } diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt index d46fec812c91..f1f795e08245 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt @@ -28,8 +28,13 @@ import javax.inject.Inject class QueryUrlConverter @Inject constructor(private val requestRewriter: RequestRewriter) : OmnibarEntryConverter { - override fun convertQueryToUrl(searchQuery: String, vertical: String?): String { - if (UriString.isWebUrl(searchQuery)) { + override fun convertQueryToUrl(searchQuery: String, vertical: String?, queryOrigin: QueryOrigin): String { + val isUrl = when (queryOrigin) { + is QueryOrigin.FromAutocomplete -> queryOrigin.nav ?: UriString.isWebUrl(searchQuery) + is QueryOrigin.FromUser -> UriString.isWebUrl(searchQuery) + } + + if (isUrl) { return convertUri(searchQuery) } From fa5a6e24ff03004b191fafa938e738803f3f5bf6 Mon Sep 17 00:00:00 2001 From: Marcos Holgado Date: Thu, 20 May 2021 14:13:20 +0100 Subject: [PATCH 2/5] Amend code as per PR comments --- .../java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 7 ++++--- .../duckduckgo/app/browser/omnibar/QueryUrlConverter.kt | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index f7406b361a44..6cdbb8d7fd6d 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -58,6 +58,7 @@ import androidx.fragment.app.transaction import androidx.lifecycle.* import androidx.recyclerview.widget.LinearLayoutManager import com.duckduckgo.app.autocomplete.api.AutoComplete.AutoCompleteSuggestion +import com.duckduckgo.app.autocomplete.api.AutoComplete.AutoCompleteSuggestion.* import com.duckduckgo.app.bookmarks.ui.EditBookmarkDialogFragment import com.duckduckgo.app.brokensite.BrokenSiteActivity import com.duckduckgo.app.brokensite.BrokenSiteData @@ -79,7 +80,7 @@ import com.duckduckgo.app.browser.model.BasicAuthenticationRequest import com.duckduckgo.app.browser.model.LongPressTarget import com.duckduckgo.app.browser.omnibar.KeyboardAwareEditText import com.duckduckgo.app.browser.omnibar.OmnibarScrolling -import com.duckduckgo.app.browser.omnibar.QueryOrigin +import com.duckduckgo.app.browser.omnibar.QueryOrigin.* import com.duckduckgo.app.browser.session.WebViewSessionStorage import com.duckduckgo.app.browser.shortcut.ShortcutBuilder import com.duckduckgo.app.browser.tabpreview.WebViewPreviewGenerator @@ -970,8 +971,8 @@ class BrowserTabFragment : viewModel.fireAutocompletePixel(suggestion) withContext(Dispatchers.Main) { val origin = when (suggestion) { - is AutoCompleteSuggestion.AutoCompleteBookmarkSuggestion -> QueryOrigin.FromAutocomplete(nav = null) - is AutoCompleteSuggestion.AutoCompleteSearchSuggestion -> QueryOrigin.FromAutocomplete(nav = suggestion.isUrl) + is AutoCompleteBookmarkSuggestion -> FromAutocomplete(nav = null) + is AutoCompleteSearchSuggestion -> FromAutocomplete(nav = suggestion.isUrl) } viewModel.onUserSubmittedQuery(suggestion.phrase, origin) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt index f1f795e08245..4ca4d81f86fa 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt @@ -30,11 +30,11 @@ class QueryUrlConverter @Inject constructor(private val requestRewriter: Request override fun convertQueryToUrl(searchQuery: String, vertical: String?, queryOrigin: QueryOrigin): String { val isUrl = when (queryOrigin) { - is QueryOrigin.FromAutocomplete -> queryOrigin.nav ?: UriString.isWebUrl(searchQuery) + is QueryOrigin.FromAutocomplete -> queryOrigin.nav is QueryOrigin.FromUser -> UriString.isWebUrl(searchQuery) } - if (isUrl) { + if (isUrl == true) { return convertUri(searchQuery) } From 61b11ead2ea5b2c798195d512096d792526ec785 Mon Sep 17 00:00:00 2001 From: Marcos Holgado Date: Thu, 20 May 2021 15:19:54 +0100 Subject: [PATCH 3/5] Change parameters to is_nav and isNav --- .../app/browser/QueryUrlConverterTest.kt | 16 ++++++++-------- .../app/autocomplete/api/AutoComplete.kt | 2 +- .../app/autocomplete/api/AutoCompleteService.kt | 4 ++-- .../duckduckgo/app/browser/BrowserTabFragment.kt | 4 ++-- .../app/browser/omnibar/OmnibarEntryConverter.kt | 2 +- .../app/browser/omnibar/QueryUrlConverter.kt | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt index d99529024575..32a20d51b722 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt @@ -96,30 +96,30 @@ class QueryUrlConverterTest { } @Test - fun whenQueryOriginIsFromAutocompleteAndNavIsFalseThenSearchQueryBuilt() { + fun whenQueryOriginIsFromAutocompleteAndIsNavIsFalseThenSearchQueryBuilt() { val input = "example.com" - val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(nav = false)) + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(isNav = false)) assertDuckDuckGoSearchQuery("example.com", result) } @Test - fun whenQueryOriginIsFromAutocompleteAndNavIsTrueThenUrlReturned() { + fun whenQueryOriginIsFromAutocompleteAndIsNavIsTrueThenUrlReturned() { val input = "http://example.com" - val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(nav = true)) + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(isNav = true)) assertEquals(input, result) } @Test - fun whenQueryOriginIsFromAutocompleteAndNavIsNullAndIsUrlThenUrlReturned() { + fun whenQueryOriginIsFromAutocompleteAndIsNavIsNullAndIsUrlThenUrlReturned() { val input = "http://example.com" - val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(nav = null)) + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(isNav = null)) assertEquals(input, result) } @Test - fun whenQueryOriginIsFromAutocompleteAndNavIsNullAndIsNotUrlThenSearchQueryBuilt() { + fun whenQueryOriginIsFromAutocompleteAndIsNavIsNullAndIsNotUrlThenSearchQueryBuilt() { val input = "foo" - val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(nav = null)) + val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(isNav = null)) assertDuckDuckGoSearchQuery("foo", result) } diff --git a/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoComplete.kt b/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoComplete.kt index 2c8cc5efc771..95ab368afaaa 100644 --- a/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoComplete.kt +++ b/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoComplete.kt @@ -71,7 +71,7 @@ class AutoCompleteApi @Inject constructor( autoCompleteService.autoComplete(query) .flatMapIterable { it } .map { - AutoCompleteSearchSuggestion(phrase = it.phrase, isUrl = (it.nav ?: UriString.isWebUrl(it.phrase))) + AutoCompleteSearchSuggestion(phrase = it.phrase, isUrl = (it.isNav ?: UriString.isWebUrl(it.phrase))) } .toList() .onErrorReturn { emptyList() } diff --git a/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoCompleteService.kt b/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoCompleteService.kt index 1de5925b783d..0056e156d256 100644 --- a/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoCompleteService.kt +++ b/app/src/main/java/com/duckduckgo/app/autocomplete/api/AutoCompleteService.kt @@ -28,8 +28,8 @@ interface AutoCompleteService { fun autoComplete( @Query("q") query: String, @Query("kl") languageCode: String = Locale.getDefault().language, - @Query("nav") nav: String = "1" + @Query("is_nav") nav: String = "1" ): Observable> } -data class AutoCompleteServiceRawResult(val phrase: String, val nav: Boolean?) +data class AutoCompleteServiceRawResult(val phrase: String, val isNav: Boolean?) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index 6cdbb8d7fd6d..ddb00ba6d733 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -971,8 +971,8 @@ class BrowserTabFragment : viewModel.fireAutocompletePixel(suggestion) withContext(Dispatchers.Main) { val origin = when (suggestion) { - is AutoCompleteBookmarkSuggestion -> FromAutocomplete(nav = null) - is AutoCompleteSearchSuggestion -> FromAutocomplete(nav = suggestion.isUrl) + is AutoCompleteBookmarkSuggestion -> FromAutocomplete(isNav = null) + is AutoCompleteSearchSuggestion -> FromAutocomplete(isNav = suggestion.isUrl) } viewModel.onUserSubmittedQuery(suggestion.phrase, origin) } diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarEntryConverter.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarEntryConverter.kt index dc919831b4bf..853378e7d5b1 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarEntryConverter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarEntryConverter.kt @@ -23,5 +23,5 @@ interface OmnibarEntryConverter { sealed class QueryOrigin { object FromUser : QueryOrigin() - data class FromAutocomplete(val nav: Boolean?) : QueryOrigin() + data class FromAutocomplete(val isNav: Boolean?) : QueryOrigin() } diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt index 4ca4d81f86fa..3051e5228e9a 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/QueryUrlConverter.kt @@ -30,7 +30,7 @@ class QueryUrlConverter @Inject constructor(private val requestRewriter: Request override fun convertQueryToUrl(searchQuery: String, vertical: String?, queryOrigin: QueryOrigin): String { val isUrl = when (queryOrigin) { - is QueryOrigin.FromAutocomplete -> queryOrigin.nav + is QueryOrigin.FromAutocomplete -> queryOrigin.isNav is QueryOrigin.FromUser -> UriString.isWebUrl(searchQuery) } From 61c7e5ab50e3fc18dfb62ed54978e27261bf77fb Mon Sep 17 00:00:00 2001 From: Marcos Holgado Date: Thu, 20 May 2021 15:27:53 +0100 Subject: [PATCH 4/5] Delete unnecessary test --- .../com/duckduckgo/app/browser/QueryUrlConverterTest.kt | 7 ------- 1 file changed, 7 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt index 32a20d51b722..84e6d3b44a45 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/QueryUrlConverterTest.kt @@ -109,13 +109,6 @@ class QueryUrlConverterTest { assertEquals(input, result) } - @Test - fun whenQueryOriginIsFromAutocompleteAndIsNavIsNullAndIsUrlThenUrlReturned() { - val input = "http://example.com" - val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(isNav = null)) - assertEquals(input, result) - } - @Test fun whenQueryOriginIsFromAutocompleteAndIsNavIsNullAndIsNotUrlThenSearchQueryBuilt() { val input = "foo" From f193531523702f0b9789445579b15a844d41fff3 Mon Sep 17 00:00:00 2001 From: Marcos Holgado Date: Fri, 21 May 2021 10:18:17 +0100 Subject: [PATCH 5/5] Bookmark from autocomplete isNav always true --- .../main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index ddb00ba6d733..e3ef0fd81909 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -971,7 +971,7 @@ class BrowserTabFragment : viewModel.fireAutocompletePixel(suggestion) withContext(Dispatchers.Main) { val origin = when (suggestion) { - is AutoCompleteBookmarkSuggestion -> FromAutocomplete(isNav = null) + is AutoCompleteBookmarkSuggestion -> FromAutocomplete(isNav = true) is AutoCompleteSearchSuggestion -> FromAutocomplete(isNav = suggestion.isUrl) } viewModel.onUserSubmittedQuery(suggestion.phrase, origin)