Onboarding: Release privacy pro onboarding dialog to all users #5222
Conversation
nalcalag
force-pushed
the
feature/noelia/release_privacy_pro_onbaording_dialog
branch
2 times, most recently
from
607fbe0
to
fcca964
Compare
| return this.getMetrics().firstOrNull { it.metric == "secondaryButtonSelected" } | ||
| } | ||
|
|
||
| suspend fun FeatureTogglesInventory.onboardingExperiments(): Toggle? { |
There was a problem hiding this comment.
You don't need this, can delete.
| } | ||
|
|
||
| @ContributesMultibinding(AppScope::class) | ||
| class ExtendedOnboardingPixelsPlugin @Inject constructor(private val inventory: FeatureTogglesInventory) : MetricsPixelPlugin { |
There was a problem hiding this comment.
Rather than injecting the inventory you can just inject ExtendedOnboardingFeatureToggles and then use the toggle for the experiment.
| VPN("vpn"), | ||
| } | ||
|
|
||
| companion object { |
| metric = "dialogShown", | ||
| value = "1", | ||
| toggle = activeToggle, | ||
| conversionWindow = listOf(ConversionWindow(lowerWindow = 0, upperWindow = Int.MAX_VALUE)), |
There was a problem hiding this comment.
You probably don't want the upper window to be the max integer. I'd talk to DS but you probably want to define something closer in time like day 7.
| @@ -470,6 +502,16 @@ class CtaViewModel @Inject constructor( | |||
|
|
|||
| fun isSuggestedSiteOption(query: String): Boolean = onboardingStore.getSitesOptions().map { it.link }.contains(query) | |||
|
|
|||
| fun getCohortOrigin(): String { | |||
| return when { | |||
| extendedOnboardingFeatureToggles.testPrivacyProOnboardingCopyNov24().isEnabled(Cohorts.PROTECTION) -> "_${Cohorts.PROTECTION.cohortName}" | |||
There was a problem hiding this comment.
You can do extendedOnboardingFeatureToggles.testPrivacyProOnboardingCopyNov24().getCohort() directly.
| @@ -128,6 +135,13 @@ class CtaViewModel @Inject constructor( | |||
| if (cta is OnboardingDaxDialogCta && cta.markAsReadOnShow) { | |||
| dismissedCtaDao.insert(DismissedCta(cta.ctaId)) | |||
| } | |||
| withContext(dispatchers.io()) { | |||
| if (cta is DaxBubbleCta.DaxPrivacyProCta || cta is DaxBubbleCta.DaxExperimentPrivacyProCta) { | |||
| extendedOnboardingPixelsPlugin.testPrivacyProOnboardingShownMetricPixel()?.getPixelDefinitions()?.forEach { | |||
There was a problem hiding this comment.
You should only send the pixels if the user is in the experiment
|
|
||
| suspend fun FeatureTogglesInventory.onboardingExperiments(): Toggle? { | ||
| return this.getAllTogglesForParent("extendedOnboarding").firstOrNull { | ||
| it.featureName().name.startsWith(EXPERIMENT_PREFIX) && it.isEnabled() |
There was a problem hiding this comment.
By deleting this it means you are not checking isEnabled() anymore so you either have to do it before trying to send the pixels or you in getMetrics you can check it there and it's false, return empty list.
nalcalag
force-pushed
the
feature/noelia/release_privacy_pro_onbaording_dialog
branch
from
fcca964
to
39c1a5e
Compare
Task/Issue URL: https://app.asana.com/0/0/1208628082622869/f
Description
testPrivacyProOnboardingCopyNov24sub-feature in remote configSteps to test this PR
Pre steps
const val PRIVACY_REMOTE_CONFIG_URLvalue for "https://www.jsonblob.com/api/1304478638945460224"control Cohort
weightto 1 and the rest to 0 in remote configprotection Cohort
weightto 1 and the rest to 0 in remote configpir Cohort
weightto 1 and the rest to 0 in remote configvpn Cohort
weightto 1 and the rest to 0 in remote configReturning users
weightto 1 in remote configExperiment pixels
weightto 1 and the rest to 0 in remote configexperiment_metrics_testPrivacyProOnboardingCopyNov24_vpn with params: {metric=dialogShown...}pixel is firedexperiment_metrics_testPrivacyProOnboardingCopyNov24_vpn with params: {metric=secondaryButtonSelected...}pixel is firedexperiment_metrics_testPrivacyProOnboardingCopyNov24_vpn with params: {metric=primaryButtonSelected...}pixel is firedUI changes