Skip to content

Commit

Permalink
Followup: New temporary pixels for "Notify Me" component / Follow-up:…
Browse files Browse the repository at this point in the history
… remove parameter for Android13 (#3007)

Task/Issue URL:  
https://app.asana.com/0/69071770703008/1203884929941926/f
https://app.asana.com/0/69071770703008/1203353347663734/f

### Description
Removed temporary pixels added as part of notifications permissions
changes on Android 13.
- Pixel `m_notify_me_component_notify_me_button_pressed` (AppTP &
Browser)
- Pixel `m_notify_me_component_close_button_pressed` (AppTP & Browser)
- Param `os_version_13_or_above` set from onboarding when default
browser set / not set (Browser)

### Steps to test this PR

DDG set as default browser
- [x] Install from this branch.
- [x] Filter logcat by `Pixel sent`.
- [ ] Start onboarding and notice the `Set DDG as default browser`.
Choose to set it.
- [x] Check that you see in the logs `Pixel sent: m_db_s with params:
{fo=true}`. It does not contain the `os_version_13_or_above` param.


DDG NOT set as default browser + notify me 
- [x] Install from this branch and don't allow notifications (if on
Android 13, else disable notifications).
- [x] Filter logcat by `Pixel sent`.
- [x] Start onboarding and notice the `Set DDG as default browser`.
Choose and set any other browser as default.
- [x] Check that you see in the logs `Pixel sent: m_db_ns with params:
{fo=true}`. It does not contain the `os_version_13_or_above` param.
- [x] Go to the `Downloads` screen.
- [x] Notice the `Notify Me` component.
- [x] Tap on `Notify Me` button. Don't allow notifications.
- [x] Check that the `m_notify_me_component_notify_me_button_pressed`
pixel is never sent.
- [x] Tap on the close button on `Notify Me` component.
- [x] Check that the `m_notify_me_component_close_button_pressed` pixel
is never sent.
- [x] Enable AppTP.
- [x] Notice the `Notify Me` component.
- [x] Tap on `Notify Me` button. Don't allow notifications.
- [x] Check that the `m_notify_me_component_notify_me_button_pressed`
pixel is never sent.
- [x] Tap on the close button on `Notify Me` component.
- [x] Check that the `m_notify_me_component_close_button_pressed` pixel
is never sent.

### NO UI changes
  • Loading branch information
anikiki authored Mar 28, 2023
1 parent a783899 commit bd53e83
Show file tree
Hide file tree
Showing 19 changed files with 14 additions and 274 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,5 @@ enum class DeviceShieldPixelNames(override val pixelName: String, val enqueue: B
ATP_REPORT_VPN_NETWORK_STACK_CREATE_ERROR("m_atp_ev_apptp_create_network_stack_error_c"),
ATP_REPORT_VPN_NETWORK_STACK_CREATE_ERROR_DAILY("m_atp_ev_apptp_create_network_stack_error_d"),

ATP_DID_PRESS_NOTIFY_ME_BUTTON("m_notify_me_component_notify_me_button_pressed"),
ATP_DID_PRESS_NOTIFY_ME_DISMISS_BUTTON("m_notify_me_component_close_button_pressed"),

;
}
Original file line number Diff line number Diff line change
Expand Up @@ -338,10 +338,6 @@ interface DeviceShieldPixels {
fun didPressOnAppTpEnabledCtaButton()

fun reportErrorCreatingVpnNetworkStack()

fun didPressOnNotifyMeButton(metadata: Map<String, String>)

fun didPressOnNotifyMeDismissButton(metadata: Map<String, String>)
}

@ContributesBinding(AppScope::class)
Expand Down Expand Up @@ -753,14 +749,6 @@ class RealDeviceShieldPixels @Inject constructor(
firePixel(DeviceShieldPixelNames.ATP_REPORT_VPN_NETWORK_STACK_CREATE_ERROR)
}

override fun didPressOnNotifyMeButton(metadata: Map<String, String>) {
firePixel(DeviceShieldPixelNames.ATP_DID_PRESS_NOTIFY_ME_BUTTON, metadata)
}

override fun didPressOnNotifyMeDismissButton(metadata: Map<String, String>) {
firePixel(DeviceShieldPixelNames.ATP_DID_PRESS_NOTIFY_ME_DISMISS_BUTTON, metadata)
}

