Bug 1476348 - Show missing field validation errors on the summary page. r=MattN

MozReview-Commit-ID: 5dOzxWH0pWp

--HG--
extra : rebase_source : 1de025d22f01c5b293355b0fe0732543769bf761
This commit is contained in:
Jared Wein 2018-08-03 18:51:31 -04:00
parent 1a485b81c7
commit 559a6cb6d4
14 changed files with 268 additions and 44 deletions

View File

@ -62,11 +62,13 @@ export default class AddressOption extends ObservedPropertiesMixin(RichOption) {
}
render() {
this._name.textContent = this.name;
this["_street-address"].textContent = `${this.streetAddress} ` +
`${this.addressLevel2} ${this.addressLevel1} ${this.postalCode} ${this.country}`;
this._email.textContent = this.email;
this._tel.textContent = this.tel;
// Fall back to empty strings to prevent 'null' from appearing.
this._name.textContent = this.name || "";
this["_street-address"].textContent =
`${this.streetAddress || ""} ${this.addressLevel2 || ""} ` +
`${this.addressLevel1 || ""} ${this.postalCode || ""} ${this.country || ""}`;
this._email.textContent = this.email || "";
this._tel.textContent = this.tel || "";
}
}

View File

@ -43,7 +43,11 @@ export default class BasicCardOption extends ObservedPropertiesMixin(RichOption)
}
static formatSingleLineLabel(basicCard) {
return basicCard["cc-number"] + " " + basicCard["cc-exp"] + " " + basicCard["cc-name"];
// Fall back to empty strings to prevent 'undefined' from appearing.
let ccNumber = basicCard["cc-number"] || "";
let ccExp = basicCard["cc-exp"] || "";
let ccName = basicCard["cc-name"] || "";
return ccNumber + " " + ccExp + " " + ccName;
}
render() {

View File

@ -29,6 +29,24 @@ export default class AddressPicker extends RichPicker {
}
}
get fieldNames() {
if (this.hasAttribute("address-fields")) {
let names = this.getAttribute("address-fields").split(/\s+/);
if (names.length) {
return names;
}
}
return [
"address-level1",
"address-level2",
"country",
"name",
"postal-code",
"street-address",
];
}
/**
* De-dupe and filter addresses for the given set of fields that will be visible
*
@ -37,14 +55,7 @@ export default class AddressPicker extends RichPicker {
* de-duping and excluding entries
* @returns {object} filtered copy of given addresses
*/
filterAddresses(addresses, fieldNames = [
"address-level1",
"address-level2",
"country",
"name",
"postal-code",
"street-address",
]) {
filterAddresses(addresses, fieldNames = this.fieldNames) {
let uniques = new Set();
let result = {};
for (let [guid, address] of Object.entries(addresses)) {
@ -72,14 +83,7 @@ export default class AddressPicker extends RichPicker {
render(state) {
let addresses = paymentRequest.getAddresses(state);
let desiredOptions = [];
let fieldNames;
if (this.hasAttribute("address-fields")) {
let names = this.getAttribute("address-fields").split(/\s+/);
if (names.length) {
fieldNames = names;
}
}
let filteredAddresses = this.filterAddresses(addresses, fieldNames);
let filteredAddresses = this.filterAddresses(addresses, this.fieldNames);
for (let [guid, address] of Object.entries(filteredAddresses)) {
let optionEl = this.dropdown.getOptionByValue(guid);
if (!optionEl) {

View File

@ -242,13 +242,17 @@ export default class PaymentDialog extends PaymentStateSubscriberMixin(HTMLEleme
case "": {
// initial/default state
this._payButton.textContent = this._payButton.dataset.label;
const INVALID_CLASS_NAME = "invalid-selected-option";
this._payButton.disabled =
(state.request.paymentOptions.requestShipping &&
(!this._shippingAddressPicker.value ||
!this._shippingOptionPicker.value)) ||
(!this._shippingAddressPicker.selectedOption ||
this._shippingAddressPicker.classList.contains(INVALID_CLASS_NAME) ||
!this._shippingOptionPicker.selectedOption)) ||
(this._isPayerRequested(state.request.paymentOptions) &&
!this._payerAddressPicker.value) ||
!this._paymentMethodPicker.value ||
(!this._payerAddressPicker.selectedOption ||
this._payerAddressPicker.classList.contains(INVALID_CLASS_NAME))) ||
!this._paymentMethodPicker.selectedOption ||
this._paymentMethodPicker.classList.contains(INVALID_CLASS_NAME) ||
state.changesPrevented;
break;
}
@ -314,7 +318,7 @@ export default class PaymentDialog extends PaymentStateSubscriberMixin(HTMLEleme
this._orderDetailsOverlay.hidden = !state.orderDetailsShowing;
let genericError = "";
if (this._shippingAddressPicker.value &&
if (this._shippingAddressPicker.selectedOption &&
(!request.paymentDetails.shippingOptions ||
!request.paymentDetails.shippingOptions.length)) {
genericError = this._errorText.dataset[shippingType + "GenericError"];

View File

@ -29,6 +29,13 @@ export default class PaymentMethodPicker extends RichPicker {
this.dropdown.after(this.securityCodeInput);
}
get fieldNames() {
let fieldNames = [...BasicCardOption.recordAttributes];
// Type is not a required field though it may be present.
fieldNames.splice(fieldNames.indexOf("type"), 1);
return fieldNames;
}
render(state) {
let basicCards = paymentRequest.getBasicCards(state);
let desiredOptions = [];

View File

@ -7,7 +7,8 @@
grid-template-columns: 5fr auto auto;
grid-template-areas:
"label edit add"
"dropdown dropdown dropdown";
"dropdown dropdown dropdown"
"invalid invalid invalid";
}
.rich-picker > label {
@ -34,12 +35,27 @@
grid-area: dropdown;
}
.invalid-selected-option > rich-select {
outline: 1px solid #c70011;
}
.rich-picker > .invalid-label {
grid-area: invalid;
font-weight: normal;
color: #c70011;
}
:not(.invalid-selected-option) > .invalid-label {
display: none;
}
/* Payment Method Picker */
payment-method-picker.rich-picker {
grid-template-columns: 20fr 1fr auto auto;
grid-template-areas:
"label spacer edit add"
"dropdown cvv cvv cvv";
"dropdown cvv cvv cvv"
"invalid invalid invalid invalid";
}
payment-method-picker > input {

View File

@ -32,6 +32,11 @@ export default class RichPicker extends PaymentStateSubscriberMixin(HTMLElement)
this.editLink.href = "javascript:void(0)";
this.editLink.textContent = this.dataset.editLinkLabel;
this.editLink.addEventListener("click", this);
this.invalidLabel = document.createElement("label");
this.invalidLabel.className = "invalid-label";
this.invalidLabel.setAttribute("for", this.dropdown.popupBox.id);
this.invalidLabel.textContent = this.dataset.invalidLabel;
}
connectedCallback() {
@ -40,6 +45,7 @@ export default class RichPicker extends PaymentStateSubscriberMixin(HTMLElement)
this.appendChild(this.dropdown);
this.appendChild(this.editLink);
this.appendChild(this.addLink);
this.appendChild(this.invalidLabel);
super.connectedCallback();
}
@ -51,10 +57,28 @@ export default class RichPicker extends PaymentStateSubscriberMixin(HTMLElement)
render(state) {
this.editLink.hidden = !this.dropdown.value;
this.classList.toggle("invalid-selected-option",
this.missingFieldsOfSelectedOption().length);
}
get value() {
get selectedOption() {
return this.dropdown &&
this.dropdown.selectedOption;
}
get fieldNames() {
return [];
}
missingFieldsOfSelectedOption() {
let selectedOption = this.selectedOption;
if (!selectedOption) {
return [];
}
let fieldNames = this.fieldNames;
// Return all field names that are empty or missing from the option.
return fieldNames.filter(name => !selectedOption.getAttribute(name));
}
}

View File

@ -200,6 +200,12 @@ let ADDRESSES_1 = {
"street-address": "P.O. Box 123",
"tel": "+1 650 555-5555",
},
"abcde12345": {
"address-level2": "Mountain View",
"country": "US",
"guid": "abcde12345",
"name": "Mrs. Fields",
},
};
let DUPED_ADDRESSES = {
@ -285,6 +291,20 @@ let BASIC_CARDS_1 = {
"cc-family-name": "Doe",
"cc-exp": "2023-05",
},
"123456789abc": {
methodName: "basic-card",
"cc-number": "************1234",
"guid": "123456789abc",
"version": 1,
"timeCreated": 1517890536491,
"timeLastModified": 1517890564518,
"timeLastUsed": 0,
"timesUsed": 0,
"cc-name": "Jane Fields",
"cc-given-name": "Jane",
"cc-additional-name": "",
"cc-family-name": "Fields",
},
};
let buttonActions = {

View File

@ -74,6 +74,7 @@
<!ENTITY timeoutErrorPage.suggestion3 "If no other solutions work, check with **host-name**.">
<!ENTITY timeoutErrorPage.doneButton.label "OK">
<!ENTITY webPaymentsBranding.label "&brandShortName; Checkout">
<!ENTITY invalidOption.label "Missing or invalid information">
]>
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
@ -128,6 +129,7 @@
data-shipping-address-label="&shippingAddressLabel;"
data-delivery-address-label="&deliveryAddressLabel;"
data-pickup-address-label="&pickupAddressLabel;"
data-invalid-label="&invalidOption.label;"
selected-state-key="selectedShippingAddress"></address-picker>
<shipping-option-picker class="shipping-related"
@ -139,12 +141,14 @@
data-add-link-label="&basicCard.addLink.label;"
data-edit-link-label="&basicCard.editLink.label;"
data-cvv-placeholder="&basicCard.cvv.placeholder;"
data-invalid-label="&invalidOption.label;"
label="&paymentMethodsLabel;">
</payment-method-picker>
<address-picker class="payer-related"
label="&payerLabel;"
data-add-link-label="&payer.addLink.label;"
data-edit-link-label="&payer.editLink.label;"
data-invalid-label="&invalidOption.label;"
selected-state-key="selectedPayerAddress"></address-picker>
</div>

View File

@ -482,6 +482,10 @@ var PaymentTestUtils = {
"cc-name": "Jane McMaster-Card",
"cc-number": "5555555555554444",
},
MissingFields: {
"cc-name": "Missy Fields",
"cc-number": "340000000000009",
},
Temp: {
"cc-exp-month": 12,
"cc-exp-year": (new Date()).getFullYear() + 9,

View File

@ -13,6 +13,7 @@ Test the address-picker component
<script src="../../res/vendor/custom-elements.min.js"></script>
<script src="../../res/unprivileged-fallbacks.js"></script>
<link rel="stylesheet" type="text/css" href="../../res/containers/rich-picker.css"/>
<link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
<link rel="stylesheet" type="text/css" href="../../res/components/address-option.css"/>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
@ -65,13 +66,20 @@ add_task(async function test_initialSet() {
"street-address": "P.O. Box 123",
"tel": "+1 650 555-5555",
},
"abcde12345": {
"address-level2": "Mountain View",
"country": "US",
"guid": "abcde12345",
"name": "Mrs. Fields",
},
},
});
await asyncElementRendered();
let options = picker1.dropdown.popupBox.children;
is(options.length, 2, "Check dropdown has both addresses");
is(options.length, 3, "Check dropdown has all addresses");
ok(options[0].textContent.includes("Mr. Foo"), "Check first address");
ok(options[1].textContent.includes("Mrs. Bar"), "Check second address");
ok(options[2].textContent.includes("Mrs. Fields"), "Check third address");
});
add_task(async function test_update() {
@ -98,11 +106,17 @@ add_task(async function test_update() {
"street-address": "P.O. Box 123",
"tel": "+1 650 555-5555",
},
"abcde12345": {
"address-level2": "Mountain View",
"country": "US",
"guid": "abcde12345",
"name": "Mrs. Fields",
},
},
});
await asyncElementRendered();
let options = picker1.dropdown.popupBox.children;
is(options.length, 2, "Check dropdown still has both addresses");
is(options.length, 3, "Check dropdown still has all addresses");
ok(options[0].textContent.includes("Mr. Foo-edit"), "Check updated name in first address");
ok(!options[0].getAttribute("address-level2"), "Check removed first address-level2");
ok(options[1].textContent.includes("Mrs. Bar"), "Check that name is the same in second address");
@ -116,16 +130,33 @@ add_task(async function test_change_selected_address() {
is(picker1.editLink.hidden, true, "Picker edit link should be hidden when no option is selected");
let {selectedShippingAddress} = picker1.requestStore.getState();
is(selectedShippingAddress, null, "store should have no option selected");
ok(!picker1.classList.contains("invalid-selected-option"), "No validation on an empty selection");
ok(isHidden(picker1.invalidLabel), "The invalid label should be hidden");
picker1.dropdown.popupBox.focus();
synthesizeKey(options[2].getAttribute("name"), {});
await asyncElementRendered();
let selectedOption = picker1.dropdown.selectedOption;
is(selectedOption, options[2], "Selected option should now be the third option");
selectedShippingAddress = picker1.requestStore.getState().selectedShippingAddress;
is(selectedShippingAddress, selectedOption.getAttribute("guid"),
"store should have third option selected");
// The third option is missing some fields. Make sure that it is marked as such.
ok(picker1.classList.contains("invalid-selected-option"), "The third option is missing fields");
ok(!isHidden(picker1.invalidLabel), "The invalid label should be visible");
picker1.dropdown.popupBox.focus();
synthesizeKey(options[1].getAttribute("name"), {});
await asyncElementRendered();
let selectedOption = picker1.dropdown.selectedOption;
selectedOption = picker1.dropdown.selectedOption;
is(selectedOption, options[1], "Selected option should now be the second option");
selectedShippingAddress = picker1.requestStore.getState().selectedShippingAddress;
is(selectedShippingAddress, selectedOption.getAttribute("guid"),
"store should have second option selected");
ok(!picker1.classList.contains("invalid-selected-option"), "The second option has all fields");
ok(isHidden(picker1.invalidLabel), "The invalid label should be hidden");
});
add_task(async function test_streetAddress_combines_street_level2_level1_postalCode_country() {
@ -134,14 +165,28 @@ add_task(async function test_streetAddress_combines_street_level2_level1_postalC
let streetAddress = richoption1.querySelector(".street-address");
/* eslint-disable max-len */
is(streetAddress.textContent,
`${options[1].getAttribute("street-address")} ${options[1].getAttribute("address-level2")} ${options[1].getAttribute("address-level1")} ${options[1].getAttribute("postal-code")} ${options[1].getAttribute("country")}`);
`${options[1].getAttribute("street-address")} ${options[1].getAttribute("address-level2")} ${options[1].getAttribute("address-level1")} ${options[1].getAttribute("postal-code")} ${options[1].getAttribute("country")}`,
"The address shown should be human readable and include all fields");
/* eslint-enable max-len */
picker1.dropdown.popupBox.focus();
synthesizeKey(options[2].getAttribute("name"), {});
await asyncElementRendered();
richoption1 = picker1.dropdown.querySelector(".rich-select-selected-option");
streetAddress = richoption1.querySelector(".street-address");
is(streetAddress.textContent, " Mountain View US",
"The address shown should be human readable and include all fields");
picker1.dropdown.popupBox.focus();
synthesizeKey(options[1].getAttribute("name"), {});
await asyncElementRendered();
});
add_task(async function test_delete() {
picker1.requestStore.setState({
savedAddresses: {
// 48bnds6854t was deleted
// 48bnds6854t and abcde12345 was deleted
"68gjdh354j": {
"address-level1": "CA",
"address-level2": "Mountain View",

View File

@ -176,7 +176,7 @@ add_task(async function test_generic_errors() {
await asyncElementRendered();
let picker = el1._shippingAddressPicker;
ok(picker.value, "Address picker should have a selected value");
ok(picker.selectedOption, "Address picker should have a selected option");
is(el1._errorText.textContent, SHIPPING_GENERIC_ERROR,
"Generic error message should be shown when no shipping options or error are provided");
});
@ -294,8 +294,6 @@ add_task(async function test_picker_labels() {
await asyncElementRendered();
is(picker.labelElement.textContent, label,
`Label should be appropriate for ${shippingType}`);
info(JSON.stringify(el1.requestStore.getState(), null, 2));
info(picker.outerHTML);
}
});

View File

@ -71,6 +71,7 @@ async function setup({shippingRequired, payerRequired}) {
}] : null;
state.request.paymentOptions.requestShipping = shippingRequired;
state.request.paymentOptions.requestPayerName = payerRequired;
state.request.paymentOptions.requestPayerPhone = payerRequired;
state.savedAddresses = shippingRequired || payerRequired ? {
"48bnds6854t": {
"address-level1": "MI",
@ -92,9 +93,24 @@ async function setup({shippingRequired, payerRequired}) {
"street-address": "P.O. Box 123",
"tel": "+1 650 555-5555",
},
"abcdef1234": {
"address-level1": "CA",
"address-level2": "Mountain View",
"country": "US",
"guid": "abcdef1234",
"name": "Mrs. Fields",
},
} : {};
state.savedBasicCards = {
"john-doe": Object.assign({methodName: "basic-card"}, deepClone(PTU.BasicCards.JohnDoe)),
"john-doe": Object.assign({
"cc-exp": (new Date()).getFullYear() + 9 + "-01",
methodName: "basic-card",
guid: "aaa1",
}, deepClone(PTU.BasicCards.JohnDoe)),
"missing-fields": Object.assign({
methodName: "basic-card",
guid: "aaa2",
}, deepClone(PTU.BasicCards.MissingFields)),
};
state.selectedPayerAddress = null;
state.selectedPaymentCard = null;
@ -107,10 +123,19 @@ async function setup({shippingRequired, payerRequired}) {
function selectFirstItemOfPicker(picker) {
picker.dropdown.popupBox.focus();
let options = picker.dropdown.popupBox.children;
info(`Selecting "${options[0].textContent}" from the options`);
synthesizeKey(options[0].textContent, {});
ok(picker.dropdown.selectedOption, `Option should be selected for ${picker.localName}`);
}
function selectLastItemOfPicker(picker) {
picker.dropdown.popupBox.focus();
let options = picker.dropdown.popupBox.children;
let lastOption = options[options.length - 1];
synthesizeKey(lastOption.textContent, {});
ok(picker.dropdown.selectedOption, `Option should be selected for ${picker.localName}`);
}
add_task(async function runTests() {
let allPickers = {
shippingAddress: el1._shippingAddressPicker,
@ -158,6 +183,24 @@ add_task(async function runTests() {
await stateChangedPromise;
ok(!payButton.disabled, "Button is enabled when required options are selected");
// Individually toggle each picker to see how the missing fields affects Pay button.
for (let picker of testCase.pickers) {
// There is no "invalid" option for shipping options.
if (picker != allPickers.shippingOption) {
stateChangedPromise = promiseStateChange(el1.requestStore);
selectLastItemOfPicker(picker);
await stateChangedPromise;
ok(payButton.disabled, "Button is disabled when selected option has missing fields");
}
stateChangedPromise = promiseStateChange(el1.requestStore);
selectFirstItemOfPicker(picker);
await stateChangedPromise;
ok(!payButton.disabled, "Button is enabled when selected option has all required fields");
}
}
});
</script>

View File

@ -13,6 +13,7 @@ Test the payment-method-picker component
<script src="../../res/vendor/custom-elements.min.js"></script>
<script src="../../res/unprivileged-fallbacks.js"></script>
<link rel="stylesheet" type="text/css" href="../../res/containers/rich-picker.css"/>
<link rel="stylesheet" type="text/css" href="../../res/components/rich-select.css"/>
<link rel="stylesheet" type="text/css" href="../../res/components/basic-card-option.css"/>
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
@ -61,13 +62,22 @@ add_task(async function test_initialSet() {
"cc-number": "***********1234",
"guid": "68gjdh354j",
},
"123456789abc": {
"cc-name": "Jane Fields",
"cc-given-name": "Jane",
"cc-additional-name": "",
"cc-family-name": "Fields",
"cc-number": "************9876",
"guid": "123456789abc",
},
},
});
await asyncElementRendered();
let options = picker1.dropdown.popupBox.children;
is(options.length, 2, "Check dropdown has both cards");
is(options.length, 3, "Check dropdown has all three cards");
ok(options[0].textContent.includes("John Doe"), "Check first card");
ok(options[1].textContent.includes("J Smith"), "Check second card");
ok(options[2].textContent.includes("Jane Fields"), "Check third card");
});
add_task(async function test_update() {
@ -90,16 +100,25 @@ add_task(async function test_update() {
"cc-number": "***********1234",
"guid": "68gjdh354j",
},
"123456789abc": {
"cc-name": "Jane Fields",
"cc-given-name": "Jane",
"cc-additional-name": "",
"cc-family-name": "Fields",
"cc-number": "************9876",
"guid": "123456789abc",
},
},
});
await asyncElementRendered();
let options = picker1.dropdown.popupBox.children;
is(options.length, 2, "Check dropdown still has both cards");
is(options.length, 3, "Check dropdown still has three cards");
ok(!options[0].textContent.includes("John Doe"), "Check cleared first cc-name");
ok(options[0].textContent.includes("9876"), "Check updated first cc-number");
ok(options[0].textContent.includes("09"), "Check updated first exp-month");
ok(options[1].textContent.includes("J Smith"), "Check second card is the same");
ok(options[2].textContent.includes("Jane Fields"), "Check third card is the same");
});
add_task(async function test_change_selected_card() {
@ -111,21 +130,42 @@ add_task(async function test_change_selected_card() {
} = picker1.requestStore.getState();
is(selectedPaymentCard, null, "store should have no option selected");
is(selectedPaymentCardSecurityCode, null, "store should have no security code");
ok(!picker1.classList.contains("invalid-selected-option"), "No validation on an empty selection");
ok(isHidden(picker1.invalidLabel), "The invalid label should be hidden");
await SimpleTest.promiseFocus();
picker1.dropdown.popupBox.focus();
synthesizeKey("************1234", {});
synthesizeKey("************9876", {});
await asyncElementRendered();
ok(true, "Focused the security code field");
ok(!picker1.open, "Picker should be closed");
let selectedOption = picker1.dropdown.selectedOption;
is(selectedOption, options[2], "Selected option should now be the third option");
selectedPaymentCard = picker1.requestStore.getState().selectedPaymentCard;
is(selectedPaymentCard, selectedOption.getAttribute("guid"),
"store should have third option selected");
selectedPaymentCardSecurityCode = picker1.requestStore.getState().selectedPaymentCardSecurityCode;
is(selectedPaymentCardSecurityCode, null, "store should have empty security code");
ok(picker1.classList.contains("invalid-selected-option"), "Missing fields for the third option");
ok(!isHidden(picker1.invalidLabel), "The invalid label should be visible");
await SimpleTest.promiseFocus();
picker1.dropdown.popupBox.focus();
synthesizeKey("***********1234", {});
await asyncElementRendered();
ok(true, "Focused the security code field");
ok(!picker1.open, "Picker should be closed");
selectedOption = picker1.dropdown.selectedOption;
is(selectedOption, options[1], "Selected option should now be the second option");
selectedPaymentCard = picker1.requestStore.getState().selectedPaymentCard;
is(selectedPaymentCard, selectedOption.getAttribute("guid"),
"store should have second option selected");
selectedPaymentCardSecurityCode = picker1.requestStore.getState().selectedPaymentCardSecurityCode;
is(selectedPaymentCardSecurityCode, null, "store should have empty security code");
ok(!picker1.classList.contains("invalid-selected-option"), "The second option has all fields");
ok(isHidden(picker1.invalidLabel), "The invalid label should be hidden");
let stateChangePromise = promiseStateChange(picker1.requestStore);
@ -149,12 +189,21 @@ add_task(async function test_delete() {
"cc-number": "***********1234",
"guid": "68gjdh354j",
},
"123456789abc": {
"cc-name": "Jane Fields",
"cc-given-name": "Jane",
"cc-additional-name": "",
"cc-family-name": "Fields",
"cc-number": "************9876",
"guid": "123456789abc",
},
},
});
await asyncElementRendered();
let options = picker1.dropdown.popupBox.children;
is(options.length, 1, "Check dropdown has one remaining card");
ok(options[0].textContent.includes("J Smith"), "Check remaining card");
is(options.length, 2, "Check dropdown has two remaining cards");
ok(options[0].textContent.includes("J Smith"), "Check remaining card #1");
ok(options[1].textContent.includes("Jane Fields"), "Check remaining card #2");
});
</script>