Skip to content

Commit

Permalink
refactor: add feature toggle to support apple-only
Browse files Browse the repository at this point in the history
  • Loading branch information
dbajpeyi committed Dec 18, 2024
1 parent 31fd22f commit fbec6fc
Show file tree
Hide file tree
Showing 15 changed files with 124 additions and 138 deletions.
42 changes: 16 additions & 26 deletions dist/autofill-debug.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 16 additions & 26 deletions dist/autofill.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions integration-test/helpers/mocks.webkit.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ export function createWebkitMocks(platform = 'macos') {
password_generation: true,
credentials_saving: true,
inlineIcon_credentials: true,
partial_form_saves: true,
email: true,
},
},
Expand Down
5 changes: 3 additions & 2 deletions integration-test/tests/save-prompts.android.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ test.describe('Android Save prompts', () => {
androidStringReplacements({
featureToggles: {
credentials_saving: true,
partial_form_saves: false,
},
availableInputTypes: {
credentials: { username: false, password: false },
Expand All @@ -81,11 +82,11 @@ test.describe('Android Save prompts', () => {
await login.submitPasswordOnlyForm(credentials);
await login.assertWasPromptedToSave(credentials);
});
test('with username only (should prompt)', async ({ page }) => {
test('with username only (should NOT prompt)', async ({ page }) => {
const { login } = await setup(page);
const credentials = { username: '123456' };
await login.submitUsernameOnlyForm(credentials.username);
await login.shouldPromptToSave();
await login.shouldNotPromptToSave();
});
});
});
Expand Down
27 changes: 27 additions & 0 deletions integration-test/tests/save-prompts.windows.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,31 @@ test.describe('Save prompts on windows', () => {
await signup.assertWasNotPromptedToSaveWindows();
});
});

test.describe('When partial form saves are disabled', () => {
test('I should not be prompted to save for username only', async ({ page }) => {
// enable in-terminal exceptions
await forwardConsoleMessages(page);

const { personalAddress } = constants.fields.email;

const credentials = {
username: personalAddress,
};

const signup = signupPage(page);
await signup.navigate();

await createWindowsMocks()
.withFeatureToggles({
partial_form_saves: false,
})
.applyTo(page);

await createAutofillScript().platform('windows').applyTo(page);

await signup.enterCredentials(credentials);
await signup.assertWasNotPromptedToSaveWindows();
});
});
});
24 changes: 10 additions & 14 deletions src/DeviceInterface/InterfacePrototype.js
Original file line number Diff line number Diff line change
Expand Up @@ -785,17 +785,6 @@ class InterfacePrototype {
}
}

/**
* Checks if partial save can be triggered
* @param {DataStorageObject} values
* @returns {boolean}
*/
shouldTriggerPartialSave(values) {
// If credentials has only username field, and no password field, then trigger is a partialSave
const isUsernameOnly = Object.keys(values?.credentials || {}).length === 1 && Boolean(values?.credentials?.username);
return isUsernameOnly && Boolean(this.settings.featureToggles.partial_form_saves);
}

/**
* `postSubmit` gives features a one-time-only opportunity to perform an
* action directly after a form submission was observed.
Expand All @@ -810,15 +799,22 @@ class InterfacePrototype {
postSubmit(values, form) {
if (!form.form) return;
if (!form.hasValues(values)) return;
const shouldPartialSave = this.shouldTriggerPartialSave(values);
const checks = [form.shouldPromptToStoreData && !form.submitHandlerExecuted, this.passwordGenerator.generated, shouldPartialSave];
const shouldTriggerPartialSave =
Object.keys(values?.credentials || {}).length === 1 &&
Boolean(values?.credentials?.username) &&
Boolean(this.settings.featureToggles.partial_form_saves);
const checks = [
form.shouldPromptToStoreData && !form.submitHandlerExecuted,
this.passwordGenerator.generated,
shouldTriggerPartialSave,
];
if (checks.some(Boolean)) {
const formData = appendGeneratedKey(values, {
password: this.passwordGenerator.password,
username: this.emailProtection.lastGenerated,
});

const trigger = shouldPartialSave ? 'partialSave' : 'formSubmission';
const trigger = shouldTriggerPartialSave ? 'partialSave' : 'formSubmission';
this.storeFormData(formData, trigger);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Form/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ class Form {
*/
getValuesReadyForStorage() {
const formValues = this.getRawValues();
return prepareFormValuesForStorage(formValues);
return prepareFormValuesForStorage(formValues, this.device.settings.featureToggles.partial_form_saves);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/Form/Form.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ describe('Test the form class reading values correctly', () => {
<input type="password" value="" autocomplete="new-password" />
<button type="submit">Sign up</button>
</form>`,
expHasValues: true,
expValues: { credentials: { username: 'testUsername' } },
expHasValues: false,
expValues: { credentials: undefined },
},
{
testCase: 'form where the password is <=3 characters long',
Expand All @@ -95,8 +95,8 @@ describe('Test the form class reading values correctly', () => {
<input type="password" value="abc" autocomplete="new-password" />
<button type="submit">Sign up</button>
</form>`,
expHasValues: true,
expValues: { credentials: { username: 'testUsername' } },
expHasValues: false,
expValues: { credentials: undefined },
},
{
testCase: 'form with hidden email field',
Expand Down
19 changes: 9 additions & 10 deletions src/Form/formatters.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,14 @@ const getMMAndYYYYFromString = (expiration) => {
};

/**
* @param {InternalDataStorageObject} credentials
* @param {InternalDataStorageObject} data
* @return {boolean}
*/
const shouldStoreIdentities = ({ identities }) =>
Boolean((identities.firstName || identities.fullName) && identities.addressStreet && identities.addressCity);

/**
* @param {InternalDataStorageObject} credentials
* @param {InternalDataStorageObject} data
* @return {boolean}
*/
const shouldStoreCreditCards = ({ creditCards }) => {
Expand All @@ -193,7 +193,7 @@ const formatPhoneNumber = (phone) => phone.replaceAll(/[^0-9|+]/g, '');
* @param {InternalDataStorageObject} formValues
* @return {DataStorageObject}
*/
const prepareFormValuesForStorage = (formValues) => {
const prepareFormValuesForStorage = (formValues, canTriggerPartialSave = false) => {
/** @type {Partial<InternalDataStorageObject>} */
let { credentials, identities, creditCards } = formValues;

Expand All @@ -203,13 +203,12 @@ const prepareFormValuesForStorage = (formValues) => {
}

/** Fixes for credentials */
if (!credentials.username && hasUsernameLikeIdentity(identities)) {
// @ts-ignore - We know that username is not a useful value here
credentials.username = identities.emailAddress || identities.phone;
}

// If we still don't have any credentials, we discard the object
if (Object.keys(credentials ?? {}).length === 0) {
if (canTriggerPartialSave || Boolean(formValues.credentials?.password)) {
if (credentials.password && !credentials.username && hasUsernameLikeIdentity(identities)) {
// @ts-ignore
credentials.username = identities.emailAddress || identities.phone;
}
} else {
credentials = undefined;
}

Expand Down
Loading

0 comments on commit fbec6fc

Please sign in to comment.