Skip to content

Commit e02df16

Browse files
aitorvsaitorvs
authored
Removed the experiment for the new default browser dialog for full release (#1072)
* Removed the experiment for the new default browser dialog for full release * Address review comments * Renamed DefaultRoleBrowserDialogExperiment -> DefaultRoleBrowserDialog * Renamed shouldShowExperiment -> shouldShowDialog * Renamed experimentShown -> dialogShown * Removed isAtLeastApiVersion * Removed references to browser dialog variant in tests
1 parent 46b990b commit e02df16

File tree

13 files changed

+68
-113
lines changed

13 files changed

+68
-113
lines changed

‎app/src/androidTest/java/com/duckduckgo/app/di/TestAppComponent.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import com.duckduckgo.app.browser.rating.di.RatingModule
2525
import com.duckduckgo.app.global.exception.UncaughtExceptionModule
2626
import com.duckduckgo.app.httpsupgrade.di.HttpsUpgraderModule
2727
import com.duckduckgo.app.onboarding.di.OnboardingModule
28+
import com.duckduckgo.app.onboarding.di.WelcomePageModule
2829
import com.duckduckgo.app.surrogates.di.ResourceSurrogateModule
2930
import com.duckduckgo.app.trackerdetection.di.TrackerDetectionModule
3031
import com.duckduckgo.app.usage.di.AppUsageModule
@@ -72,7 +73,8 @@ import javax.inject.Singleton
7273
UncaughtExceptionModule::class,
7374
PlayStoreReferralModule::class,
7475
CoroutinesModule::class,
75-
CertificateTrustedStoreModule::class
76+
CertificateTrustedStoreModule::class,
77+
WelcomePageModule::class
7678
]
7779
)
7880
interface TestAppComponent : AppComponent {

‎app/src/androidTest/java/com/duckduckgo/app/onboarding/ui/OnboardingPageManagerPageCountTest.kt‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
package com.duckduckgo.app.onboarding.ui
1818

1919
import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector
20+
import com.duckduckgo.app.global.DefaultRoleBrowserDialog
2021
import com.duckduckgo.app.statistics.Variant
21-
import com.duckduckgo.app.statistics.VariantManager
2222
import com.nhaarman.mockitokotlin2.mock
2323
import com.nhaarman.mockitokotlin2.whenever
2424
import org.junit.Assert.assertEquals
@@ -33,11 +33,11 @@ class OnboardingPageManagerPageCountTest(private val testCase: TestCase) {
3333
private lateinit var testee: OnboardingPageManager
3434
private val onboardingPageBuilder: OnboardingPageBuilder = mock()
3535
private val mockDefaultBrowserDetector: DefaultBrowserDetector = mock()
36-
private val variantManager: VariantManager = mock()
36+
private val defaultRoleBrowserDialog: DefaultRoleBrowserDialog = mock()
3737

3838
@Before
3939
fun setup() {
40-
testee = OnboardingPageManagerWithTrackerBlocking(variantManager, onboardingPageBuilder, mockDefaultBrowserDetector)
40+
testee = OnboardingPageManagerWithTrackerBlocking(defaultRoleBrowserDialog, onboardingPageBuilder, mockDefaultBrowserDetector)
4141
}
4242

4343
@Test
@@ -49,7 +49,6 @@ class OnboardingPageManagerPageCountTest(private val testCase: TestCase) {
4949
}
5050

5151
private fun configureDefaultBrowserPageConfig() {
52-
whenever(variantManager.getVariant()).thenReturn(testCase.variant)
5352
if (testCase.defaultBrowserPage) {
5453
configureDeviceSupportsDefaultBrowser()
5554
} else {
@@ -60,20 +59,13 @@ class OnboardingPageManagerPageCountTest(private val testCase: TestCase) {
6059
companion object {
6160

6261
private val otherVariant = Variant(key = "variant", features = listOf(), filterBy = { true })
63-
private val defaultBrowserVariant = Variant(
64-
key = "variant",
65-
features = listOf(VariantManager.VariantFeature.SetDefaultBrowserDialog),
66-
filterBy = { true }
67-
)
6862

6963
@JvmStatic
7064
@Parameterized.Parameters(name = "Test case: {index} - {0}")
7165
fun testData(): Array<TestCase> {
7266
return arrayOf(
7367
TestCase(false, 1, otherVariant),
74-
TestCase(true, 2, otherVariant),
75-
TestCase(false, 1, defaultBrowserVariant),
76-
TestCase(true, 1, defaultBrowserVariant)
68+
TestCase(true, 2, otherVariant)
7769
)
7870
}
7971
}

‎app/src/androidTest/java/com/duckduckgo/app/onboarding/ui/OnboardingPageManagerTest.kt‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@
1717
package com.duckduckgo.app.onboarding.ui
1818

1919
import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector
20-
import com.duckduckgo.app.statistics.Variant
21-
import com.duckduckgo.app.statistics.VariantManager
20+
import com.duckduckgo.app.global.DefaultRoleBrowserDialog
2221
import com.nhaarman.mockitokotlin2.mock
2322
import com.nhaarman.mockitokotlin2.whenever
2423
import org.junit.Assert.assertEquals
@@ -30,35 +29,29 @@ class OnboardingPageManagerTest {
3029
private lateinit var testee: OnboardingPageManager
3130
private val onboardingPageBuilder: OnboardingPageBuilder = mock()
3231
private val mockDefaultBrowserDetector: DefaultBrowserDetector = mock()
33-
private val variantManager: VariantManager = mock()
34-
private val defaultBrowserVariant = Variant(
35-
key = "variant",
36-
features = listOf(VariantManager.VariantFeature.SetDefaultBrowserDialog),
37-
filterBy = { true }
38-
)
39-
private val otherVariant = Variant(key = "variant", features = listOf(), filterBy = { true })
32+
private val defaultRoleBrowserDialog: DefaultRoleBrowserDialog = mock()
4033

4134
@Before
4235
fun setup() {
43-
testee = OnboardingPageManagerWithTrackerBlocking(variantManager, onboardingPageBuilder, mockDefaultBrowserDetector)
36+
testee = OnboardingPageManagerWithTrackerBlocking(defaultRoleBrowserDialog, onboardingPageBuilder, mockDefaultBrowserDetector)
4437
}
4538

4639
@Test
4740
fun whenDDGIsNotDefaultBrowserThenExpectedOnboardingPagesAreTwo() {
4841
configureDeviceSupportsDefaultBrowser()
4942
whenever(mockDefaultBrowserDetector.isDefaultBrowser()).thenReturn(false)
50-
whenever(variantManager.getVariant()).thenReturn(otherVariant)
43+
whenever(defaultRoleBrowserDialog.shouldShowDialog()).thenReturn(false)
5144

5245
testee.buildPageBlueprints()
5346

5447
assertEquals(2, testee.pageCount())
5548
}
5649

5750
@Test
58-
fun whenDDGIsNotDefaultBrowserAndBrowserDialogVariantThenExpectedOnboardingPagesAre1() {
51+
fun whenDDGIsNotDefaultBrowserAndShouldShowBrowserDialogThenExpectedOnboardingPagesAre1() {
5952
configureDeviceSupportsDefaultBrowser()
6053
whenever(mockDefaultBrowserDetector.isDefaultBrowser()).thenReturn(false)
61-
whenever(variantManager.getVariant()).thenReturn(defaultBrowserVariant)
54+
whenever(defaultRoleBrowserDialog.shouldShowDialog()).thenReturn(true)
6255

6356
testee.buildPageBlueprints()
6457

@@ -69,18 +62,18 @@ class OnboardingPageManagerTest {
6962
fun whenDDGAsDefaultBrowserThenSinglePageOnBoarding() {
7063
configureDeviceSupportsDefaultBrowser()
7164
whenever(mockDefaultBrowserDetector.isDefaultBrowser()).thenReturn(true)
72-
whenever(variantManager.getVariant()).thenReturn(otherVariant)
65+
whenever(defaultRoleBrowserDialog.shouldShowDialog()).thenReturn(false)
7366

7467
testee.buildPageBlueprints()
7568

7669
assertEquals(1, testee.pageCount())
7770
}
7871

7972
@Test
80-
fun whenDDGAsDefaultBrowserAndBrowserDialogVariantThenSinglePageOnBoarding() {
73+
fun whenDDGAsDefaultBrowserAndShouldShowBrowserDialogThenSinglePageOnBoarding() {
8174
configureDeviceSupportsDefaultBrowser()
8275
whenever(mockDefaultBrowserDetector.isDefaultBrowser()).thenReturn(true)
83-
whenever(variantManager.getVariant()).thenReturn(defaultBrowserVariant)
76+
whenever(defaultRoleBrowserDialog.shouldShowDialog()).thenReturn(true)
8477

8578
testee.buildPageBlueprints()
8679

@@ -90,17 +83,17 @@ class OnboardingPageManagerTest {
9083
@Test
9184
fun whenDeviceDoesNotSupportDefaultBrowserThenSinglePageOnBoarding() {
9285
configureDeviceDoesNotSupportDefaultBrowser()
93-
whenever(variantManager.getVariant()).thenReturn(otherVariant)
86+
whenever(defaultRoleBrowserDialog.shouldShowDialog()).thenReturn(false)
9487

9588
testee.buildPageBlueprints()
9689

9790
assertEquals(1, testee.pageCount())
9891
}
9992

10093
@Test
101-
fun whenDeviceDoesNotSupportDefaultBrowserAndBrowserDialogVariantThenSinglePageOnBoarding() {
94+
fun whenDeviceDoesNotSupportDefaultBrowserAndShouldShowBrowserDialogThenSinglePageOnBoarding() {
10295
configureDeviceDoesNotSupportDefaultBrowser()
103-
whenever(variantManager.getVariant()).thenReturn(defaultBrowserVariant)
96+
whenever(defaultRoleBrowserDialog.shouldShowDialog()).thenReturn(true)
10497

10598
testee.buildPageBlueprints()
10699

‎app/src/androidTest/java/com/duckduckgo/app/onboarding/ui/page/WelcomePageViewModelTest.kt‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import android.content.Intent
2020
import androidx.arch.core.executor.testing.InstantTaskExecutorRule
2121
import androidx.test.platform.app.InstrumentationRegistry
2222
import com.duckduckgo.app.CoroutineTestRule
23-
import com.duckduckgo.app.global.DefaultRoleBrowserDialogExperiment
23+
import com.duckduckgo.app.global.DefaultRoleBrowserDialog
2424
import com.duckduckgo.app.global.install.AppInstallStore
2525
import com.duckduckgo.app.runBlocking
2626
import com.duckduckgo.app.statistics.pixels.Pixel
@@ -58,7 +58,7 @@ class WelcomePageViewModelTest {
5858
private lateinit var appInstallStore: AppInstallStore
5959

6060
@Mock
61-
private lateinit var defaultRoleBrowserDialogExperiment: DefaultRoleBrowserDialogExperiment
61+
private lateinit var defaultRoleBrowserDialog: DefaultRoleBrowserDialog
6262

6363
private val events = ConflatedBroadcastChannel<WelcomePageView.Event>()
6464

@@ -73,15 +73,15 @@ class WelcomePageViewModelTest {
7373
appInstallStore,
7474
InstrumentationRegistry.getInstrumentation().targetContext,
7575
pixel,
76-
defaultRoleBrowserDialogExperiment
76+
defaultRoleBrowserDialog
7777
)
7878

7979
viewEvents = events.asFlow().flatMapLatest { viewModel.reduce(it) }
8080
}
8181

8282
@Test
83-
fun whenOnPrimaryCtaClickedAndShouldNotShowXpThenFinish() = coroutineRule.runBlocking {
84-
whenever(defaultRoleBrowserDialogExperiment.shouldShowExperiment())
83+
fun whenOnPrimaryCtaClickedAndShouldNotShowDialogThenFinish() = coroutineRule.runBlocking {
84+
whenever(defaultRoleBrowserDialog.shouldShowDialog())
8585
.thenReturn(false)
8686

8787
val launch = launch {
@@ -95,11 +95,11 @@ class WelcomePageViewModelTest {
9595
}
9696

9797
@Test
98-
fun whenOnPrimaryCtaClickedAndShouldShowXpAndShowThenEmitShowDialog() = coroutineRule.runBlocking {
99-
whenever(defaultRoleBrowserDialogExperiment.shouldShowExperiment())
98+
fun whenOnPrimaryCtaClickedAndShouldShowDialogAndShowThenEmitShowDialog() = coroutineRule.runBlocking {
99+
whenever(defaultRoleBrowserDialog.shouldShowDialog())
100100
.thenReturn(true)
101101
val intent = Intent()
102-
whenever(defaultRoleBrowserDialogExperiment.createIntent(any()))
102+
whenever(defaultRoleBrowserDialog.createIntent(any()))
103103
.thenReturn(intent)
104104

105105
val launch = launch {
@@ -113,10 +113,10 @@ class WelcomePageViewModelTest {
113113
}
114114

115115
@Test
116-
fun whenOnPrimaryCtaClickedAndShouldShowXpNullIntentThenFireAndFinish() = coroutineRule.runBlocking {
117-
whenever(defaultRoleBrowserDialogExperiment.shouldShowExperiment())
116+
fun whenOnPrimaryCtaClickedAndShouldShowDialogNullIntentThenFireAndFinish() = coroutineRule.runBlocking {
117+
whenever(defaultRoleBrowserDialog.shouldShowDialog())
118118
.thenReturn(true)
119-
whenever(defaultRoleBrowserDialogExperiment.createIntent(any()))
119+
whenever(defaultRoleBrowserDialog.createIntent(any()))
120120
.thenReturn(null)
121121

122122
val launch = launch {
@@ -132,15 +132,15 @@ class WelcomePageViewModelTest {
132132
}
133133

134134
@Test
135-
fun whenOnDefaultBrowserSetThenCallExperimentShownFireAndFinish() = coroutineRule.runBlocking {
135+
fun whenOnDefaultBrowserSetThenCallDialogShownFireAndFinish() = coroutineRule.runBlocking {
136136
val launch = launch {
137137
viewEvents.collect { state ->
138138
assertTrue(state == WelcomePageView.State.Finish)
139139
}
140140
}
141141
events.send(WelcomePageView.Event.OnDefaultBrowserSet)
142142

143-
verify(defaultRoleBrowserDialogExperiment).experimentShown()
143+
verify(defaultRoleBrowserDialog).dialogShown()
144144
verify(pixel).fire(
145145
Pixel.PixelName.DEFAULT_BROWSER_SET,
146146
mapOf(Pixel.PixelParameter.DEFAULT_BROWSER_SET_FROM_ONBOARDING to true.toString())
@@ -150,15 +150,15 @@ class WelcomePageViewModelTest {
150150
}
151151

152152
@Test
153-
fun whenOnDefaultBrowserNotSetThenCallExperimentShownFireAndFinish() = coroutineRule.runBlocking {
153+
fun whenOnDefaultBrowserNotSetThenCallDialogShownFireAndFinish() = coroutineRule.runBlocking {
154154
val launch = launch {
155155
viewEvents.collect { state ->
156156
assertTrue(state == WelcomePageView.State.Finish)
157157
}
158158
}
159159
events.send(WelcomePageView.Event.OnDefaultBrowserNotSet)
160160

161-
verify(defaultRoleBrowserDialogExperiment).experimentShown()
161+
verify(defaultRoleBrowserDialog).dialogShown()
162162
verify(pixel).fire(
163163
Pixel.PixelName.DEFAULT_BROWSER_NOT_SET,
164164
mapOf(Pixel.PixelParameter.DEFAULT_BROWSER_SET_FROM_ONBOARDING to true.toString())

‎app/src/androidTest/java/com/duckduckgo/app/statistics/VariantManagerTest.kt‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,6 @@ class VariantManagerTest {
6767
assertEquals(SerpHeaderQueryReplacement, variant.features[0])
6868
}
6969

70-
@Test
71-
fun roleManagerDefaultBrowserDialogControlHasExpectedWeightAndFeatures() {
72-
val variant = variants.first { it.key == "zt" }
73-
assertEqualsDouble(1.0, variant.weight)
74-
assertTrue(variant.features.isEmpty())
75-
}
76-
77-
@Test
78-
fun roleManagerDefaultBrowserDialogTreatmentHasExpectedWeightAndFeatures() {
79-
val variant = variants.first { it.key == "zu" }
80-
assertEqualsDouble(1.0, variant.weight)
81-
assertTrue(variant.features == listOf(SetDefaultBrowserDialog))
82-
}
83-
8470
@Test
8571
fun verifyNoDuplicateVariantNames() {
8672
val existingNames = mutableSetOf<String>()

‎app/src/main/java/com/duckduckgo/app/di/AndroidBindingModule.kt‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ import com.duckduckgo.app.launch.LaunchBridgeActivity
4141
import com.duckduckgo.app.location.ui.LocationPermissionsActivity
4242
import com.duckduckgo.app.location.ui.SiteLocationPermissionDialog
4343
import com.duckduckgo.app.notification.NotificationHandlerService
44-
import com.duckduckgo.app.onboarding.di.WelcomePageModule
4544
import com.duckduckgo.app.onboarding.ui.OnboardingActivity
4645
import com.duckduckgo.app.onboarding.ui.page.DefaultBrowserPage
4746
import com.duckduckgo.app.onboarding.ui.page.WelcomePage
@@ -185,9 +184,7 @@ abstract class AndroidBindingModule {
185184
@ContributesAndroidInjector
186185
abstract fun brokenSiteNegativeFeedbackFragment(): BrokenSiteNegativeFeedbackFragment
187186

188-
@ContributesAndroidInjector(
189-
modules = [WelcomePageModule::class]
190-
)
187+
@ContributesAndroidInjector
191188
abstract fun welcomePage(): WelcomePage
192189

193190
@ContributesAndroidInjector

‎app/src/main/java/com/duckduckgo/app/di/AppComponent.kt‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import com.duckduckgo.app.global.DuckDuckGoApplication
2626
import com.duckduckgo.app.global.exception.UncaughtExceptionModule
2727
import com.duckduckgo.app.httpsupgrade.di.HttpsUpgraderModule
2828
import com.duckduckgo.app.onboarding.di.OnboardingModule
29+
import com.duckduckgo.app.onboarding.di.WelcomePageModule
2930
import com.duckduckgo.app.surrogates.di.ResourceSurrogateModule
3031
import com.duckduckgo.app.trackerdetection.di.TrackerDetectionModule
3132
import com.duckduckgo.app.usage.di.AppUsageModule
@@ -70,7 +71,8 @@ import javax.inject.Singleton
7071
UncaughtExceptionModule::class,
7172
PlayStoreReferralModule::class,
7273
CoroutinesModule::class,
73-
CertificateTrustedStoreModule::class
74+
CertificateTrustedStoreModule::class,
75+
WelcomePageModule::class
7476
]
7577
)
7678
interface AppComponent : AndroidInjector<DuckDuckGoApplication> {

‎app/src/main/java/com/duckduckgo/app/global/DefaultRoleBrowserDialogExperiment.kt‎ renamed to ‎app/src/main/java/com/duckduckgo/app/global/DefaultRoleBrowserDialog.kt‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,17 @@ import android.content.Context
2121
import android.content.Intent
2222
import android.os.Build
2323
import com.duckduckgo.app.global.install.AppInstallStore
24-
import com.duckduckgo.app.statistics.VariantManager
2524
import timber.log.Timber
2625

27-
interface DefaultRoleBrowserDialogExperiment {
26+
interface DefaultRoleBrowserDialog {
2827
fun createIntent(context: Context): Intent?
29-
fun shouldShowExperiment(): Boolean
30-
fun experimentShown()
28+
fun shouldShowDialog(): Boolean
29+
fun dialogShown()
3130
}
3231

33-
class RealDefaultRoleBrowserDialogExperiment(
34-
private val appInstallStore: AppInstallStore,
35-
private val variantManager: VariantManager
36-
) : DefaultRoleBrowserDialogExperiment {
32+
class RealDefaultRoleBrowserDialog(
33+
private val appInstallStore: AppInstallStore
34+
) : DefaultRoleBrowserDialog {
3735

3836
/**
3937
* @return an Intent to launch the role browser dialog
@@ -56,14 +54,14 @@ class RealDefaultRoleBrowserDialogExperiment(
5654
return null
5755
}
5856

59-
override fun shouldShowExperiment(): Boolean {
57+
override fun shouldShowDialog(): Boolean {
6058
// The second and subsequent times the dialog is shown, the system allows the user to click on "don't show again"
6159
// we will get the same result as if the dialog was just dismissed.
62-
return variantManager.getVariant().hasFeature(VariantManager.VariantFeature.SetDefaultBrowserDialog) &&
60+
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q &&
6361
appInstallStore.newDefaultBrowserDialogCount < DEFAULT_BROWSER_DIALOG_MAX_ATTEMPTS
6462
}
6563

66-
override fun experimentShown() {
64+
override fun dialogShown() {
6765
appInstallStore.newDefaultBrowserDialogCount++
6866
}
6967

‎app/src/main/java/com/duckduckgo/app/onboarding/di/OnboardingModule.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
package com.duckduckgo.app.onboarding.di
1818

1919
import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector
20+
import com.duckduckgo.app.global.DefaultRoleBrowserDialog
2021
import com.duckduckgo.app.onboarding.ui.OnboardingFragmentPageBuilder
2122
import com.duckduckgo.app.onboarding.ui.OnboardingPageBuilder
2223
import com.duckduckgo.app.onboarding.ui.OnboardingPageManager
2324
import com.duckduckgo.app.onboarding.ui.OnboardingPageManagerWithTrackerBlocking
24-
import com.duckduckgo.app.statistics.VariantManager
2525
import dagger.Module
2626
import dagger.Provides
2727
import javax.inject.Singleton
@@ -31,11 +31,11 @@ class OnboardingModule {
3131

3232
@Provides
3333
fun onboardingPageManger(
34-
variantManager: VariantManager,
34+
defaultRoleBrowserDialog: DefaultRoleBrowserDialog,
3535
onboardingPageBuilder: OnboardingPageBuilder,
3636
defaultBrowserDetector: DefaultBrowserDetector
3737
): OnboardingPageManager {
38-
return OnboardingPageManagerWithTrackerBlocking(variantManager, onboardingPageBuilder, defaultBrowserDetector)
38+
return OnboardingPageManagerWithTrackerBlocking(defaultRoleBrowserDialog, onboardingPageBuilder, defaultBrowserDetector)
3939
}
4040

4141
@Provides

0 commit comments

Comments
 (0)