search

w3c / payment-request

Payment Request API
https://www.w3.org/TR/payment-request/
Other
488 stars 135 forks source link

Support requesting billing address #749

Closed marcoscaceres closed 5 years ago

marcoscaceres commented 6 years ago

Part 1 for #27 (part 2 is #750)

The following tasks have been completed:

Implementation commitment:

Impact on Payment Handler spec?

Preview | Diff

domenic commented 6 years ago

Hmm, substantive concerns at https://github.com/w3c/payment-request/issues/27#issuecomment-403614316; I'd appreciate your (or others') thoughts there before merging this.

marcoscaceres commented 6 years ago

Gecko bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1477117

marcoscaceres commented 6 years ago

Closing, due to lack of consensus.

marcoscaceres commented 6 years ago

@adrianhopebailie, @ianbjacobs, @rsolomakhin:

Ok, so given feedback in #27 and teleconferences we've had. How about I do:

Agree?

rsolomakhin commented 6 years ago

This looks good to me as is.

marcoscaceres commented 6 years ago

@rsolomakhin, thanks for the feedback. The issue that come up previously is that billing address is always bound to an instrument, so it might make more sense to just attach the billing address to the "paymentmethodchange" event.

Basically, the model that is currently specified stays the same... the deck chairs just get rearranged a bit.

marcoscaceres commented 6 years ago

Ok, now matches https://github.com/w3c/payment-request/pull/749#issuecomment-412771286 and I've also updated w3c/payment-method-basic-card#53

rsolomakhin commented 6 years ago

I do prefer a paymentmethodchange event that includes a redacting billing address if necessary. @marcoscaceres :+1:

marcoscaceres commented 6 years ago

@domenic, the updated PR is basically a rewrite (smaller tho:)). Do you have cycles to re-review it?

@rsolomakhin, do you want to give it a good look over and see if the text matches how you would implement this?

ianbjacobs commented 6 years ago

@domenic,

billingAddress should be part of the payment method data. The thing is that in some cases, the Total depends on the billingAddress (e.g., VAT computation). The merchant wants to know about the billingAddress while the sheet is open to display a correct total.

I believe the solution of this pull request will work for those payment methods build into the browser (e.g., basic card). However it will not work for some payment methods supported by payment handlers (e.g., for push payments or real-time credit financing) until we do something else. I'm synthesizing what I'm hearing here: https://github.com/w3c/payment-request/wiki/PushPayments

Ian

marcoscaceres commented 6 years ago

@domenic wrote:

I guess my biggest substantive question is why we're adding requestBillingAddress to PaymentOptions when it sounds like we're keeping billing address as a payment method-specific thing. (E.g., it is not returned in any property of PaymentResponse.) If that's the case, shouldn't whether or not to request the billing address be part of the payment method data?

Yes, you are absolutely right! It should be part of the method data. I'll fix that.

@ianbjacobs, wrote:

I believe the solution of this pull request will work for those payment methods build into the browser (e.g., basic card). However it will not work for some payment methods supported by payment handlers (e.g., for push payments or real-time credit financing) until we do something else.

I really need your help understanding what "something else" could be. I've read over everything provided thus far, and tried to make sure the updated PR addresses the push payment use case (as I understand it). If it doesn't address the use case, then I want to fix that - but I need your help showing me that what I've proposed here is actually unable to address the use case.

