Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blocks API: get billing details from order if not in request #3778

Merged
merged 4 commits into from
Jan 29, 2025

Conversation

annemirasol
Copy link
Contributor

@annemirasol annemirasol commented Jan 27, 2025

Fixes #3777

Changes proposed in this Pull Request:

With express checkout's shift to Blocks API for processing the order, the billing details required for Stripe customer creation logic is no longer present in the request (POST) data.

This PR adds logic to retrieve the billing data from the order object, if it is available.

Testing instructions

  1. As a guest shopper, make any purchase using an express payment method, e.g. Google Pay.
    • In develop, you will see a Undefined index: name PHP notice in your logs.
    • In this branch, the PHP notice should be gone.
  2. Go to the Stripe customers dashboard (https://dashboard.stripe.com/test/customers):
    • In develop, the customer's name is Name: , Guest (missing name) and the billing details are also missing.
    • In this branch, the customer's name is present, e.g. Card Holder Name if using Google Pay test account. Verify inside the customer's page that their billing details are complete.
  3. Make another purchase using the same express payment method and card.
    • In develop, another nameless customer with no billing details gets created in Stripe.
    • In this branch, the existing Stripe customer gets retrieved and reused, as expected.
  4. Regression check: as a guest shopper, purchase an item using a credit card, a different name and email address.
    • The Stripe customer should be created with complete billing details, and reused whenever possible.

Note: I wanted to add some unit tests, but the testable methods involve calls to Stripe API. I'm somewhat hesitant to add code to enable us to inject an API library, but LMK if you feel strongly one way or another.


  • Covered with tests (or have a good reason not to test in description ☝️)
  • Added changelog entry in both changelog.txt and readme.txt (or does not apply)
  • Tested on mobile (or does not apply)

Post merge

@annemirasol annemirasol self-assigned this Jan 27, 2025
@annemirasol annemirasol marked this pull request as ready for review January 27, 2025 21:37
@annemirasol annemirasol force-pushed the fix/3777-blocks-api-missing-billing-data branch from 3e8b832 to 7d349a4 Compare January 27, 2025 21:41
@annemirasol annemirasol requested a review from wjrosa January 27, 2025 21:45
Copy link
Contributor

@wjrosa wjrosa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Code is good and works as expected 👍

One thing I noticed: all ECE purchases now set the order status to "pending payment", which allows me to use the Pay for Order page. Is this expected? Maybe due the previous PR?

@annemirasol
Copy link
Contributor Author

One thing I noticed: all ECE purchases now set the order status to "pending payment", which allows me to use the Pay for Order page. Is this expected? Maybe due the previous PR?

@wjrosa Does this happen to you even for the regular payment flow? In my testing, it happens only when on the confirmation tokens flow.

@wjrosa
Copy link
Contributor

wjrosa commented Jan 28, 2025

One thing I noticed: all ECE purchases now set the order status to "pending payment", which allows me to use the Pay for Order page. Is this expected? Maybe due the previous PR?

@wjrosa Does this happen to you even for the regular payment flow? In my testing, it happens only when on the confirmation tokens flow.

Hum, maybe I forgot to build the assets after reverting the change needed for testing 🤔 . Sorry about that! It is working fine now

@annemirasol annemirasol merged commit d22f038 into develop Jan 29, 2025
37 checks passed
@annemirasol annemirasol deleted the fix/3777-blocks-api-missing-billing-data branch January 29, 2025 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PHP Notice when using express checkout as a guest
2 participants