Show the billing address page on on-boarding when requestShipping is false and there are no saved addresses

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P1
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: prathiksha, Assigned: prathiksha)

Tracking

(Blocks 1 bug)

unspecified
Firefox 62
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox62 fixed)

Details

(Whiteboard: [webpayments])

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

Last year
When requestShipping is false and there are no saved addresses, we show the add_billing_address page before the basic card page on on-boarding. This way we can avoid having an empty address picker when the user enters the basic card page.
Assignee

Updated

Last year
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Priority: -- → P1
Whiteboard: [webpayments]
Btw. bug 1462779 is adding:
> <!ENTITY billingAddress.addPage.title   "Add Billing Address">
> <!ENTITY billingAddress.editPage.title  "Edit Billing Address">

so depending upon which lands first this can be shared.

Comment 3

Last year
mozreview-review
Comment on attachment 8979412 [details]
Bug 1462779 - Show the billing address page during on-boarding if requestShipping is false and there are no saved addresses.

https://reviewboard.mozilla.org/r/245586/#review251626

::: browser/components/payments/res/paymentRequest.js:141
(Diff revision 1)
> +      } else {
> +        state.page.title = paymentDialog.dataset.billingAddressTitleAdd;

I think you forgot to take into account whether there are cards or not… we shouldn't show the billing address form if the user has a saved card.
Attachment #8979412 - Flags: review?(MattN+bmo)
Comment hidden (mozreview-request)

Comment 5

Last year
mozreview-review
Comment on attachment 8979412 [details]
Bug 1462779 - Show the billing address page during on-boarding if requestShipping is false and there are no saved addresses.

https://reviewboard.mozilla.org/r/245586/#review251986

::: browser/components/payments/res/paymentRequest.js:119
(Diff revision 2)
> +    let noSavedAddresses = Object.keys(detail.savedAddresses).length == 0;
> +    let noSavedCards = Object.keys(detail.savedBasicCards).length == 0;

I think it would be easier to read if we avoid double-negatives by making these variables indicate that there is saved data.

::: browser/components/payments/res/paymentRequest.js
(Diff revision 2)
> -      page: {
> -        id: "payment-summary",
> -      },
>      };
>  
>      // Onboarding wizard flow.
> -    if (Object.keys(detail.savedAddresses).length == 0) {
> +    if ((!noSavedCards && !noSavedAddresses) || (!shippingRequested && !noSavedCards)) {
> +      state.page = {
> +        id: "payment-summary",

I think it was easier to understand if we leave the default page as payment-summary but override for the FTU in specific cases. e.g. (not necessarily accurate)
```js
if (!hasSavedAddress && (shippingRequested || !hasSavedCards) {
  // address-page
} else if (!hasSavedCards) {
  // basic-card-page
}
```
Attachment #8979412 - Flags: review?(MattN+bmo) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

Last year
Attachment #8979412 - Attachment is obsolete: true

Comment 8

Last year
mozreview-review
Comment on attachment 8980467 [details]
Bug 1462779 - Auto-select appropriate addresses in select dropdowns in on-boarding forms.

https://reviewboard.mozilla.org/r/246630/#review252756

Hello, I don't have time to manually test this yet but I have some comments for now.

::: commit-message-3dc6c:1
(Diff revision 1)
> +Bug 1462779 - Auto-select appropriate addresses in menulists on on-boarding forms. r?MattN

Nit: we don't call them menulists usually

::: browser/components/payments/res/containers/basic-card-form.js:162
(Diff revision 1)
> -      let addressGuid = basicCardPage.billingAddressGUID;
> -      let billingAddressSelect = this.form.querySelector("#billingAddressGUID");
> +    let billingAddressSelect = this.form.querySelector("#billingAddressGUID");
> -      billingAddressSelect.value = addressGuid;
> +    if (basicCardPage.billingAddressGUID) {
> +      billingAddressSelect.value = basicCardPage.billingAddressGUID;
> +    } else {
> +      billingAddressSelect.value = Object.keys(addresses)[0];

I think we shouldn't auto-select an address when editing as the user may not know that clicking back would mean that the billing address would not be associated with the card.

::: browser/components/payments/res/paymentRequest.js:148
(Diff revision 1)
> +        Object.assign(state.page, {selectedStateKey: ["selectedShippingAddress"]});
>        } else {
>          Object.assign(state["address-page"], {
>            title: paymentDialog.dataset.billingAddressTitleAdd,
>          });
> +        Object.assign(state.page, {selectedStateKey: ["basic-card-page", "billingAddressGUID"]});

Nit: These two places don't need Object.assign and would be easier to read as follows:
```js
state.page.selectedStateKey = ["…"];
```
Attachment #8980467 - Flags: review?(MattN+bmo)
Assignee

Updated

Last year
Attachment #8979412 - Attachment is obsolete: false
Attachment #8979412 - Attachment is patch: true
Attachment #8979412 - Attachment mime type: text/x-review-board-request → text/plain
Comment hidden (mozreview-request)
Comment on attachment 8980467 [details]
Bug 1462779 - Auto-select appropriate addresses in select dropdowns in on-boarding forms.

https://reviewboard.mozilla.org/r/246630/#review253728

Please rebase the two commits for this bug onto latest mozilla-central since there are some conflicts with Jared's changes. Also, make sure when you push for review that you push both commits using `-r` (`-c` is only for one commit). e.g. `hg push review central -r 84567af75:985fed43`
Comment on attachment 8980467 [details]
Bug 1462779 - Auto-select appropriate addresses in select dropdowns in on-boarding forms.

https://reviewboard.mozilla.org/r/246630/#review253738

::: browser/components/payments/res/paymentRequest.js:153
(Diff revision 2)
> +        state.page.selectedStateKey = ["selectedShippingAddress"];
>        } else {
>          Object.assign(state["address-page"], {
>            title: paymentDialog.dataset.billingAddressTitleAdd,
>          });
> +        state.page.selectedStateKey = ["basic-card-page", "billingAddressGUID"];

Can you please add a test for this behaviour since I think it would be easy to regress the behaviour of the selected state key getting carried over to the next (card) page since it's in different code than what actually sets decides which page to show.
Attachment #8980467 - Flags: review?(MattN+bmo)
Assignee

Updated

Last year
Attachment #8979412 - Attachment is obsolete: true
Attachment #8979412 - Attachment is patch: false
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8981618 [details]
Bug 1462779 - Show the billing address page during on-boarding if requestShipping is false and there are no saved addresses.

https://reviewboard.mozilla.org/r/247730/#review253844

Thanks

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:45
(Diff revision 2)
>                 state["address-page"].selectedStateKey == "selectedShippingAddress";
>        }, "Address page is shown first during on-boarding if there are no saved addresses");
>  
> +      info("Checking if the address page has been rendered");
> +      let addressSaveButton = content.document.querySelector("address-form .save-button");
> +      ok(content.isVisible(addressSaveButton), "Address page is rendered");

The old message was better: "Address save button is rendered"
Attachment #8981618 - Flags: review?(MattN+bmo) → review+
Comment on attachment 8980467 [details]
Bug 1462779 - Auto-select appropriate addresses in select dropdowns in on-boarding forms.

https://reviewboard.mozilla.org/r/246630/#review253846

::: browser/components/payments/test/browser/browser_payments_onboarding_wizard.js:342
(Diff revision 3)
>        ok(content.isVisible(cardSaveButton), "Basic card page is rendered");
>  
>        for (let [key, val] of Object.entries(PTU.BasicCards.JohnDoe)) {

What I mean to test is here that the billing address is selected by default in the dropdown.
Attachment #8980467 - Flags: review?(MattN+bmo) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 19

Last year
Pushed by prathikshaprasadsuman@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/def9442b2a81
Show the billing address page during on-boarding if requestShipping is false and there are no saved addresses. r=MattN
https://hg.mozilla.org/integration/autoland/rev/df80a62cc91f
Auto-select appropriate addresses in select dropdowns in on-boarding forms. r=MattN

Comment 20

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/def9442b2a81
https://hg.mozilla.org/mozilla-central/rev/df80a62cc91f
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
You need to log in before you can comment on or make changes to this bug.