Skip to content

Commit

Permalink
Convert enum to data class and store tag/pixel name (#5057)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1125189844152671/1208390039947645/f

### Description
This PRs changes the PixelType from enum to sealed class to add the
option of storing a tag rather than the pixel name to avoid sending
duplicated pixels (unique/daily)

### Steps to test this PR
- [ ] Check code changes
- [ ] Tests pass
  • Loading branch information
marcosholgado authored Sep 26, 2024
1 parent cc3b3cc commit f5fab29
Show file tree
Hide file tree
Showing 53 changed files with 370 additions and 296 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import androidx.core.content.edit
import androidx.test.ext.junit.runners.AndroidJUnit4
import com.duckduckgo.adclick.impl.Exemption
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.COUNT
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Count
import com.duckduckgo.common.test.api.InMemorySharedPreferences
import java.time.Instant
import java.util.concurrent.TimeUnit
Expand Down Expand Up @@ -248,7 +248,7 @@ class RealAdClickPixelsTest {
pixel = eq(AdClickPixelName.AD_CLICK_PAGELOADS_WITH_AD_ATTRIBUTION),
parameters = any(),
encodedParameters = any(),
type = eq(COUNT),
type = eq(Count),
)
}

Expand All @@ -264,7 +264,7 @@ class RealAdClickPixelsTest {
pixel = eq(AdClickPixelName.AD_CLICK_PAGELOADS_WITH_AD_ATTRIBUTION),
parameters = eq(mapOf(AdClickPixelParameters.AD_CLICK_PAGELOADS_WITH_AD_ATTRIBUTION_COUNT to "1")),
encodedParameters = any(),
type = eq(COUNT),
type = eq(Count),
)
}

Expand All @@ -283,7 +283,7 @@ class RealAdClickPixelsTest {
pixel = eq(AdClickPixelName.AD_CLICK_PAGELOADS_WITH_AD_ATTRIBUTION),
parameters = any(),
encodedParameters = any(),
type = eq(COUNT),
type = eq(Count),
)
}

Expand All @@ -302,7 +302,7 @@ class RealAdClickPixelsTest {
pixel = eq(AdClickPixelName.AD_CLICK_PAGELOADS_WITH_AD_ATTRIBUTION),
parameters = eq(mapOf(AdClickPixelParameters.AD_CLICK_PAGELOADS_WITH_AD_ATTRIBUTION_COUNT to "1")),
encodedParameters = any(),
type = eq(COUNT),
type = eq(Count),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import com.duckduckgo.anrs.api.AnrRepository
import com.duckduckgo.app.statistics.api.OfflinePixel
import com.duckduckgo.app.statistics.api.PixelSender
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.COUNT
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Count
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesMultibinding
import io.reactivex.Completable
Expand All @@ -47,7 +47,7 @@ class AnrOfflinePixelSender @Inject constructor(
ANR_CUSTOM_TAB to it.customTab.toString(),
),
mapOf(),
COUNT,
Count,
).ignoreElement().doOnComplete {
anrRepository.removeMostRecentAnr()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import com.duckduckgo.app.anr.CrashPixel.APPLICATION_CRASH_GLOBAL_VERIFIED_INSTA
import com.duckduckgo.app.anrs.store.UncaughtExceptionDao
import com.duckduckgo.app.statistics.api.OfflinePixel
import com.duckduckgo.app.statistics.api.PixelSender
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.COUNT
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Count
import com.duckduckgo.browser.api.WebViewVersionProvider
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.verifiedinstallation.IsVerifiedPlayStoreInstall
Expand Down Expand Up @@ -63,13 +63,13 @@ class CrashOfflinePixelSender @Inject constructor(
pixelName = APPLICATION_CRASH_GLOBAL_VERIFIED_INSTALL.pixelName,
parameters = params,
encodedParameters = emptyMap(),
type = COUNT,
type = Count,
)
pixels.add(verifiedPixel.ignoreElement())
}

