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

ECE - Set the order payment method title to the wallet used to process payments #3482

Merged
merged 16 commits into from
Oct 17, 2024

Conversation

james-allan
Copy link
Contributor

@james-allan james-allan commented Sep 30, 2024

Fixes #3469

Changes proposed in this Pull Request:

This PR makes the following changes:

  1. Removes code added in a previous ECE PR (Add support for ECE Payment buttons on Pay for Order page #3440) which set the payment method title when processing payments via the Pay for Order payment. This is no longer necessary because we're setting the payment method title in a more general way that will work for all pages.
  2. Stripe payment method objects record the type of express payment wallet is used to process the payment (see example below). When processing a payment with wallet information, set the payment method title to the corresponding wallet type.
Stripe Payment Method object

stdClass Object
(
    [id] => pm_1Q4c4tEYkV5SNaUVCGyhKgeg
    [object] => payment_method
    [billing_details] => stdClass Object
        (
            [address] => stdClass Object
                (
                   ...
                )

            [email] => ...
            [name] => Card Holder Name
            [phone] => +1 650-555-5555
        )

    [card] => stdClass Object
        (
            [brand] => visa
            ...

            [wallet] => stdClass Object
                (
                    [dynamic_last4] => 1111
                    [google_pay] => stdClass Object
                        (
                        )

                    [type] => google_pay <------ SEE HERE
                )

        )
     ...

    [type] => card
)

Testing instructions

  1. Set the ECE feature flag option (_wcstripe_feature_ece) to 'yes'.
  2. Enable Express payment methods Google and Apple Pay.
  3. Complete a purchase using Google and/or Apple Pay
  4. Confirm on the order received page that the order's payment method title is set to Google or Apple Pay.
Screenshot 2024-09-30 at 3 35 37 pm

While working on this, I've also confirmed that standard card payments still set the title to the default Credit / Debit Card and if the title is customised, it is still set for standard card payments.


  • 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

@james-allan james-allan changed the title ECE - Set the order payment method title ECE - Set the order payment method title to the wallet used to process payments Sep 30, 2024
Copy link
Contributor

@Mayisha Mayisha left a comment

Choose a reason for hiding this comment

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

Nice fix James 🎉

Payment method title is correctly set as Google Pay (Stripe) when paid via ECE button. I have tested with Google Pay.

  • Shortcode cart page ✅
  • Shortcode checkout page ✅
  • Block cart page ✅
  • Block checkout page ✅
  • Pay for order page ✅

Additionally I have tested with card, AliPay and CashApp. Payment method title is correctly set for them as well.

Base automatically changed from fix/3480-set-custom-payment-method-titles to develop October 2, 2024 02:04
@james-allan
Copy link
Contributor Author

Thanks for the review @Mayisha. I'm going to reopen this for review because I've made a substantial change to this PR.

The WC_Stripe_UPE_Payment_Method_CC::get_title() function previously supported passing $payment_details that could be used to alter the resulting title. If for example it was passed payment method details that indicated that the payment method was a Mastercard Credit Card the resulting order payment method would be Mastercard credit card rather than the default title.

Doing a search of the code base for ->get_title(... I wasn't able to find any instances where we were passing this addition information other than the change in this PR and in unit tests, so its safe (imo) to assume this feature was not actually being used.

That feature of customising the title to be Mastercard credit card also breaks with the customisable title that merchants can set via the settings. ie if a merchant changes the default title of the credit card payment method, then the way this code worked, it would override that to "Visa debit card".

@james-allan james-allan requested a review from Mayisha October 2, 2024 03:51
Copy link
Contributor

@Mayisha Mayisha left a comment

Choose a reason for hiding this comment

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

The new changes look good and things are working perfectly.
I also had a quick search and did not find any instances where the additional information was being sent to the get_title function. 👍

@james-allan james-allan merged commit 3a1da1f into develop Oct 17, 2024
34 of 35 checks passed
@james-allan james-allan deleted the fix/ece-3469-set-payment-method-title branch October 17, 2024 00:08
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.

[ECE] Fix payment method title for orders paid with ECE button
2 participants