Skip to content

Commit

Permalink
New privacyProEligible filter for experiments (#4967)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1201807753394693/1208158029105828/f

### Description
Add new filter `privacyProEligible` to remote experiment framework

### Steps to test this PR
_Pre steps_
- [x] Change privacy-config URL for `PRIVACY_REMOTE_CONFIG_URL =
"https://jsonblob.com/api/1279046082855034880"`
- [x] Make sure you remove DuckDuckGo folder under Downloads directory

_privacyProEligible filter disabled_
- [x] In the JsonBlob, under `experimentalVariants` set
`privacyProEligible` to false for `mq` and `mr` variants
>         "filters": {
>           "privacyProEligible": false
>         }
- [x] Fresh install
- [x] Filter by 'variant' in Logcat
- [x] If you are **not** eligible for privacyPro, then `mq` or `mr`
**should** be assigned
- [x] If you **are** eligible, then **no** variant should be assigned

_privacyProEligible filter enabled_
- [ ] In the JsonBlob, under `experimentalVariants` set
`privacyProEligible` to true for `mq` and `mr` variants
>         "filters": {
>           "privacyProEligible": true
>         }
- [ ] Fresh install
- [ ] Filter by 'variant' in Logcat
- [ ] If you are **not eligible** for privacyPro, then `mq` or `mr`
**should not** be assigned
- [ ] If you **are** eligible, then a variant **should** be assigned

### No UI changes
  • Loading branch information
nalcalag authored Sep 3, 2024
1 parent 4917ff5 commit 70b723d
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,5 @@ data class VariantConfig(
data class VariantFilters(
val locale: List<String> = emptyList(),
val androidVersion: List<String> = emptyList(),
val privacyProEligible: Boolean? = null,
)
4 changes: 2 additions & 2 deletions experiments/experiments-impl/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ dependencies {

implementation project(path: ':di')
implementation project(path: ':common-utils')
implementation project(path: ':experiments-api')
implementation project(path: ':statistics-api')
implementation project(path: ':app-build-config-api')
implementation project(path: ':browser-api')
implementation project(path: ':experiments-api')
implementation project(path: ':subscriptions-api')

// Room
implementation AndroidX.room.runtime
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@
package com.duckduckgo.experiments.impl

import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.common.utils.DispatcherProvider
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.experiments.api.VariantConfig
import com.duckduckgo.experiments.impl.ExperimentFiltersManagerImpl.ExperimentFilterType.ANDROID_VERSION
import com.duckduckgo.experiments.impl.ExperimentFiltersManagerImpl.ExperimentFilterType.LOCALE
import com.duckduckgo.experiments.impl.ExperimentFiltersManagerImpl.ExperimentFilterType.PRIVACY_PRO_ELIGIBLE
import com.duckduckgo.subscriptions.api.Subscriptions
import com.squareup.anvil.annotations.ContributesBinding
import java.util.Locale
import javax.inject.Inject
import kotlinx.coroutines.runBlocking

interface ExperimentFiltersManager {
fun addFilters(entity: VariantConfig): (AppBuildConfig) -> Boolean
Expand All @@ -32,6 +36,8 @@ interface ExperimentFiltersManager {
@ContributesBinding(AppScope::class)
class ExperimentFiltersManagerImpl @Inject constructor(
private val appBuildConfig: AppBuildConfig,
private val subscriptions: Subscriptions,
private val dispatcherProvider: DispatcherProvider,
) : ExperimentFiltersManager {
override fun addFilters(entity: VariantConfig): (AppBuildConfig) -> Boolean {
if (entity.variantKey == "sc" || entity.variantKey == "se") {
Expand All @@ -41,6 +47,7 @@ class ExperimentFiltersManagerImpl @Inject constructor(
val filters: MutableMap<ExperimentFilterType, Boolean> = mutableMapOf(
LOCALE to true,
ANDROID_VERSION to true,
PRIVACY_PRO_ELIGIBLE to true,
)

if (!entity.filters?.locale.isNullOrEmpty()) {
Expand All @@ -51,6 +58,10 @@ class ExperimentFiltersManagerImpl @Inject constructor(
val userAndroidVersion = appBuildConfig.sdkInt.toString()
filters[ANDROID_VERSION] = entity.filters!!.androidVersion.contains(userAndroidVersion)
}
if (entity.filters?.privacyProEligible != null) {
val privacyProEligible = runBlocking(dispatcherProvider.io()) { subscriptions.isEligible() }
filters[PRIVACY_PRO_ELIGIBLE] = entity.filters?.privacyProEligible == privacyProEligible
}

return { filters.filter { !it.value }.isEmpty() }
}
Expand Down Expand Up @@ -79,5 +90,6 @@ class ExperimentFiltersManagerImpl @Inject constructor(
enum class ExperimentFilterType {
LOCALE,
ANDROID_VERSION,
PRIVACY_PRO_ELIGIBLE,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,36 @@
package com.duckduckgo.experiments.impl

import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.experiments.api.VariantConfig
import com.duckduckgo.experiments.api.VariantFilters
import com.duckduckgo.subscriptions.api.Subscriptions
import java.util.Locale
import junit.framework.TestCase.assertFalse
import junit.framework.TestCase.assertTrue
import kotlinx.coroutines.test.runTest
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.mockito.kotlin.mock
import org.mockito.kotlin.whenever

class ExperimentFiltersManagerImplTest {

@get:Rule
val coroutineRule = CoroutineTestRule()

private lateinit var testee: ExperimentFiltersManager

private val mockAppBuildConfig: AppBuildConfig = mock()
private val mockSubscriptions: Subscriptions = mock()

@Before
fun setup() {
testee = ExperimentFiltersManagerImpl(
mockAppBuildConfig,
mockSubscriptions,
coroutineRule.testDispatcherProvider,
)
}

Expand Down Expand Up @@ -75,11 +85,32 @@ class ExperimentFiltersManagerImplTest {
}

@Test
fun whenVariantComplyWithBothFiltersThenAddFiltersReturnsTrue() {
fun whenVariantComplyWithPrivacyProEligibleFilterThenAddFiltersReturnsTrue() = runTest {
whenever(mockSubscriptions.isEligible()).thenReturn(true)
val testEntity = addActiveVariant(privacyProEligible = true)

assertTrue(testee.addFilters(testEntity).invoke(mockAppBuildConfig))
}

@Test
fun whenVariantDoesNotComplyWithPrivacyProEligibleFilterThenAddFiltersReturnsFalse() = runTest {
whenever(mockSubscriptions.isEligible()).thenReturn(false)
val testEntity = addActiveVariant(privacyProEligible = true)

assertFalse(testee.addFilters(testEntity).invoke(mockAppBuildConfig))
}

@Test
fun whenVariantComplyWithAllFiltersThenAddFiltersReturnsTrue() = runTest {
val locale = Locale("en", "US")
Locale.setDefault(locale)
whenever(mockAppBuildConfig.sdkInt).thenReturn(33)
val testEntity = addActiveVariant(localeFilter = listOf("en_US"), androidVersionFilter = listOf("33", "34"))
whenever(mockSubscriptions.isEligible()).thenReturn(false)
val testEntity = addActiveVariant(
localeFilter = listOf("en_US"),
androidVersionFilter = listOf("33", "34"),
privacyProEligible = false,
)

assertTrue(testee.addFilters(testEntity).invoke(mockAppBuildConfig))
}
Expand All @@ -104,10 +135,26 @@ class ExperimentFiltersManagerImplTest {
assertFalse(testee.addFilters(testEntity).invoke(mockAppBuildConfig))
}

@Test
fun whenVariantComplyWithLocaleAndAndroidVersionFiltersAndDoesNotComplyWithPrivacyProEligibleThenAddFiltersReturnsFalse() = runTest {
val locale = Locale("en", "US")
Locale.setDefault(locale)
whenever(mockAppBuildConfig.sdkInt).thenReturn(33)
whenever(mockSubscriptions.isEligible()).thenReturn(true)
val testEntity = addActiveVariant(
localeFilter = listOf("en_US"),
androidVersionFilter = listOf("33", "34"),
privacyProEligible = false,
)

assertFalse(testee.addFilters(testEntity).invoke(mockAppBuildConfig))
}

private fun addActiveVariant(
localeFilter: List<String> = listOf(),
androidVersionFilter: List<String> = listOf(),
privacyProEligible: Boolean? = null,
): VariantConfig {
return VariantConfig("key", 1.0, VariantFilters(localeFilter, androidVersionFilter))
return VariantConfig("key", 1.0, VariantFilters(localeFilter, androidVersionFilter, privacyProEligible))
}
}
1 change: 0 additions & 1 deletion statistics/statistics-impl/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ dependencies {
implementation project(path: ':app-build-config-api')
implementation project(path: ':browser-api')
implementation project(path: ':autofill-api')
implementation project(path: ':experiments-api')
implementation project(path: ':privacy-config-api')
implementation project(path: ':data-store-api')
implementation project(path: ':anrs-api')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import com.duckduckgo.app.statistics.store.PixelFiredRepository
import com.duckduckgo.app.statistics.store.StatisticsDataStore
import com.duckduckgo.common.utils.device.DeviceInfo
import com.duckduckgo.di.scopes.AppScope
import com.duckduckgo.experiments.api.VariantManager
import com.squareup.anvil.annotations.ContributesBinding
import com.squareup.anvil.annotations.ContributesMultibinding
import dagger.SingleInstanceIn
Expand All @@ -51,7 +50,6 @@ class RxPixelSender @Inject constructor(
private val api: PixelService,
private val pendingPixelDao: PendingPixelDao,
private val statisticsDataStore: StatisticsDataStore,
private val variantManager: VariantManager,
private val deviceInfo: DeviceInfo,
private val statisticsLibraryConfig: StatisticsLibraryConfig?,
private val pixelFiredRepository: PixelFiredRepository,
Expand Down Expand Up @@ -159,7 +157,7 @@ class RxPixelSender @Inject constructor(
return defaultParameters.plus(parameters)
}

private fun getAtbInfo() = statisticsDataStore.atb?.formatWithVariant(variantManager.getVariantKey()) ?: ""
private fun getAtbInfo() = statisticsDataStore.atb?.formatWithVariant(statisticsDataStore.variant) ?: ""

private fun getDeviceFactor() = deviceInfo.formFactor().description

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import com.duckduckgo.app.statistics.store.StatisticsDataStore
import com.duckduckgo.common.test.CoroutineTestRule
import com.duckduckgo.common.test.InstantSchedulersRule
import com.duckduckgo.common.utils.device.DeviceInfo
import com.duckduckgo.experiments.api.VariantManager
import io.reactivex.Completable
import java.util.concurrent.TimeoutException
import kotlinx.coroutines.test.runTest
Expand Down Expand Up @@ -74,9 +73,6 @@ class RxPixelSenderTest {
@Mock
val mockStatisticsDataStore: StatisticsDataStore = mock()

@Mock
val mockVariantManager: VariantManager = mock()

@Mock
val mockDeviceInfo: DeviceInfo = mock()

Expand All @@ -97,7 +93,6 @@ class RxPixelSenderTest {
api,
pendingPixelDao,
mockStatisticsDataStore,
mockVariantManager,
mockDeviceInfo,
object : StatisticsLibraryConfig {
override fun shouldFirePixelsAsDev() = true
Expand Down Expand Up @@ -397,7 +392,7 @@ class RxPixelSenderTest {
}

private fun givenVariant(variantKey: String) {
whenever(mockVariantManager.getVariantKey()).thenReturn(variantKey)
whenever(mockStatisticsDataStore.variant).thenReturn(variantKey)
}

private fun givenAtbVariant(atb: Atb) {
Expand Down

0 comments on commit 70b723d

Please sign in to comment.