Skip to content

Commit

Permalink
UI for autofill breakage reports (#4741)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/608920331025315/1207767049586685/f 

### Description
Adds the UI for showing the option to send an autofill breakage report.

Note, this branch doesn't actually submit the report or suppress the prompt from showing again.

### Steps to test this PR

ℹ️  Local hack to enable feature: In `AutofillSiteBreakageReportingFeature`, set default value to `true`

---

**Verifying it doesn't show unexpectedly**
- [ ] Fresh install
- [ ] Tap on overflow->Passwords; verify you do **not** see the "report a problem" view
- [ ] Manually add a password for `fill.dev`, and press back after saving. verify you do **not** see the "report a problem" view
- [ ] Return to browser and visit a different site, e.g., `example.com`
- [ ] Tap on overflow->Passwords; verify you do **not** see the "report a problem" view
- [ ] Visit `fill.dev`. This time, access passwords from `Settings` (not directly from the overflow.) verify you do **not** see the "report a problem" view

**Verifying it does show when it should**
- [ ] With a password saved for `fill.dev` already, visit `fill.dev`
- [ ] Tap on overflow->Passwords; verify you **do** see the "report a problem" view

**Verifying it behaves when tapped on**
- [ ] Tap on "Report a problem with autofill"
- [ ] Verify prompt looks good against designs
- [ ] Tap `Send Report` and verify confirmation snackbar looks good

**Verifying feature flag works**
- [ ] Remove your local hack so that the feature defaults to `false` (and remote config isn't yet set up for it anyway)
- [ ] With a password saved for `fill.dev` already, visit `fill.dev`
- [ ] Tap on overflow->Passwords; verify you do **not** see the "report a problem" view


![output](https://github.com/user-attachments/assets/6ae3326b-81dc-4f61-a762-7f57ddaaca32)
  • Loading branch information
CDRussell authored and laghee committed Aug 3, 2024
1 parent 67a7fb7 commit 9e82ad9
Show file tree
Hide file tree
Showing 10 changed files with 268 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import com.duckduckgo.autofill.impl.R
import com.duckduckgo.autofill.impl.databinding.ItemRowAutofillCredentialsManagementScreenBinding
import com.duckduckgo.autofill.impl.databinding.ItemRowAutofillCredentialsManagementScreenDividerBinding
import com.duckduckgo.autofill.impl.databinding.ItemRowAutofillCredentialsManagementScreenHeaderBinding
import com.duckduckgo.autofill.impl.databinding.ItemRowAutofillReportBreakageManagementScreenBinding
import com.duckduckgo.autofill.impl.databinding.ItemRowSearchNoResultsBinding
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillManagementRecyclerAdapter.ContextMenuAction.CopyPassword
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillManagementRecyclerAdapter.ContextMenuAction.CopyUsername
Expand All @@ -42,6 +43,7 @@ import com.duckduckgo.autofill.impl.ui.credential.management.AutofillManagementR
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillManagementRecyclerAdapter.ListItem.Divider
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillManagementRecyclerAdapter.ListItem.GroupHeading
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillManagementRecyclerAdapter.ListItem.NoMatchingSearchResults
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillManagementRecyclerAdapter.ListItem.ReportAutofillBreakage
import com.duckduckgo.autofill.impl.ui.credential.management.sorting.CredentialGrouper
import com.duckduckgo.autofill.impl.ui.credential.management.sorting.InitialExtractor
import com.duckduckgo.autofill.impl.ui.credential.management.suggestion.SuggestionListBuilder
Expand All @@ -59,6 +61,7 @@ class AutofillManagementRecyclerAdapter(
private val suggestionListBuilder: SuggestionListBuilder,
private val onCredentialSelected: (credentials: LoginCredentials) -> Unit,
private val onContextMenuItemClicked: (ContextMenuAction) -> Unit,
private val onReportBreakageClicked: () -> Unit,
) : Adapter<RecyclerView.ViewHolder>() {

private var listItems = listOf<ListItem>()
Expand Down Expand Up @@ -93,6 +96,11 @@ class AutofillManagementRecyclerAdapter(
NoMatchingSearchResultsViewHolder(binding)
}

ITEM_VIEW_TYPE_REPORT_AUTOFILL_BREAKAGE -> {
val binding = ItemRowAutofillReportBreakageManagementScreenBinding.inflate(LayoutInflater.from(parent.context), parent, false)
ReportBreakageViewHolder(binding)
}

else -> throw IllegalArgumentException("Unknown view type")
}
}
Expand All @@ -106,6 +114,7 @@ class AutofillManagementRecyclerAdapter(
is CredentialsViewHolder -> onBindViewHolderCredential(position, viewHolder)
is HeadingViewHolder -> onBindViewHolderHeading(position, viewHolder)
is NoMatchingSearchResultsViewHolder -> onBindViewHolderNoMatchingSearchResults(position, viewHolder)
is ReportBreakageViewHolder -> onBindViewHolderReportBreakage(viewHolder)
}
}

Expand All @@ -118,6 +127,12 @@ class AutofillManagementRecyclerAdapter(
viewHolder.binding.noMatchingLoginsHint.text = formattedNoResultsText
}

private fun onBindViewHolderReportBreakage(viewHolder: ReportBreakageViewHolder) {
viewHolder.binding.root.setOnClickListener {
onReportBreakageClicked()
}
}

private fun onBindViewHolderCredential(
position: Int,
viewHolder: CredentialsViewHolder,
Expand Down Expand Up @@ -169,6 +184,7 @@ class AutofillManagementRecyclerAdapter(
is SuggestedCredential -> ITEM_VIEW_TYPE_SUGGESTED_CREDENTIAL
is Divider -> ITEM_VIEW_TYPE_DIVIDER
is NoMatchingSearchResults -> ITEM_VIEW_TYPE_NO_MATCHING_SEARCH_RESULTS
is ReportAutofillBreakage -> ITEM_VIEW_TYPE_REPORT_AUTOFILL_BREAKAGE
}
}

Expand Down Expand Up @@ -206,10 +222,11 @@ class AutofillManagementRecyclerAdapter(
unsortedCredentials: List<LoginCredentials>,
unsortedDirectSuggestions: List<LoginCredentials>,
unsortedSharableSuggestions: List<LoginCredentials>,
allowBreakageReporting: Boolean,
) {
val newList = mutableListOf<ListItem>()

val directSuggestionsListItems = suggestionListBuilder.build(unsortedDirectSuggestions, unsortedSharableSuggestions)
val directSuggestionsListItems = suggestionListBuilder.build(unsortedDirectSuggestions, unsortedSharableSuggestions, allowBreakageReporting)
newList.addAll(directSuggestionsListItems)

val groupedCredentials = grouper.group(unsortedCredentials)
Expand Down Expand Up @@ -239,16 +256,17 @@ class AutofillManagementRecyclerAdapter(
data class Credential(override val credentials: LoginCredentials) : CredentialListItem(credentials)
data class SuggestedCredential(override val credentials: LoginCredentials) : CredentialListItem(credentials)
}

data object ReportAutofillBreakage : ListItem
data class GroupHeading(val label: String) : ListItem
object Divider : ListItem
data object Divider : ListItem
data class NoMatchingSearchResults(val query: String) : ListItem
}

open class CredentialsViewHolder(open val binding: ItemRowAutofillCredentialsManagementScreenBinding) : RecyclerView.ViewHolder(binding.root)
class SuggestedCredentialsViewHolder(override val binding: ItemRowAutofillCredentialsManagementScreenBinding) : CredentialsViewHolder(binding)
class HeadingViewHolder(val binding: ItemRowAutofillCredentialsManagementScreenHeaderBinding) : RecyclerView.ViewHolder(binding.root)
class DividerViewHolder(val binding: ItemRowAutofillCredentialsManagementScreenDividerBinding) : RecyclerView.ViewHolder(binding.root)
class ReportBreakageViewHolder(val binding: ItemRowAutofillReportBreakageManagementScreenBinding) : RecyclerView.ViewHolder(binding.root)
class NoMatchingSearchResultsViewHolder(val binding: ItemRowSearchNoResultsBinding) : RecyclerView.ViewHolder(binding.root)

companion object {
Expand All @@ -257,5 +275,6 @@ class AutofillManagementRecyclerAdapter(
private const val ITEM_VIEW_TYPE_SUGGESTED_CREDENTIAL = 2
private const val ITEM_VIEW_TYPE_DIVIDER = 3
private const val ITEM_VIEW_TYPE_NO_MATCHING_SEARCH_RESULTS = 4
private const val ITEM_VIEW_TYPE_REPORT_AUTOFILL_BREAKAGE = 5
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import com.duckduckgo.autofill.impl.pixel.AutofillPixelNames.AUTOFILL_MANUALLY_S
import com.duckduckgo.autofill.impl.pixel.AutofillPixelNames.AUTOFILL_NEVER_SAVE_FOR_THIS_SITE_CONFIRMATION_PROMPT_CONFIRMED
import com.duckduckgo.autofill.impl.pixel.AutofillPixelNames.AUTOFILL_NEVER_SAVE_FOR_THIS_SITE_CONFIRMATION_PROMPT_DISMISSED
import com.duckduckgo.autofill.impl.pixel.AutofillPixelNames.AUTOFILL_NEVER_SAVE_FOR_THIS_SITE_CONFIRMATION_PROMPT_DISPLAYED
import com.duckduckgo.autofill.impl.reporting.remoteconfig.AutofillSiteBreakageReportingFeature
import com.duckduckgo.autofill.impl.store.InternalAutofillStore
import com.duckduckgo.autofill.impl.store.NeverSavedSiteRepository
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillSettingsViewModel.Command.ExitCredentialMode
Expand Down Expand Up @@ -74,6 +75,7 @@ import com.duckduckgo.autofill.impl.ui.credential.management.AutofillSettingsVie
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillSettingsViewModel.DuckAddressStatus.SettingActivationStatus
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillSettingsViewModel.ListModeCommand.LaunchDeleteAllPasswordsConfirmation
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillSettingsViewModel.ListModeCommand.LaunchImportPasswords
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillSettingsViewModel.ListModeCommand.LaunchReportAutofillBreakageConfirmation
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillSettingsViewModel.ListModeCommand.LaunchResetNeverSaveListConfirmation
import com.duckduckgo.autofill.impl.ui.credential.management.AutofillSettingsViewModel.ListModeCommand.PromptUserToAuthenticateMassDeletion
import com.duckduckgo.autofill.impl.ui.credential.management.neversaved.NeverSavedSitesViewState
Expand All @@ -83,6 +85,8 @@ import com.duckduckgo.autofill.impl.ui.credential.management.survey.SurveyDetail
import com.duckduckgo.autofill.impl.ui.credential.management.viewing.duckaddress.DuckAddressIdentifier
import com.duckduckgo.autofill.impl.ui.credential.repository.DuckAddressStatusRepository
import com.duckduckgo.autofill.impl.ui.credential.repository.DuckAddressStatusRepository.ActivationStatusResult
import com.duckduckgo.autofill.impl.urlmatcher.AutofillUrlMatcher
import com.duckduckgo.autofill.store.reporting.AutofillSiteBreakageReportingFeatureRepository
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.ActivityScope
import com.duckduckgo.sync.api.engine.SyncEngine
Expand Down Expand Up @@ -117,6 +121,9 @@ class AutofillSettingsViewModel @Inject constructor(
private val syncEngine: SyncEngine,
private val neverSavedSiteRepository: NeverSavedSiteRepository,
private val autofillSurvey: AutofillSurvey,
private val reportBreakageFeature: AutofillSiteBreakageReportingFeature,
private val reportBreakageFeatureExceptions: AutofillSiteBreakageReportingFeatureRepository,
private val urlMatcher: AutofillUrlMatcher,
) : ViewModel() {

private val _viewState = MutableStateFlow(ViewState())
Expand Down Expand Up @@ -407,13 +414,16 @@ class AutofillSettingsViewModel @Inject constructor(
if (combineJob != null) return
combineJob = viewModelScope.launch(dispatchers.io()) {
_viewState.value = _viewState.value.copy(autofillEnabled = autofillStore.autofillEnabled)

val allCredentials = autofillStore.getAllCredentials().distinctUntilChanged()
val combined = allCredentials.combine(searchQueryFilter) { credentials, filter ->
credentialListFilter.filter(credentials, filter)
}
combined.collect { credentials ->
val updatedBreakageState = _viewState.value.reportBreakageState.copy(allowBreakageReporting = isBreakageReportingAllowed())
_viewState.value = _viewState.value.copy(
logins = credentials,
reportBreakageState = updatedBreakageState,
)
}
}
Expand All @@ -425,6 +435,15 @@ class AutofillSettingsViewModel @Inject constructor(
}
}

private fun isBreakageReportingAllowed(): Boolean {
val url = _viewState.value.reportBreakageState.currentUrl ?: return false
val urlParts = urlMatcher.extractUrlPartsForAutofill(url)

return (reportBreakageFeature.self().isEnabled() && !reportBreakageFeatureExceptions.exceptions.contains(urlParts.eTldPlus1)).also {
Timber.v("Allow breakage reporting for [%s]: %s", urlParts, it)
}
}

fun onDeleteCurrentCredentials() {
getCurrentCredentials()?.let {
onDeleteCredentials(it)
Expand Down Expand Up @@ -688,15 +707,45 @@ class AutofillSettingsViewModel @Inject constructor(
addCommand(LaunchImportPasswords)
}

fun onReportBreakageClicked() {
val currentUrl = _viewState.value.reportBreakageState.currentUrl
val eTldPlusOne = urlMatcher.extractUrlPartsForAutofill(currentUrl).eTldPlus1
if (eTldPlusOne != null) {
addCommand(LaunchReportAutofillBreakageConfirmation(eTldPlusOne))
}
}

fun updateCurrentUrl(currentUrl: String?) {
val updatedReportBreakageState = _viewState.value.reportBreakageState.copy(currentUrl = currentUrl)
_viewState.value = _viewState.value.copy(reportBreakageState = updatedReportBreakageState)
}

fun userConfirmedSendBreakageReport(eTldPlusOne: String) {
// todo send the pixel

// todo record feedback sent timestamp for this domain. todo - work out where to record this

val updatedReportBreakageState = _viewState.value.reportBreakageState.copy(allowBreakageReporting = false)
_viewState.value = _viewState.value.copy(reportBreakageState = updatedReportBreakageState)

addCommand(ListModeCommand.ShowUserReportSentMessage)
}

data class ViewState(
val autofillEnabled: Boolean = true,
val showAutofillEnabledToggle: Boolean = true,
val logins: List<LoginCredentials>? = null,
val credentialMode: CredentialMode? = null,
val credentialSearchQuery: String = "",
val reportBreakageState: ReportBreakageState = ReportBreakageState(),
val survey: SurveyDetails? = null,
)

data class ReportBreakageState(
val currentUrl: String? = null,
val allowBreakageReporting: Boolean = false,
)

/**
* Supported credentials modes, each of which might have a different UI and different subtypes.
*
Expand Down Expand Up @@ -764,6 +813,8 @@ class AutofillSettingsViewModel @Inject constructor(
data class LaunchDeleteAllPasswordsConfirmation(val numberToDelete: Int) : ListModeCommand()
data class PromptUserToAuthenticateMassDeletion(val authConfiguration: AuthConfiguration) : ListModeCommand()
data object LaunchImportPasswords : ListModeCommand()
data class LaunchReportAutofillBreakageConfirmation(val eTldPlusOne: String) : ListModeCommand()
data object ShowUserReportSentMessage : ListModeCommand()
}

sealed class DuckAddressStatus {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class SuggestionListBuilder @Inject constructor(
fun build(
unsortedDirectSuggestions: List<LoginCredentials>,
unsortedSharableSuggestions: List<LoginCredentials>,
allowBreakageReporting: Boolean,
): List<ListItem> {
val list = mutableListOf<ListItem>()

Expand All @@ -46,6 +47,10 @@ class SuggestionListBuilder @Inject constructor(
val allSuggestions = sortedDirectSuggestions + sortedSharableSuggestions
list.addAll(allSuggestions.map { SuggestedCredential(it) })

if (allowBreakageReporting) {
list.add(ListItem.ReportAutofillBreakage)
}

list.add(Divider)
}

Expand Down
Loading

0 comments on commit 9e82ad9

Please sign in to comment.