Ian, would it be best to have a call about it? We've successfully resolved issues like this before when we've chatted (or you've successfully explained to me how wrong I am 😅). We only get one real shot at this, so we gotta make sure we get it right.

rsolomakhin commented 6 years ago

@marcoscaceres : I think @ianbjacobs wants to make changes to Payment Handler, but this PR should work.

@ianbjacobs : Did I understand you correctly? Please add me to the call, if you have one at a convenient time.

ianbjacobs commented 6 years ago

After chatting with @marcoscaceres and @rsolomakhin we would like clarification on one point: if a merchant needs a billing address (e.g., for VAT computation), will that merchant need the billing address for EVERY payment method the merchant would accept?

The answer to this question affects the design, roughly as follows:

As an example, would a merchant ever need billing address for a basic card payment, and not need a billing address for payment by credit transfer, or cryptocurrency, or real-time financing, etc.?

@lyverovski, @Krystosterone, @michelle-stripe, @asolove-stripe, @mattsaxon, @vkuntz, @frank-hoffmann we welcome your input here.

asolove-stripe commented 6 years ago

I think there are probably some cases where you only need billing address for one method. I could imagine wanting it for card so you do AVS on a card, but don't care about it for other payment methods.

That said, the main use-case here is still where the billing address affects tax or other calculations controlling the total. For that use-case, it's really the same across all payment methods.

stpeter commented 6 years ago

Thanks @asolove-stripe! As @adrianhopebailie mentioned on a call right now, if billing address is needed for VAT then it would be needed no matter what the payment method might be. I still worry a bit about non-VAT use cases where a user might be asked for a billing address when that's unexpected for their chosen payment method, but maybe we don't have a good handle on those yet.

marcoscaceres commented 6 years ago

Ok, we can go back to having it as a "global" requestBillingAddress on PaymentOptions.

In the future, if we need to, we can go more fine grained by adding requestBillingAddress on specific PaymentMethodData's if the case arrises.

stpeter commented 6 years ago

This resolution is consistent with discussion on the WPWG call today, too: https://www.w3.org/2018/08/23-wpwg-minutes#item04

marcoscaceres commented 6 years ago

Ok, awesome! Thanks for getting wider opinions and review from the WG. Will make these changes hopefully today.

marcoscaceres commented 6 years ago

Ok, updated based on recent discussions. This is how it works in theory (it's a bit ugly, because paymentMethodErrors 🤷🏽‍♂️):

const options = {
  requestBillingAddress: true,
};
const request = new PaymentRequest(methods, details, options);

request.onpaymentmethodchange = ev => {
  // get billing address
  const { billingAddress } = ev.methodDetails;
  // get new total
  const updatedTotalPromise = asynCalcNewTotal(billingAddress);
  ev.updateWith(updatedTotalPromise);
};

const response = await request.show();
await validAddresses(response);
await response.complete("success");

async function validAddresses(response) {
  const {
    shippingAddress,
    details: { billingAddress },
  } = response;
  const [shipping, billingErrors] = await Promise.all([
    validateAddress(shippingAddress),
    validateAddress(billingAddress),
  ]);
  const billing = billingErrors
    ? { paymentMethodErrors: { billing: billingErrors } }
    : undefined;
  const errors = { shipping, billing };
  if (Object.getOwnPropertyNames(errors).length) {
    await response.retry(errors);
  }
  return validAddresses(response);
}
marcoscaceres commented 6 years ago

Again, we do have the opportunity to move billingAddress to PaymentResponse, and we deprecate "basic-card"'s .billingAddress. That would make things a bit cleaner from a developer perspective.

marcoscaceres commented 6 years ago

@domenic, @rsolomakhin, opinions on?: https://github.com/w3c/payment-request/pull/749#issuecomment-415663572

marcoscaceres commented 6 years ago

fixed a couple of typos above

rsolomakhin commented 6 years ago

Thank you, @marcoscaceres!

Re: code sample https://github.com/w3c/payment-request/pull/749#issuecomment-415663572 -- Ideally the merchant would also validate the shipping address in shippingaddresschange event and validate the billing address in paymentmethodchange event, which happens before the full response is received, so the chance of using .retry() would be reduced. (By the way, is validAddresses calling itself recursively without a termination condition?)

Re: moving billingAddress to PaymentResponse and/or PaymentRequest, similar to shippingAddress https://github.com/w3c/payment-request/pull/749#issuecomment-415664294 -- This would imply that the user agent needs to read the billingAddress field from 3rd party payment handler response as well, which is OK, but adds some complexity to keep in mind.

adrianhopebailie commented 6 years ago

Again, we do have the opportunity to move billingAddress to PaymentResponse, and we deprecate "basic-card"'s .billingAddress. That would make things a bit cleaner from a developer perspective.

Yes please!

@rsolomakhin wouldn't it be cleaner to have a billingaddresschange event (if we do what @marcoscaceres suggests above)? Seems much less surprising for a developer than different mechanisms for dealing with (to a developer) very similar data elements.

I can imagine implementations then deciding if a payment method change automatically changes the selected billing address or not, and if it does, firing both events. In that case, how would (could) the browser handle two updateWith calls?

rsolomakhin commented 6 years ago

@adrianhopebailie: It seems that a billing address is always tied to a payment instrument, so decoupling the two events would result in paymentmethodchange event always being followed by billingaddresschange event, which feels unnecessary. If A and B always happen at the same time, then A==B is my thinking.

adrianhopebailie commented 6 years ago

@rsolomakhin I think you are right but I'm not sure if that is ALWAYS true.

What if I pick a payment handler that has multiple underlying instruments? If we de-couple this we open up the possibility of allowing payment handlers to explicitly fire this event.

e.g. I pick BobPay which has two different instruments internally.

If the payment handler never calls 'billingAddressChanged()' then the browser can still fire the billingaddresschanged event when the payment handler returns (but before passing the response to the website). In this case, if the website returns with a new total the browser knows that the payment needs to be retried with the payment handler (some design work to figure out how that would be dealt with).

We could consider s similar mechanism for shipping address

marcoscaceres commented 6 years ago

Browser fires paymentmethodchanged event to signal the selection of BobPay.

Currently, "paymentmethodchanged" only fires when the user picks an instrument, not when they pick a different or new handler.

Sidebar: Does this event still need an updateWith() if it doesn't signal address changes?

I would say, yes, for consistency - but only in as far as the event might contain useful information that the merchant would modify the total on (or modify some other aspect of the request on).

marcoscaceres commented 6 years ago

Re: code sample #749 (comment) -- Ideally the merchant would also validate the shipping address in shippingaddresschange event and validate the billing address in paymentmethodchange event, which happens before the full response is received, so the chance of using .retry() would be reduced.

Agree.

(By the way, is validAddresses calling itself recursively without a termination condition?)

oops! will fix.

Re: moving billingAddress to PaymentResponse and/or PaymentRequest, similar to shippingAddress #749 (comment) -- This would imply that the user agent needs to read the billingAddress field from 3rd party payment handler response as well, which is OK, but adds some complexity to keep in mind.

I'll just leave it as is.

rsolomakhin commented 6 years ago

I've hooked this up into 3rd party payment handlers via https://github.com/w3c/payment-handler/pull/318.

marcoscaceres commented 5 years ago

@sonakshisaxena1 has graciously offered to assist us with the testing. I'll work with her on it.

marcoscaceres commented 5 years ago

@rsolomakhin @aestes, @zouhir just wanted to confirm if you are also planning to support this (requestBillingAddress)?

Firefox's plan is always return null for billing address unless this is set to true.

rsolomakhin commented 5 years ago

Yes, we plan to support this in Chrome, but the exact behavior needs to be thought through and fine-tuned on our end. The Firefox decision seems reasonable is in the realm of possibilities for us.

snez commented 5 years ago

I am currently using Stripe's javascript API with the paymentRequestButton, and I have found the billing address to be available in the returned json object when the customer selects a card. I.e. the billing address registered with the specific card comes back as part of the card details json object. Not sure if the underlying implementation is something that Stripe has done, or if it is part of the paymentRequest API, but it solves the problem in my case.

marcoscaceres commented 5 years ago

Hi @snez, you wrote:

Not sure if the underlying implementation is something that Stripe has done, or if it is part of the paymentRequest API, but it solves the problem in my case.

If I may ask, could you help us understand what you are using the billing address for?

marcoscaceres commented 5 years ago

Put up a test for review: https://github.com/web-platform-tests/wpt/pull/14255

marcoscaceres commented 5 years ago

Just a quick update: Firefox patches for requestBillingAddress should be landing hopefully tomorrow.

marcoscaceres commented 5 years ago

File chrome bug, based on @rsolomakhin comment https://github.com/w3c/payment-request/pull/749#issuecomment-442093344

snez commented 5 years ago

Hi @snez, you wrote:

Not sure if the underlying implementation is something that Stripe has done, or if it is part of the paymentRequest API, but it solves the problem in my case.

If I may ask, could you help us understand what you are using the billing address for?

The billing address is a minimum requirement for many large eCommerce systems in order to place an order (in my case Magento). I wouldn't be able to programmatically create an order without it, and thus the whole idea of accepting payments from the product page could not be implemented (see https://magento2.cryozonic.com/radiant-tee.html for an example), which is a perfect use for the paymentRequest API, and something that many merchants are trying to implement to improve their conversions.

marcoscaceres commented 5 years ago

Thanks, @snez, for the example and details. That’s super helpful.