Skip to content

Commit

Permalink
Add scoring algorithm for history suggestions (extracted from BSK) (#…
Browse files Browse the repository at this point in the history
…4341)

Task/Issue URL: https://app.asana.com/0/0/1206816228247419/f 

### Description
Copy the scoring algorithm from BSK

### Steps to test this PR
_Bookmarks shouldn't show in top hits (top section, above DDG
suggestions)_
- [ ] Add wikipedia.org as a bookmark (not favorite)
- [ ] Type wiki on the search bar
- [ ] Check the newly added bookmark is not shown in top hits, and is
instead shown in the bottom section (below DDG suggestions)

_Favorites should show in top hits_
- [ ] Add wikipedia.org as a favorite
- [ ] Type wiki on the search bar
- [ ] Check the newly added favorite is shown in top hits and uses the
bookmark+favorite icon

### Steps to test this PR

_Test pixel fired when selecting a favorite_
- [ ] Select a favorite from autocomplete
- [ ] Check m_autocomplete_favorite_selection has been fired

_Test pixel fired when selecting a bookmark_
- [ ] Select a bookmark from autocomplete
- [ ] Checkk m_aut_s_b has been fired

_Website (not SERP)_
- [ ] Visit a page (not SERP)
- [ ] Open the history database using inspector or flipper, and check
there's a new entry in `history_entries`. Take note of the ID. Check the
title field contains the same title as the tab, the query field is empty
and `isSerp` is 0
- [ ] Check there's an entry in `visits_list` with that same
`historyEntryId`
- [ ] Reload the page
- [ ] Check the `history_entries` table still has only one entry for
that site
- [ ] Check the `visits_list` contains 2 entries for that same
`historyEntryId`

_SERP_
- [ ] Perform a search
- [ ] Open the history database using inspector or flipper, and check
there's a new entry in `history_entries`. Take note of the ID. Check the
title field contains the same title as the tab, the query field contains
your query, and `isSerp` is 0
- [ ] Check there's an entry in `visits_list` with that same
`historyEntryId`
- [ ] Reload the page
- [ ] Check the `history_entries` table still has only one entry for
that site (In some cases, reload doesn't produce the same exact query
params,* in case a second entry is added, check URLs are different)
- [ ] Check the `visits_list` contains 2 entries for that same
`historyEntryId`

_DuckDuckGo URL (not SERP)_
- [ ] Visit https://duckduckgo.com/settings
- [ ] Open the history database using inspector or flipper, and check
the query field is empty and `isSerp` is 0

**Clear app data between tests, fire button doesn't clear history
(yet)**

_Feature 1_
- [ ] Visit wikipedia.org
- [ ] Type wiki
- [ ] Check a history result is shown for wikipedia.org

_Feature 2_
- [ ] Visit https://en.wikipedia.org/wiki/Cat
- [ ] Type wiki
- [ ] Check there's no top hits result for Cat at wikipedia
- [ ] Visit https://en.wikipedia.org/wiki/Cat 3 more times
- [ ] Check there's a top hits result for Cat at wikipedia

_Feature 3_
- [ ] Add a random site as a bookmark
- [ ] Then go to bookmarks and edit it, Set https://wikipedia.org as the
URL and Wikipedia as the title
- [ ] Type wiki
- [ ] Check wikipedia is shown on the bottom section with a bookmark
icon, not in top hits
- [ ] Visit wikipedia.org
- [ ] Check now wikipedia is shown in top hits with the bookmark icon

_Feature 4_
- [ ] Add a random site as a favorite
- [ ] Then go to bookmarks and edit it, Set https://wikipedia.org as the
URL and Wikipedia as the title
- [ ] Type wiki
- [ ] Check now wikipedia is shown in top hits with the favorite icon

_Feature 5_

- [ ] Type something on the search bar
- [ ] Check history/bookmark/favorite suggestions aren't shown before
search suggestions are loaded

_Feature 6_
- [ ] Type something on the search bar
- [ ] Quickly delete everything
- [ ] Check history/bookmark/favorite suggestions aren't shown before
search suggestions and no "No suggestions found" message is shown either

### UI changes

