Skip to content

Commit

Permalink
Bug: autocomplete pixels firing when Adding Bookmark (#5165)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/0/1208581391309609/f

### Description
Fixed autocomplete flow and pixels.

### Steps to test this PR

Steps to repro:
- [x] Install from branch
`feature/ana/android_migrate_auto_complete_to_flow_and_coroutines`
- [x] Visit a site in a new tab
- [x] Open ... menu
- [x] Click add bookmark
- [x] Observe that autocomplete pixels are firing (they should not):
m_autocomplete_displayed_bookmark
m_autocomplete_displayed_switch_to_tab

Test the fix:
- [x] Install from this branch
- [x] Visit a site in a new tab
- [x] Open ... menu
- [x] Click add bookmark
- [x] Autocomplete pixels are not firing (check for "m_autocomplete" in
logs)

### NO UI changes
  • Loading branch information
anikiki authored Oct 22, 2024
1 parent 55b2ba0 commit f6acf55
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1470,6 +1470,7 @@ class BrowserTabViewModelTest {
whenever(mockAutoCompleteScorer.score("title", "https://foo.com".toUri(), 1, "title")).thenReturn(1)
whenever(mockUserStageStore.getUserAppStage()).thenReturn(ESTABLISHED)

testee.onAutoCompleteSuggestionsChanged()
testee.autoCompleteStateFlow.value = "title"
delay(500)
testee.autoCompleteSuggestionsGone()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1777,6 +1777,7 @@ class BrowserTabFragment :
}

private fun autocompleteItemRemoved() {
viewModel.onAutoCompleteSuggestionsChanged()
showKeyboardAndRestorePosition(autocompleteFirstVisibleItemPosition, autocompleteItemOffsetTop)
}

Expand Down Expand Up @@ -3710,6 +3711,7 @@ class BrowserTabFragment :
binding.autoCompleteSuggestionsList.gone()
} else {
binding.autoCompleteSuggestionsList.show()
viewModel.onAutoCompleteSuggestionsChanged()
autoCompleteSuggestionsAdapter.updateData(viewState.searchResults.query, viewState.searchResults.suggestions)
hideFocusedView()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,6 @@ class BrowserTabViewModel @Inject constructor(

init {
initializeViewStates()
configureAutoComplete()
logVoiceSearchAvailability()

fireproofWebsiteState.observeForever(fireproofWebsitesObserver)
Expand Down Expand Up @@ -3697,6 +3696,10 @@ class BrowserTabViewModel @Inject constructor(
}
}

fun onAutoCompleteSuggestionsChanged() {
configureAutoComplete()
}

fun autoCompleteSuggestionsGone() {
viewModelScope.launch(dispatchers.io()) {
if (hasUserSeenHistoryIAM) {
Expand Down Expand Up @@ -3725,6 +3728,7 @@ class BrowserTabViewModel @Inject constructor(
}
}
lastAutoCompleteState = null
autoCompleteJob?.cancel()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,10 @@ class SystemSearchActivity : DuckDuckGoActivity() {
}
}

fun onAutoCompleteSuggestionsChanged() {
configureAutoComplete()
}

private fun sendLaunchPixels(intent: Intent) {
when {
launchedFromAssist(intent) -> pixel.fire(AppPixelName.APP_ASSIST_LAUNCH)
Expand Down Expand Up @@ -487,6 +491,7 @@ class SystemSearchActivity : DuckDuckGoActivity() {
}

private fun autocompleteItemRemoved() {
viewModel.onAutoCompleteSuggestionsChanged()
showKeyboardAndRestorePosition()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ class SystemSearchViewModel @Inject constructor(
}
}

fun onAutoCompleteSuggestionsChanged() {
configureResults()
}

private fun showRemoveSearchSuggestionDialog(suggestion: AutoCompleteSuggestion) {
appCoroutineScope.launch(dispatchers.main()) {
command.value = Command.ShowRemoveSearchSuggestionDialog(suggestion)
Expand Down
5 changes: 0 additions & 5 deletions app/src/main/java/com/duckduckgo/app/tabs/db/TabsDao.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import com.duckduckgo.app.tabs.model.TabSelectionEntity
import com.duckduckgo.common.utils.swap
import com.duckduckgo.di.scopes.AppScope
import dagger.SingleInstanceIn
import io.reactivex.Single
import kotlinx.coroutines.flow.Flow

@Dao
Expand All @@ -45,10 +44,6 @@ abstract class TabsDao {
@Query("select * from tabs where deletable is 0 order by position")
abstract fun flowTabs(): Flow<List<TabEntity>>

// TODO: ANA to remove this
@Query("select * from tabs where deletable is 0 order by position")
abstract fun singleTabs(): Single<List<TabEntity>>

@Query("select * from tabs where deletable is 0 order by position")
abstract fun liveTabs(): LiveData<List<TabEntity>>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.AppScope
import dagger.SingleInstanceIn
import io.reactivex.Scheduler
import io.reactivex.Single
import io.reactivex.schedulers.Schedulers
import java.util.UUID
import javax.inject.Inject
Expand Down Expand Up @@ -82,8 +81,6 @@ class TabDataRepository @Inject constructor(

private var purgeDeletableTabsJob = ConflatedJob()

override fun getTabsObservable(): Single<List<TabEntity>> = tabsDao.singleTabs()

override suspend fun add(
url: String?,
skipHome: Boolean,
Expand Down
4 changes: 0 additions & 4 deletions browser-api/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,6 @@ dependencies {

// LiveData
implementation AndroidX.lifecycle.liveDataKtx

// TODO: ANA to remove this
implementation "io.reactivex.rxjava2:rxjava:_"
implementation "io.reactivex.rxjava2:rxandroid:_"
}
android {
namespace 'com.duckduckgo.browser.api'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import com.duckduckgo.app.global.model.Site
import com.duckduckgo.app.tabs.model.TabSwitcherData.LayoutType
import io.reactivex.Single
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.SharedFlow

Expand All @@ -44,8 +43,6 @@ interface TabRepository {

val tabSwitcherData: Flow<TabSwitcherData>

fun getTabsObservable(): Single<List<TabEntity>>

/**
* @return tabId of new record
*/
Expand Down

0 comments on commit f6acf55

Please sign in to comment.