Skip to content

Commit

Permalink
Add simplified ("toggle") broken site reporting (#5280)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1201870266890790/1208769785329643/f

### Description
We're implementing the simplified broken site reporting flow that
already exists on other platforms here, prompting users to report site
breakage on URLs where they toggle off protections.

### Steps to test this PR

_Toggle Reports_
- [x] Set up your local environment by checking out the
[privacy-dashboard
branch](https://github.com/duckduckgo/privacy-dashboard/tree/pr-releases/pr-196)
and editing `app/build.gradle` to point to its location ([replacing this
bit](https://github.com/duckduckgo/Android/blob/4677ba3e51108db4d4f20071977c73baaaa5bb40/app/build.gradle#L44)).
- [x] Navigate to any non-DuckDuckGo site and tap the privacy shield,
then toggle off protections
   - [x] Confirm that you're shown the report prompting screen
- [x] Click on "See what's sent" and confirm the data details expand
into view
- [x] Tap "Send report" and confirm that the
`m_protection-toggled-off-breakage-report` pixel is sent with
`reportFlow=on_protections_off_dashboard_main`
- [x] Re-enable protections
- [x] Tap the 3-dot menu and then "Disable privacy protection"
   - [x] Confirm that you're again shown the prompt screen
- [x] Tap "Send report" and confirm that the above pixel is sent with
`reportFlow=on_protections_off_menu`
- [x] Re-enable protections
- [x] Toggle off protections again from either entry point and confirm
that you are again show the prompt
   - [x] Tap "Don't send"
- [x] Confirm that you are no longer shown the prompt from either
toggle-off point (block should last 48 hours, but that value can be
overridden by the remote privacy config -- there is [an existing
JSONblob](https://jsonblob.com/api/1306645031426056192) with both
intervals set to 2 minutes if you'd like to test more exhaustively, as I
did)

---------

Co-authored-by: Aitor Viana <[email protected]>
  • Loading branch information
laghee and aitorvs authored Dec 6, 2024
1 parent 9e578a3 commit c21801c
Show file tree
Hide file tree
Showing 39 changed files with 1,635 additions and 5,277 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ import com.duckduckgo.app.browser.commands.Command.HideBrokenSitePromptCta
import com.duckduckgo.app.browser.commands.Command.HideOnboardingDaxDialog
import com.duckduckgo.app.browser.commands.Command.LaunchPrivacyPro
import com.duckduckgo.app.browser.commands.Command.LoadExtractedUrl
import com.duckduckgo.app.browser.commands.Command.RefreshAndShowPrivacyProtectionDisabledConfirmation
import com.duckduckgo.app.browser.commands.Command.RefreshAndShowPrivacyProtectionEnabledConfirmation
import com.duckduckgo.app.browser.commands.Command.ShareLink
import com.duckduckgo.app.browser.commands.Command.ShowBackNavigationHistory
import com.duckduckgo.app.browser.commands.Command.ShowPrivacyProtectionDisabledConfirmation
import com.duckduckgo.app.browser.commands.Command.ShowPrivacyProtectionEnabledConfirmation
import com.duckduckgo.app.browser.commands.NavigationCommand
import com.duckduckgo.app.browser.commands.NavigationCommand.Navigate
import com.duckduckgo.app.browser.customtabs.CustomTabPixelNames
Expand Down Expand Up @@ -228,6 +228,7 @@ import com.duckduckgo.privacy.config.impl.features.gpc.RealGpc.Companion.GPC_HEA
import com.duckduckgo.privacy.config.impl.features.gpc.RealGpc.Companion.GPC_HEADER_VALUE
import com.duckduckgo.privacy.dashboard.api.PrivacyProtectionTogglePlugin
import com.duckduckgo.privacy.dashboard.api.PrivacyToggleOrigin
import com.duckduckgo.privacy.dashboard.api.ui.ToggleReports
import com.duckduckgo.privacy.dashboard.impl.pixels.PrivacyDashboardPixels
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopupExperimentExternalPixels
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopupManager
Expand Down Expand Up @@ -499,6 +500,7 @@ class BrowserTabViewModelTest {
private var fakeAndroidConfigBrowserFeature = FakeFeatureToggleFactory.create(AndroidBrowserConfigFeature::class.java)
private val mockAutocompleteTabsFeature: AutocompleteTabsFeature = mock()
private val fakeCustomHeadersPlugin = FakeCustomHeadersProvider(emptyMap())
private val mockToggleReports: ToggleReports = mock()
private val mockBrokenSitePrompt: BrokenSitePrompt = mock()
private val mockTabStatsBucketing: TabStatsBucketing = mock()
private val extendedOnboardingFeatureToggles = FeatureToggles.Builder(FakeToggleStore(), featureName = "extendedOnboarding").build()
Expand Down Expand Up @@ -553,6 +555,7 @@ class BrowserTabViewModelTest {
whenever(mockAutocompleteTabsFeature.self()).thenReturn(mockEnabledToggle)
whenever(mockAutocompleteTabsFeature.self().isEnabled()).thenReturn(true)
whenever(mockSitePermissionsManager.hasSitePermanentPermission(any(), any())).thenReturn(false)
whenever(mockToggleReports.shouldPrompt()).thenReturn(false)

remoteMessagingModel = givenRemoteMessagingModel(mockRemoteMessagingRepository, mockPixel, coroutineRule.testDispatcherProvider)

Expand Down Expand Up @@ -673,6 +676,7 @@ class BrowserTabViewModelTest {
privacyProtectionTogglePlugin = protectionTogglePluginPoint,
showOnAppLaunchOptionHandler = mockShowOnAppLaunchHandler,
customHeadersProvider = fakeCustomHeadersPlugin,
toggleReports = mockToggleReports,
brokenSitePrompt = mockBrokenSitePrompt,
tabStatsBucketing = mockTabStatsBucketing,
)
Expand Down Expand Up @@ -2015,33 +2019,55 @@ class BrowserTabViewModelTest {
}

@Test
fun whenPrivacyProtectionMenuClickedAndSiteNotInAllowListThenShowDisabledConfirmationMessage() = runTest {
fun whenPrivacyProtectionMenuClickedToTurnOffProtectionsThenTogglePromptIsShownWhenCheckIsTrue() = runTest {
whenever(mockUserAllowListRepository.isDomainInUserAllowList("www.example.com")).thenReturn(false)
loadUrl("http://www.example.com/home.html")
testee.onPrivacyProtectionMenuClicked()
assertCommandIssued<ShowPrivacyProtectionDisabledConfirmation> {
assertEquals("www.example.com", this.domain)
verify(mockToggleReports).shouldPrompt()
}

@Test
fun whenPrivacyProtectionMenuClickedForNonAllowListedSiteThenRefreshAndShowDisabledConfirmationMessage() = runTest {
val domain = "www.example.com"
val url = "http://www.example.com/home.html"

val allowlistFlow = MutableStateFlow(listOf<String>())
whenever(mockUserAllowListRepository.domainsInUserAllowListFlow()).thenReturn(allowlistFlow)

loadUrl(url)
testee.onPrivacyProtectionMenuClicked()
allowlistFlow.value = listOf(domain)

assertCommandIssued<RefreshAndShowPrivacyProtectionDisabledConfirmation> {
assertEquals(domain, this.domain)
}
}

@Test
fun whenPrivacyProtectionMenuClickedForAllowListedSiteThenSiteRemovedFromAllowListAndPixelSentAndPageRefreshed() = runTest {
whenever(mockUserAllowListRepository.isDomainInUserAllowList("www.example.com")).thenReturn(true)
loadUrl("http://www.example.com/home.html")
fun whenPrivacyProtectionMenuClickedForAllowListedSiteThenRefreshAndShowEnabledConfirmationMessage() = runTest {
val domain = "www.example.com"
val url = "http://www.example.com/home.html"

val allowlistFlow = MutableStateFlow(listOf(domain))
whenever(mockUserAllowListRepository.domainsInUserAllowListFlow()).thenReturn(allowlistFlow)

loadUrl(url)
testee.onPrivacyProtectionMenuClicked()
verify(mockUserAllowListRepository).removeDomainFromUserAllowList("www.example.com")
verify(mockPixel).fire(AppPixelName.BROWSER_MENU_ALLOWLIST_REMOVE)
assertEquals(1, protectionTogglePlugin.toggleOn)
allowlistFlow.value = emptyList()

assertCommandIssued<RefreshAndShowPrivacyProtectionEnabledConfirmation> {
assertEquals(domain, this.domain)
}
}

@Test
fun whenPrivacyProtectionMenuClickedForAllowListedSiteThenShowDisabledConfirmationMessage() = runTest {
fun whenPrivacyProtectionMenuClickedForAllowListedSiteThenSiteRemovedFromAllowListAndPixelSentAndPageRefreshed() = runTest {
whenever(mockUserAllowListRepository.isDomainInUserAllowList("www.example.com")).thenReturn(true)
loadUrl("http://www.example.com/home.html")
testee.onPrivacyProtectionMenuClicked()
assertCommandIssued<ShowPrivacyProtectionEnabledConfirmation> {
assertEquals("www.example.com", this.domain)
}
verify(mockUserAllowListRepository).removeDomainFromUserAllowList("www.example.com")
verify(mockPixel).fire(AppPixelName.BROWSER_MENU_ALLOWLIST_REMOVE)
assertEquals(1, protectionTogglePlugin.toggleOn)
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow.DASHBOARD
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow.MENU
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow.PROMPT
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow.TOGGLE_DASHBOARD
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow.TOGGLE_MENU
import com.duckduckgo.browser.api.brokensite.BrokenSiteOpenerContext
import com.duckduckgo.common.utils.SingleLiveEvent
import com.duckduckgo.common.utils.extractDomain
Expand Down Expand Up @@ -212,7 +214,7 @@ class BrokenSiteViewModel @Inject constructor(

val brokenSite = getBrokenSite(url, description, loginSiteFinal)

brokenSiteSender.submitBrokenSiteFeedback(brokenSite)
brokenSiteSender.submitBrokenSiteFeedback(brokenSite, toggle = false)
}
command.value = Command.ConfirmAndFinish
}
Expand Down Expand Up @@ -287,4 +289,6 @@ private fun ReportFlow.mapToBrokenSiteModelReportFlow(): BrokenSiteModelReportFl
MENU -> BrokenSiteModelReportFlow.MENU
DASHBOARD -> BrokenSiteModelReportFlow.DASHBOARD
PROMPT -> BrokenSiteModelReportFlow.PROMPT
TOGGLE_MENU -> BrokenSiteModelReportFlow.TOGGLE_MENU
TOGGLE_DASHBOARD -> BrokenSiteModelReportFlow.TOGGLE_DASHBOARD
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import com.duckduckgo.brokensite.api.ReportFlow
import com.duckduckgo.brokensite.api.ReportFlow.DASHBOARD
import com.duckduckgo.brokensite.api.ReportFlow.MENU
import com.duckduckgo.brokensite.api.ReportFlow.PROMPT
import com.duckduckgo.brokensite.api.ReportFlow.TOGGLE_DASHBOARD
import com.duckduckgo.brokensite.api.ReportFlow.TOGGLE_MENU
import com.duckduckgo.browser.api.WebViewVersionProvider
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.absoluteString
Expand Down Expand Up @@ -82,7 +84,7 @@ class BrokenSiteSubmitter @Inject constructor(
private val inventory: FeatureTogglesInventory,
) : BrokenSiteSender {

override fun submitBrokenSiteFeedback(brokenSite: BrokenSite) {
override fun submitBrokenSiteFeedback(brokenSite: BrokenSite, toggle: Boolean) {
appCoroutineScope.launch(dispatcherProvider.io()) {
val isGpcEnabled = (featureToggle.isFeatureEnabled(PrivacyFeatureName.GpcFeatureName.value) && gpc.isEnabled()).toString()

Expand Down Expand Up @@ -149,7 +151,7 @@ class BrokenSiteSubmitter @Inject constructor(
params[LAST_SENT_DAY] = lastSentDay
}

if (appBuildConfig.deviceLocale.language == Locale.ENGLISH.language) {
if (appBuildConfig.deviceLocale.language == Locale.ENGLISH.language && !toggle) {
params[LOGIN_SITE] = brokenSite.loginSite.orEmpty()
}

Expand All @@ -160,7 +162,15 @@ class BrokenSiteSubmitter @Inject constructor(
SURROGATES_KEY to brokenSite.surrogates,
)
runCatching {
pixel.fire(AppPixelName.BROKEN_SITE_REPORT.pixelName, params.toMap(), encodedParams)
if (toggle) {
val unnecessaryKeys = listOf(CATEGORY_KEY, DESCRIPTION_KEY, PROTECTIONS_STATE)
for (key in unnecessaryKeys) {
params.remove(key)
}
pixel.fire(AppPixelName.PROTECTION_TOGGLE_BROKEN_SITE_REPORT.pixelName, params.toMap(), encodedParams)
} else {
pixel.fire(AppPixelName.BROKEN_SITE_REPORT.pixelName, params.toMap(), encodedParams)
}
}
.onSuccess {
Timber.v("Feedback submission succeeded")
Expand Down Expand Up @@ -222,4 +232,6 @@ private fun ReportFlow.toStringValue(): String = when (this) {
DASHBOARD -> "dashboard"
MENU -> "menu"
PROMPT -> "prompt"
TOGGLE_DASHBOARD -> "on_protections_off_dashboard_main"
TOGGLE_MENU -> "on_protections_off_menu"
}
14 changes: 11 additions & 3 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.playstore.PlayStoreUtils
import com.duckduckgo.di.scopes.ActivityScope
import com.duckduckgo.navigation.api.GlobalActivityStarter
import com.duckduckgo.privacy.dashboard.api.ui.DashboardOpener
import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams.PrivacyDashboardPrimaryScreen
import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams.PrivacyDashboardToggleReportScreen
import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -465,9 +467,15 @@ open class BrowserActivity : DuckDuckGoActivity() {
return intent.getBooleanExtra(NEW_SEARCH_EXTRA, false)
}

fun launchPrivacyDashboard() {
currentTab?.tabId?.let {
val params = PrivacyDashboardPrimaryScreen(it)
fun launchPrivacyDashboard(toggle: Boolean) {
currentTab?.tabId?.let { tabId ->
val params = if (toggle) {
PrivacyDashboardToggleReportScreen(tabId, opener = DashboardOpener.DASHBOARD)
} else {
PrivacyDashboardPrimaryScreen(
tabId,
)
}
val intent = globalActivityStarter.startIntent(this, params)
intent?.let { startActivity(it) }
}
Expand Down
27 changes: 23 additions & 4 deletions app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,11 @@ import com.duckduckgo.js.messaging.api.SubscriptionEventData
import com.duckduckgo.mobile.android.app.tracking.ui.AppTrackingProtectionScreens.AppTrackerOnboardingActivityWithEmptyParamsParams
import com.duckduckgo.navigation.api.GlobalActivityStarter
import com.duckduckgo.navigation.api.GlobalActivityStarter.DeeplinkActivityParams
import com.duckduckgo.privacy.dashboard.api.ui.DashboardOpener
import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams
import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams.BrokenSiteForm
import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams.BrokenSiteForm.BrokenSiteFormReportFlow
import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams.PrivacyDashboardToggleReportScreen
import com.duckduckgo.privacy.dashboard.api.ui.WebBrokenSiteForm
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopup
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopupFactory
Expand Down Expand Up @@ -940,7 +942,7 @@ class BrowserTabFragment :

private fun onOmnibarPrivacyShieldButtonPressed() {
contentScopeScripts.sendSubscriptionEvent(createBreakageReportingEventData())
browserActivity?.launchPrivacyDashboard()
browserActivity?.launchPrivacyDashboard(toggle = false)
if (!changeOmnibarPositionFeature.refactor().isEnabled()) {
viewModel.onPrivacyShieldSelected()
}
Expand Down Expand Up @@ -1527,8 +1529,14 @@ class BrowserTabFragment :

is Command.ShowFireproofWebSiteConfirmation -> fireproofWebsiteConfirmation(it.fireproofWebsiteEntity)
is Command.DeleteFireproofConfirmation -> removeFireproofWebsiteConfirmation(it.fireproofWebsiteEntity)
is Command.ShowPrivacyProtectionEnabledConfirmation -> privacyProtectionEnabledConfirmation(it.domain)
is Command.ShowPrivacyProtectionDisabledConfirmation -> privacyProtectionDisabledConfirmation(it.domain)
is Command.RefreshAndShowPrivacyProtectionEnabledConfirmation -> {
refresh()
privacyProtectionEnabledConfirmation(it.domain)
}
is Command.RefreshAndShowPrivacyProtectionDisabledConfirmation -> {
refresh()
privacyProtectionDisabledConfirmation(it.domain)
}
is NavigationCommand.Navigate -> {
dismissAppLinkSnackBar()
navigate(it.url, it.headers)
Expand Down Expand Up @@ -1591,6 +1599,10 @@ class BrowserTabFragment :
launchBrokenSiteFeedback(it.data)
}

is Command.ToggleReportFeedback -> {
launchToggleReportFeedback(it.opener)
}

is Command.ShowFullScreen -> {
binding.webViewFullScreenContainer.addView(
it.view,
Expand Down Expand Up @@ -1912,14 +1924,19 @@ class BrowserTabFragment :
PROMPT -> BrokenSiteFormReportFlow.PROMPT
else -> BrokenSiteFormReportFlow.MENU
}
globalActivityStarter.startIntent(context, BrokenSiteForm(tabId, reportFlow))
globalActivityStarter.startIntent(context, BrokenSiteForm(tabId = tabId, reportFlow = reportFlow))
?.let { startActivity(it) }
} else {
val options = ActivityOptions.makeSceneTransitionAnimation(browserActivity).toBundle()
startActivity(BrokenSiteActivity.intent(context, data), options)
}
}

private fun launchToggleReportFeedback(opener: DashboardOpener) {
globalActivityStarter.startIntent(requireContext(), PrivacyDashboardToggleReportScreen(tabId, opener))
?.let { startActivity(it) }
}

private fun showErrorSnackbar(command: Command.ShowErrorWithAction) {
// Snackbar is global and it should appear only the foreground fragment
if (!errorSnackbar.view.isAttachedToWindow && isVisible) {
Expand Down Expand Up @@ -3540,6 +3557,8 @@ class BrowserTabFragment :

private const val AUTOCOMPLETE_PADDING_DP = 6

private const val TOGGLE_REPORT_TOAST_DELAY = 3000L

fun newInstance(
tabId: String,
query: String? = null,
Expand Down
Loading

0 comments on commit c21801c

Please sign in to comment.