[Video](https://github.com/duckduckgo/Android/assets/6297834/645d3666-82a0-490a-9bf6-02901d14a117)

![favorites](https://github.com/duckduckgo/Android/assets/6297834/c142ec6b-ff0f-4ce9-8cc1-a1864d5a4de1)

![bookmarks](https://github.com/duckduckgo/Android/assets/6297834/945a9cb4-5f9d-4beb-9a07-953403d9251f)
  • Loading branch information
CrisBarreiro committed Apr 30, 2024
1 parent 69befa7 commit 66c4a75
Show file tree
Hide file tree
Showing 31 changed files with 1,538 additions and 140 deletions.
2 changes: 2 additions & 0 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ fladle {
}

dependencies {
implementation project(":history-impl")
implementation project(":history-api")
implementation project(":verified-installation-impl")
implementation project(":verified-installation-api")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import com.duckduckgo.app.accessibility.data.AccessibilitySettingsDataStore
import com.duckduckgo.app.accessibility.data.AccessibilitySettingsSharedPreferences
import com.duckduckgo.app.autocomplete.api.AutoComplete.AutoCompleteResult
import com.duckduckgo.app.autocomplete.api.AutoComplete.AutoCompleteSuggestion.AutoCompleteBookmarkSuggestion
import com.duckduckgo.app.autocomplete.api.AutoComplete.AutoCompleteSuggestion.AutoCompleteHistoryRelatedSuggestion.AutoCompleteHistorySearchSuggestion
import com.duckduckgo.app.autocomplete.api.AutoComplete.AutoCompleteSuggestion.AutoCompleteSearchSuggestion
import com.duckduckgo.app.autocomplete.api.AutoCompleteApi
import com.duckduckgo.app.autocomplete.api.AutoCompleteService
Expand Down Expand Up @@ -136,6 +137,7 @@ import com.duckduckgo.app.privacy.model.TestEntity
import com.duckduckgo.app.settings.db.SettingsDataStore
import com.duckduckgo.app.statistics.api.StatisticsUpdater
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.COUNT
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.UNIQUE
import com.duckduckgo.app.surrogates.SurrogateResponse
Expand Down Expand Up @@ -166,6 +168,7 @@ import com.duckduckgo.downloads.api.FileDownloader
import com.duckduckgo.downloads.api.FileDownloader.PendingFileDownload
import com.duckduckgo.feature.toggles.api.FeatureToggle
import com.duckduckgo.feature.toggles.api.Toggle
import com.duckduckgo.history.api.NavigationHistory
import com.duckduckgo.privacy.config.api.*
import com.duckduckgo.privacy.config.impl.features.gpc.RealGpc
import com.duckduckgo.privacy.config.impl.features.gpc.RealGpc.Companion.GPC_HEADER
Expand Down Expand Up @@ -266,6 +269,9 @@ class BrowserTabViewModelTest {
@Mock
private lateinit var mockSavedSitesRepository: SavedSitesRepository

@Mock
private lateinit var mockNavigationHistory: NavigationHistory

@Mock
private lateinit var mockLongPressHandler: LongPressHandler

Expand Down Expand Up @@ -471,7 +477,7 @@ class BrowserTabViewModelTest {
fireproofWebsiteDao = db.fireproofWebsiteDao()
locationPermissionsDao = db.locationPermissionsDao()

mockAutoCompleteApi = AutoCompleteApi(mockAutoCompleteService, mockSavedSitesRepository)
mockAutoCompleteApi = AutoCompleteApi(mockAutoCompleteService, mockSavedSitesRepository, mockNavigationHistory)
val fireproofWebsiteRepositoryImpl = FireproofWebsiteRepositoryImpl(
fireproofWebsiteDao,
coroutineRule.testDispatcherProvider,
Expand Down Expand Up @@ -2169,7 +2175,43 @@ class BrowserTabViewModelTest {
val suggestion = AutoCompleteBookmarkSuggestion("example", "Example", "https://example.com")
testee.autoCompleteViewState.value = autoCompleteViewState().copy(searchResults = AutoCompleteResult("", listOf(suggestion)))
testee.fireAutocompletePixel(suggestion)
verify(mockPixel).fire(AppPixelName.AUTOCOMPLETE_BOOKMARK_SELECTION, pixelParams(showedBookmarks = true, bookmarkCapable = true))
val argumentCaptor = argumentCaptor<Map<String, String>>()
verify(mockPixel).fire(eq(AppPixelName.AUTOCOMPLETE_BOOKMARK_SELECTION), argumentCaptor.capture(), any(), any())

assertEquals("true", argumentCaptor.firstValue[PixelParameter.SHOWED_BOOKMARKS])
assertEquals("true", argumentCaptor.firstValue[PixelParameter.BOOKMARK_CAPABLE])
}

@Test
fun whenBookmarkFavoriteSubmittedThenAutoCompleteFavoriteSelectionPixelSent() = runTest {
whenever(mockSavedSitesRepository.hasBookmarks()).thenReturn(true)
whenever(mockSavedSitesRepository.hasFavorites()).thenReturn(true)
val suggestion = AutoCompleteBookmarkSuggestion("example", "Example", "https://example.com", isFavorite = true)
testee.autoCompleteViewState.value = autoCompleteViewState().copy(searchResults = AutoCompleteResult("", listOf(suggestion)))
testee.fireAutocompletePixel(suggestion)

val argumentCaptor = argumentCaptor<Map<String, String>>()
verify(mockPixel).fire(eq(AppPixelName.AUTOCOMPLETE_FAVORITE_SELECTION), argumentCaptor.capture(), any(), any())

assertEquals("false", argumentCaptor.firstValue[PixelParameter.SHOWED_BOOKMARKS])
assertEquals("true", argumentCaptor.firstValue[PixelParameter.SHOWED_FAVORITES])
assertEquals("true", argumentCaptor.firstValue[PixelParameter.BOOKMARK_CAPABLE])
assertEquals("true", argumentCaptor.firstValue[PixelParameter.FAVORITE_CAPABLE])
}

@Test
fun whenHistorySubmittedThenAutoCompleteHistorySelectionPixelSent() = runTest {
whenever(mockSavedSitesRepository.hasBookmarks()).thenReturn(true)
val suggestion = AutoCompleteHistorySearchSuggestion("example")
testee.autoCompleteViewState.value = autoCompleteViewState().copy(searchResults = AutoCompleteResult("", listOf(suggestion)))
testee.fireAutocompletePixel(suggestion)

val argumentCaptor = argumentCaptor<Map<String, String>>()
verify(mockPixel).fire(eq(AppPixelName.AUTOCOMPLETE_HISTORY_SELECTION), argumentCaptor.capture(), any(), any())

assertEquals("false", argumentCaptor.firstValue[PixelParameter.SHOWED_BOOKMARKS])
assertEquals("true", argumentCaptor.firstValue[PixelParameter.BOOKMARK_CAPABLE])
assertEquals("true", argumentCaptor.firstValue[PixelParameter.SHOWED_HISTORY])
}

@Test
Expand All @@ -2179,7 +2221,11 @@ class BrowserTabViewModelTest {
testee.autoCompleteViewState.value = autoCompleteViewState().copy(searchResults = AutoCompleteResult("", suggestions))
testee.fireAutocompletePixel(AutoCompleteSearchSuggestion("example", false))

verify(mockPixel).fire(AppPixelName.AUTOCOMPLETE_SEARCH_SELECTION, pixelParams(showedBookmarks = true, bookmarkCapable = true))
val argumentCaptor = argumentCaptor<Map<String, String>>()
verify(mockPixel).fire(eq(AppPixelName.AUTOCOMPLETE_SEARCH_SELECTION), argumentCaptor.capture(), any(), any())

assertEquals("true", argumentCaptor.firstValue[PixelParameter.SHOWED_BOOKMARKS])
assertEquals("true", argumentCaptor.firstValue[PixelParameter.BOOKMARK_CAPABLE])
}

@Test
Expand All @@ -2188,7 +2234,11 @@ class BrowserTabViewModelTest {
testee.autoCompleteViewState.value = autoCompleteViewState().copy(searchResults = AutoCompleteResult("", emptyList()))
testee.fireAutocompletePixel(AutoCompleteSearchSuggestion("example", false))

verify(mockPixel).fire(AppPixelName.AUTOCOMPLETE_SEARCH_SELECTION, pixelParams(showedBookmarks = false, bookmarkCapable = false))
val argumentCaptor = argumentCaptor<Map<String, String>>()
verify(mockPixel).fire(eq(AppPixelName.AUTOCOMPLETE_SEARCH_SELECTION), argumentCaptor.capture(), any(), any())

assertEquals("false", argumentCaptor.firstValue[PixelParameter.SHOWED_BOOKMARKS])
assertEquals("false", argumentCaptor.firstValue[PixelParameter.BOOKMARK_CAPABLE])
}

@Test
Expand Down Expand Up @@ -5429,9 +5479,11 @@ class BrowserTabViewModelTest {
private fun pixelParams(
showedBookmarks: Boolean,
bookmarkCapable: Boolean,
showedHistory: Boolean = false,
) = mapOf(
Pixel.PixelParameter.SHOWED_BOOKMARKS to showedBookmarks.toString(),
Pixel.PixelParameter.BOOKMARK_CAPABLE to bookmarkCapable.toString(),
Pixel.PixelParameter.SHOWED_HISTORY to showedHistory.toString(),
)

private fun givenExpectedCtaAddWidgetInstructions() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import org.junit.Test
import org.mockito.ArgumentMatchers.anyString
import org.mockito.kotlin.any
import org.mockito.kotlin.argumentCaptor
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.never
import org.mockito.kotlin.times
Expand Down Expand Up @@ -162,7 +163,7 @@ class BrowserWebViewClientTest {
testee.webViewClientListener = listener
whenever(webResourceRequest.url).thenReturn(Uri.EMPTY)
whenever(cookieManagerProvider.get()).thenReturn(cookieManager)
whenever(currentTimeProvider.getTimeInMillis()).thenReturn(0)
whenever(currentTimeProvider.elapsedRealtime()).thenReturn(0)
whenever(webViewVersionProvider.getMajorVersion()).thenReturn("1")
whenever(deviceInfo.appVersion).thenReturn("1")
}
Expand Down Expand Up @@ -744,7 +745,7 @@ class BrowserWebViewClientTest {
fun whenPageFinishesBeforeStartingThenPixelIsNotFired() {
val mockWebView = getImmediatelyInvokedMockWebView()
testee.onPageFinished(mockWebView, EXAMPLE_URL)
verify(pageLoadedHandler, never()).invoke(any(), any(), any())
verify(pageLoadedHandler, never()).onPageLoaded(any(), any(), any(), any())
}

@Test
Expand All @@ -754,11 +755,11 @@ class BrowserWebViewClientTest {
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())
whenever(mockWebView.settings).thenReturn(mock())
testee.onPageStarted(mockWebView, EXAMPLE_URL, null)
whenever(currentTimeProvider.getTimeInMillis()).thenReturn(10)
whenever(currentTimeProvider.elapsedRealtime()).thenReturn(10)
testee.onPageFinished(mockWebView, EXAMPLE_URL)
val startArgumentCaptor = argumentCaptor<Long>()
val endArgumentCaptor = argumentCaptor<Long>()
verify(pageLoadedHandler).invoke(any(), startArgumentCaptor.capture(), endArgumentCaptor.capture())
verify(pageLoadedHandler).onPageLoaded(any(), eq(null), startArgumentCaptor.capture(), endArgumentCaptor.capture())
assertEquals(0L, startArgumentCaptor.firstValue)
assertEquals(10L, endArgumentCaptor.firstValue)
}
Expand All @@ -771,7 +772,7 @@ class BrowserWebViewClientTest {
whenever(mockWebView.settings).thenReturn(mock())
testee.onPageStarted(mockWebView, "about:blank", null)
testee.onPageFinished(mockWebView, "about:blank")
verify(pageLoadedHandler, never()).invoke(any(), any(), any())
verify(pageLoadedHandler, never()).onPageLoaded(any(), any(), any(), any())
}

@Test
Expand All @@ -780,7 +781,7 @@ class BrowserWebViewClientTest {
whenever(mockWebView.settings).thenReturn(mock())
testee.onPageStarted(mockWebView, EXAMPLE_URL, null)
testee.onPageFinished(mockWebView, EXAMPLE_URL)
verify(pageLoadedHandler, never()).invoke(any(), any(), any())
verify(pageLoadedHandler, never()).onPageLoaded(any(), any(), any(), any())
}

@Test
Expand All @@ -790,14 +791,14 @@ class BrowserWebViewClientTest {
whenever(mockWebView.safeCopyBackForwardList()).thenReturn(TestBackForwardList())
whenever(mockWebView.settings).thenReturn(mock())
testee.onPageStarted(mockWebView, EXAMPLE_URL, null)
whenever(currentTimeProvider.getTimeInMillis()).thenReturn(5)
whenever(currentTimeProvider.elapsedRealtime()).thenReturn(5)
testee.onPageStarted(mockWebView, EXAMPLE_URL, null)
whenever(currentTimeProvider.getTimeInMillis()).thenReturn(10)
whenever(currentTimeProvider.elapsedRealtime()).thenReturn(10)
testee.onPageFinished(mockWebView, EXAMPLE_URL)

val startArgumentCaptor = argumentCaptor<Long>()
val endArgumentCaptor = argumentCaptor<Long>()
verify(pageLoadedHandler).invoke(any(), startArgumentCaptor.capture(), endArgumentCaptor.capture())
verify(pageLoadedHandler).onPageLoaded(any(), eq(null), startArgumentCaptor.capture(), endArgumentCaptor.capture())
assertEquals(0L, startArgumentCaptor.firstValue)
assertEquals(10L, endArgumentCaptor.firstValue)
}
Expand All @@ -813,13 +814,13 @@ class BrowserWebViewClientTest {
whenever(webResourceError.errorCode).thenReturn(ERROR_HOST_LOOKUP)
whenever(webResourceRequest.isForMainFrame).thenReturn(true)
testee.onReceivedError(mockWebView, webResourceRequest, webResourceError)
whenever(currentTimeProvider.getTimeInMillis()).thenReturn(5)
whenever(currentTimeProvider.elapsedRealtime()).thenReturn(5)
testee.onPageStarted(mockWebView, EXAMPLE_URL, null)
whenever(currentTimeProvider.getTimeInMillis()).thenReturn(10)
whenever(currentTimeProvider.elapsedRealtime()).thenReturn(10)
testee.onPageFinished(mockWebView, EXAMPLE_URL)
val startArgumentCaptor = argumentCaptor<Long>()
val endArgumentCaptor = argumentCaptor<Long>()
verify(pageLoadedHandler).invoke(any(), startArgumentCaptor.capture(), endArgumentCaptor.capture())
verify(pageLoadedHandler).onPageLoaded(any(), eq(null), startArgumentCaptor.capture(), endArgumentCaptor.capture())
assertEquals(5L, startArgumentCaptor.firstValue)
assertEquals(10L, endArgumentCaptor.firstValue)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.duckduckgo.autoconsent.api.Autoconsent
import com.duckduckgo.browser.api.WebViewVersionProvider
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.common.utils.device.DeviceInfo
import com.duckduckgo.history.api.NavigationHistory
import kotlinx.coroutines.test.TestScope
import org.junit.Assert
import org.junit.Before
Expand All @@ -28,6 +29,7 @@ class PageLoadedHandlerTest {
private val webViewVersionProvider: WebViewVersionProvider = mock()
private val pageLoadedPixelDao: PageLoadedPixelDao = mock()
private val autoconsent: Autoconsent = mock()
private val navigationHistory: NavigationHistory = mock()

private val testee = RealPageLoadedHandler(
deviceInfo,
Expand All @@ -40,6 +42,7 @@ class PageLoadedHandlerTest {
override val enabled: Boolean
get() = true
},
navigationHistory,
)

@Before
Expand All @@ -51,15 +54,15 @@ class PageLoadedHandlerTest {

@Test
fun whenInvokingWithValidUrlThenPixelIsAdded() {
testee.invoke(VALID_URL, 0L, 10L)
testee.onPageLoaded(VALID_URL, "title", 0L, 10L)
val argumentCaptor = argumentCaptor<PageLoadedPixelEntity>()
verify(pageLoadedPixelDao).add(argumentCaptor.capture())
Assert.assertEquals(10L, argumentCaptor.firstValue.elapsedTime)
}

@Test
fun whenInvokingWithInvalidUrlThenPixelIsAdded() {
testee.invoke(INVALID_URL, 0L, 10L)
testee.onPageLoaded(INVALID_URL, "title", 0L, 10L)
verify(pageLoadedPixelDao, never()).add(any())
}
}

0 comments on commit 66c4a75

Please sign in to comment.