Skip to content

Commit

Permalink
Fix UX when removing subs from device (#4350)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/488551667048375/1206887290583787/f

### Description
This PR fixes UX when subs are removed from the device

### Steps to test this PR
See [test cases](https://app.asana.com/0/0/1206910653710828/f)
  • Loading branch information
aitorvs authored Mar 26, 2024
1 parent 7634ba4 commit 6768d9d
Show file tree
Hide file tree
Showing 33 changed files with 777 additions and 562 deletions.
1 change: 0 additions & 1 deletion app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@ dependencies {
implementation project(':web-compat-store')

implementation project(':subscriptions-api')
implementation project(':network-protection-subscription-internal')

implementation project(':subscriptions-impl')
internalImplementation project(':subscriptions-internal')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ class SettingsViewModel @Inject constructor(
if (isPrivacyProEnabled()) Hidden else getNetworkProtectionEntryState(this)
},
isAutoconsentEnabled = autoconsent.isSettingEnabled(),
isPrivacyProEnabled = isPrivacyProEnabled() && subscriptions.isEligible(),
),
)
networkProtectionState.getConnectionStateFlow()
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ subprojects {
if (dependencyPath == projectPath) continue
// internal modules have to use internalImplementation
// when a non internal configuration is built (i.e. PlayDebug) no internal dependencies should be found
def internalExceptions = [":network-protection-subscription-internal"]
def internalExceptions = []
if (dependencyPath.endsWith('internal') && !internalExceptions.contains(dependencyPath) && !name.toLowerCase().contains("internal")) {
throw new GradleException("Invalid dependency $projectPath -> $dependencyPath. " +
"'internal' modules must use internalImplementation")
Expand Down
1 change: 1 addition & 0 deletions network-protection/network-protection-impl/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ dependencies {
implementation project(':privacy-config-api')
implementation project(':navigation-api')
implementation project(':subscriptions-api')
implementation project(':settings-api')

implementation AndroidX.appCompat
implementation AndroidX.constraintLayout
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.appbuildconfig.api.isInternalBuild
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.networkprotection.impl.BuildConfig
import com.duckduckgo.networkprotection.impl.store.NetworkProtectionRepository
import com.duckduckgo.networkprotection.impl.waitlist.store.NetPWaitlistRepository
import com.duckduckgo.subscriptions.api.Subscriptions
import com.squareup.anvil.annotations.ContributesMultibinding
Expand All @@ -39,7 +38,6 @@ import okhttp3.Response
class NetpWaitlistRequestInterceptor @Inject constructor(
private val netpWaitlistRepository: NetPWaitlistRepository,
private val appBuildConfig: AppBuildConfig,
private val networkProtectionRepository: NetworkProtectionRepository,
private val subscriptions: Subscriptions,
) : ApiInterceptorPlugin, Interceptor {

Expand All @@ -65,11 +63,7 @@ class NetpWaitlistRequestInterceptor @Inject constructor(

chain.proceed(
newRequest.build().also { logcat { "headers: ${it.headers}" } },
).also {
if (runBlocking { subscriptions.isEnabled() }) {
networkProtectionRepository.vpnAccessRevoked = (it.code == 403)
}
}
)
} else {
chain.proceed(newRequest.build())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,17 @@ import com.duckduckgo.browser.api.ui.BrowserScreens.SettingsScreenNoParams
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.navigation.api.GlobalActivityStarter
import com.duckduckgo.networkprotection.impl.R
import com.duckduckgo.networkprotection.impl.store.NetworkProtectionRepository
import com.duckduckgo.networkprotection.impl.subscription.NetpSubscriptionManager
import com.duckduckgo.networkprotection.impl.subscription.isActive
import com.duckduckgo.networkprotection.impl.subscription.isExpired
import com.squareup.anvil.annotations.ContributesBinding
import java.text.DateFormat
import java.text.SimpleDateFormat
import java.util.Date
import javax.inject.Inject

interface NetPDisabledNotificationBuilder {
fun buildDisabledNotification(context: Context): Notification
suspend fun buildDisabledNotification(context: Context): Notification?

fun buildSnoozeNotification(
context: Context,
Expand All @@ -55,7 +57,7 @@ interface NetPDisabledNotificationBuilder {
class RealNetPDisabledNotificationBuilder @Inject constructor(
private val netPNotificationActions: NetPNotificationActions,
private val globalActivityStarter: GlobalActivityStarter,
private val netpRepository: NetworkProtectionRepository,
private val netpSubscriptionManager: NetpSubscriptionManager,
) : NetPDisabledNotificationBuilder {
private val defaultDateTimeFormatter = SimpleDateFormat.getTimeInstance(DateFormat.SHORT)

Expand All @@ -72,11 +74,14 @@ class RealNetPDisabledNotificationBuilder @Inject constructor(
}
}

override fun buildDisabledNotification(context: Context): Notification {
return if (netpRepository.vpnAccessRevoked) {
override suspend fun buildDisabledNotification(context: Context): Notification? {
val vpnStatus = netpSubscriptionManager.getVpnStatus()
return if (vpnStatus.isExpired()) {
buildVpnAccessRevokedNotification(context)
} else {
} else if (vpnStatus.isActive()) {
buildVpnDisabledNotification(context)
} else {
null
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import com.duckduckgo.mobile.android.vpn.service.VpnServiceCallbacks
import com.duckduckgo.mobile.android.vpn.state.VpnStateMonitor.VpnStopReason
import com.duckduckgo.networkprotection.api.NetworkProtectionState
import com.duckduckgo.networkprotection.impl.settings.NetPSettingsLocalConfig
import com.duckduckgo.networkprotection.impl.store.NetworkProtectionRepository
import com.duckduckgo.networkprotection.impl.waitlist.NetPRemoteFeature
import com.squareup.anvil.annotations.ContributesMultibinding
import java.util.concurrent.atomic.AtomicReference
Expand All @@ -47,7 +46,6 @@ class NetPDisabledNotificationScheduler @Inject constructor(
@AppCoroutineScope private val coroutineScope: CoroutineScope,
private val dispatcherProvider: DispatcherProvider,
private val netPRemoteFeature: NetPRemoteFeature,
private val networkProtectionRepository: NetworkProtectionRepository,
) : VpnServiceCallbacks {

private var isNetPEnabled: AtomicReference<Boolean> = AtomicReference(false)
Expand Down Expand Up @@ -80,20 +78,6 @@ class NetPDisabledNotificationScheduler @Inject constructor(
}
}

override fun onVpnStartFailed(coroutineScope: CoroutineScope) {
coroutineScope.launch(dispatcherProvider.io()) {
if (networkProtectionRepository.vpnAccessRevoked) {
notificationManager.checkPermissionAndNotify(
context,
NETP_REMINDER_NOTIFICATION_ID,
netPDisabledNotificationBuilder.buildVpnAccessRevokedNotification(context),
)
// This is to clear the registered features and remove VPN
networkProtectionState.stop()
}
}
}

private suspend fun shouldShowImmediateNotification(): Boolean {
// When VPN is stopped and if AppTP has been enabled AND user has been onboarded, then we show the disabled notif
return isNetPEnabled.get() && networkProtectionState.isOnboarded() && netPRemoteFeature.waitlistBetaActive().isEnabled()
Expand Down Expand Up @@ -140,11 +124,13 @@ class NetPDisabledNotificationScheduler @Inject constructor(
coroutineScope.launch(dispatcherProvider.io()) {
logcat { "Showing disabled notification for NetP" }
if (!netPSettingsLocalConfig.vpnNotificationAlerts().isEnabled()) return@launch
notificationManager.checkPermissionAndNotify(
context,
NETP_REMINDER_NOTIFICATION_ID,
netPDisabledNotificationBuilder.buildDisabledNotification(context),
)
netPDisabledNotificationBuilder.buildDisabledNotification(context)?.let { notification ->
notificationManager.checkPermissionAndNotify(
context,
NETP_REMINDER_NOTIFICATION_ID,
notification,
)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,37 +17,76 @@
package com.duckduckgo.networkprotection.impl.revoked

import android.app.Activity
import android.content.SharedPreferences
import androidx.core.content.edit
import com.duckduckgo.app.di.AppCoroutineScope
import com.duckduckgo.common.ui.view.dialog.TextAlertDialogBuilder
import com.duckduckgo.common.ui.view.dialog.TextAlertDialogBuilder.EventListener
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.mobile.android.vpn.prefs.VpnSharedPreferencesProvider
import com.duckduckgo.navigation.api.GlobalActivityStarter
import com.duckduckgo.networkprotection.impl.R
import com.duckduckgo.networkprotection.impl.pixels.NetworkProtectionPixels
import com.duckduckgo.networkprotection.impl.store.NetworkProtectionRepository
import com.duckduckgo.subscriptions.api.SubscriptionScreens.SubscriptionScreenNoParams
import com.squareup.anvil.annotations.ContributesBinding
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext

interface AccessRevokedDialog {
fun show(activity: Activity)
/**
* Call this method to always show the dialog
*/
fun showAlways(activity: Activity)

/**
* Call this method to show the dialog only once.
* Use [clearIsShown] to reset that state
*/
fun showOnce(activity: Activity)

/**
* Call this method to allow [showOnce] to show the dialog more than once
*/
fun clearIsShown()
}

@ContributesBinding(AppScope::class)
class RealAccessRevokedDialog @Inject constructor(
@AppCoroutineScope private val coroutineScope: CoroutineScope,
private val dispatcherProvider: DispatcherProvider,
private val networkProtectionRepository: NetworkProtectionRepository,
private val globalActivityStarter: GlobalActivityStarter,
private val networkProtectionPixels: NetworkProtectionPixels,
private val vpnSharedPreferencesProvider: VpnSharedPreferencesProvider,
) : AccessRevokedDialog {

private val preferences: SharedPreferences by lazy {
vpnSharedPreferencesProvider.getSharedPreferences(
FILENAME,
multiprocess = true,
migrate = false,
)
}

private var boundActivity: Activity? = null

override fun show(activity: Activity) {
override fun showAlways(activity: Activity) {
showInternal(activity)
}

override fun showOnce(activity: Activity) {
coroutineScope.launch {
if (!isShown()) {
withContext(dispatcherProvider.main()) {
showInternal(activity)
}
}
}
}

private fun showInternal(activity: Activity) {
if (boundActivity == activity) return

boundActivity = activity
Expand All @@ -62,11 +101,14 @@ class RealAccessRevokedDialog @Inject constructor(
override fun onPositiveButtonClicked() {
// Commenting this for now since this is still behind the subs build
globalActivityStarter.start(activity, SubscriptionScreenNoParams)
resetVpnAccessRevokedState()
}

override fun onNegativeButtonClicked() {
resetVpnAccessRevokedState()
override fun onNegativeButtonClicked() {}

override fun onDialogDismissed() {
coroutineScope.launch {
markAsShown()
}
}
},
)
Expand All @@ -75,9 +117,26 @@ class RealAccessRevokedDialog @Inject constructor(
networkProtectionPixels.reportAccessRevokedDialogShown()
}

private fun resetVpnAccessRevokedState() {
override fun clearIsShown() {
coroutineScope.launch(dispatcherProvider.io()) {
networkProtectionRepository.vpnAccessRevoked = false
preferences.edit(commit = true) {
putBoolean(KEY_END_DIALOG_SHOWN, false)
}
}
}

private suspend fun isShown(): Boolean = withContext(dispatcherProvider.io()) {
return@withContext preferences.getBoolean(KEY_END_DIALOG_SHOWN, false)
}

private suspend fun markAsShown() = withContext(dispatcherProvider.io()) {
preferences.edit(commit = true) {
putBoolean(KEY_END_DIALOG_SHOWN, true)
}
}

companion object {
private const val FILENAME = "com.duckduckgo.networkprotection.dialog.access.revoked.store.v1"
private const val KEY_END_DIALOG_SHOWN = "KEY_END_DIALOG_SHOWN"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.mobile.android.vpn.prefs.VpnSharedPreferencesProvider
import com.duckduckgo.networkprotection.impl.R.string
import com.duckduckgo.networkprotection.impl.pixels.NetworkProtectionPixels
import com.duckduckgo.networkprotection.impl.store.NetworkProtectionRepository
import com.squareup.anvil.annotations.ContributesBinding
import javax.inject.Inject
import kotlinx.coroutines.CoroutineScope
Expand All @@ -42,7 +41,6 @@ interface BetaEndedDialog {
class RealBetaEndedDialog @Inject constructor(
@AppCoroutineScope private val coroutineScope: CoroutineScope,
private val dispatcherProvider: DispatcherProvider,
private val networkProtectionRepository: NetworkProtectionRepository,
private val networkProtectionPixels: NetworkProtectionPixels,
private val vpnSharedPreferencesProvider: VpnSharedPreferencesProvider,
) : BetaEndedDialog {
Expand Down Expand Up @@ -83,7 +81,6 @@ class RealBetaEndedDialog @Inject constructor(

private fun resetVpnAccessRevokedState() {
coroutineScope.launch(dispatcherProvider.io()) {
networkProtectionRepository.vpnAccessRevoked = false
storeBetaEndDialogShown()
}
}
Expand All @@ -96,7 +93,7 @@ class RealBetaEndedDialog @Inject constructor(
private fun hasShownBetaEndDialog(): Boolean = preferences.getBoolean(KEY_END_DIALOG_SHOWN, false)

companion object {
const val FILENAME = "com.duckduckgo.networkprotection.impl.waitlist.end.store.v1"
const val KEY_END_DIALOG_SHOWN = "KEY_END_DIALOG_SHOWN"
private const val FILENAME = "com.duckduckgo.networkprotection.impl.waitlist.end.store.v1"
private const val KEY_END_DIALOG_SHOWN = "KEY_END_DIALOG_SHOWN"
}
}
Loading

0 comments on commit 6768d9d

Please sign in to comment.