Skip to content

Conversation

wjrosa
Copy link
Contributor

@wjrosa wjrosa commented Sep 11, 2025

Inspired by #4658 (comment)

Changes proposed in this Pull Request:

In this PR, I am simplifying the logic we have today to compare new payment method tokens with existing ones. I am also fixing comparisons for the CashApp Pay tokens and the value it sets for the gateway ID.

The CashApp Pay token comparison was checking the type property against itself. The Gateway ID parameter was being set with the wrong values.

Testing instructions

  • Code review
  • Check if the tests are still passing
  • Perform some basic smoke-testing for CashApp Pay tokens:
    -- Confirm you can complete a purchase and save it for later use
    -- Add it as a saved payment method using the My Account screen

  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Changelog entry

  • This Pull Request does not require a changelog entry. (Comment required below)
Changelog Entry Comment

Comment

Post merge

@wjrosa wjrosa self-assigned this Sep 15, 2025
@wjrosa wjrosa marked this pull request as ready for review September 15, 2025 17:46
@wjrosa wjrosa changed the title Simplifying payment tokens comparison Fixing and simplifying payment tokens comparison Sep 15, 2025
$gateway_id = $this->is_oc_enabled() ? 'stripe' : $this->id;

$token->set_gateway_id( $gateway_id );
$token->set_gateway_id( WC_Stripe_Payment_Tokens::UPE_REUSABLE_GATEWAYS_BY_PAYMENT_METHOD[ self::STRIPE_ID ] );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the first fix here.

if ( WC_Stripe_Payment_Methods::CASHAPP_PAY === $this->get_type()
&& ( $payment_method->cashapp->cashtag ?? null ) === $this->get_cashtag() ) {
return true;
if ( WC_Stripe_Payment_Methods::CASHAPP_PAY !== $payment_method->type ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And that's the second fix.

@wjrosa wjrosa requested review from a team, annemirasol and Mayisha and removed request for a team September 15, 2025 17:58
Copy link
Contributor

@daledupreez daledupreez left a comment

Choose a reason for hiding this comment

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

Two quick questions from me from inspection. Otherwise generally looking good, but I'll let the assigned reviewers test.

@wjrosa wjrosa requested a review from daledupreez September 16, 2025 14:54
Copy link
Contributor

@annemirasol annemirasol left a comment

Choose a reason for hiding this comment

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

LGTM, I think the early returns are more readable!

Smoke tested saving the following payment methods:

  • CashApp
  • Klarna
  • credit card
  • credit card 3DS
  • Link

Smoke tested using the saved CashApp payment method during checkout.

@Mayisha Mayisha removed their request for review September 16, 2025 18:19
Copy link
Contributor

@daledupreez daledupreez left a comment

Choose a reason for hiding this comment

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

Changes look good to me, and testing CashApp flows worked well for me.

@wjrosa wjrosa merged commit 67c924b into develop Sep 17, 2025
9 of 10 checks passed
@wjrosa wjrosa deleted the dev/simplifying-payment-token-comparison branch September 17, 2025 12:15
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.

3 participants