private fun firePixel(
p: DeviceShieldPixelNames,
payload: Map<String, String> = emptyMap(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,6 @@ class DeviceShieldTrackerActivity :
binding.ctaShowAll.setOnClickListener {
viewModel.onViewEvent(ViewEvent.LaunchMostRecentActivity)
}

binding.deviceShieldTrackerNotifyMe.onNotifyMeClicked {
viewModel.onViewEvent(ViewEvent.NotifyMeClicked)
}

binding.deviceShieldTrackerNotifyMe.onDismissClicked {
viewModel.onViewEvent(ViewEvent.NotifyMeDismissClicked)
}
}

override fun onActivityResult(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,6 @@ class DeviceShieldTrackerActivityViewModel @Inject constructor(
ViewEvent.PromoteAlwaysOnCancelled -> onAlwaysOnPromotionDialogCancelled()
is ViewEvent.AlwaysOnInitialState -> onAlwaysOnInitialState(viewEvent.alwaysOnState)
ViewEvent.LaunchTrackingProtectionExclusionListActivity -> sendCommand(Command.LaunchTrackingProtectionExclusionListActivity)
ViewEvent.NotifyMeClicked -> firePixel(viewEvent)
ViewEvent.NotifyMeDismissClicked -> firePixel(viewEvent)
}
}

Expand Down Expand Up @@ -195,15 +193,6 @@ class DeviceShieldTrackerActivityViewModel @Inject constructor(
}
}

private fun firePixel(viewEvent: ViewEvent) {
val metadata = mapOf(PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_NAME to PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_VALUE)
if (viewEvent == ViewEvent.NotifyMeClicked) {
deviceShieldPixels.didPressOnNotifyMeButton(metadata)
} else if (viewEvent == ViewEvent.NotifyMeDismissClicked) {
deviceShieldPixels.didPressOnNotifyMeDismissButton(metadata)
}
}

