Share validation between the form pages and the summary page and surface the errors on both views
Categories
(Firefox :: WebPayments UI, enhancement, P5)
Tracking
()
People
(Reporter: jaws, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [webpayments])
User Story
Errors to handle * field-specific merchant errors * missing required fields * custom Fx field validation (Luhn, postal/zip code, etc.)
Attachments
(1 file, 6 obsolete files)
Currently the summary view will show errors if the selected option is missing required fields, and the form pages will show errors if the record being added/edited is missing fields or has other validation issues such as invalid CC number (Luhn algorithm), invalid email address, valid postal code, and valid region. We will need to create a shared module of some sorts that can do this validation in one place, and then can be referenced by the two pages such that we don't have to duplicate this validation logic.
Updated•6 years ago
|
Comment 1•6 years ago
|
||
One idea I had would be to have a separate <address-form> for each address picker so behind the scenes we populate the fields with the selected address and then we can use the same form validation logic for the summary screen by checking the validity of the corresponding form.
Comment 2•6 years ago
|
||
I also noticed that field-specific merchant errors also need to be surfaced on the summary page.
Updated•6 years ago
|
Updated•6 years ago
|
Comment 5•6 years ago
|
||
Broadly, the plan is to create separate forms for the shipping address, billing address and payer address forms. This will help as each has different rules and navigating these is the source of some of the complexity we are dealing with today in this code. We can then re-use these forms when validating an address picker by delegating to the relevant form's checkValidity() method.
Updated•6 years ago
|
Comment 6•6 years ago
|
||
* New AddressForm instances for each address type * Move selected-state-key out of state and into a form attribute * Remove passing form title via state - form can use state (for editing or not, and for shippingType) to pick correct data-attribute TODO: * Fix up remaining tests in browser/ * Moving validation logic to the forms should be in a patch layered on this one
Updated•6 years ago
|
Comment 7•6 years ago
|
||
I've updated the first patch so that it breaks out the different forms, moves the selectedStateKey out of the page state and into the form attribute. There should be no other functional changes. We could land something like this ahead of the other validation changes to reduce conflicts with other work. Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=838ad2014e5521e032c1e566c42f5c9319035a4e
Comment 8•6 years ago
|
||
* New AddressForm instances for each address type * Move selected-state-key out of state for the address forms, and into an attribute * Remove passing form title via state - form can use state (for editing or not, and for shippingType) to pick correct da * Remove form title logic from address picker. * Amend onboarding logic to direct to billing address form when necessary * Fix-up tests
Comment 9•6 years ago
|
||
* New AddressForm instances for each address type * Move selected-state-key out of state for the address forms, and into an attribute * Remove passing form title via state, just use data-title-edit and data-title-add on the element * Remove shippingOption form title logic from address picker, just set the right attribute values from PaymentDialog * Move setting the extraRequiredFields data attribute the payer form needs out to the payment-dialog's render. * Amend onboarding logic to direct to billing address form when necessary * Fix-up tests
Updated•6 years ago
|
Updated•6 years ago
|
Comment 10•6 years ago
|
||
(In reply to Sam Foster [:sfoster] from comment #9) > Created attachment 9026634 [details] > Bug 1481481 - Create separate forms for shipping, payer and billing address. > r?MattN Sorry about the noise. Attachment 9024156 [details] (D11545) has now been updated with all the requested changes.
Comment 11•6 years ago
|
||
Pushed by sfoster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c026a6f2c6cd Create separate forms for shipping, payer and billing address r=MattN
Updated•6 years ago
|
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c026a6f2c6cd
Comment 13•6 years ago
|
||
* Remove .guid property from the page state use the value referenced by selectedStateKey as the current value in AddressForm and BasicCardForm * Fix up mochitests, browser tests
Comment 14•6 years ago
|
||
Depends on D13326
Comment 15•5 years ago
|
||
attachment 9028473 [details] is part II and just removes the {pageid}.guid property in favor of using the selected-state-key to references the record the given picker and form should be loading. That is pretty stable probably ready for review. attachment 9028474 [details] is WIP still. All the tests/mochitests are now passing, and manual testing looks good. But I still need to fix up tests/browser. I'm not sure why moz-phab created 2 revisions for these patches :/
Updated•5 years ago
|
Comment 16•5 years ago
|
||
MattN: I've updated the first patch (D13327 Bug 1481481 - Use form to validate the picker selected option) to make sure all the selectedStateKey -related stuff is in there, and all the tests are passing at that point. The 2nd patch is WIP still as discussed.
Comment 17•5 years ago
|
||
(In reply to Sam Foster [:sfoster] from comment #16) > The 2nd patch is WIP still as discussed. Rebased and updated. More detail in the phab revision comments.
Comment 18•5 years ago
|
||
- Dispatch the same messages when add/updating temporary collections
- Wait for both the response message and updateState message when saving records from card and address forms
Depends on D13326
Updated•5 years ago
|
Comment 19•5 years ago
|
||
I've added another patch to the series which defers resolving paymentRequest.updateAutofillRecord until after the updateState event has been received.
The selectedStateKey / .guid patch is unchanged bar restoring the test skippage.
The form validation patch is now ready for review also. I'll update the commit message in phabricator, but basically the only thing that I'm aware of that is outstanding is a test failure: test_address_edit in browser_change_shipping.js is failing locally; the expected selected option index is wrong after updating the record in the formautofill store. I'm not sure this patch caused that but I'll look some more before trying to land anything.
This is a large patch, apologies for that. Let me know if you want me to walk through it, break it up some more or whatever will help.
Updated•5 years ago
|
Comment 20•5 years ago
|
||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 21•5 years ago
|
||
Last time I checked, it was becoming increasingly difficult to get the (disabled) test suite to run. I don't anticipate updating these patches. If that changes I'll re-open.
Comment 22•5 years ago
|
||
We haven't closed the other Web Payments bugs so I think this should stay open for when work on this component resumes.
Comment 23•4 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:jimm, maybe it's time to close this bug?
Updated•4 years ago
|
Comment 24•3 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:jimm, maybe it's time to close this bug?
Updated•3 years ago
|
Comment 25•3 years ago
|
||
The leave-open keyword is there and there is no activity for 6 months.
:jimm, maybe it's time to close this bug?
Comment 26•3 years ago
|
||
This code is getting removed in bug 1721229 but clearing leave-open in the meantime.
Updated•2 years ago
|
Description
•