1429260 - Handle request changes with updateWith - verify that state stays consistent

Status: RESOLVED FIXED

(Firefox :: WebPayments UI, enhancement, P1)

Description

[Matthew N. [:MattN]]

This bug tracks auditing the dialog behaviour to ensure that dialog state remains consistent when the Payment Request is changed.

Bug 1429190 is one example where there could be a problem but there are possibly others.

Comment 1

[Matthew N. [:MattN]]

https://dxr.mozilla.org/mozilla-central/rev/0ac953fcddf10132eaecdb753d72b2ba5a43c32a/toolkit/components/payments/paymentUIService.js#82-84 will have to eventually lead to setStateFromParent getting called with the updated `request` property.

Comment 2

[Sam Foster [:sfoster] (he/him)]

This should entail reviewing existing test coverage and adding any necessary tests to ensure we have implemented updateWith handling, and that it doesn't regress.

Comment 3

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1429260 - Refactor PTU.Details data to only include the expected properties.

Review commit: https://reviewboard.mozilla.org/r/246638/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246638/

Comment 4

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 1429260 - Add tests to verify handling of payment detail properties it is possible to updateWith.

* Check update of displayItems, additionalDisplay items and total (via a modifier.)

Review commit: https://reviewboard.mozilla.org/r/246640/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246640/

Comment 5

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8980473 [details]
Bug 1429260 - Refactor PTU.Details data to only include the expected properties.

https://reviewboard.mozilla.org/r/246638/#review253044

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8980473 [details]
Bug 1429260 - Refactor PTU.Details data to only include the expected properties.

https://reviewboard.mozilla.org/r/246638/#review253046

This will need to be rebased on top of the shippingAddressErrors patch that landed in bug 1427959.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8980474 [details]
Bug 1429260 - Add tests to verify handling of payment detail properties it is possible to updateWith.

https://reviewboard.mozilla.org/r/246640/#review253112

::: browser/components/payments/test/browser/browser_change_shipping.js:78
(Diff revision 1)
> +         "\u20AC2.50",
> +         "Check updated total currency amount");
> +
> +      let btn = content.document.querySelector("#view-all");
> +      btn.click();
> +      await content.Promise.resolve();

This should use
```
      await PTU.DialogContentUtils.waitForState(content, (state) => {
        return state.orderDetailsShowing;
      }, "Order details show be showing now");
```

Comment 8

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8980473 [details]
Bug 1429260 - Refactor PTU.Details data to only include the expected properties.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246638/diff/1-2/

Comment 9

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8980474 [details]
Bug 1429260 - Add tests to verify handling of payment detail properties it is possible to updateWith.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246640/diff/1-2/

Comment 10

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8980473 [details]
Bug 1429260 - Refactor PTU.Details data to only include the expected properties.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246638/diff/2-3/

Comment 11

[Sam Foster [:sfoster] (he/him)]

Comment on attachment 8980474 [details]
Bug 1429260 - Add tests to verify handling of payment detail properties it is possible to updateWith.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246640/diff/2-3/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42fb8fd8d64b
Refactor PTU.Details data to only include the expected properties. r=jaws
https://hg.mozilla.org/integration/autoland/rev/d7379d70b963
Add tests to verify handling of payment detail properties it is possible to updateWith. r=jaws

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/42fb8fd8d64b
https://hg.mozilla.org/mozilla-central/rev/d7379d70b963