Add nav parameter to autocomplete (#1247)

marcosholgado · web-flow · commit ac377f5c0a71 · 2021-05-21T14:01:31.000+01:00
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
@@ -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
@@ -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
@@ -110,7 +110,7 @@ class BrowserViewModelTest {
 
         runBlocking<Unit> {
             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
@@ -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,41 @@ 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 whenQueryOriginIsFromAutocompleteAndIsNavIsFalseThenSearchQueryBuilt() {
+        val input = "example.com"
+        val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(isNav = false))
+        assertDuckDuckGoSearchQuery("example.com", result)
+    }
+
+    @Test
+    fun whenQueryOriginIsFromAutocompleteAndIsNavIsTrueThenUrlReturned() {
+        val input = "http://example.com"
+        val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(isNav = true))
+        assertEquals(input, result)
+    }
+
+    @Test
+    fun whenQueryOriginIsFromAutocompleteAndIsNavIsNullAndIsNotUrlThenSearchQueryBuilt() {
+        val input = "foo"
+        val result = testee.convertQueryToUrl(input, queryOrigin = QueryOrigin.FromAutocomplete(isNav = 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
@@ -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.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
@@ -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("is_nav") nav: String = "1"
     ): Observable<List<AutoCompleteServiceRawResult>>
 }
 
-data class AutoCompleteServiceRawResult(val phrase: String)
+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
@@ -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,6 +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.session.WebViewSessionStorage
 import com.duckduckgo.app.browser.shortcut.ShortcutBuilder
 import com.duckduckgo.app.browser.tabpreview.WebViewPreviewGenerator
@@ -968,7 +970,11 @@ class BrowserTabFragment :
         GlobalScope.launch {
             viewModel.fireAutocompletePixel(suggestion)
             withContext(Dispatchers.Main) {
-                viewModel.onUserSubmittedQuery(suggestion.phrase)
+                val origin = when (suggestion) {
+                    is AutoCompleteBookmarkSuggestion -> FromAutocomplete(isNav = true)
+                    is AutoCompleteSearchSuggestion -> FromAutocomplete(isNav = 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
@@ -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
@@ -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 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
@@ -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.isNav
+            is QueryOrigin.FromUser -> UriString.isWebUrl(searchQuery)
+        }
+
+        if (isUrl == true) {
             return convertUri(searchQuery)
         }
 

Original file line number	Diff line number	Diff line change
`@@ -110,7 +110,7 @@ class BrowserViewModelTest {`
`110`	`110`
`111`	`111`	`runBlocking<Unit> {`
`112`	`112`	`whenever(mockTabRepository.add()).thenReturn(TAB_ID)`
`113`		`- whenever(mockOmnibarEntryConverter.convertQueryToUrl(any(), any())).then { it.arguments.first() }`
	`113`	`+ whenever(mockOmnibarEntryConverter.convertQueryToUrl(any(), any(), any())).then { it.arguments.first() }`
`114`	`114`	`}`
`115`	`115`	`}`
`116`	`116`