val pixel =
pixelSender.sendPixel(APPLICATION_CRASH_GLOBAL.pixelName, params, emptyMap(), COUNT).ignoreElement().doOnComplete {
pixelSender.sendPixel(APPLICATION_CRASH_GLOBAL.pixelName, params, emptyMap(), Count).ignoreElement().doOnComplete {
logcat { "Sent pixel with params: $params containing exception; deleting exception with id=${exception.hash}" }
uncaughtExceptionDao.delete(exception)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ import com.duckduckgo.app.settings.db.SettingsDataStore
import com.duckduckgo.app.statistics.api.StatisticsUpdater
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.COUNT
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.DAILY
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.UNIQUE
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Count
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Daily
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Unique
import com.duckduckgo.app.surrogates.SurrogateResponse
import com.duckduckgo.app.tabs.model.TabEntity
import com.duckduckgo.app.tabs.model.TabRepository
Expand Down Expand Up @@ -5226,8 +5226,8 @@ class BrowserTabViewModelTest {
whenever(mockUserAllowListRepository.isDomainInUserAllowList("www.example.com")).thenReturn(true)
testee.onPrivacyProtectionMenuClicked()

verify(mockPixel).fire(AppPixelName.BROWSER_MENU_ALLOWLIST_ADD, params, type = COUNT)
verify(mockPixel).fire(AppPixelName.BROWSER_MENU_ALLOWLIST_REMOVE, params, type = COUNT)
verify(mockPixel).fire(AppPixelName.BROWSER_MENU_ALLOWLIST_ADD, params, type = Count)
verify(mockPixel).fire(AppPixelName.BROWSER_MENU_ALLOWLIST_REMOVE, params, type = Count)
verify(privacyProtectionsPopupExperimentExternalPixels).tryReportProtectionsToggledFromBrowserMenu(protectionsEnabled = false)
verify(privacyProtectionsPopupExperimentExternalPixels).tryReportProtectionsToggledFromBrowserMenu(protectionsEnabled = true)
}
Expand Down Expand Up @@ -5512,7 +5512,7 @@ class BrowserTabViewModelTest {
val testParams = mapOf("daysSinceInstall" to "0", "from_onboarding" to "true")

testee.onPrivacyShieldSelected()
verify(mockPixel).fire(pixel = PrivacyDashboardPixels.PRIVACY_DASHBOARD_FIRST_TIME_OPENED, parameters = testParams, type = UNIQUE)
verify(mockPixel).fire(pixel = PrivacyDashboardPixels.PRIVACY_DASHBOARD_FIRST_TIME_OPENED, parameters = testParams, type = Unique())
}

@Test
Expand Down Expand Up @@ -5545,7 +5545,7 @@ class BrowserTabViewModelTest {

testee.onUserSubmittedQuery("foo")

verify(mockPixel).fire(ONBOARDING_SEARCH_CUSTOM, type = UNIQUE)
verify(mockPixel).fire(ONBOARDING_SEARCH_CUSTOM, type = Unique())
}

@Test
Expand All @@ -5556,7 +5556,7 @@ class BrowserTabViewModelTest {

testee.onUserSubmittedQuery("foo")

verify(mockPixel).fire(ONBOARDING_VISIT_SITE_CUSTOM, type = UNIQUE)
verify(mockPixel).fire(ONBOARDING_VISIT_SITE_CUSTOM, type = Unique())
}

@Test
Expand Down Expand Up @@ -5607,7 +5607,7 @@ class BrowserTabViewModelTest {

assertCommandIssued<Command.LaunchTabSwitcher>()
verify(mockPixel).fire(AppPixelName.TAB_MANAGER_CLICKED)
verify(mockPixel).fire(AppPixelName.TAB_MANAGER_CLICKED_DAILY, emptyMap(), emptyMap(), DAILY)
verify(mockPixel).fire(AppPixelName.TAB_MANAGER_CLICKED_DAILY, emptyMap(), emptyMap(), Daily())
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ import com.duckduckgo.app.privacy.model.HttpsStatus
import com.duckduckgo.app.privacy.model.TestEntity
import com.duckduckgo.app.settings.db.SettingsDataStore
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.COUNT
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.UNIQUE
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Count
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Unique
import com.duckduckgo.app.tabs.model.TabEntity
import com.duckduckgo.app.tabs.model.TabRepository
import com.duckduckgo.app.trackerdetection.model.Entity
Expand Down Expand Up @@ -173,32 +173,32 @@ class CtaViewModelTest {
@Test
fun whenCtaShownAndCtaIsDaxAndCanNotSendPixelThenPixelIsNotFired() {
testee.onCtaShown(DaxBubbleCta.DaxIntroSearchOptionsCta(mockOnboardingStore, mockAppInstallStore))
verify(mockPixel, never()).fire(eq(SURVEY_CTA_SHOWN), any(), any(), eq(COUNT))
verify(mockPixel, never()).fire(eq(SURVEY_CTA_SHOWN), any(), any(), eq(Count))
}

@Test
fun whenCtaShownAndCtaIsDaxAndCanSendPixelThenPixelIsFired() {
whenever(mockOnboardingStore.onboardingDialogJourney).thenReturn("s:0")
testee.onCtaShown(DaxBubbleCta.DaxEndCta(mockOnboardingStore, mockAppInstallStore))
verify(mockPixel, never()).fire(eq(SURVEY_CTA_SHOWN), any(), any(), eq(COUNT))
verify(mockPixel, never()).fire(eq(SURVEY_CTA_SHOWN), any(), any(), eq(Count))
}

@Test
fun whenCtaShownAndCtaIsNotDaxThenPixelIsFired() {
testee.onCtaShown(HomePanelCta.AddWidgetAuto)
verify(mockPixel).fire(eq(WIDGET_CTA_SHOWN), any(), any(), eq(COUNT))
verify(mockPixel).fire(eq(WIDGET_CTA_SHOWN), any(), any(), eq(Count))
}

@Test
fun whenCtaLaunchedPixelIsFired() {
testee.onUserClickCtaOkButton(HomePanelCta.AddWidgetAuto)
verify(mockPixel).fire(eq(WIDGET_CTA_LAUNCHED), any(), any(), eq(COUNT))
verify(mockPixel).fire(eq(WIDGET_CTA_LAUNCHED), any(), any(), eq(Count))
}

@Test
fun whenCtaDismissedPixelIsFired() = runTest {
testee.onUserDismissedCta(HomePanelCta.AddWidgetAuto)
verify(mockPixel).fire(eq(WIDGET_CTA_DISMISSED), any(), any(), eq(COUNT))
verify(mockPixel).fire(eq(WIDGET_CTA_DISMISSED), any(), any(), eq(Count))
}

@Test
Expand Down Expand Up @@ -745,7 +745,7 @@ class CtaViewModelTest {
val site = site(url = privacyProUrl)

val value = testee.refreshCta(coroutineRule.testDispatcher, isBrowserShowing = true, site = site)
verify(mockPixel, never()).fire(eq(ONBOARDING_SKIP_MAJOR_NETWORK_UNIQUE), any(), any(), eq(UNIQUE))
verify(mockPixel, never()).fire(eq(ONBOARDING_SKIP_MAJOR_NETWORK_UNIQUE), any(), any(), eq(Unique()))
assertNull(value)
}

Expand All @@ -761,7 +761,7 @@ class CtaViewModelTest {
whenever(mockDuckPlayer.isSimulatedYoutubeNoCookie(anyString())).thenReturn(false)

val value = testee.refreshCta(coroutineRule.testDispatcher, isBrowserShowing = true, site = site)
verify(mockPixel).fire(eq(ONBOARDING_SKIP_MAJOR_NETWORK_UNIQUE), any(), any(), eq(UNIQUE))
verify(mockPixel).fire(eq(ONBOARDING_SKIP_MAJOR_NETWORK_UNIQUE), any(), any(), eq(Unique()))
assertNull(value)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import com.duckduckgo.app.notification.model.SchedulableNotificationPlugin
import com.duckduckgo.app.pixels.AppPixelName
import com.duckduckgo.app.settings.db.SettingsDataStore
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.COUNT
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Count
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.common.utils.plugins.PluginPoint
Expand Down Expand Up @@ -76,31 +76,31 @@ class NotificationRegistrarTest {
fun whenNotificationsPreviouslyOffAndNowOnThenPixelIsFiredAndSettingsUpdated() {
whenever(mockSettingsDataStore.appNotificationsEnabled).thenReturn(false)
testee.updateStatus(true)
verify(mockPixel).fire(eq(AppPixelName.NOTIFICATIONS_ENABLED), any(), any(), eq(COUNT))
verify(mockPixel).fire(eq(AppPixelName.NOTIFICATIONS_ENABLED), any(), any(), eq(Count))
verify(mockSettingsDataStore).appNotificationsEnabled = true
}

@Test
fun whenNotificationsPreviouslyOffAndStillOffThenNoPixelIsFiredAndSettingsUnchanged() {
whenever(mockSettingsDataStore.appNotificationsEnabled).thenReturn(false)
testee.updateStatus(false)
verify(mockPixel, never()).fire(any<Pixel.PixelName>(), any(), any(), eq(COUNT))
verify(mockPixel, never()).fire(any<Pixel.PixelName>(), any(), any(), eq(Count))
verify(mockSettingsDataStore, never()).appNotificationsEnabled = true
}

@Test
fun whenNotificationsPreviouslyOnAndStillOnThenNoPixelIsFiredAndSettingsUnchanged() {
whenever(mockSettingsDataStore.appNotificationsEnabled).thenReturn(true)
testee.updateStatus(true)
verify(mockPixel, never()).fire(any<Pixel.PixelName>(), any(), any(), eq(COUNT))
verify(mockPixel, never()).fire(any<Pixel.PixelName>(), any(), any(), eq(Count))
verify(mockSettingsDataStore, never()).appNotificationsEnabled = false
}

@Test
fun whenNotificationsPreviouslyOnAndNowOffPixelIsFiredAndSettingsUpdated() {
whenever(mockSettingsDataStore.appNotificationsEnabled).thenReturn(true)
testee.updateStatus(false)
verify(mockPixel).fire(eq(AppPixelName.NOTIFICATIONS_DISABLED), any(), any(), eq(COUNT))
verify(mockPixel).fire(eq(AppPixelName.NOTIFICATIONS_DISABLED), any(), any(), eq(Count))
verify(mockSettingsDataStore).appNotificationsEnabled = false
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import com.duckduckgo.app.brokensite.model.SiteProtectionsState.ENABLED
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.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
Expand Down Expand Up @@ -187,10 +187,10 @@ class BrokenSiteViewModel @Inject constructor(
val pixelParams = privacyProtectionsPopupExperimentExternalPixels.getPixelParams()
if (protectionsEnabled) {
userAllowListRepository.removeDomainFromUserAllowList(domain)
pixel.fire(AppPixelName.BROKEN_SITE_ALLOWLIST_REMOVE, pixelParams, type = COUNT)
pixel.fire(AppPixelName.BROKEN_SITE_ALLOWLIST_REMOVE, pixelParams, type = Count)
} else {
userAllowListRepository.addDomainToUserAllowList(domain)
pixel.fire(AppPixelName.BROKEN_SITE_ALLOWLIST_ADD, pixelParams, type = COUNT)
pixel.fire(AppPixelName.BROKEN_SITE_ALLOWLIST_ADD, pixelParams, type = Count)
}
privacyProtectionsPopupExperimentExternalPixels.tryReportProtectionsToggledFromBrokenSiteReport(protectionsEnabled)
protectionsToggleUsageListener.onPrivacyProtectionsToggleUsed()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ import com.duckduckgo.app.settings.db.SettingsDataStore
import com.duckduckgo.app.statistics.pixels.Pixel
import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter.FIRE_BUTTON_STATE
import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter.LOADING_BAR_EXPERIMENT
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.DAILY
import com.duckduckgo.app.statistics.pixels.Pixel.PixelType.Daily
import com.duckduckgo.app.tabs.model.TabEntity
import com.duckduckgo.app.tabs.ui.GridViewColumnCalculator
import com.duckduckgo.app.tabs.ui.TabSwitcherActivity
Expand Down Expand Up @@ -2798,11 +2798,11 @@ class BrowserTabFragment :
pixel.fire(
AppPixelName.REFRESH_ACTION_DAILY_PIXEL.pixelName,
mapOf(LOADING_BAR_EXPERIMENT to loadingBarExperimentManager.variant.toBinaryString()),
type = DAILY,
type = Daily(),
)
} else {
pixel.fire(AppPixelName.BROWSER_PULL_TO_REFRESH.pixelName)
pixel.fire(AppPixelName.REFRESH_ACTION_DAILY_PIXEL.pixelName, type = DAILY)
pixel.fire(AppPixelName.REFRESH_ACTION_DAILY_PIXEL.pixelName, type = Daily())
}
}

Expand Down Expand Up @@ -3656,11 +3656,11 @@ class BrowserTabFragment :
pixel.fire(
AppPixelName.REFRESH_ACTION_DAILY_PIXEL.pixelName,
mapOf(LOADING_BAR_EXPERIMENT to loadingBarExperimentManager.variant.toBinaryString()),
type = DAILY,
type = Daily(),
)
} else {
pixel.fire(AppPixelName.MENU_ACTION_REFRESH_PRESSED.pixelName)
pixel.fire(AppPixelName.REFRESH_ACTION_DAILY_PIXEL.pixelName, type = DAILY)
pixel.fire(AppPixelName.REFRESH_ACTION_DAILY_PIXEL.pixelName, type = Daily())
}
}
}
Expand Down
Loading

0 comments on commit f5fab29

Please sign in to comment.