Skip to content

Commit

Permalink
[FeatureToggle] Add new autofill config for partial save trigger (#724)
Browse files Browse the repository at this point in the history
* fix: call credentials flow call first, and then close parent

* chore: move partial save behind feature toggle

* refactor: add feature toggle to support apple-only

* chore:PR comments
  • Loading branch information
dbajpeyi authored Dec 18, 2024
1 parent 7623023 commit 47c26dc
Show file tree
Hide file tree
Showing 17 changed files with 144 additions and 78 deletions.
32 changes: 17 additions & 15 deletions dist/autofill-debug.js

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

29 changes: 15 additions & 14 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.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export function androidStringReplacements(overrides = {}) {
inputType_credentials: true,
inputType_identities: false,
inputType_creditCards: false,
partial_form_saves: true,
emailProtection: true,
emailProtection_incontext_signup: true,
password_generation: false,
Expand Down
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
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();
});
});
});
15 changes: 10 additions & 5 deletions src/DeviceInterface/InterfacePrototype.js
Original file line number Diff line number Diff line change
Expand Up @@ -799,17 +799,22 @@ class InterfacePrototype {
postSubmit(values, form) {
if (!form.form) return;
if (!form.hasValues(values)) return;

const isUsernameOnly = Object.keys(values?.credentials || {}).length === 1 && values?.credentials?.username;
const checks = [form.shouldPromptToStoreData && !form.submitHandlerExecuted, this.passwordGenerator.generated, isUsernameOnly];
const shouldTriggerPartialSave =
Object.keys(values?.credentials || {}).length === 1 &&
Boolean(values?.credentials?.username) &&
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,
});

// If credentials has only username field, and no password field, then trigger is a partialSave
const trigger = isUsernameOnly ? '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
15 changes: 8 additions & 7 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,14 @@ const prepareFormValuesForStorage = (formValues) => {
}

/** Fixes for credentials */
if (!credentials.username && hasUsernameLikeIdentity(identities)) {
// @ts-ignore - We know that username is not a useful value here
// If we don't have a username to match a password, let's see if email or phone are available
if (credentials.password && !credentials.username && hasUsernameLikeIdentity(identities)) {
// @ts-ignore - username will be likely undefined, but needs to be specifically assigned to a string value
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 there's no password, and we shouldn't trigger a partial save, let's discard the object
if (!credentials.password && !canTriggerPartialSave) {
credentials = undefined;
}

Expand Down
18 changes: 16 additions & 2 deletions src/Form/formatters.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ describe('Can strip phone formatting characters', () => {

describe('prepareFormValuesForStorage()', () => {
describe('handling credentials', () => {
it('accepts for username only', () => {
it('rejects for username only', () => {
const values = prepareFormValuesForStorage({
credentials: { username: '[email protected]' },
// @ts-ignore
creditCards: {},
// @ts-ignore
identities: {},
});
expect(values.credentials?.username).toBe('[email protected]');
expect(values.credentials).toBeUndefined();
});
it('accepts password only', () => {
const values = prepareFormValuesForStorage({
Expand All @@ -93,5 +93,19 @@ describe('prepareFormValuesForStorage()', () => {
});
expect(values.credentials).toEqual(inputCredentials);
});
it('accepts username only with partial_form_saves feature', () => {
const inputCredentials = { username: '[email protected]' };
const values = prepareFormValuesForStorage(
{
credentials: inputCredentials,
// @ts-ignore
creditCards: {},
// @ts-ignore
identities: {},
},
true,
);
expect(values.credentials).toEqual(inputCredentials);
});
});
});
1 change: 1 addition & 0 deletions src/Settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ export class Settings {
inputType_creditCards: false,
inlineIcon_credentials: false,
unknown_username_categorization: false,
partial_form_saves: false,
},
/** @type {AvailableInputTypes} */
availableInputTypes: {
Expand Down
1 change: 1 addition & 0 deletions src/Settings.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ describe('Settings', () => {
"inputType_credentials": false,
"inputType_creditCards": false,
"inputType_identities": false,
"partial_form_saves": false,
"password_generation": false,
"unknown_username_categorization": false,
}
Expand Down
4 changes: 4 additions & 0 deletions src/deviceApiCalls/__generated__/validators-ts.ts

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

Loading

0 comments on commit 47c26dc

Please sign in to comment.