Lost functionality billing information on anonymous checkout

I reimplemented this functionality in the attached patch.

I reused the data-stripe attributes added to shipping address fields (which are currently unused) but modify them slightly, and attach them to the billing address as well. When the form is submitted, I detect the "copy fields" checkbox, pull out the relevant name and address fields, and includes them in the call to stripe.createPaymentMethod(). In manual testing the name and addresses then show up correctly in the Stripe admin UI.

The Stripe docs suggest you can also send billing address in the call to stripe.handleCardSetup() and I see them in the API logs, but this didn't seem to affect the address that is actually displayed in the Stripe UI. This flow only seems to be used if the user goes back from the review page and re-enters their card details, so not sure it is that important.

I think the test failures are unrelated, other patches in this queue seem to have the same fails.

Self-review:

1. +++ b/commerce_stripe.module
   @@ -13,3 +16,10 @@ function commerce_stripe_page_attachments(array &$page) {
   +  $form['shipping_information']['shipping_profile']['#after_build'][] = [PaymentMethodAddForm::class, 'addAddressAttributes'];
   

   This could probably do with an if around it as the pane may not exist.

2. +++ b/src/Plugin/Commerce/CheckoutPane/StripeReview.php
   @@ -145,6 +145,9 @@ class StripeReview extends CheckoutPaneBase {
   +    if (isset($profiles['billing'])) {
   +      $pane_form['#attached']['drupalSettings']['commerceStripe']['billing'] = $profiles['billing']->get('address')->first()->toArray();
   +    }
   

   This isn't needed, this is left over from an earlier attempt where I tried to send the billing address at the review stage, but I got an error back from Stripe when trying this.

Addressed #6. Also fixed an issue where Stripe returns unhelpful error messages if you leave fields blank, so the code no longer sends empty name or address fields to Stripe. Validation is still carried out as normal by Drupal.

Hi Dave @longwave,

I hope you are well.

I just tried this patch and it fails due to the following error:
You passed an empty string for 'billing_details[name]'. We assume empty values are an attempt to unset a parameter; however 'billing_details[name]' cannot be unset. You should remove 'billing_details[name]' from your request or supply a non-empty value.

This is returned from stripe.
tried with RC4 and dev versions of module, same issue with both.

This is on a test account on stripe, not sure if that is what makes the difference.

Regards

Tim

Yeah, that's the error I saw and fixed in #7 if the customer leaves the billing name field empty. The change in #7 fixed this error for me so I'm not sure what could be happening here without access to the site to debug it I'm afraid.

It's possible that if you are using custom checkout panes or have otherwise modified the checkout that the jQuery selectors are wrong and finding empty values somehow, I guess.

Hi

I can give you access to the site at whatever level you wish for debugging I have a dev version on a public server.

I am not sure what changes to the checkout would cause this, can you suggest what sort of changes.

This was the first D8 Commerce site I built more than a year ago and maybe I made a mistake in the checkout flow, here is our current checkout flow.

Regards

Tim

Ah, I have single page checkout with payment information and shipping information on the same page, I guess that's probably the issue as the fields aren't all there on the payment page. If I get time I will have a play with other checkout flows and see if I can fix it.

Moving shipping info to the same page as payment info worked. No error

You are a star.

I'd buy you a drink but no idea when we will ever be in the same pub again.

Have fun

Tim

Hi @longwave just working through this, coming from #3078099: Send order information to Stripe.

For anonymous orders: the patch at #7 successfully gives me the billing name as the customer name for the Stripe payment overview (it does not create an actual Stripe customer, expected), and as the owner on the Stripe payment method. It does not give me an owner email under payment method or a customer email under Risk Insights. Should it?

For authenticated orders: I have the email as the customer name for the Stripe payment overview, the billing name as the owner on the payment method, and the email as the owner email under payment method and customer email under Risk Insights. This is the same as without the patch.

It would be good to clarify whether we are aiming for any consistency between anonymous/authenticated payments. At present it looks difficult for an admin to reconcile, with a mix of email address vs name as the "customer name".

Yeah, this doesn't populate the email address at Stripe, it probably should. This wasn't important for the client I implemented this for, who only uses anonymous checkout. Marking "needs work" to add the email field too.

It probably could also do with extra work to handle the situation in #12 where some or all of the fields might have been entered on a previous checkout pane. I guess we could build a default object server side containing existing details from the order and then overwrite in JavaScript if updated fields are available on the page.

I think if we are going to populate the email for anonymous payments, that needs to be used as the customer name, for consistency with authenticated payments.

It might be good to provide a configurable mapping of fields (though perhaps not a UI for that). My client would also prefer the billing name as customer name, for any payment. Probably a separate issue.

I'll be in a position to help out in a week or so, if we decide Stripe is the way we will go.

According to comment #9 in the parent issue, using data-stripe is not the correct way to go because it does not support Stripe Elements (?) Ignore that...

Here's a simple modification that adds the entered email address to the payment method. It works if the default contact information pane is on the same page, and the user is anonymous (if the user is authenticated, the email address will be obtained from the user account).

Stripe preferably uses the payment method owner email over the payment method owner name on its payment overview screen, so this change also makes the overview behave the same for anonymous payments and customer payments, with the email address shown in the "customer" column.

Work is still needed to persist billing details and email if those were entered on a previous checkout step, but I will not be needing that functionality. A followup issue perhaps? Marking needs review...

Just noticed the administrative_area billing element is still missing, patch to come.

This patch adds the administrative_area billing element. I don't think the test failures are us.

Nice work :)

+++ b/js/commerce_stripe.form.js
@@ -98,15 +98,48 @@
+          details.email = $('.checkout-pane-contact-information [type=email]', $form).first().val();

I think you might want a defensive if around this to only set it if there is a value like we do for details.name, the Stripe API returned errors for me when I sent empty strings in other fields and I assume it might do the same here.

Also I wonder if it would be neater to just set a data-stripe attribute on this field server side and then pick up the client side field the same way we do the other fields, this means the JS is more flexible if users do server-side custom panes or form alters.

Agree that the test failures are unrelated.

I've added the defensive "if" you suggested @longwave and also added a backup email collection selector as in my situation the contact pane is not present.

Thanks Dan. I have a feeling that backup email address collection could possibly be problematic in some situations.

If there isn't appropriate data present, we should probably assume it was collected in a previous checkout step and try to get it from the existing order server-side and populate a js object at pane build, as mentioned in #16. Any implementation that removes the default contact info pane in favor of something else should be ensuring the order owner is correctly populated before payment information is collected.

The difficulty here is that the stripe form js runs on form submit before any Drupal checkout pane handlers get to run, and tries to create a payment method and customer immediately, which effectively turns this all into a nasty architectural arms race.

There is a patch to add a Symfony event server-side after js form submit, before the payment method and customer get created at #3191993: Add event to allow customising payment method and customer creation, but I wonder if perhaps we might also trigger a jQuery event on form submit before any processing, to allow client-side overrides to get in early (and cleanly) to do exactly the sort of thing your patch does, specific to your own implementation?

I think I've worked around the collection issue in the attached patch, by adding a data attribute to the email address in the same way we add one to the address fields. This data attribute can then also be attached to a different email field (even a hidden one if required).

Before I apply this patch can someone confirm it enables the Order Number to be passed to Stripe so it appears in the Description there?
Thanks,
Martin

No, this sends the billing email, name and address to Stripe, but not the order number.

Thanks @longwave. If anyone knows how to pass order number across to Stripe, I'd appreciate a pointer to the right source.

The order ID (not number, though) is sent across in the Stripe metadata. I suggest opening a new issue for your request as it's likely that can be solved separately from this issue, which is more about improving Stripe's fraud protection.

The patch at #27 won't work to pick up email fields with a data-stripe- attribute, because they won't be in the payment-information checkout pane.

I'm thinking it might be better to remove that checkout pane restriction entirely and allow the stripeBillingDetails() method to identify and use any field anywhere in the form that has an appropriate data-stripe- attribute set? That would allow modules providing custom panes to opt-in.

Edit: we need to distinguish between payment information and shipping information, so that won't be appropriate. I think getting the email value just needs to be moved outside that jQuery parent selector. Patch to come.

This patch gets billing information from the payment or shipping pane (same as #27), then if no email was found, looks for the first email field in any checkout pane that has the appropriate data-stripe attribute.

Ah yeah, I forgot that on our site we have modified the payment pane to include the email address field.

path #33 dont work for me. i alway get a empty address in stripe with anonymous customer.
any walkaround?

Where are you looking at Stripe? The address is on the payment, not the customer.

im looking there

https://i.ibb.co/tqrPwLZ/Capture-d-e-cran-le-2021-04-08-a-10-12-07.png

Patch no longer applies to latest dev.

Re-roll, no changes. Note this re-adds the commerce_stripe.module file, removed by another commit.

I'm getting what I expect for anonymous payments. Note however my requirements only collect region data, not a full address:

I rerolled the patch in 40 to apply to the latest dev branch.

When a user checks out anonymously, customer information is now showing in Stripe with this patch.

"Billing same as shipping checkbox" functionality extended with
"4) Work with the tax_number field (which is usually hidden on shipping and shown on billing)" functionality from https://www.drupal.org/project/commerce_shipping/issues/2852207
AND
Commerce Stripe with this patch has a conflict, doesn't work for me.
Will try to work on it later, just letting you know the experience.

Patch needs a reroll for current dev now the commerce_stripe.module file is already present. On it.

Patch against current dev.

This looks ok in principle, but I'd like to see test coverage for the shipping address case.

The changes to addAddressAttributes() feel familiar from some other bug.

Since the patch has been sitting in the queue for a while and seems to fix an important issue, decided to go ahead and commit it.

Thanks for your contribution @John Pitcairn.