Commit afd13085 authored by sandromaggi's avatar sandromaggi Committed by Commit Bot

[Autofill Assistant] Check object completeness in native

We no longer want to rely on the inbuilt |isComplete| check of Autofill
objects, instead we want to consistently use the same verifier in the
native action as well as the view to prevent scenarios where the action
and the UI have different states.

Bug: b/154068342
Change-Id: Ibc3572946c50e1d5181642155cedd1c21a42975e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2201602Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#771541}
parent cba88486
...@@ -174,12 +174,21 @@ class AssistantCollectUserDataBinder ...@@ -174,12 +174,21 @@ class AssistantCollectUserDataBinder
view.mContactDetailsSection.setListener(collectUserDataDelegate != null view.mContactDetailsSection.setListener(collectUserDataDelegate != null
? collectUserDataDelegate::onContactInfoChanged ? collectUserDataDelegate::onContactInfoChanged
: null); : null);
view.mContactDetailsSection.setCompletenessDelegate(collectUserDataDelegate != null
? collectUserDataDelegate::isContactComplete
: null);
view.mPaymentMethodSection.setListener(collectUserDataDelegate != null view.mPaymentMethodSection.setListener(collectUserDataDelegate != null
? collectUserDataDelegate::onPaymentMethodChanged ? collectUserDataDelegate::onPaymentMethodChanged
: null); : null);
view.mPaymentMethodSection.setCompletenessDelegate(collectUserDataDelegate != null
? collectUserDataDelegate::isPaymentInstrumentComplete
: null);
view.mShippingAddressSection.setListener(collectUserDataDelegate != null view.mShippingAddressSection.setListener(collectUserDataDelegate != null
? collectUserDataDelegate::onShippingAddressChanged ? collectUserDataDelegate::onShippingAddressChanged
: null); : null);
view.mShippingAddressSection.setCompletenessDelegate(collectUserDataDelegate != null
? collectUserDataDelegate::isShippingAddressComplete
: null);
view.mLoginSection.setListener(collectUserDataDelegate != null view.mLoginSection.setListener(collectUserDataDelegate != null
? collectUserDataDelegate::onLoginChoiceChanged ? collectUserDataDelegate::onLoginChoiceChanged
: null); : null);
......
...@@ -53,4 +53,22 @@ public interface AssistantCollectUserDataDelegate { ...@@ -53,4 +53,22 @@ public interface AssistantCollectUserDataDelegate {
/** The focus on a text field has been lost */ /** The focus on a text field has been lost */
void onTextFocusLost(); void onTextFocusLost();
/**
* Returns true if the contact is complete.
* TODO(b/154068342): Remove this method and send the error message from |Controller|.
*/
boolean isContactComplete(@Nullable AutofillContact contact);
/**
* Returns true if the shipping address is complete.
* TODO(b/154068342): Remove this method and send the error message from |Controller|.
*/
boolean isShippingAddressComplete(@Nullable AutofillAddress address);
/**
* Returns true if the payment instrument is complete.
* TODO(b/154068342): Remove this method and send the error message from |Controller|.
*/
boolean isPaymentInstrumentComplete(@Nullable AutofillPaymentInstrument paymentInstrument);
} }
...@@ -169,6 +169,44 @@ public class AssistantCollectUserDataNativeDelegate implements AssistantCollectU ...@@ -169,6 +169,44 @@ public class AssistantCollectUserDataNativeDelegate implements AssistantCollectU
} }
} }
@Override
public boolean isContactComplete(@Nullable AutofillContact contact) {
if (mNativeAssistantCollectUserDataDelegate != 0) {
return AssistantCollectUserDataNativeDelegateJni.get().isContactComplete(
mNativeAssistantCollectUserDataDelegate,
AssistantCollectUserDataNativeDelegate.this,
contact != null ? contact.getProfile() : null);
}
return false;
}
@Override
public boolean isShippingAddressComplete(@Nullable AutofillAddress address) {
if (mNativeAssistantCollectUserDataDelegate != 0) {
return AssistantCollectUserDataNativeDelegateJni.get().isShippingAddressComplete(
mNativeAssistantCollectUserDataDelegate,
AssistantCollectUserDataNativeDelegate.this,
address != null ? address.getProfile() : null);
}
return false;
}
@Override
public boolean isPaymentInstrumentComplete(
@Nullable AutofillPaymentInstrument paymentInstrument) {
if (mNativeAssistantCollectUserDataDelegate != 0) {
return AssistantCollectUserDataNativeDelegateJni.get().isPaymentInstrumentComplete(
mNativeAssistantCollectUserDataDelegate,
AssistantCollectUserDataNativeDelegate.this,
paymentInstrument != null ? paymentInstrument.getCard() : null,
paymentInstrument != null ? paymentInstrument.getBillingProfile() : null);
}
return false;
}
@CalledByNative @CalledByNative
private void clearNativePtr() { private void clearNativePtr() {
mNativeAssistantCollectUserDataDelegate = 0; mNativeAssistantCollectUserDataDelegate = 0;
...@@ -212,5 +250,15 @@ public class AssistantCollectUserDataNativeDelegate implements AssistantCollectU ...@@ -212,5 +250,15 @@ public class AssistantCollectUserDataNativeDelegate implements AssistantCollectU
AssistantCollectUserDataNativeDelegate caller, String key, AssistantValue value); AssistantCollectUserDataNativeDelegate caller, String key, AssistantValue value);
void onTextFocusLost(long nativeAssistantCollectUserDataDelegate, void onTextFocusLost(long nativeAssistantCollectUserDataDelegate,
AssistantCollectUserDataNativeDelegate caller); AssistantCollectUserDataNativeDelegate caller);
boolean isContactComplete(long nativeAssistantCollectUserDataDelegate,
AssistantCollectUserDataNativeDelegate caller,
@Nullable PersonalDataManager.AutofillProfile address);
boolean isShippingAddressComplete(long nativeAssistantCollectUserDataDelegate,
AssistantCollectUserDataNativeDelegate caller,
@Nullable PersonalDataManager.AutofillProfile address);
boolean isPaymentInstrumentComplete(long nativeAssistantCollectUserDataDelegate,
AssistantCollectUserDataNativeDelegate caller,
@Nullable PersonalDataManager.CreditCard card,
@Nullable PersonalDataManager.AutofillProfile address);
} }
} }
...@@ -31,9 +31,23 @@ import java.util.List; ...@@ -31,9 +31,23 @@ import java.util.List;
* such as |AutofillContact|, |AutofillPaymentMethod|, etc. * such as |AutofillContact|, |AutofillPaymentMethod|, etc.
*/ */
public abstract class AssistantCollectUserDataSection<T extends EditableOption> { public abstract class AssistantCollectUserDataSection<T extends EditableOption> {
interface Delegate<T> {
boolean isComplete(T element);
}
private class Item {
View mFullView;
T mOption;
Item(View fullView, T option) {
this.mFullView = fullView;
this.mOption = option;
}
}
private final @Nullable View mTitleAddButton; private final @Nullable View mTitleAddButton;
protected final AssistantVerticalExpander mSectionExpander; private final AssistantVerticalExpander mSectionExpander;
protected final AssistantChoiceList mItemsView; private final AssistantChoiceList mItemsView;
private final View mSummaryView; private final View mSummaryView;
private final int mFullViewResId; private final int mFullViewResId;
private final int mTitleToContentPadding; private final int mTitleToContentPadding;
...@@ -46,15 +60,7 @@ public abstract class AssistantCollectUserDataSection<T extends EditableOption> ...@@ -46,15 +60,7 @@ public abstract class AssistantCollectUserDataSection<T extends EditableOption>
private Callback<T> mListener; private Callback<T> mListener;
private int mTopPadding; private int mTopPadding;
private int mBottomPadding; private int mBottomPadding;
private Delegate<T> mCompletenessDelegate;
private class Item {
Item(View fullView, T option) {
this.mFullView = fullView;
this.mOption = option;
}
View mFullView;
T mOption;
}
/** /**
* *
...@@ -127,10 +133,18 @@ public abstract class AssistantCollectUserDataSection<T extends EditableOption> ...@@ -127,10 +133,18 @@ public abstract class AssistantCollectUserDataSection<T extends EditableOption>
mSectionExpander.setVisibility(visible ? View.VISIBLE : View.GONE); mSectionExpander.setVisibility(visible ? View.VISIBLE : View.GONE);
} }
void setListener(Callback<T> listener) { void setListener(@Nullable Callback<T> listener) {
mListener = listener; mListener = listener;
} }
void setCompletenessDelegate(@Nullable Delegate<T> completenessDelegate) {
mCompletenessDelegate = completenessDelegate;
}
boolean isComplete(T element) {
return mCompletenessDelegate != null && mCompletenessDelegate.isComplete(element);
}
void setTitle(String title) { void setTitle(String title) {
TextView titleView = mSectionExpander.findViewById(R.id.section_title); TextView titleView = mSectionExpander.findViewById(R.id.section_title);
AssistantTextUtils.applyVisualAppearanceTags(titleView, title, null); AssistantTextUtils.applyVisualAppearanceTags(titleView, title, null);
......
...@@ -67,12 +67,14 @@ public class AssistantContactDetailsSection ...@@ -67,12 +67,14 @@ public class AssistantContactDetailsSection
if (contact == null || mFullOptions == null) { if (contact == null || mFullOptions == null) {
return; return;
} }
TextView fullViewText = fullView.findViewById(R.id.contact_full); TextView fullViewText = fullView.findViewById(R.id.contact_full);
String description = createContactDescription(mFullOptions, contact); String description = createContactDescription(mFullOptions, contact);
fullViewText.setText(description); fullViewText.setText(description);
hideIfEmpty(fullViewText); hideIfEmpty(fullViewText);
fullView.findViewById(R.id.incomplete_error) fullView.findViewById(R.id.incomplete_error)
.setVisibility(contact.isComplete() ? View.GONE : View.VISIBLE); .setVisibility(isComplete(contact) ? View.GONE : View.VISIBLE);
} }
@Override @Override
...@@ -80,12 +82,14 @@ public class AssistantContactDetailsSection ...@@ -80,12 +82,14 @@ public class AssistantContactDetailsSection
if (contact == null || mSummaryOptions == null) { if (contact == null || mSummaryOptions == null) {
return; return;
} }
TextView contactSummaryView = summaryView.findViewById(R.id.contact_summary); TextView contactSummaryView = summaryView.findViewById(R.id.contact_summary);
String description = createContactDescription(mSummaryOptions, contact); String description = createContactDescription(mSummaryOptions, contact);
contactSummaryView.setText(description); contactSummaryView.setText(description);
hideIfEmpty(contactSummaryView); hideIfEmpty(contactSummaryView);
summaryView.findViewById(R.id.incomplete_error) summaryView.findViewById(R.id.incomplete_error)
.setVisibility(contact.isComplete() ? View.GONE : View.VISIBLE); .setVisibility(isComplete(contact) ? View.GONE : View.VISIBLE);
} }
@Override @Override
...@@ -129,6 +133,7 @@ public class AssistantContactDetailsSection ...@@ -129,6 +133,7 @@ public class AssistantContactDetailsSection
if (mIgnoreProfileChangeNotifications) { if (mIgnoreProfileChangeNotifications) {
return; return;
} }
int selectedContactIndex = -1; int selectedContactIndex = -1;
if (mSelectedOption != null) { if (mSelectedOption != null) {
for (int i = 0; i < contacts.size(); i++) { for (int i = 0; i < contacts.size(); i++) {
...@@ -138,6 +143,7 @@ public class AssistantContactDetailsSection ...@@ -138,6 +143,7 @@ public class AssistantContactDetailsSection
} }
} }
} }
// Replace current set of items, keep selection if possible. // Replace current set of items, keep selection if possible.
setItems(contacts, selectedContactIndex); setItems(contacts, selectedContactIndex);
} }
......
...@@ -80,7 +80,9 @@ public class AssistantPaymentMethodSection ...@@ -80,7 +80,9 @@ public class AssistantPaymentMethodSection
if (method == null) { if (method == null) {
return; return;
} }
updateSummaryView(fullView, method);
updateView(fullView, method);
TextView cardNameView = fullView.findViewById(R.id.credit_card_name); TextView cardNameView = fullView.findViewById(R.id.credit_card_name);
cardNameView.setText(method.getCard().getName()); cardNameView.setText(method.getCard().getName());
hideIfEmpty(cardNameView); hideIfEmpty(cardNameView);
...@@ -92,34 +94,34 @@ public class AssistantPaymentMethodSection ...@@ -92,34 +94,34 @@ public class AssistantPaymentMethodSection
return; return;
} }
ImageView cardIssuerImageView = summaryView.findViewById(R.id.credit_card_issuer_icon); updateView(summaryView, method);
}
private void updateView(View view, AutofillPaymentInstrument method) {
ImageView cardIssuerImageView = view.findViewById(R.id.credit_card_issuer_icon);
try { try {
cardIssuerImageView.setImageDrawable( cardIssuerImageView.setImageDrawable(view.getContext().getResources().getDrawable(
summaryView.getContext().getResources().getDrawable( method.getCard().getIssuerIconDrawableId()));
method.getCard().getIssuerIconDrawableId()));
} catch (Resources.NotFoundException e) { } catch (Resources.NotFoundException e) {
cardIssuerImageView.setImageDrawable(null); cardIssuerImageView.setImageDrawable(null);
} }
/** // By default, the obfuscated number contains the issuer (e.g., 'Visa'). This is needlessly
* By default, the obfuscated number contains the issuer (e.g., 'Visa'). This is needlessly // verbose, so we strip it away. See |PersonalDataManagerTest::testAddAndEditCreditCards|
* verbose, so we strip it away. See |PersonalDataManagerTest::testAddAndEditCreditCards| // for explanation of "\u0020...\u2060".
* for explanation of "\u0020...\u2060".
*/
String obfuscatedNumber = method.getCard().getObfuscatedNumber(); String obfuscatedNumber = method.getCard().getObfuscatedNumber();
int beginningOfObfuscatedNumber = int beginningOfObfuscatedNumber =
Math.max(obfuscatedNumber.indexOf("\u0020\u202A\u2022\u2060"), 0); Math.max(obfuscatedNumber.indexOf("\u0020\u202A\u2022\u2060"), 0);
obfuscatedNumber = obfuscatedNumber.substring(beginningOfObfuscatedNumber); obfuscatedNumber = obfuscatedNumber.substring(beginningOfObfuscatedNumber);
TextView cardNumberView = summaryView.findViewById(R.id.credit_card_number); TextView cardNumberView = view.findViewById(R.id.credit_card_number);
cardNumberView.setText(obfuscatedNumber); cardNumberView.setText(obfuscatedNumber);
hideIfEmpty(cardNumberView); hideIfEmpty(cardNumberView);
TextView cardExpirationView = summaryView.findViewById(R.id.credit_card_expiration); TextView cardExpirationView = view.findViewById(R.id.credit_card_expiration);
cardExpirationView.setText( cardExpirationView.setText(method.getCard().getFormattedExpirationDate(view.getContext()));
method.getCard().getFormattedExpirationDate(summaryView.getContext()));
hideIfEmpty(cardExpirationView); hideIfEmpty(cardExpirationView);
TextView errorMessageView = summaryView.findViewById(R.id.incomplete_error); TextView errorMessageView = view.findViewById(R.id.incomplete_error);
setErrorMessage(errorMessageView, method); setErrorMessage(errorMessageView, method);
hideIfEmpty(errorMessageView); hideIfEmpty(errorMessageView);
} }
...@@ -186,7 +188,7 @@ public class AssistantPaymentMethodSection ...@@ -186,7 +188,7 @@ public class AssistantPaymentMethodSection
int selectedMethodIndex = -1; int selectedMethodIndex = -1;
if (mSelectedOption != null) { if (mSelectedOption != null) {
for (int i = 0; i < paymentMethods.size(); ++i) { for (int i = 0; i < paymentMethods.size(); ++i) {
if (areEqual(mSelectedOption, paymentMethods.get(i))) { if (areEqual(paymentMethods.get(i), mSelectedOption)) {
selectedMethodIndex = i; selectedMethodIndex = i;
break; break;
} }
...@@ -218,12 +220,10 @@ public class AssistantPaymentMethodSection ...@@ -218,12 +220,10 @@ public class AssistantPaymentMethodSection
} }
private void setErrorMessage(TextView errorMessageView, AutofillPaymentInstrument method) { private void setErrorMessage(TextView errorMessageView, AutofillPaymentInstrument method) {
if (!method.isComplete()) { // TODO(b/154068342): Remove these granular checks and send the error message directly
errorMessageView.setText(R.string.autofill_assistant_payment_information_missing); // from |Controller|.
return; if (!method.isComplete() || method.getBillingProfile() == null
} || AutofillAddress.checkAddressCompletionStatus(method.getBillingProfile(),
if (method.getBillingProfile() != null
&& AutofillAddress.checkAddressCompletionStatus(method.getBillingProfile(),
AutofillAddress.CompletenessCheckType.IGNORE_PHONE) AutofillAddress.CompletenessCheckType.IGNORE_PHONE)
!= AutofillAddress.CompletionStatus.COMPLETE) { != AutofillAddress.CompletionStatus.COMPLETE) {
errorMessageView.setText(R.string.autofill_assistant_payment_information_missing); errorMessageView.setText(R.string.autofill_assistant_payment_information_missing);
...@@ -231,8 +231,7 @@ public class AssistantPaymentMethodSection ...@@ -231,8 +231,7 @@ public class AssistantPaymentMethodSection
} }
if (mRequiresBillingPostalCode if (mRequiresBillingPostalCode
&& (method.getBillingProfile() == null && TextUtils.isEmpty(method.getBillingProfile().getPostalCode())) {
|| TextUtils.isEmpty(method.getBillingProfile().getPostalCode()))) {
errorMessageView.setText(mBillingPostalCodeMissingText); errorMessageView.setText(mBillingPostalCodeMissingText);
return; return;
} }
...@@ -244,6 +243,12 @@ public class AssistantPaymentMethodSection ...@@ -244,6 +243,12 @@ public class AssistantPaymentMethodSection
return; return;
} }
// Final check to catch things we might have missed above.
if (!isComplete(method)) {
errorMessageView.setText(R.string.autofill_assistant_payment_information_missing);
return;
}
errorMessageView.setText(""); errorMessageView.setText("");
} }
} }
\ No newline at end of file
...@@ -70,7 +70,7 @@ public class AssistantShippingAddressSection ...@@ -70,7 +70,7 @@ public class AssistantShippingAddressSection
hideIfEmpty(fullAddressView); hideIfEmpty(fullAddressView);
TextView methodIncompleteView = fullView.findViewById(R.id.incomplete_error); TextView methodIncompleteView = fullView.findViewById(R.id.incomplete_error);
methodIncompleteView.setVisibility(address.isComplete() ? View.GONE : View.VISIBLE); methodIncompleteView.setVisibility(isComplete(address) ? View.GONE : View.VISIBLE);
} }
@Override @Override
...@@ -89,7 +89,7 @@ public class AssistantShippingAddressSection ...@@ -89,7 +89,7 @@ public class AssistantShippingAddressSection
hideIfEmpty(shortAddressView); hideIfEmpty(shortAddressView);
TextView methodIncompleteView = summaryView.findViewById(R.id.incomplete_error); TextView methodIncompleteView = summaryView.findViewById(R.id.incomplete_error);
methodIncompleteView.setVisibility(address.isComplete() ? View.GONE : View.VISIBLE); methodIncompleteView.setVisibility(isComplete(address) ? View.GONE : View.VISIBLE);
} }
@Override @Override
...@@ -131,6 +131,7 @@ public class AssistantShippingAddressSection ...@@ -131,6 +131,7 @@ public class AssistantShippingAddressSection
if (mIgnoreProfileChangeNotifications) { if (mIgnoreProfileChangeNotifications) {
return; return;
} }
int selectedAddressIndex = -1; int selectedAddressIndex = -1;
if (mSelectedOption != null) { if (mSelectedOption != null) {
for (int i = 0; i < addresses.size(); i++) { for (int i = 0; i < addresses.size(); i++) {
...@@ -140,6 +141,7 @@ public class AssistantShippingAddressSection ...@@ -140,6 +141,7 @@ public class AssistantShippingAddressSection
} }
} }
} }
// Replace current set of items, keep selection if possible. // Replace current set of items, keep selection if possible.
setItems(addresses, selectedAddressIndex); setItems(addresses, selectedAddressIndex);
} }
......
...@@ -178,6 +178,22 @@ public class AutofillAssistantCollectUserDataTestHelper { ...@@ -178,6 +178,22 @@ public class AutofillAssistantCollectUserDataTestHelper {
@Override @Override
public void onTextFocusLost() {} public void onTextFocusLost() {}
@Override
public boolean isContactComplete(@Nullable AutofillContact contact) {
return contact != null && contact.isComplete();
}
@Override
public boolean isShippingAddressComplete(@Nullable AutofillAddress address) {
return address != null && address.isComplete();
}
@Override
public boolean isPaymentInstrumentComplete(
@Nullable AutofillPaymentInstrument paymentInstrument) {
return paymentInstrument != null && paymentInstrument.isComplete();
}
} }
public AutofillAssistantCollectUserDataTestHelper() throws TimeoutException { public AutofillAssistantCollectUserDataTestHelper() throws TimeoutException {
......
...@@ -969,20 +969,24 @@ public class AutofillAssistantCollectUserDataUiTest { ...@@ -969,20 +969,24 @@ public class AutofillAssistantCollectUserDataUiTest {
} }
/** /**
* For expired credit cards, an error message should be displayed. * Check the order in which errors are shown:
* - Incomplete card / missing or incomplete address -> Generic information missing
* - Missing required zip code
* - Expired card
*/ */
@Test @Test
@MediumTest @MediumTest
public void testExpiredCreditCard() throws Exception { public void testOrderOfCreditCardErrorMessages() throws Exception {
// add credit card without postcode.
PersonalDataManager.AutofillProfile profile = PersonalDataManager.AutofillProfile profile =
mHelper.createDummyProfile("John Doe", "john@gmail.com", ""); mHelper.createDummyProfile("John Doe", "john@gmail.com", /* postcode= */ "");
String profileId = mHelper.setProfile(profile); String profileId = mHelper.setProfile(profile);
PersonalDataManager.CreditCard creditCard = mHelper.createDummyCreditCard(profileId); PersonalDataManager.CreditCard creditCard = mHelper.createDummyCreditCard(profileId);
creditCard.setYear("2019"); creditCard.setYear("2000");
AssistantCollectUserDataModel model = new AssistantCollectUserDataModel(); AssistantCollectUserDataModel model = new AssistantCollectUserDataModel();
AssistantCollectUserDataCoordinator coordinator = createCollectUserDataCoordinator(model); AssistantCollectUserDataCoordinator coordinator = createCollectUserDataCoordinator(model);
AutofillAssistantCollectUserDataTestHelper.MockDelegate delegate =
new AutofillAssistantCollectUserDataTestHelper.MockDelegate();
AutofillAssistantCollectUserDataTestHelper AutofillAssistantCollectUserDataTestHelper
.ViewHolder viewHolder = TestThreadUtils.runOnUiThreadBlocking( .ViewHolder viewHolder = TestThreadUtils.runOnUiThreadBlocking(
() -> new AutofillAssistantCollectUserDataTestHelper.ViewHolder(coordinator)); () -> new AutofillAssistantCollectUserDataTestHelper.ViewHolder(coordinator));
...@@ -990,26 +994,60 @@ public class AutofillAssistantCollectUserDataUiTest { ...@@ -990,26 +994,60 @@ public class AutofillAssistantCollectUserDataUiTest {
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
// WEB_CONTENTS are necessary for the creation of the editors. // WEB_CONTENTS are necessary for the creation of the editors.
model.set(AssistantCollectUserDataModel.WEB_CONTENTS, mTestRule.getWebContents()); model.set(AssistantCollectUserDataModel.WEB_CONTENTS, mTestRule.getWebContents());
model.set(AssistantCollectUserDataModel.DELEGATE, delegate);
model.set(AssistantCollectUserDataModel.REQUEST_PAYMENT, true); model.set(AssistantCollectUserDataModel.REQUEST_PAYMENT, true);
model.set(AssistantCollectUserDataModel.BILLING_POSTAL_CODE_MISSING_TEXT,
"Missing Zip Code");
model.set(AssistantCollectUserDataModel.REQUIRE_BILLING_POSTAL_CODE, true);
model.set(AssistantCollectUserDataModel.CREDIT_CARD_EXPIRED_TEXT, "Card is expired"); model.set(AssistantCollectUserDataModel.CREDIT_CARD_EXPIRED_TEXT, "Card is expired");
model.set(AssistantCollectUserDataModel.VISIBLE, true); model.set(AssistantCollectUserDataModel.VISIBLE, true);
AutofillPaymentInstrument expiredPaymentInstrument = AutofillPaymentInstrument paymentInstrument =
AssistantCollectUserDataModel.createAutofillPaymentInstrument(
mTestRule.getWebContents(), creditCard, /* billingProfile= */ null);
model.set(AssistantCollectUserDataModel.AVAILABLE_PAYMENT_INSTRUMENTS,
Collections.singletonList(paymentInstrument));
model.set(AssistantCollectUserDataModel.SELECTED_PAYMENT_INSTRUMENT, paymentInstrument);
});
// Without billing profile, a generic information missing error should be shown.
onView(is(getPaymentSummaryErrorView(viewHolder))).check(matches(isDisplayed()));
onView(is(getPaymentSummaryErrorView(viewHolder)))
.check(matches(withText("Information missing")));
TestThreadUtils.runOnUiThreadBlocking(() -> {
AutofillPaymentInstrument paymentInstrument =
AssistantCollectUserDataModel.createAutofillPaymentInstrument( AssistantCollectUserDataModel.createAutofillPaymentInstrument(
mTestRule.getWebContents(), creditCard, profile); mTestRule.getWebContents(), creditCard, profile);
model.set(AssistantCollectUserDataModel.AVAILABLE_PAYMENT_INSTRUMENTS, model.set(AssistantCollectUserDataModel.AVAILABLE_PAYMENT_INSTRUMENTS,
Collections.singletonList(expiredPaymentInstrument)); Collections.singletonList(paymentInstrument));
model.set(AssistantCollectUserDataModel.SELECTED_PAYMENT_INSTRUMENT, paymentInstrument);
});
// A missing zip code with an otherwise valid billing address should show a specialized
// error message.
onView(is(getPaymentSummaryErrorView(viewHolder))).check(matches(isDisplayed()));
onView(is(getPaymentSummaryErrorView(viewHolder)))
.check(matches(withText("Missing Zip Code")));
TestThreadUtils.runOnUiThreadBlocking(() -> {
profile.setPostalCode("90210");
AutofillPaymentInstrument validPaymentInstrument =
AssistantCollectUserDataModel.createAutofillPaymentInstrument(
mTestRule.getWebContents(), creditCard, profile);
model.set(AssistantCollectUserDataModel.AVAILABLE_PAYMENT_INSTRUMENTS,
Collections.singletonList(validPaymentInstrument));
model.set(AssistantCollectUserDataModel.SELECTED_PAYMENT_INSTRUMENT, model.set(AssistantCollectUserDataModel.SELECTED_PAYMENT_INSTRUMENT,
expiredPaymentInstrument); validPaymentInstrument);
}); });
// check that the card is not accepted (i.e. an error message is shown). // An expired card should only show a specialized error message if everything else is
// complete.
onView(is(getPaymentSummaryErrorView(viewHolder))).check(matches(isDisplayed())); onView(is(getPaymentSummaryErrorView(viewHolder))).check(matches(isDisplayed()));
onView(is(getPaymentSummaryErrorView(viewHolder))) onView(is(getPaymentSummaryErrorView(viewHolder)))
.check(matches(withText("Card is expired"))); .check(matches(withText("Card is expired")));
creditCard.setYear("2050");
TestThreadUtils.runOnUiThreadBlocking(() -> { TestThreadUtils.runOnUiThreadBlocking(() -> {
creditCard.setYear("2050");
AutofillPaymentInstrument validPaymentInstrument = AutofillPaymentInstrument validPaymentInstrument =
AssistantCollectUserDataModel.createAutofillPaymentInstrument( AssistantCollectUserDataModel.createAutofillPaymentInstrument(
mTestRule.getWebContents(), creditCard, profile); mTestRule.getWebContents(), creditCard, profile);
......
...@@ -26,6 +26,7 @@ import static android.support.test.espresso.matcher.ViewMatchers.withId; ...@@ -26,6 +26,7 @@ import static android.support.test.espresso.matcher.ViewMatchers.withId;
import static android.support.test.espresso.matcher.ViewMatchers.withParent; import static android.support.test.espresso.matcher.ViewMatchers.withParent;
import static android.support.test.espresso.matcher.ViewMatchers.withText; import static android.support.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.anything; import static org.hamcrest.Matchers.anything;
...@@ -50,6 +51,7 @@ import org.junit.runner.RunWith; ...@@ -50,6 +51,7 @@ import org.junit.runner.RunWith;
import org.chromium.base.test.util.CommandLineFlags; import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.autofill_assistant.R; import org.chromium.chrome.autofill_assistant.R;
import org.chromium.chrome.browser.autofill.PersonalDataManager.CreditCard;
import org.chromium.chrome.browser.autofill_assistant.proto.ActionProto; import org.chromium.chrome.browser.autofill_assistant.proto.ActionProto;
import org.chromium.chrome.browser.autofill_assistant.proto.ChipProto; import org.chromium.chrome.browser.autofill_assistant.proto.ChipProto;
import org.chromium.chrome.browser.autofill_assistant.proto.CollectUserDataProto; import org.chromium.chrome.browser.autofill_assistant.proto.CollectUserDataProto;
...@@ -589,8 +591,8 @@ public class AutofillAssistantPersonalDataManagerTest { ...@@ -589,8 +591,8 @@ public class AutofillAssistantPersonalDataManagerTest {
/** /**
* Catch the insert of a credit card with a billing address missing the postal code added * Catch the insert of a credit card with a billing address missing the postal code added
* outside of the Autofill Assistant, e.g. with the Chrome settings UI, and fill it into the * outside of the Autofill Assistant, e.g. with the Chrome settings UI and verify the error
* form. * message.
*/ */
@Test @Test
@MediumTest @MediumTest
...@@ -623,7 +625,79 @@ public class AutofillAssistantPersonalDataManagerTest { ...@@ -623,7 +625,79 @@ public class AutofillAssistantPersonalDataManagerTest {
waitUntilViewMatchesCondition(allOf(withText("Missing Billing Code"), waitUntilViewMatchesCondition(allOf(withText("Missing Billing Code"),
isDescendantOfA(withId(R.id.payment_method_summary))), isDescendantOfA(withId(R.id.payment_method_summary))),
isDisplayed()); isDisplayed());
waitUntilViewMatchesCondition(withText("Continue"), isEnabled()); onView(withContentDescription("Continue")).check(matches(not(isEnabled())));
// TODO(b/154068342): Update billing zip, verify that error message is gone and Continue
// is enabled.
}
/**
* Catch the insert of a credit card with an invalid expiration date added outside of the
* Autofill Assistant, e.g. with the Chrome settings UI and verify the error message.
*/
@Test
@MediumTest
public void testExternalAddExpiredCreditCard() throws Exception {
// Add address for card.
String profileId = mHelper.addDummyProfile("John Doe", "johndoe@google.com");
ArrayList<ActionProto> list = new ArrayList<>();
list.add((ActionProto) ActionProto.newBuilder()
.setCollectUserData(CollectUserDataProto.newBuilder()
.setRequestPaymentMethod(true)
.setBillingAddressName("billing_address")
.setCreditCardExpiredText("Card is expired")
.setRequestTermsAndConditions(false))
.build());
AutofillAssistantTestScript script = new AutofillAssistantTestScript(
(SupportedScriptProto) SupportedScriptProto.newBuilder()
.setPath("form_target_website.html")
.setPresentation(PresentationProto.newBuilder().setAutostart(true).setChip(
ChipProto.newBuilder().setText("Payment")))
.build(),
list);
runScript(script);
waitUntilViewMatchesCondition(
allOf(withId(R.id.section_title_add_button_label), withText("Add card")),
isCompletelyDisplayed());
CreditCard card = new CreditCard("", "https://example.com", /* isLocal= */ true,
/* isCached= */ true, "John Doe", "4111111111111111", "1111", "12", "2000", "visa",
org.chromium.chrome.autofill_assistant.R.drawable.visa_card, profileId,
/* serverId= */ "");
mHelper.setCreditCard(card);
waitUntilViewMatchesCondition(allOf(withText("Card is expired"),
isDescendantOfA(withId(R.id.payment_method_summary))),
isDisplayed());
onView(withContentDescription("Continue")).check(matches(not(isEnabled())));
onView(withText("Payment method")).perform(click());
waitUntilViewMatchesCondition(
allOf(withContentDescription("Edit card"),
isNextAfterSibling(hasDescendant(withText(containsString("John Doe"))))),
isDisplayed());
onView(allOf(withContentDescription("Edit card"),
isNextAfterSibling(hasDescendant(withText(containsString("John Doe"))))))
.perform(click());
waitUntilViewMatchesCondition(
withContentDescription("Card number*"), allOf(isDisplayed(), isEnabled()));
onView(allOf(withId(org.chromium.chrome.R.id.spinner), withChild(withText("2000"))))
.perform(click());
// Select 2 years in the future, 0 is the currently invalid year, 1 is the current year.
onData(anything())
.atPosition(3)
.inRoot(withDecorView(withClassName(containsString("Popup"))))
.perform(click());
onView(withId(org.chromium.chrome.R.id.editor_dialog_done_button))
.perform(scrollTo(), click());
// Updating the card does not collapse the card section.
waitUntilViewMatchesCondition(allOf(withId(R.id.credit_card_number),
isDescendantOfA(withId(R.id.payment_method_full))),
allOf(withText(containsString("1111")), isDisplayed()));
waitUntilViewMatchesCondition(allOf(withId(R.id.incomplete_error),
isDescendantOfA(withId(R.id.payment_method_full))),
not(isDisplayed()));
onView(withContentDescription("Continue")).check(matches(isEnabled()));
} }
/** /**
......
...@@ -191,4 +191,58 @@ AssistantCollectUserDataDelegate::GetJavaObject() { ...@@ -191,4 +191,58 @@ AssistantCollectUserDataDelegate::GetJavaObject() {
return java_assistant_collect_user_data_delegate_; return java_assistant_collect_user_data_delegate_;
} }
bool AssistantCollectUserDataDelegate::IsContactComplete(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jobject>& jcontact_profile) {
if (!jcontact_profile) {
return ui_controller_->IsContactComplete(nullptr);
}
autofill::AutofillProfile contact;
autofill::PersonalDataManagerAndroid::PopulateNativeProfileFromJava(
jcontact_profile, env, &contact);
return ui_controller_->IsContactComplete(&contact);
}
bool AssistantCollectUserDataDelegate::IsShippingAddressComplete(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jobject>& jaddress) {
if (!jaddress) {
return ui_controller_->IsShippingAddressComplete(nullptr);
}
autofill::AutofillProfile address;
autofill::PersonalDataManagerAndroid::PopulateNativeProfileFromJava(
jaddress, env, &address);
return ui_controller_->IsShippingAddressComplete(&address);
}
bool AssistantCollectUserDataDelegate::IsPaymentInstrumentComplete(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jobject>& jcard,
const base::android::JavaParamRef<jobject>& jaddress) {
if (!jcard) {
return ui_controller_->IsPaymentInstrumentComplete(nullptr, nullptr);
}
autofill::CreditCard card;
autofill::PersonalDataManagerAndroid::PopulateNativeCreditCardFromJava(
jcard, env, &card);
if (jaddress) {
autofill::AutofillProfile address;
autofill::PersonalDataManagerAndroid::PopulateNativeProfileFromJava(
jaddress, env, &address);
return ui_controller_->IsPaymentInstrumentComplete(&card, &address);
}
return ui_controller_->IsPaymentInstrumentComplete(&card, nullptr);
}
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -93,6 +93,22 @@ class AssistantCollectUserDataDelegate { ...@@ -93,6 +93,22 @@ class AssistantCollectUserDataDelegate {
void OnTextFocusLost(JNIEnv* env, void OnTextFocusLost(JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller); const base::android::JavaParamRef<jobject>& jcaller);
bool IsContactComplete(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jobject>& jcontact_profile);
bool IsShippingAddressComplete(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jobject>& jaddress);
bool IsPaymentInstrumentComplete(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller,
const base::android::JavaParamRef<jobject>& jcard,
const base::android::JavaParamRef<jobject>& jaddress);
base::android::ScopedJavaGlobalRef<jobject> GetJavaObject(); base::android::ScopedJavaGlobalRef<jobject> GetJavaObject();
private: private:
......
...@@ -965,6 +965,34 @@ void UiControllerAndroid::OnTextFocusLost() { ...@@ -965,6 +965,34 @@ void UiControllerAndroid::OnTextFocusLost() {
base::TimeDelta::FromMilliseconds(50)); base::TimeDelta::FromMilliseconds(50));
} }
bool UiControllerAndroid::IsContactComplete(
autofill::AutofillProfile* contact) {
auto* options = ui_delegate_->GetCollectUserDataOptions();
if (options == nullptr) {
return false;
}
return IsCompleteContact(contact, *options);
}
bool UiControllerAndroid::IsShippingAddressComplete(
autofill::AutofillProfile* address) {
auto* options = ui_delegate_->GetCollectUserDataOptions();
if (options == nullptr) {
return false;
}
return IsCompleteShippingAddress(address, *options);
}
bool UiControllerAndroid::IsPaymentInstrumentComplete(
autofill::CreditCard* card,
autofill::AutofillProfile* address) {
auto* options = ui_delegate_->GetCollectUserDataOptions();
if (options == nullptr) {
return false;
}
return IsCompleteCreditCard(card, address, *options);
}
void UiControllerAndroid::HideKeyboardIfFocusNotOnText() { void UiControllerAndroid::HideKeyboardIfFocusNotOnText() {
Java_AutofillAssistantUiController_hideKeyboardIfFocusNotOnText( Java_AutofillAssistantUiController_hideKeyboardIfFocusNotOnText(
AttachCurrentThread(), java_object_); AttachCurrentThread(), java_object_);
......
...@@ -149,6 +149,10 @@ class UiControllerAndroid : public ControllerObserver { ...@@ -149,6 +149,10 @@ class UiControllerAndroid : public ControllerObserver {
void OnDateTimeRangeEndTimeSlotCleared(); void OnDateTimeRangeEndTimeSlotCleared();
void OnKeyValueChanged(const std::string& key, const ValueProto& value); void OnKeyValueChanged(const std::string& key, const ValueProto& value);
void OnTextFocusLost(); void OnTextFocusLost();
bool IsContactComplete(autofill::AutofillProfile* contact);
bool IsShippingAddressComplete(autofill::AutofillProfile* address);
bool IsPaymentInstrumentComplete(autofill::CreditCard* card,
autofill::AutofillProfile* address);
// Called by AssistantFormDelegate: // Called by AssistantFormDelegate:
void OnCounterChanged(int input_index, int counter_index, int value); void OnCounterChanged(int input_index, int counter_index, int value);
......
...@@ -98,107 +98,6 @@ bool OnlyLoginRequested( ...@@ -98,107 +98,6 @@ bool OnlyLoginRequested(
.has_value(); .has_value();
} }
bool IsCompleteContact(
const autofill::AutofillProfile* profile,
const CollectUserDataOptions& collect_user_data_options) {
if (!collect_user_data_options.request_payer_name &&
!collect_user_data_options.request_payer_email &&
!collect_user_data_options.request_payer_phone) {
return true;
}
if (!profile) {
return false;
}
if (collect_user_data_options.request_payer_name &&
!profile->HasInfo(autofill::NAME_FULL)) {
return false;
}
if (collect_user_data_options.request_payer_email &&
!profile->HasInfo(autofill::EMAIL_ADDRESS)) {
return false;
}
if (collect_user_data_options.request_payer_phone &&
!profile->HasInfo(autofill::PHONE_HOME_WHOLE_NUMBER)) {
return false;
}
return true;
}
bool IsCompleteAddress(const autofill::AutofillProfile* profile,
bool require_postal_code) {
if (!profile) {
return false;
}
// We use a hard coded locale here since it's not used in the autofill:: code
// anyway. I.e. creating this profile ends up in FormGroup::GetInfoImpl, which
// simply ignores the app_locale.
auto address_data =
autofill::i18n::CreateAddressDataFromAutofillProfile(*profile, "en-US");
if (!autofill::addressinput::HasAllRequiredFields(*address_data)) {
return false;
}
if (require_postal_code && address_data->postal_code.empty()) {
return false;
}
return true;
}
bool IsCompleteShippingAddress(
const autofill::AutofillProfile* profile,
const CollectUserDataOptions& collect_user_data_options) {
return !collect_user_data_options.request_shipping ||
IsCompleteAddress(profile, /* require_postal_code = */ false);
}
bool IsCompleteCreditCard(
const autofill::CreditCard* credit_card,
const autofill::AutofillProfile* billing_profile,
const CollectUserDataOptions& collect_user_data_options) {
if (!collect_user_data_options.request_payment_method) {
return true;
}
if (!credit_card || !billing_profile) {
return false;
}
if (!IsCompleteAddress(
billing_profile,
collect_user_data_options.require_billing_postal_code)) {
return false;
}
if (credit_card->record_type() != autofill::CreditCard::MASKED_SERVER_CARD &&
!credit_card->HasValidCardNumber()) {
// Can't check validity of masked server card numbers because they are
// incomplete until decrypted.
return false;
}
if (!credit_card->HasValidExpirationDate() ||
credit_card->billing_address_id().empty()) {
return false;
}
std::string basic_card_network =
autofill::data_util::GetPaymentRequestData(credit_card->network())
.basic_card_issuer_network;
if (!collect_user_data_options.supported_basic_card_networks.empty() &&
std::find(collect_user_data_options.supported_basic_card_networks.begin(),
collect_user_data_options.supported_basic_card_networks.end(),
basic_card_network) ==
collect_user_data_options.supported_basic_card_networks.end()) {
return false;
}
return true;
}
bool IsValidLoginChoice( bool IsValidLoginChoice(
const std::string& choice_identifier, const std::string& choice_identifier,
const CollectUserDataOptions& collect_user_data_options) { const CollectUserDataOptions& collect_user_data_options) {
......
...@@ -133,6 +133,26 @@ bool CompletenessComparePaymentInstruments( ...@@ -133,6 +133,26 @@ bool CompletenessComparePaymentInstruments(
return complete_fields_a > complete_fields_b; return complete_fields_a > complete_fields_b;
} }
bool IsCompleteAddress(const autofill::AutofillProfile* profile,
bool require_postal_code) {
if (!profile) {
return false;
}
// We use a hard coded locale here since we are only interested in whether
// fields are empty or not.
auto address_data =
autofill::i18n::CreateAddressDataFromAutofillProfile(*profile, "en-US");
if (!autofill::addressinput::HasAllRequiredFields(*address_data)) {
return false;
}
if (require_postal_code && address_data->postal_code.empty()) {
return false;
}
return true;
}
} // namespace } // namespace
std::vector<int> SortContactsByCompleteness( std::vector<int> SortContactsByCompleteness(
...@@ -252,4 +272,84 @@ bool CompareContactDetails( ...@@ -252,4 +272,84 @@ bool CompareContactDetails(
return true; return true;
} }
bool IsCompleteContact(
const autofill::AutofillProfile* profile,
const CollectUserDataOptions& collect_user_data_options) {
if (!collect_user_data_options.request_payer_name &&
!collect_user_data_options.request_payer_email &&
!collect_user_data_options.request_payer_phone) {
return true;
}
if (!profile) {
return false;
}
if (collect_user_data_options.request_payer_name &&
!profile->HasInfo(autofill::NAME_FULL)) {
return false;
}
if (collect_user_data_options.request_payer_email &&
!profile->HasInfo(autofill::EMAIL_ADDRESS)) {
return false;
}
if (collect_user_data_options.request_payer_phone &&
!profile->HasInfo(autofill::PHONE_HOME_WHOLE_NUMBER)) {
return false;
}
return true;
}
bool IsCompleteShippingAddress(
const autofill::AutofillProfile* profile,
const CollectUserDataOptions& collect_user_data_options) {
return !collect_user_data_options.request_shipping ||
IsCompleteAddress(profile, /* require_postal_code = */ false);
}
bool IsCompleteCreditCard(
const autofill::CreditCard* credit_card,
const autofill::AutofillProfile* billing_profile,
const CollectUserDataOptions& collect_user_data_options) {
if (!collect_user_data_options.request_payment_method) {
return true;
}
if (!credit_card || !billing_profile) {
return false;
}
if (!IsCompleteAddress(
billing_profile,
collect_user_data_options.require_billing_postal_code)) {
return false;
}
if (credit_card->record_type() != autofill::CreditCard::MASKED_SERVER_CARD &&
!credit_card->HasValidCardNumber()) {
// Can't check validity of masked server card numbers because they are
// incomplete until decrypted.
return false;
}
if (!credit_card->HasValidExpirationDate() ||
credit_card->billing_address_id().empty()) {
return false;
}
std::string basic_card_network =
autofill::data_util::GetPaymentRequestData(credit_card->network())
.basic_card_issuer_network;
if (!collect_user_data_options.supported_basic_card_networks.empty() &&
std::find(collect_user_data_options.supported_basic_card_networks.begin(),
collect_user_data_options.supported_basic_card_networks.end(),
basic_card_network) ==
collect_user_data_options.supported_basic_card_networks.end()) {
return false;
}
return true;
}
} // namespace autofill_assistant } // namespace autofill_assistant
...@@ -62,6 +62,18 @@ bool CompareContactDetails( ...@@ -62,6 +62,18 @@ bool CompareContactDetails(
const autofill::AutofillProfile* a, const autofill::AutofillProfile* a,
const autofill::AutofillProfile* b); const autofill::AutofillProfile* b);
bool IsCompleteContact(const autofill::AutofillProfile* profile,
const CollectUserDataOptions& collect_user_data_options);
bool IsCompleteShippingAddress(
const autofill::AutofillProfile* profile,
const CollectUserDataOptions& collect_user_data_options);
bool IsCompleteCreditCard(
const autofill::CreditCard* credit_card,
const autofill::AutofillProfile* billing_profile,
const CollectUserDataOptions& collect_user_data_options);
} // namespace autofill_assistant } // namespace autofill_assistant
#endif // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_USER_DATA_UTIL_H_ #endif // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_USER_DATA_UTIL_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment