Skip to content

Commit

Permalink
VPN-5520: Decouple data collection checkbox from setting during onboa…
Browse files Browse the repository at this point in the history 
…rding (#8283)
  • Loading branch information
@MattLichtenstein
MattLichtenstein authored Oct 18, 2023
1 parent c36e439 commit d337aac
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 35 deletions.
25 changes: 11 additions & 14 deletions src/mozillavpn.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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";
}
Expand Down
22 changes: 11 additions & 11 deletions src/settingslist.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
9 changes: 4 additions & 5 deletions src/ui/screens/onboarding/OnboardingDataSlide.qml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
99 changes: 94 additions & 5 deletions tests/functional/testOnboarding.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,47 @@ 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
await vpn.waitForQuery(queries.screenOnboarding.DATA_SLIDE.visible());
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());
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
});
});

0 comments on commit d337aac

Please sign in to comment.