From 178a34cfefe2fa9700bc105aacd5f153aab9a9d3 Mon Sep 17 00:00:00 2001 From: Marcos Holgado Date: Tue, 27 Apr 2021 10:58:39 +0100 Subject: [PATCH 1/2] Add 2nd experiment for uoa adding all notifications --- .../AndroidNotificationSchedulerTest.kt | 69 ++++++++++++++++++- .../app/statistics/VariantManagerTest.kt | 22 +++++- .../duckduckgo/app/di/NotificationModule.kt | 7 +- .../AndroidNotificationScheduler.kt | 31 +++++++-- .../app/statistics/VariantManager.kt | 9 ++- 5 files changed, 123 insertions(+), 15 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt index 9f52de9e79bf..b6569b913028 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/notification/AndroidNotificationSchedulerTest.kt @@ -26,6 +26,7 @@ import androidx.work.WorkManager import androidx.work.impl.utils.SynchronousExecutor import androidx.work.testing.WorkManagerTestInitHelper import com.duckduckgo.app.CoroutineTestRule +import com.duckduckgo.app.global.install.AppInstallStore import com.duckduckgo.app.notification.NotificationScheduler.ClearDataNotificationWorker import com.duckduckgo.app.notification.NotificationScheduler.PrivacyNotificationWorker import com.duckduckgo.app.notification.model.SchedulableNotification @@ -36,10 +37,12 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test +import java.util.concurrent.TimeUnit import kotlin.reflect.jvm.jvmName class AndroidNotificationSchedulerTest { @@ -52,6 +55,7 @@ class AndroidNotificationSchedulerTest { private val privacyNotification: SchedulableNotification = mock() private val useOurAppNotification: SchedulableNotification = mock() private val variantManager: VariantManager = mock() + private val appInstallStore: AppInstallStore = mock() private val context = InstrumentationRegistry.getInstrumentation().targetContext private lateinit var workManager: WorkManager @@ -66,7 +70,8 @@ class AndroidNotificationSchedulerTest { clearNotification, privacyNotification, useOurAppNotification, - variantManager + variantManager, + appInstallStore ) } @@ -204,6 +209,67 @@ class AndroidNotificationSchedulerTest { assertNoNotificationScheduled() } + @Test + fun whenInAppVariantAndGetDurationForInactiveNotificationForDay1AndAppHasBeenInstalled0DaysThenReturn1() { + setInAppUsageVariant() + givenAppHasBeenInstalledForDays(days = 0) + + assertEquals(1, testee.getDurationForInactiveNotification(1)) + } + + @Test + fun whenInAppVariantAndGetDurationForInactiveNotificationForDay1AndAppHasBeenInstalled1DayThenReturn1() { + setInAppUsageVariant() + givenAppHasBeenInstalledForDays(days = 1) + + assertEquals(1, testee.getDurationForInactiveNotification(1)) + } + + @Test + fun whenInAppVariantAndGetDurationForInactiveNotificationForDay1AndAppHasBeenInstalled2DaysThenReturn2() { + setInAppUsageVariant() + givenAppHasBeenInstalledForDays(days = 2) + + assertEquals(2, testee.getDurationForInactiveNotification(1)) + } + + @Test + fun whenInAppVariantAndGetDurationForInactiveNotificationForDay3AndAppHasBeenInstalled0DaysThenReturn4() { + setInAppUsageVariant() + givenAppHasBeenInstalledForDays(days = 0) + + assertEquals(4, testee.getDurationForInactiveNotification(3)) + } + + @Test + fun whenInAppVariantAndGetDurationForInactiveNotificationForDay3AndAppHasBeenInstalled1DayThenReturn3() { + setInAppUsageVariant() + givenAppHasBeenInstalledForDays(days = 1) + + assertEquals(3, testee.getDurationForInactiveNotification(3)) + } + + @Test + fun whenDefaultVariantAndGetDurationForInactiveNotificationForDay1AndAppHasBeenInstalled2DaysThenReturn1() { + setDefaultVariant() + givenAppHasBeenInstalledForDays(days = 2) + + assertEquals(1, testee.getDurationForInactiveNotification(1)) + } + + @Test + fun whenDefaultVariantAndGetDurationForInactiveNotificationForDay3AndAppHasBeenInstalled0DaysThenReturn3() { + setDefaultVariant() + givenAppHasBeenInstalledForDays(days = 0) + + assertEquals(3, testee.getDurationForInactiveNotification(3)) + } + + private fun givenAppHasBeenInstalledForDays(days: Long) { + val timeSinceInstallation = System.currentTimeMillis() - TimeUnit.DAYS.toMillis(days) + whenever(appInstallStore.installTimestamp).thenReturn(timeSinceInstallation) + } + private suspend fun givenNoInactiveUserNotifications() { whenever(privacyNotification.canShow()).thenReturn(false) whenever(clearNotification.canShow()).thenReturn(false) @@ -215,7 +281,6 @@ class AndroidNotificationSchedulerTest { "test", features = listOf( VariantManager.VariantFeature.InAppUsage, - VariantManager.VariantFeature.RemoveDay1AndDay3Notifications, VariantManager.VariantFeature.KillOnboarding ), filterBy = { true } diff --git a/app/src/androidTest/java/com/duckduckgo/app/statistics/VariantManagerTest.kt b/app/src/androidTest/java/com/duckduckgo/app/statistics/VariantManagerTest.kt index 40b0b0b7e68d..11e50781d205 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/statistics/VariantManagerTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/statistics/VariantManagerTest.kt @@ -47,14 +47,14 @@ class VariantManagerTest { @Test fun inBrowserControlVariantHasExpectedWeightAndNoFeatures() { val variant = variants.first { it.key == "ma" } - assertEqualsDouble(1.0, variant.weight) + assertEqualsDouble(0.0, variant.weight) assertEquals(0, variant.features.size) } @Test fun inBrowserSecondControlVariantHasExpectedWeightAndRemoveDay1And3NotificationsAndKillOnboardingFeatures() { val variant = variants.first { it.key == "mb" } - assertEqualsDouble(1.0, variant.weight) + assertEqualsDouble(0.0, variant.weight) assertEquals(2, variant.features.size) assertTrue(variant.hasFeature(KillOnboarding)) assertTrue(variant.hasFeature(RemoveDay1AndDay3Notifications)) @@ -63,13 +63,29 @@ class VariantManagerTest { @Test fun inBrowserInAppUsageVariantHasExpectedWeightAndRemoveDay1And3NotificationsAndKillOnboardingAndInAppUsageFeatures() { val variant = variants.first { it.key == "mc" } - assertEqualsDouble(1.0, variant.weight) + assertEqualsDouble(0.0, variant.weight) assertEquals(3, variant.features.size) assertTrue(variant.hasFeature(KillOnboarding)) assertTrue(variant.hasFeature(RemoveDay1AndDay3Notifications)) assertTrue(variant.hasFeature(InAppUsage)) } + @Test + fun newInBrowserControlVariantHasExpectedWeightAndNoFeatures() { + val variant = variants.first { it.key == "zx" } + assertEqualsDouble(1.0, variant.weight) + assertEquals(0, variant.features.size) + } + + @Test + fun newInBrowserInAppUsageVariantHasExpectedWeightAndKillOnboardingAndInAppUsageFeatures() { + val variant = variants.first { it.key == "zy" } + assertEqualsDouble(1.0, variant.weight) + assertEquals(2, variant.features.size) + assertTrue(variant.hasFeature(KillOnboarding)) + assertTrue(variant.hasFeature(InAppUsage)) + } + @Test fun verifyNoDuplicateVariantNames() { val existingNames = mutableSetOf() diff --git a/app/src/main/java/com/duckduckgo/app/di/NotificationModule.kt b/app/src/main/java/com/duckduckgo/app/di/NotificationModule.kt index fb40b498e744..a41256b425f9 100644 --- a/app/src/main/java/com/duckduckgo/app/di/NotificationModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/NotificationModule.kt @@ -22,6 +22,7 @@ import androidx.core.app.NotificationManagerCompat import androidx.localbroadcastmanager.content.LocalBroadcastManager import androidx.work.WorkManager import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector +import com.duckduckgo.app.global.install.AppInstallStore import com.duckduckgo.app.notification.AndroidNotificationScheduler import com.duckduckgo.app.notification.NotificationFactory import com.duckduckgo.app.notification.NotificationScheduler @@ -92,14 +93,16 @@ class NotificationModule { clearDataNotification: ClearDataNotification, privacyProtectionNotification: PrivacyProtectionNotification, useOurAppNotification: UseOurAppNotification, - variantManager: VariantManager + variantManager: VariantManager, + appInstallStore: AppInstallStore ): AndroidNotificationScheduler { return NotificationScheduler( workManager, clearDataNotification, privacyProtectionNotification, useOurAppNotification, - variantManager + variantManager, + appInstallStore ) } diff --git a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt index 302d73a9fad1..69fd6748bcaf 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -17,9 +17,12 @@ package com.duckduckgo.app.notification import android.content.Context +import androidx.annotation.VisibleForTesting import androidx.annotation.WorkerThread import androidx.core.app.NotificationManagerCompat import androidx.work.* +import com.duckduckgo.app.global.install.AppInstallStore +import com.duckduckgo.app.global.install.daysInstalled import com.duckduckgo.app.global.plugins.worker.WorkerInjectorPlugin import com.duckduckgo.app.notification.db.NotificationDao import com.duckduckgo.app.notification.model.ClearDataNotification @@ -48,7 +51,8 @@ class NotificationScheduler( private val clearDataNotification: SchedulableNotification, private val privacyNotification: SchedulableNotification, private val useOurAppNotification: SchedulableNotification, - private val variantManager: VariantManager + private val variantManager: VariantManager, + private val appInstallStore: AppInstallStore ) : AndroidNotificationScheduler { override suspend fun scheduleNextNotification() { @@ -60,7 +64,7 @@ class NotificationScheduler( if (variant().hasFeature(VariantManager.VariantFeature.InAppUsage) && useOurAppNotification.canShow()) { val operation = scheduleUniqueNotification( OneTimeWorkRequestBuilder(), - 3, + UOA_DURATION, TimeUnit.DAYS, USE_OUR_APP_WORK_REQUEST_TAG ) @@ -77,15 +81,29 @@ class NotificationScheduler( when { (!variant().hasFeature(VariantManager.VariantFeature.RemoveDay1AndDay3Notifications) && privacyNotification.canShow()) -> { - scheduleNotification(OneTimeWorkRequestBuilder(), 1, TimeUnit.DAYS, UNUSED_APP_WORK_REQUEST_TAG) + val duration = getDurationForInactiveNotification(PRIVACY_DURATION) + scheduleNotification(OneTimeWorkRequestBuilder(), duration, TimeUnit.DAYS, UNUSED_APP_WORK_REQUEST_TAG) } (!variant().hasFeature(VariantManager.VariantFeature.RemoveDay1AndDay3Notifications) && clearDataNotification.canShow()) -> { - scheduleNotification(OneTimeWorkRequestBuilder(), 3, TimeUnit.DAYS, UNUSED_APP_WORK_REQUEST_TAG) + val duration = getDurationForInactiveNotification(CLEAR_DATA_DURATION) + scheduleNotification(OneTimeWorkRequestBuilder(), duration, TimeUnit.DAYS, UNUSED_APP_WORK_REQUEST_TAG) } else -> Timber.v("Notifications not enabled for this variant") } } + @VisibleForTesting + fun getDurationForInactiveNotification(day: Long): Long { + Timber.d("Inactive notification days installed is ${appInstallStore.daysInstalled()} day is $day") + var duration = day + if (variantHasInAppUsage() && (appInstallStore.daysInstalled() + day) == UOA_DURATION) { + duration += 1 + } + return duration + } + + private fun variantHasInAppUsage() = variant().hasFeature(VariantManager.VariantFeature.InAppUsage) + private fun variant() = variantManager.getVariant() private fun scheduleUniqueNotification(builder: OneTimeWorkRequest.Builder, duration: Long, unit: TimeUnit, tag: String): Operation { @@ -99,7 +117,7 @@ class NotificationScheduler( } private fun scheduleNotification(builder: OneTimeWorkRequest.Builder, duration: Long, unit: TimeUnit, tag: String) { - Timber.v("Scheduling notification") + Timber.v("Scheduling notification for $duration") val request = builder .addTag(tag) .setInitialDelay(duration, unit) @@ -146,6 +164,9 @@ class NotificationScheduler( companion object { const val UNUSED_APP_WORK_REQUEST_TAG = "com.duckduckgo.notification.schedule" const val USE_OUR_APP_WORK_REQUEST_TAG = "com.duckduckgo.notification.useOurApp" + const val UOA_DURATION = 3L + const val CLEAR_DATA_DURATION = 3L + const val PRIVACY_DURATION = 1L } } diff --git a/statistics/src/main/java/com/duckduckgo/app/statistics/VariantManager.kt b/statistics/src/main/java/com/duckduckgo/app/statistics/VariantManager.kt index 8d2a8f0a019b..29a6d040e41b 100644 --- a/statistics/src/main/java/com/duckduckgo/app/statistics/VariantManager.kt +++ b/statistics/src/main/java/com/duckduckgo/app/statistics/VariantManager.kt @@ -47,9 +47,12 @@ interface VariantManager { Variant(key = "se", weight = 0.0, features = emptyList(), filterBy = { isSerpRegionToggleCountry() }), // InAppUsage Experiments - Variant(key = "ma", weight = 1.0, features = emptyList(), filterBy = { isEnglishLocale() }), - Variant(key = "mb", weight = 1.0, features = listOf(VariantFeature.KillOnboarding, VariantFeature.RemoveDay1AndDay3Notifications), filterBy = { isEnglishLocale() }), - Variant(key = "mc", weight = 1.0, features = listOf(VariantFeature.KillOnboarding, VariantFeature.InAppUsage, VariantFeature.RemoveDay1AndDay3Notifications), filterBy = { isEnglishLocale() }) + Variant(key = "ma", weight = 0.0, features = emptyList(), filterBy = { isEnglishLocale() }), + Variant(key = "mb", weight = 0.0, features = listOf(VariantFeature.KillOnboarding, VariantFeature.RemoveDay1AndDay3Notifications), filterBy = { isEnglishLocale() }), + Variant(key = "mc", weight = 0.0, features = listOf(VariantFeature.KillOnboarding, VariantFeature.InAppUsage, VariantFeature.RemoveDay1AndDay3Notifications), filterBy = { isEnglishLocale() }), + + Variant(key = "zx", weight = 1.0, features = emptyList(), filterBy = { isEnglishLocale() }), + Variant(key = "zy", weight = 1.0, features = listOf(VariantFeature.KillOnboarding, VariantFeature.InAppUsage), filterBy = { isEnglishLocale() }) ) val REFERRER_VARIANTS = listOf( From 0b96dd1172ff082ee5b00c6a455236fd0635d547 Mon Sep 17 00:00:00 2001 From: Marcos Holgado Date: Wed, 28 Apr 2021 11:26:36 +0100 Subject: [PATCH 2/2] Rename consts --- .../notification/AndroidNotificationScheduler.kt | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt index 69fd6748bcaf..e46169a33662 100644 --- a/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt +++ b/app/src/main/java/com/duckduckgo/app/notification/AndroidNotificationScheduler.kt @@ -64,7 +64,7 @@ class NotificationScheduler( if (variant().hasFeature(VariantManager.VariantFeature.InAppUsage) && useOurAppNotification.canShow()) { val operation = scheduleUniqueNotification( OneTimeWorkRequestBuilder(), - UOA_DURATION, + UOA_DELAY_DURATION_IN_DAYS, TimeUnit.DAYS, USE_OUR_APP_WORK_REQUEST_TAG ) @@ -81,11 +81,11 @@ class NotificationScheduler( when { (!variant().hasFeature(VariantManager.VariantFeature.RemoveDay1AndDay3Notifications) && privacyNotification.canShow()) -> { - val duration = getDurationForInactiveNotification(PRIVACY_DURATION) + val duration = getDurationForInactiveNotification(PRIVACY_DELAY_DURATION_IN_DAYS) scheduleNotification(OneTimeWorkRequestBuilder(), duration, TimeUnit.DAYS, UNUSED_APP_WORK_REQUEST_TAG) } (!variant().hasFeature(VariantManager.VariantFeature.RemoveDay1AndDay3Notifications) && clearDataNotification.canShow()) -> { - val duration = getDurationForInactiveNotification(CLEAR_DATA_DURATION) + val duration = getDurationForInactiveNotification(CLEAR_DATA_DELAY_DURATION_IN_DAYS) scheduleNotification(OneTimeWorkRequestBuilder(), duration, TimeUnit.DAYS, UNUSED_APP_WORK_REQUEST_TAG) } else -> Timber.v("Notifications not enabled for this variant") @@ -96,7 +96,7 @@ class NotificationScheduler( fun getDurationForInactiveNotification(day: Long): Long { Timber.d("Inactive notification days installed is ${appInstallStore.daysInstalled()} day is $day") var duration = day - if (variantHasInAppUsage() && (appInstallStore.daysInstalled() + day) == UOA_DURATION) { + if (variantHasInAppUsage() && (appInstallStore.daysInstalled() + day) == UOA_DELAY_DURATION_IN_DAYS) { duration += 1 } return duration @@ -164,9 +164,9 @@ class NotificationScheduler( companion object { const val UNUSED_APP_WORK_REQUEST_TAG = "com.duckduckgo.notification.schedule" const val USE_OUR_APP_WORK_REQUEST_TAG = "com.duckduckgo.notification.useOurApp" - const val UOA_DURATION = 3L - const val CLEAR_DATA_DURATION = 3L - const val PRIVACY_DURATION = 1L + const val UOA_DELAY_DURATION_IN_DAYS = 3L + const val CLEAR_DATA_DELAY_DURATION_IN_DAYS = 3L + const val PRIVACY_DELAY_DURATION_IN_DAYS = 1L } }