fun bannerState(): BannerState {
return if (vpnStore.getAndSetOnboardingSession()) {
BannerState.OnboardingBanner
Expand Down Expand Up @@ -245,8 +234,6 @@ class DeviceShieldTrackerActivityViewModel @Inject constructor(
object RemoveFeature : ViewEvent()
object StartVpn : ViewEvent()
object AskToRemoveFeature : ViewEvent()
object NotifyMeClicked : ViewEvent()
object NotifyMeDismissClicked : ViewEvent()

object PromoteAlwaysOnOpenSettings : ViewEvent()
object PromoteAlwaysOnCancelled : ViewEvent()
Expand Down Expand Up @@ -274,11 +261,6 @@ class DeviceShieldTrackerActivityViewModel @Inject constructor(
object OpenVpnSettings : Command()
object ShowAppTpEnabledCta : Command()
}

companion object {
internal const val PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_NAME = "from_screen"
internal const val PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_VALUE = "apptp"
}
}

internal inline class TrackerCount(val value: Int)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,14 @@

package com.duckduckgo.mobile.android.vpn.pixels

import com.duckduckgo.mobile.android.vpn.pixels.DeviceShieldPixelNames.ATP_DID_PRESS_NOTIFY_ME_BUTTON
import com.duckduckgo.mobile.android.vpn.pixels.DeviceShieldPixelNames.ATP_DID_PRESS_NOTIFY_ME_DISMISS_BUTTON
import org.junit.Assert.assertTrue
import org.junit.Test

class DeviceShieldPixelNamesTest {
@Test
fun allAppTrackingProtectionPixelsShallBePrefixed() {
DeviceShieldPixelNames.values()
// These 2 pixels have the same names across browser and AppTP and should not have the "m_atp" prefix.
.filter { it != ATP_DID_PRESS_NOTIFY_ME_BUTTON && it != ATP_DID_PRESS_NOTIFY_ME_DISMISS_BUTTON }
.map { it.pixelName }.forEach { pixel ->
assertTrue(pixel.startsWith("m_atp"))
}
DeviceShieldPixelNames.values().map { it.pixelName }.forEach { pixel ->
assertTrue(pixel.startsWith("m_atp"))
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ import com.duckduckgo.mobile.android.vpn.state.VpnStateMonitor
import com.duckduckgo.mobile.android.vpn.stats.AppTrackerBlockingStatsRepository
import com.duckduckgo.mobile.android.vpn.ui.onboarding.VpnStore
import com.duckduckgo.mobile.android.vpn.ui.tracker_activity.DeviceShieldTrackerActivityViewModel.BannerState
import com.duckduckgo.mobile.android.vpn.ui.tracker_activity.DeviceShieldTrackerActivityViewModel.Companion.PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_NAME
import com.duckduckgo.mobile.android.vpn.ui.tracker_activity.DeviceShieldTrackerActivityViewModel.Companion.PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_VALUE
import com.duckduckgo.mobile.android.vpn.ui.tracker_activity.DeviceShieldTrackerActivityViewModel.ViewEvent
import kotlin.time.ExperimentalTime
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand Down Expand Up @@ -342,30 +340,4 @@ class DeviceShieldTrackerActivityViewModelTest {

assertEquals(BannerState.OnboardingBanner, bannerState)
}

@Test
fun whenUserClickedOnNotifyMeThenPixelIsSentWithCorrectParams() = runBlocking {
viewModel.commands().test {
viewModel.onViewEvent(ViewEvent.NotifyMeClicked)

verify(deviceShieldPixels).didPressOnNotifyMeButton(
mapOf(PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_NAME to PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_VALUE),
)

cancelAndConsumeRemainingEvents()
}
}

@Test
fun whenUserClickedOnDismissNotifyMeThenPixelIsSentWithCorrectParams() = runBlocking {
viewModel.commands().test {
viewModel.onViewEvent(ViewEvent.NotifyMeDismissClicked)

verify(deviceShieldPixels).didPressOnNotifyMeDismissButton(
mapOf(PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_NAME to PIXEL_PARAM_NOTIFY_ME_FROM_SCREEN_VALUE),
)

cancelAndConsumeRemainingEvents()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -192,14 +192,6 @@ class DownloadsAdapter @Inject constructor(
}
},
)

binding.root.onNotifyMeClicked {
listener.onNotifyMeButtonClicked()
}

binding.root.onDismissClicked {
listener.onNotifyMeDismissButtonClicked()
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,4 @@ interface DownloadsItemListener {
fun onCancelItemClicked(item: DownloadItem)

fun onItemVisibilityChanged(visible: Boolean)

fun onNotifyMeButtonClicked()

fun onNotifyMeDismissButtonClicked()
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ import com.duckduckgo.app.downloads.DownloadsViewModel.Command.OpenFile
import com.duckduckgo.app.downloads.DownloadsViewModel.Command.ShareFile
import com.duckduckgo.app.global.DispatcherProvider
import com.duckduckgo.app.global.formatters.time.TimeDiffFormatter
import com.duckduckgo.app.pixels.AppPixelName
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter.NOTIFY_ME_FROM_SCREEN
import com.duckduckgo.app.statistics.pixels.Pixel.PixelValues.NOTIFY_ME_DOWNLOADS_SCREEN
import com.duckduckgo.di.scopes.ActivityScope
import com.duckduckgo.downloads.api.DownloadsRepository
import com.duckduckgo.downloads.api.model.DownloadItem
Expand All @@ -60,7 +56,6 @@ class DownloadsViewModel @Inject constructor(
private val timeDiffFormatter: TimeDiffFormatter,
private val downloadsRepository: DownloadsRepository,
private val dispatcher: DispatcherProvider,
private val pixel: Pixel,
) : ViewModel(), DownloadsItemListener {

data class ViewState(
Expand Down Expand Up @@ -205,14 +200,6 @@ class DownloadsViewModel @Inject constructor(
}
}

override fun onNotifyMeButtonClicked() {
pixel.fire(AppPixelName.NOTIFY_ME_BUTTON_PRESSED, mapOf(NOTIFY_ME_FROM_SCREEN to NOTIFY_ME_DOWNLOADS_SCREEN))
}

override fun onNotifyMeDismissButtonClicked() {
pixel.fire(AppPixelName.NOTIFY_ME_DISMISS_BUTTON_PRESSED, mapOf(NOTIFY_ME_FROM_SCREEN to NOTIFY_ME_DOWNLOADS_SCREEN))
}

private fun DownloadItem.mapToDownloadViewItem(): DownloadViewItem = Item(this)

private fun List<DownloadItem>.mapToDownloadViewItems(): List<DownloadViewItem> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ class WelcomePageModule {
context: Context,
pixel: Pixel,
defaultRoleBrowserDialog: DefaultRoleBrowserDialog,
appBuildConfig: AppBuildConfig,
) = WelcomePageViewModelFactory(appInstallStore, context, pixel, defaultRoleBrowserDialog, appBuildConfig)
) = WelcomePageViewModelFactory(appInstallStore, context, pixel, defaultRoleBrowserDialog)

@Provides
fun defaultRoleBrowserDialog(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import com.duckduckgo.app.global.SingleLiveEvent
import com.duckduckgo.app.global.install.AppInstallStore
import com.duckduckgo.app.pixels.AppPixelName
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.di.scopes.FragmentScope
import javax.inject.Inject

Expand All @@ -33,7 +32,6 @@ class DefaultBrowserPageViewModel @Inject constructor(
private val defaultBrowserDetector: DefaultBrowserDetector,
private val pixel: Pixel,
private val installStore: AppInstallStore,
private val appBuildConfig: AppBuildConfig,
) : ViewModel() {

sealed class ViewState {
Expand Down Expand Up @@ -173,7 +171,6 @@ class DefaultBrowserPageViewModel @Inject constructor(
val params = mapOf(
Pixel.PixelParameter.DEFAULT_BROWSER_SET_FROM_ONBOARDING to true.toString(),
Pixel.PixelParameter.DEFAULT_BROWSER_SET_ORIGIN to originValue,
Pixel.PixelParameter.DEFAULT_BROWSER_SET_ON_ANDROID_13_OR_ABOVE to appBuildConfig.isAndroid13OrAbove().toString(),
)
pixel.fire(AppPixelName.DEFAULT_BROWSER_SET, params)
} else {
Expand All @@ -199,8 +196,6 @@ class DefaultBrowserPageViewModel @Inject constructor(
}
}

private fun AppBuildConfig.isAndroid13OrAbove(): Boolean = sdkInt >= android.os.Build.VERSION_CODES.TIRAMISU

companion object {
const val MAX_DIALOG_ATTEMPTS = 2
const val DEFAULT_URL = "https://duckduckgo.com"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import com.duckduckgo.app.global.DefaultRoleBrowserDialog
import com.duckduckgo.app.global.install.AppInstallStore
import com.duckduckgo.app.pixels.AppPixelName
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flow

Expand All @@ -34,7 +33,6 @@ class WelcomePageViewModel(
private val context: Context,
private val pixel: Pixel,
private val defaultRoleBrowserDialog: DefaultRoleBrowserDialog,
private val appBuildConfig: AppBuildConfig,
) : ViewModel() {

fun reduce(event: WelcomePageView.Event): Flow<WelcomePageView.State> {
Expand Down Expand Up @@ -68,7 +66,6 @@ class WelcomePageViewModel(
AppPixelName.DEFAULT_BROWSER_SET,
mapOf(
Pixel.PixelParameter.DEFAULT_BROWSER_SET_FROM_ONBOARDING to true.toString(),
Pixel.PixelParameter.DEFAULT_BROWSER_SET_ON_ANDROID_13_OR_ABOVE to appBuildConfig.isAndroid13OrAbove().toString(),
),
)

Expand All @@ -84,14 +81,11 @@ class WelcomePageViewModel(
AppPixelName.DEFAULT_BROWSER_NOT_SET,
mapOf(
Pixel.PixelParameter.DEFAULT_BROWSER_SET_FROM_ONBOARDING to true.toString(),
Pixel.PixelParameter.DEFAULT_BROWSER_SET_ON_ANDROID_13_OR_ABOVE to appBuildConfig.isAndroid13OrAbove().toString(),
),
)

emit(WelcomePageView.State.Finish)
}

private fun AppBuildConfig.isAndroid13OrAbove(): Boolean = sdkInt >= android.os.Build.VERSION_CODES.TIRAMISU
}

@Suppress("UNCHECKED_CAST")
Expand All @@ -100,7 +94,6 @@ class WelcomePageViewModelFactory(
private val context: Context,
private val pixel: Pixel,
private val defaultRoleBrowserDialog: DefaultRoleBrowserDialog,
private val appBuildConfig: AppBuildConfig,
) : ViewModelProvider.NewInstanceFactory() {

override fun <T : ViewModel> create(modelClass: Class<T>): T {
Expand All @@ -111,7 +104,6 @@ class WelcomePageViewModelFactory(
context,
pixel,
defaultRoleBrowserDialog,
appBuildConfig,
)
else -> throw IllegalArgumentException("Unknown ViewModel class: ${modelClass.name}")
}
Expand Down
3 changes: 0 additions & 3 deletions app/src/main/java/com/duckduckgo/app/pixels/AppPixelName.kt
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,4 @@ enum class AppPixelName(override val pixelName: String) : Pixel.PixelName {
REMOTE_MESSAGE_SECONDARY_ACTION_CLICKED("m_remote_message_secondary_action_clicked"),

CREATE_BLOOM_FILTER_ERROR("m_create_bloom_filter_error"),

NOTIFY_ME_BUTTON_PRESSED("m_notify_me_component_notify_me_button_pressed"),
NOTIFY_ME_DISMISS_BUTTON_PRESSED("m_notify_me_component_close_button_pressed"),
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ import com.duckduckgo.app.downloads.DownloadsViewModel.Command.ShareFile
import com.duckduckgo.app.global.R as CommonR
import com.duckduckgo.app.global.formatters.time.RealTimeDiffFormatter
import com.duckduckgo.app.global.formatters.time.TimeDiffFormatter
import com.duckduckgo.app.pixels.AppPixelName
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter.NOTIFY_ME_FROM_SCREEN
import com.duckduckgo.app.statistics.pixels.Pixel.PixelValues.NOTIFY_ME_DOWNLOADS_SCREEN
import com.duckduckgo.downloads.api.DownloadsRepository
import com.duckduckgo.downloads.api.model.DownloadItem
import com.duckduckgo.downloads.store.DownloadStatus.FINISHED
Expand Down Expand Up @@ -64,7 +60,6 @@ class DownloadsViewModelTest {
var coroutineRule = CoroutineTestRule()

private val mockDownloadsRepository: DownloadsRepository = mock()
private val mockPixel: Pixel = mock()

private val context: Context = mock()

Expand All @@ -74,7 +69,6 @@ class DownloadsViewModelTest {
FakeTimeDiffFormatter(TODAY, RealTimeDiffFormatter(context)),
mockDownloadsRepository,
coroutineRule.testDispatcherProvider,
mockPixel,
)
model
}
Expand Down Expand Up @@ -342,24 +336,6 @@ class DownloadsViewModelTest {
}
}

@Test
fun whenNotifyMeButtonClickedThenPixelIsSentWithCorrectParams() = runTest {
testee.onNotifyMeButtonClicked()

testee.commands().test {
verify(mockPixel).fire(AppPixelName.NOTIFY_ME_BUTTON_PRESSED, mapOf(NOTIFY_ME_FROM_SCREEN to NOTIFY_ME_DOWNLOADS_SCREEN))
}
}

@Test
fun whenNotifyMeDismissButtonClickedThenPixelIsSentWithCorrectParams() = runTest {
testee.onNotifyMeDismissButtonClicked()

testee.commands().test {
verify(mockPixel).fire(AppPixelName.NOTIFY_ME_DISMISS_BUTTON_PRESSED, mapOf(NOTIFY_ME_FROM_SCREEN to NOTIFY_ME_DOWNLOADS_SCREEN))
}
}

@Test
fun whenRemoveFromDownloadManagerThenRemoveIt() = runTest {
val downloadId = 1L
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@ class AtpPixelRemovalInterceptorTest {
companion object {
private const val PIXEL_TEMPLATE = "https://improving.duckduckgo.com/t/%s_android_phone?atb=v255-7zu&appVersion=5.74.0&test=1"

private val PIXELS_WITH_ATB_INFO = listOf<String>(
DeviceShieldPixelNames.ATP_DID_PRESS_NOTIFY_ME_BUTTON.pixelName,
DeviceShieldPixelNames.ATP_DID_PRESS_NOTIFY_ME_DISMISS_BUTTON.pixelName,
)
private val PIXELS_WITH_ATB_INFO = listOf<String>()
}
}
Loading

0 comments on commit bd53e83

Please sign in to comment.