1476571 - Billing address is incorrectly pre-filled starting from the second flow

Status: VERIFIED FIXED

(Firefox :: WebPayments UI, defect, P1)

Description

[Timea Cernea [:tbabos][inactive]]

Attachment: Video of the issue

[Affected versions]:
Nightly 63.0a1

[Affected platforms]:
Windows 10 x64, Mac OS X 10.12, Ubuntu 18.04 x64

[Prerequisites]:
- set the pref dom.payments.request.enabled to "true"
- make sure to have at least 2 Shipping Address saved on your profile

[Steps to reproduce]:
1. Go to “https://rsolomakhin.github.io/pr/single/” and click on “Buy”
2. Select the second (or third and so on) Shipping Address from the dropdown menu
3. Click on “Add” for a new Payment method

[Expected Result]:
As per the requirements document:
“Billing address: default would be as same as shipping address...”
“General credit card form includes credit card details and pre-filled billing address”
Thus, the pre-filled billing address should be the shipping address that was selected in Step 2.

[Actual Result]: 
The pre-filled billing address is always the first shipping address from the drop-down menu.

Comment 1

[:prathiksha]

Attachment: Bug 1476571 - Auto-select selectedShippingAddress as billing address when it is not null.

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

Comment 2

[Matthew N. [:MattN]]

Comment on attachment 8994722 [details]
Bug 1476571 - Auto-select selectedShippingAddress as billing address when it is not null.

https://reviewboard.mozilla.org/r/259222/#review266238

::: browser/components/payments/res/containers/basic-card-form.js:172
(Diff revision 1)
> +  checkAddressExists(guid) {
> +    if (!guid) {
> +      return false;
> +    }
> +
> +    let state = this.requestStore.getState();

I don't think we need to get the state again as you could pass it in but really I think you could do the following instead:
```js
if (paymentRequest.getAddresses(state)[selectedShippingAddress]) {
  billingAddressSelect.value = selectedShippingAddress;
} else {
  billingAddressSelect.value = Object.keys(addresses)[0];
}
```

This also avoid an unnecessary negation which helps readability.

::: browser/components/payments/test/mochitest/test_basic_card_form.html:187
(Diff revision 1)
>    checkCCForm(form, {
>      billingAddressGUID: address1.guid,
>    });
>  
> +  let select = document.getElementById("billingAddressGUID");
> +  ok(select.value, address1.guid, "Correct billing addresss shown");

I don't think this test is actually testing anything new and would have passed even before your fix as  line 188 was already testing this and the first address is getting selected here. You need to test with multiple addresses where the selectedShippingAddress isn't the first address that would get naturallly selected.

Comment 3

[:prathiksha]

Comment on attachment 8994722 [details]
Bug 1476571 - Auto-select selectedShippingAddress as billing address when it is not null.

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

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8994722 [details]
Bug 1476571 - Auto-select selectedShippingAddress as billing address when it is not null.

https://reviewboard.mozilla.org/r/259222/#review266522

Thanks

::: browser/components/payments/test/mochitest/test_basic_card_form.html:190
(Diff revision 2)
>    checkCCForm(form, {
> -    billingAddressGUID: address1.guid,
> +    billingAddressGUID: address2.guid,
>    });
>  
> +  let select = document.getElementById("billingAddressGUID");
> +  ok(select.value, address2.guid, "Correct billing addresss shown");

I think this addition can be deleted because doesn't the `checkCCForm` handle this like I mentioned in my last review?

Comment 5

[:prathiksha]

Comment on attachment 8994722 [details]
Bug 1476571 - Auto-select selectedShippingAddress as billing address when it is not null.

https://reviewboard.mozilla.org/r/259222/#review266522

> I think this addition can be deleted because doesn't the `checkCCForm` handle this like I mentioned in my last review?

Yeah, looks like it does. Removing it, sorry. :)

Comment 6

[:prathiksha]

Comment on attachment 8994722 [details]
Bug 1476571 - Auto-select selectedShippingAddress as billing address when it is not null.

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c92cbb9c1b4d
Auto-select selectedShippingAddress as billing address when it is not null. r=MattN

Comment 8

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/c92cbb9c1b4d

Comment 9

[Marco Mucci [:MarcoM]]

Hi Matt, can you review and set the QE flag.

Comment 10

[Hani Yacoub, Desktop QA]

Build ID: 20180730221422
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0

Verified as fixed on Firefox Nightly 63.0a1 on Windows 10 x 64, Mac OS X 10.13 and Ubuntu 18.04 x64.