Skip to content

Commit

Permalink
Integrate web version of the breakage reporting form (#4927)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1205648422731273/1208115613742938/f

### Description

This PR adds web implementation of the site breakage reporting form. The
web form needs adjustments before it's ready to ship, so it should be
hidden behind a feature flag for now.

### Steps to test this PR

#### Native form
Privacy Dashboard flow:
- [x] Navigate to a website
- [x] Tap on the shield icon to open Privacy Dashboard
- [x] Tap "Website not working?" to open broken site form
- [x] Verify that the native form is opened
- [x] Select category, enter description and submit report
- [x] Verify that both `epbf` and `m_bsr` pixels are sent
- [x] Verify that `reportFlow` param on the `epbf` pixel is set to
`dashboard`

Browser menu flow:
- [x] Navigate to a website
- [x] Tap on "Report Broken Site" in the options menu
- [x] Verify that the native form is opened
- [x] Select category, enter description  and submit report
- [x] Verify that `reportFlow` param on the `epbf` pixel is set to
`menu`

#### Web form

- [x] To enable web form, override remote config url with this:
https://jsonblob.com/api/1276082387480862720

Privacy Dashboard flow:
- [x] Navigate to a website
- [x] Tap on the shield icon to open Privacy Dashboard
- [x] Tap "Website not working?" to open broken site form
- [x] Verify that the web form is opened
- [x] Select category, enter description  and submit report
- [x] Verify that both `epbf` and `m_bsr` pixels are sent
- [x] Verify that `reportFlow` param on the epbf pixel is set to
`dashboard`

Browser menu flow:
- [x] Navigate to a website
- [x] Tap on "Report Broken Site" in the options menu
- [x] Verify that the web form is opened
- [x] Select category, enter description  and submit report
- [x] Verify that `reportFlow` param on the epbf pixel is set to `menu`

### UI changes

There are no changes to the native form UI. Web UI remains disabled
until design is finalized.
  • Loading branch information
lmac012 authored Aug 23, 2024
1 parent 1458d21 commit de3d33f
Show file tree
Hide file tree
Showing 27 changed files with 827 additions and 320 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import com.duckduckgo.app.brokensite.model.BrokenSiteCategory
import com.duckduckgo.app.browser.R
import com.duckduckgo.app.browser.databinding.ActivityBrokenSiteBinding
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.browser.api.WebViewVersionProvider
import com.duckduckgo.browser.api.brokensite.BrokenSiteData
import com.duckduckgo.browser.api.brokensite.BrokenSiteData.ReportFlow
import com.duckduckgo.browser.api.brokensite.BrokenSiteOpenerContext
Expand All @@ -55,8 +54,6 @@ class BrokenSiteActivity : DuckDuckGoActivity() {
private val binding: ActivityBrokenSiteBinding by viewBinding()
private val viewModel: BrokenSiteViewModel by bindViewModel()

@Inject lateinit var webViewVersionProvider: WebViewVersionProvider

@Inject lateinit var appBuildConfig: AppBuildConfig

private val toolbar
Expand Down Expand Up @@ -144,10 +141,9 @@ class BrokenSiteActivity : DuckDuckGoActivity() {
}
brokenSites.submitButton.setOnClickListener {
if (!submitted) {
val webViewVersion = webViewVersionProvider.getFullVersion()
val description = brokenSites.brokenSiteFormFeedbackInput.text
val loginSite = brokenSites.brokenSiteFormLoginInput.text
viewModel.onSubmitPressed(webViewVersion, description, loginSite)
viewModel.onSubmitPressed(description, loginSite)
submitted = true
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,8 @@ import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.duckduckgo.anvil.annotations.ContributesViewModel
import com.duckduckgo.app.brokensite.BrokenSiteViewModel.ViewState
import com.duckduckgo.app.brokensite.api.BrokenSiteSender
import com.duckduckgo.app.brokensite.model.BrokenSite
import com.duckduckgo.app.brokensite.model.BrokenSiteCategory
import com.duckduckgo.app.brokensite.model.BrokenSiteCategory.*
import com.duckduckgo.app.brokensite.model.ReportFlow as BrokenSiteModelReportFlow
import com.duckduckgo.app.brokensite.model.SiteProtectionsState
import com.duckduckgo.app.brokensite.model.SiteProtectionsState.DISABLED
import com.duckduckgo.app.brokensite.model.SiteProtectionsState.DISABLED_BY_REMOTE_CONFIG
Expand All @@ -35,6 +32,9 @@ import com.duckduckgo.app.pixels.AppPixelName
import com.duckduckgo.app.privacy.db.UserAllowListRepository
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.COUNT
import com.duckduckgo.brokensite.api.BrokenSite
import com.duckduckgo.brokensite.api.BrokenSiteSender
import com.duckduckgo.brokensite.api.ReportFlow as BrokenSiteModelReportFlow
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
Expand All @@ -43,7 +43,6 @@ import com.duckduckgo.common.utils.SingleLiveEvent
import com.duckduckgo.common.utils.extractDomain
import com.duckduckgo.di.scopes.ActivityScope
import com.duckduckgo.feature.toggles.api.FeatureToggle
import com.duckduckgo.privacy.config.api.AmpLinks
import com.duckduckgo.privacy.config.api.ContentBlocking
import com.duckduckgo.privacy.config.api.PrivacyFeatureName
import com.duckduckgo.privacy.config.api.UnprotectedTemporary
Expand All @@ -62,7 +61,6 @@ import kotlinx.coroutines.launch
class BrokenSiteViewModel @Inject constructor(
private val pixel: Pixel,
private val brokenSiteSender: BrokenSiteSender,
private val ampLinks: AmpLinks,
private val featureToggle: FeatureToggle,
private val contentBlocking: ContentBlocking,
private val unprotectedTemporary: UnprotectedTemporary,
Expand Down Expand Up @@ -199,29 +197,21 @@ class BrokenSiteViewModel @Inject constructor(
}
}

fun onSubmitPressed(webViewVersion: String, description: String?, loginSite: String?) {
fun onSubmitPressed(
description: String?,
loginSite: String?,
) {
viewState.value?.submitAllowed = false
if (url.isNotEmpty()) {
val lastAmpLinkInfo = ampLinks.lastAmpLinkInfo

val loginSiteFinal = if (shuffledCategories.elementAtOrNull(viewValue.indexSelected)?.key == BrokenSiteCategory.LOGIN_CATEGORY_KEY) {
loginSite
} else {
""
}

val brokenSite = if (lastAmpLinkInfo?.destinationUrl == url) {
getBrokenSite(lastAmpLinkInfo.ampLink, webViewVersion, description, loginSiteFinal)
} else {
getBrokenSite(url, webViewVersion, description, loginSiteFinal)
}
val brokenSite = getBrokenSite(url, description, loginSiteFinal)

brokenSiteSender.submitBrokenSiteFeedback(brokenSite)

pixel.fire(
AppPixelName.BROKEN_SITE_REPORTED,
mapOf(Pixel.PixelParameter.URL to brokenSite.siteUrl),
)
}
command.value = Command.ConfirmAndFinish
}
Expand Down Expand Up @@ -256,7 +246,6 @@ class BrokenSiteViewModel @Inject constructor(
@VisibleForTesting
fun getBrokenSite(
urlString: String,
webViewVersion: String,
description: String?,
loginSite: String?,
): BrokenSite {
Expand All @@ -268,7 +257,6 @@ class BrokenSiteViewModel @Inject constructor(
upgradeHttps = upgradedHttps,
blockedTrackers = blockedTrackers,
surrogates = surrogates,
webViewVersion = webViewVersion,
siteType = if (isDesktopMode) DESKTOP_SITE else MOBILE_SITE,
urlParametersRemoved = urlParametersRemoved,
consentManaged = consentManaged,
Expand All @@ -279,8 +267,8 @@ class BrokenSiteViewModel @Inject constructor(
loginSite = loginSite,
reportFlow = reportFlow?.mapToBrokenSiteModelReportFlow(),
userRefreshCount = userRefreshCount,
openerContext = openerContext,
jsPerformance = jsPerformance,
openerContext = openerContext?.context,
jsPerformance = jsPerformance?.toList(),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,20 @@ package com.duckduckgo.app.brokensite.api

import android.net.Uri
import androidx.core.net.toUri
import com.duckduckgo.app.brokensite.model.BrokenSite
import com.duckduckgo.app.brokensite.model.ReportFlow
import com.duckduckgo.app.brokensite.model.ReportFlow.DASHBOARD
import com.duckduckgo.app.brokensite.model.ReportFlow.MENU
import com.duckduckgo.app.di.AppCoroutineScope
import com.duckduckgo.app.pixels.AppPixelName
import com.duckduckgo.app.privacy.db.UserAllowListRepository
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.store.StatisticsDataStore
import com.duckduckgo.app.trackerdetection.db.TdsMetadataDao
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.brokensite.api.BrokenSite
import com.duckduckgo.brokensite.api.BrokenSiteLastSentReport
import com.duckduckgo.brokensite.api.BrokenSiteSender
import com.duckduckgo.brokensite.api.ReportFlow
import com.duckduckgo.brokensite.api.ReportFlow.DASHBOARD
import com.duckduckgo.brokensite.api.ReportFlow.MENU
import com.duckduckgo.browser.api.WebViewVersionProvider
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.common.utils.absoluteString
import com.duckduckgo.common.utils.domain
Expand All @@ -38,6 +40,7 @@ import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.experiments.api.VariantManager
import com.duckduckgo.feature.toggles.api.FeatureToggle
import com.duckduckgo.networkprotection.api.NetworkProtectionState
import com.duckduckgo.privacy.config.api.AmpLinks
import com.duckduckgo.privacy.config.api.ContentBlocking
import com.duckduckgo.privacy.config.api.Gpc
import com.duckduckgo.privacy.config.api.PrivacyConfig
Expand All @@ -51,10 +54,6 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import timber.log.Timber

interface BrokenSiteSender {
fun submitBrokenSiteFeedback(brokenSite: BrokenSite)
}

@ContributesBinding(AppScope::class)
class BrokenSiteSubmitter @Inject constructor(
private val statisticsStore: StatisticsDataStore,
Expand All @@ -73,18 +72,26 @@ class BrokenSiteSubmitter @Inject constructor(
private val brokenSiteLastSentReport: BrokenSiteLastSentReport,
private val privacyProtectionsPopupExperimentExternalPixels: PrivacyProtectionsPopupExperimentExternalPixels,
private val networkProtectionState: NetworkProtectionState,
private val webViewVersionProvider: WebViewVersionProvider,
private val ampLinks: AmpLinks,
) : BrokenSiteSender {

override fun submitBrokenSiteFeedback(brokenSite: BrokenSite) {
appCoroutineScope.launch(dispatcherProvider.io()) {
val isGpcEnabled = (featureToggle.isFeatureEnabled(PrivacyFeatureName.GpcFeatureName.value) && gpc.isEnabled()).toString()
val absoluteUrl = Uri.parse(brokenSite.siteUrl).absoluteString

val domain = brokenSite.siteUrl.toUri().domain()
val ampLink = ampLinks.lastAmpLinkInfo
?.takeIf { it.destinationUrl == brokenSite.siteUrl }
?.ampLink

val siteUrl = ampLink ?: brokenSite.siteUrl
val absoluteUrl = Uri.parse(siteUrl).absoluteString
val domain = siteUrl.toUri().domain()

val protectionsState = !userAllowListRepository.isDomainInUserAllowList(domain) &&
!unprotectedTemporary.isAnException(brokenSite.siteUrl) &&
!unprotectedTemporary.isAnException(siteUrl) &&
featureToggle.isFeatureEnabled(PrivacyFeatureName.ContentBlockingFeatureName.value) &&
!contentBlocking.isAnException(brokenSite.siteUrl)
!contentBlocking.isAnException(siteUrl)

val vpnOn = runCatching { networkProtectionState.isRunning() }.getOrNull()
val locale = appBuildConfig.deviceLocale.toSanitizedLanguageTag()
Expand All @@ -100,7 +107,7 @@ class BrokenSiteSubmitter @Inject constructor(
OS_KEY to appBuildConfig.sdkInt.toString(),
MANUFACTURER_KEY to appBuildConfig.manufacturer,
MODEL_KEY to appBuildConfig.model,
WEBVIEW_VERSION_KEY to brokenSite.webViewVersion,
WEBVIEW_VERSION_KEY to webViewVersionProvider.getFullVersion(),
SITE_TYPE_KEY to brokenSite.siteType,
GPC to isGpcEnabled,
URL_PARAMETERS_REMOVED to brokenSite.urlParametersRemoved.toBinaryString(),
Expand All @@ -115,7 +122,7 @@ class BrokenSiteSubmitter @Inject constructor(
VPN_ON to vpnOn.toString(),
LOCALE to locale,
USER_REFRESH_COUNT to brokenSite.userRefreshCount.toString(),
OPENER_CONTEXT to brokenSite.openerContext?.context.orEmpty(),
OPENER_CONTEXT to brokenSite.openerContext.orEmpty(),
JS_PERFORMANCE to brokenSite.jsPerformance?.joinToString(",").orEmpty(),
)

Expand Down Expand Up @@ -148,6 +155,11 @@ class BrokenSiteSubmitter @Inject constructor(
}
}
.onFailure { Timber.w(it, "Feedback submission failed") }

pixel.fire(
AppPixelName.BROKEN_SITE_REPORTED,
mapOf(Pixel.PixelParameter.URL to siteUrl),
)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,6 @@ package com.duckduckgo.app.brokensite.model

import androidx.annotation.StringRes
import com.duckduckgo.app.browser.R
import com.duckduckgo.browser.api.brokensite.BrokenSiteOpenerContext

data class BrokenSite(
val category: String?,
val description: String?,
val siteUrl: String,
val upgradeHttps: Boolean,
val blockedTrackers: String,
val surrogates: String,
val webViewVersion: String,
val siteType: String,
val urlParametersRemoved: Boolean,
val consentManaged: Boolean,
val consentOptOutFailed: Boolean,
val consentSelfTestFailed: Boolean,
val errorCodes: String,
val httpErrorCodes: String,
val loginSite: String?,
val reportFlow: ReportFlow?,
val userRefreshCount: Int,
val openerContext: BrokenSiteOpenerContext?,
val jsPerformance: DoubleArray?,
)

sealed class BrokenSiteCategory(
@StringRes val category: Int,
Expand Down Expand Up @@ -68,5 +45,3 @@ sealed class BrokenSiteCategory(
const val OTHER_CATEGORY_KEY = "other"
}
}

enum class ReportFlow { DASHBOARD, MENU }
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ 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.PrivacyDashboardHybridScreen.PrivacyDashboardHybridWithTabIdParam
import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams.PrivacyDashboardPrimaryScreen
import com.duckduckgo.savedsites.impl.bookmarks.BookmarksActivity.Companion.SAVED_SITE_URL_EXTRA
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -434,7 +434,7 @@ open class BrowserActivity : DuckDuckGoActivity() {

fun launchPrivacyDashboard() {
currentTab?.tabId?.let {
val params = PrivacyDashboardHybridWithTabIdParam(it)
val params = PrivacyDashboardPrimaryScreen(it)
val intent = globalActivityStarter.startIntent(this, params)
intent?.let { startActivity(it) }
}
Expand Down
18 changes: 14 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 @@ -260,7 +260,9 @@ 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.PrivacyDashboardHybridScreen
import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams
import com.duckduckgo.privacy.dashboard.api.ui.PrivacyDashboardHybridScreenParams.BrokenSiteForm
import com.duckduckgo.privacy.dashboard.api.ui.WebBrokenSiteForm
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopup
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopupFactory
import com.duckduckgo.privacyprotectionspopup.api.PrivacyProtectionsPopupViewState
Expand Down Expand Up @@ -500,6 +502,9 @@ class BrowserTabFragment :
@Inject
lateinit var safeWebViewFeature: SafeWebViewFeature

@Inject
lateinit var webBrokenSiteForm: WebBrokenSiteForm

/**
* We use this to monitor whether the user was seeing the in-context Email Protection signup prompt
* This is needed because the activity stack will be cleared if an external link is opened in our browser
Expand Down Expand Up @@ -895,7 +900,7 @@ class BrowserTabFragment :
}

omnibar.customTabToolbarContainer.customTabShieldIcon.setOnClickListener { _ ->
val params = PrivacyDashboardHybridScreen.PrivacyDashboardHybridWithTabIdParam(tabId)
val params = PrivacyDashboardHybridScreenParams.PrivacyDashboardPrimaryScreen(tabId)
val intent = globalActivityStarter.startIntent(requireContext(), params)
contentScopeScripts.sendSubscriptionEvent(createBreakageReportingEventData())
intent?.let { startActivity(it) }
Expand Down Expand Up @@ -1819,9 +1824,14 @@ class BrowserTabFragment :
}

private fun launchBrokenSiteFeedback(data: BrokenSiteData) {
context?.let {
val context = context ?: return

if (webBrokenSiteForm.shouldUseWebBrokenSiteForm()) {
globalActivityStarter.startIntent(context, BrokenSiteForm(tabId))
?.let { startActivity(it) }
} else {
val options = ActivityOptions.makeSceneTransitionAnimation(browserActivity).toBundle()
startActivity(BrokenSiteActivity.intent(it, data), options)
startActivity(BrokenSiteActivity.intent(context, data), options)
}
}

Expand Down
Loading

0 comments on commit de3d33f

Please sign in to comment.