From d337aac7a5ba47ec24e47e13a1b2f2b6ebe60387 Mon Sep 17 00:00:00 2001 From: Matt Lichtenstein Date: Wed, 18 Oct 2023 10:47:24 -0400 Subject: [PATCH] VPN-5520: Decouple data collection checkbox from setting during onboarding (#8283) --- src/mozillavpn.cpp | 25 +++-- src/settingslist.h | 22 ++--- .../onboarding/OnboardingDataSlide.qml | 9 +- tests/functional/testOnboarding.js | 99 ++++++++++++++++++- 4 files changed, 120 insertions(+), 35 deletions(-) diff --git a/src/mozillavpn.cpp b/src/mozillavpn.cpp index 2c9f1510a9..be7b792bc0 100644 --- a/src/mozillavpn.cpp +++ b/src/mozillavpn.cpp @@ -401,15 +401,6 @@ void MozillaVPN::maybeStateMain() { // getting to the home screen if (Feature::get(Feature::Feature_newOnboarding)->isSupported()) { if (!settingsHolder->onboardingCompleted()) { - // We turn telemetry off when onboarding starts so that the user has to - // specifically opt in to all future telemetry *Note: This is needed - // because telemetry is on prior to onboarding to record metrics regarding - // auth - if (!settingsHolder->onboardingStarted()) { - settingsHolder->setGleanEnabled(false); - settingsHolder->setOnboardingStarted(true); - } - setState(StateOnboarding); return; } @@ -931,16 +922,22 @@ void MozillaVPN::onboardingCompleted() { logger.debug() << "onboarding completed"; settingsHolder->setOnboardingCompleted(true); - // Resetting for the benefit of testing so that we only have to reset one - // setting (onboardingCompleted) manually. No real affect on user since they - // will never see onboarding again - settingsHolder->setOnboardingStep(0); + // Toggle glean on or off at the end of onboarding, depending on what the + // user selected + settingsHolder->setGleanEnabled( + settingsHolder->onboardingDataCollectionEnabled()); - // Mark the old onboarding expereince as completed as well, ensuring that + // Mark the old onboarding experience as completed as well, ensuring that // users do not have to go through it if the new onboaring feature is turned // off settingsHolder->setPostAuthenticationShown(true); + // Resetting for the benefit of testing so that we only have to reset one + // setting (onboardingCompleted) manually. No real affect on user since they + // *should* never see onboarding more than once + settingsHolder->setOnboardingStep(0); + settingsHolder->setOnboardingDataCollectionEnabled(false); + } else { logger.debug() << "telemetry policy completed"; } diff --git a/src/settingslist.h b/src/settingslist.h index fac293f595..9fd14f8e0d 100644 --- a/src/settingslist.h +++ b/src/settingslist.h @@ -368,17 +368,6 @@ SETTING_STRINGLIST(missingApps, // getter false // sensitive (do not log) ) -SETTING_BOOL(onboardingStarted, // getter - setOnboardingStarted, // setter - removeOnboardingStarted, // remover - hasOnboardingStarted, // has - "onboardingStarted", // key - false, // default value - false, // user setting - false, // remove when reset - false // sensitive (do not log) -) - SETTING_BOOL(onboardingCompleted, // getter setOnboardingCompleted, // setter removeOnboardingCompleted, // remover @@ -390,6 +379,17 @@ SETTING_BOOL(onboardingCompleted, // getter false // sensitive (do not log) ) +SETTING_BOOL(onboardingDataCollectionEnabled, // getter + setOnboardingDataCollectionEnabled, // setter + removeOnboardingDataCollectionEnabled, // remover + hasOnboardingDataCollectionEnabled, // has + "onboardingDataCollectionEnabled", // key + false, // default value + false, // user setting + false, // remove when reset + false // sensitive (do not log) +) + SETTING_INT(onboardingStep, // getter setOnboardingStep, // setter removeOnboardingStep, // remover diff --git a/src/ui/screens/onboarding/OnboardingDataSlide.qml b/src/ui/screens/onboarding/OnboardingDataSlide.qml index aa570f7510..537dc67212 100644 --- a/src/ui/screens/onboarding/OnboardingDataSlide.qml +++ b/src/ui/screens/onboarding/OnboardingDataSlide.qml @@ -61,13 +61,12 @@ ColumnLayout { Layout.rightMargin: MZTheme.theme.windowMargin * 2 Layout.fillWidth: true - subLabelText: MZI18n.OnboardingDataSlideCheckboxLabel leftMargin: 0 - isChecked: MZSettings.gleanEnabled + subLabelText: MZI18n.OnboardingDataSlideCheckboxLabel showDivider: false - onClicked: { - MZSettings.gleanEnabled = !MZSettings.gleanEnabled - } + isChecked: MZSettings.onboardingDataCollectionEnabled + + onClicked: MZSettings.onboardingDataCollectionEnabled = !MZSettings.onboardingDataCollectionEnabled } Item { diff --git a/tests/functional/testOnboarding.js b/tests/functional/testOnboarding.js index d8ffccfc58..633a1f07fc 100644 --- a/tests/functional/testOnboarding.js +++ b/tests/functional/testOnboarding.js @@ -12,16 +12,35 @@ describe('Onboarding', function() { beforeEach(async () => { await vpn.flipFeatureOn("newOnboarding"); - assert.equal(await vpn.getSetting('onboardingStarted'), false); assert.equal(await vpn.getSetting('onboardingCompleted'), false); + assert.equal(await vpn.getSetting('onboardingStep'), 0); await vpn.authenticateInApp(); }); + async function completeOnboarding(currentStep = 0) { + switch(currentStep) { + case 0: + await vpn.waitForQueryAndClick(queries.screenOnboarding.DATA_NEXT_BUTTON.visible()); + await vpn.waitForQuery(queries.screenOnboarding.STEP_NAV_STACK_VIEW.ready()); + case 1: + await vpn.waitForQueryAndClick(queries.screenOnboarding.PRIVACY_NEXT_BUTTON.visible()); + await vpn.waitForQuery(queries.screenOnboarding.STEP_NAV_STACK_VIEW.ready()); + case 2: + await vpn.waitForQueryAndClick(queries.screenOnboarding.DEVICES_NEXT_BUTTON.visible()); + await vpn.waitForQuery(queries.screenOnboarding.STEP_NAV_STACK_VIEW.ready()); + case 3: + await vpn.waitForQueryAndClick(queries.screenOnboarding.START_NEXT_BUTTON.visible()); + await vpn.waitForQuery(queries.screenHome.SCREEN.visible()); + assert.equal(await vpn.getSetting('onboardingCompleted'), true); + default: + break; + } + } + //Tests pretty much all of the new onboarding, including all slides, controls, navigation, and ensuring settings persist into StateMain it('Complete onboarding', async () => { await vpn.waitForQuery(queries.screenOnboarding.SCREEN.visible()); await vpn.waitForQuery(queries.screenOnboarding.ONBOARDING_VIEW.visible()); - assert.equal(await vpn.getSetting('onboardingStarted'), true); //DATA SLIDE //Do some checks on the initial state of the data slide @@ -29,12 +48,11 @@ describe('Onboarding', function() { assert.equal(await vpn.getQueryProperty(queries.screenOnboarding.ONBOARDING_VIEW, 'currentIndex'), 0); assert.equal(await vpn.getSetting('onboardingStep'), 0); assert.equal(await vpn.getQueryProperty(queries.screenOnboarding.DATA_CHECKBOX, 'isChecked'), 'false'); - assert.equal(await vpn.getSetting('gleanEnabled'), false); + assert.equal(await vpn.getSetting('gleanEnabled'), true); //Check data collection checkbox await vpn.waitForQueryAndClick(queries.screenOnboarding.DATA_CHECKBOX.visible()); assert.equal(await vpn.getQueryProperty(queries.screenOnboarding.DATA_CHECKBOX, 'isChecked'), 'true'); - assert.equal(await vpn.getSetting('gleanEnabled'), true); //Check privacy link await vpn.waitForQueryAndClick(queries.screenOnboarding.DATA_PRIVACY_LINK.visible()); @@ -71,6 +89,8 @@ describe('Onboarding', function() { //Clicks block trackers checkbox await vpn.waitForQueryAndClick(queries.screenOnboarding.PRIVACY_BLOCK_MALWARE_CHECKBOX.visible()); assert.equal(await vpn.getQueryProperty(queries.screenOnboarding.PRIVACY_BLOCK_MALWARE_CHECKBOX, 'checked'), 'true'); + //dnsProviderFlags is a bitfield mapping of hex flags that can handle bitwise operations to combine multiple flags + //here, we combine blockAds (0x02), blockTrackers (0x04), and blockMalware (0x08) which totals to 0xE or 14 in decimal assert.equal(await vpn.getSetting('dnsProviderFlags'), 14); //Test back button @@ -104,7 +124,6 @@ describe('Onboarding', function() { assert.equal(await vpn.getSetting('onboardingStep'), 2); //Switch between toggle buttons - console.log(await vpn.getQueryProperty(queries.screenOnboarding.DEVICES_DEVICE_TYPE_TOGGLE.visible(), 'selectedIndex')) if (await vpn.getQueryProperty(queries.screenOnboarding.DEVICES_DEVICE_TYPE_TOGGLE.visible(), 'selectedIndex') === 0) { //Starting with Android - switch to iOS and back to Android await vpn.waitForQueryAndClick(queries.screenOnboarding.DEVICES_TOGGLE_BTN_IOS.visible()); @@ -205,13 +224,17 @@ describe('Onboarding', function() { assert.equal(await vpn.getQueryProperty(queries.screenSettings.privacyView.BLOCK_ADS_CHECKBOX, 'checked'), 'true'); assert.equal(await vpn.getQueryProperty(queries.screenSettings.privacyView.BLOCK_TRACKERS_CHECKBOX, 'checked'), 'true'); assert.equal(await vpn.getQueryProperty(queries.screenSettings.privacyView.BLOCK_MALWARE_CHECKBOX, 'checked'), 'true'); + assert.equal(await vpn.getSetting('dnsProviderFlags'), 14); await vpn.waitForQueryAndClick(queries.screenSettings.BACK.visible()); await vpn.waitForQuery(queries.screenSettings.STACKVIEW.ready()); //Check data collection and start at boot await vpn.waitForQueryAndClick(queries.screenSettings.APP_PREFERENCES.visible()); assert.equal(await vpn.getQueryProperty(queries.screenSettings.appPreferencesView.DATA_COLLECTION, 'isChecked'), 'true'); + assert.equal(await vpn.getSetting('gleanEnabled'), true); assert.equal(await vpn.getQueryProperty(queries.screenSettings.appPreferencesView.START_AT_BOOT, 'isChecked'), 'true'); + assert.equal(await vpn.getSetting('startAtBoot'), true); + }); //Tests restoring onboarding to current step after quitting @@ -277,4 +300,70 @@ describe('Onboarding', function() { assert.equal(await vpn.getQueryProperty(queries.screenOnboarding.ONBOARDING_VIEW, 'currentIndex'), 3); assert.equal(await vpn.getSetting('onboardingStep'), 3); }); + + it('Complete onboarding with data collection disabled', async () => { + + //Ensure we are in onboarding + await vpn.waitForQuery(queries.screenOnboarding.DATA_SLIDE.visible()); + + //Ensure that the data collection checkbox state is bound to onboardingDataCollectionEnabled, which defaults to false + //and that this checkboxes state does not affect gleanEnabled + assert.equal(await vpn.getQueryProperty(queries.screenOnboarding.DATA_CHECKBOX, 'isChecked'), 'false'); + assert.equal(await vpn.getSetting('onboardingDataCollectionEnabled'), false); + assert.equal(await vpn.getSetting('gleanEnabled'), true); + + //Complete onboarding and get to the home screen + await completeOnboarding(await vpn.getSetting('onboardingStep')); + + //Ensure glean is disabled + assert.equal(await vpn.getSetting('gleanEnabled'), false); + }); + + it('Complete onboarding with data collection enabled', async () => { + + //Ensure we are in onboarding + await vpn.waitForQuery(queries.screenOnboarding.DATA_SLIDE.visible()); + + //Ensure that the data collection checkbox state remains bound to onboardingDataCollectionEnabled when clicked + //and that this checkboxes state does not affect gleanEnabled + await vpn.waitForQueryAndClick(queries.screenOnboarding.DATA_CHECKBOX.visible()); + assert.equal(await vpn.getQueryProperty(queries.screenOnboarding.DATA_CHECKBOX, 'isChecked'), 'true'); + assert.equal(await vpn.getSetting('onboardingDataCollectionEnabled'), true); + assert.equal(await vpn.getSetting('gleanEnabled'), true); + + //Complete onboarding and get to the home screen + await completeOnboarding(await vpn.getSetting('onboardingStep')); + + //Ensure glean is enabled + assert.equal(await vpn.getSetting('gleanEnabled'), true); + }); + + it('Data collection checkbox state persists across app sessions', async () => { + // Skip WASM because this test involves quitting and relaunching + if (this.ctx.wasm) { + return; + } + //Ensure we are in onboarding + await vpn.waitForQuery(queries.screenOnboarding.DATA_SLIDE.visible()); + + //Click the data collection checkbox to set onboardingDataCollectionEnabled to true + await vpn.waitForQueryAndClick(queries.screenOnboarding.DATA_CHECKBOX.visible()); + + //Quit and relaunch the app + await vpn.quit(); + await setup.startAndConnect(); + + //Ensure we are still in onboarding + await vpn.waitForQuery(queries.screenOnboarding.DATA_SLIDE.visible()); + + //Ensure that the data collection checkbox state remains bound to onboardingDataCollectionEnabled upon relaunch + //and that this checkboxes state does not affect gleanEnabled + assert.equal(await vpn.getQueryProperty(queries.screenOnboarding.DATA_CHECKBOX, 'isChecked'), 'true'); + assert.equal(await vpn.getSetting('onboardingDataCollectionEnabled'), true); + assert.equal(await vpn.getSetting('gleanEnabled'), true); + + //Ensure the data collection checkbox is checked and onboardingDataCollectionEnabled is set to true + assert.equal(await vpn.getQueryProperty(queries.screenOnboarding.DATA_CHECKBOX, 'isChecked'), 'true'); + assert.equal(await vpn.getSetting('onboardingDataCollectionEnabled'), true); + }); }); \ No newline at end of file