-
Notifications
You must be signed in to change notification settings - Fork 216
Refactor WC_Stripe_Subscriptions_Trait->maybe_render_subscription_payment_method() #4568
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
base: develop
Are you sure you want to change the base?
Changes from 16 commits
91d638b
35837db
9cfccaf
68bee81
c622d76
2e20c28
5dc3c1e
80bfbbf
0ddecfb
58e88e5
e87b5c9
7511081
9af152c
83cd681
5aabff5
b0f5b3a
3a2a111
9e6ebdd
2920871
3aecb6c
8d2077e
7522668
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -977,7 +977,6 @@ public function maybe_render_subscription_payment_method( $payment_method_to_dis | |||||||||||||||||||||||
$subscription->save(); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
$stripe_customer = new WC_Stripe_Customer(); | ||||||||||||||||||||||||
$stripe_customer_id = $subscription->get_meta( '_stripe_customer_id', true ); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// If we couldn't find a Stripe customer linked to the subscription, fallback to the user meta data. | ||||||||||||||||||||||||
|
@@ -1011,82 +1010,124 @@ public function maybe_render_subscription_payment_method( $payment_method_to_dis | |||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
$stripe_customer->set_id( $stripe_customer_id ); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
$payment_method_to_display = __( 'N/A', 'woocommerce-gateway-stripe' ); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
try { | ||||||||||||||||||||||||
// Retrieve all possible payment methods for subscriptions. | ||||||||||||||||||||||||
foreach ( WC_Stripe_Customer::STRIPE_PAYMENT_METHODS as $payment_method_type ) { | ||||||||||||||||||||||||
foreach ( $stripe_customer->get_payment_methods( $payment_method_type ) as $source ) { | ||||||||||||||||||||||||
if ( $source->id !== $stripe_source_id ) { | ||||||||||||||||||||||||
continue; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Legacy handling for Stripe Card objects. ref: https://docs.stripe.com/api/cards/object | ||||||||||||||||||||||||
if ( isset( $source->object ) && WC_Stripe_Payment_Methods::CARD === $source->object ) { | ||||||||||||||||||||||||
/* translators: 1) card brand 2) last 4 digits */ | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( __( 'Via %1$s card ending in %2$s', 'woocommerce-gateway-stripe' ), ( isset( $source->brand ) ? wc_get_credit_card_type_label( $source->brand ) : __( 'N/A', 'woocommerce-gateway-stripe' ) ), $source->last4 ); | ||||||||||||||||||||||||
break 2; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
$saved_payment_method = $this->get_subscription_payment_method_details( $stripe_customer_id, $stripe_source_id ); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
switch ( $source->type ) { | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::CARD: | ||||||||||||||||||||||||
/* translators: 1) card brand 2) last 4 digits */ | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( __( 'Via %1$s card ending in %2$s', 'woocommerce-gateway-stripe' ), ( isset( $source->card->brand ) ? wc_get_credit_card_type_label( $source->card->brand ) : __( 'N/A', 'woocommerce-gateway-stripe' ) ), $source->card->last4 ); | ||||||||||||||||||||||||
break 3; | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::SEPA_DEBIT: | ||||||||||||||||||||||||
/* translators: 1) last 4 digits of SEPA Direct Debit */ | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( __( 'Via SEPA Direct Debit ending in %1$s', 'woocommerce-gateway-stripe' ), $source->sepa_debit->last4 ); | ||||||||||||||||||||||||
break 3; | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::CASHAPP_PAY: | ||||||||||||||||||||||||
/* translators: 1) Cash App Cashtag */ | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( __( 'Via Cash App Pay (%1$s)', 'woocommerce-gateway-stripe' ), $source->cashapp->cashtag ); | ||||||||||||||||||||||||
break 3; | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::LINK: | ||||||||||||||||||||||||
/* translators: 1) email address associated with the Stripe Link payment method */ | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( __( 'Via Stripe Link (%1$s)', 'woocommerce-gateway-stripe' ), $source->link->email ); | ||||||||||||||||||||||||
break 3; | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::ACH: | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( | ||||||||||||||||||||||||
/* translators: 1) account type (checking, savings), 2) last 4 digits of account. */ | ||||||||||||||||||||||||
__( 'Via %1$s Account ending in %2$s', 'woocommerce-gateway-stripe' ), | ||||||||||||||||||||||||
ucfirst( $source->us_bank_account->account_type ), | ||||||||||||||||||||||||
$source->us_bank_account->last4 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
break 3; | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::BECS_DEBIT: | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( | ||||||||||||||||||||||||
/* translators: last 4 digits of account. */ | ||||||||||||||||||||||||
__( 'BECS Direct Debit ending in %s', 'woocommerce-gateway-stripe' ), | ||||||||||||||||||||||||
$source->au_becs_debit->last4 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
break 3; | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::ACSS_DEBIT: | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( | ||||||||||||||||||||||||
/* translators: 1) bank name, 2) last 4 digits of account. */ | ||||||||||||||||||||||||
__( 'Via %1$s ending in %2$s', 'woocommerce-gateway-stripe' ), | ||||||||||||||||||||||||
$source->acss_debit->bank_name, | ||||||||||||||||||||||||
$source->acss_debit->last4 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
break 3; | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::BACS_DEBIT: | ||||||||||||||||||||||||
/* translators: 1) the Bacs Direct Debit payment method's last 4 numbers */ | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( __( 'Via Bacs Direct Debit ending in (%1$s)', 'woocommerce-gateway-stripe' ), $source->bacs_debit->last4 ); | ||||||||||||||||||||||||
break 3; | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::AMAZON_PAY: | ||||||||||||||||||||||||
/* translators: 1) the Amazon Pay payment method's email */ | ||||||||||||||||||||||||
$payment_method_to_display = sprintf( __( 'Via Amazon Pay (%1$s)', 'woocommerce-gateway-stripe' ), $source->billing_details->email ?? '' ); | ||||||||||||||||||||||||
break 3; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
if ( null !== $saved_payment_method ) { | ||||||||||||||||||||||||
return $this->get_payment_method_to_display_for_payment_method( $saved_payment_method ); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} catch ( WC_Stripe_Exception $e ) { | ||||||||||||||||||||||||
wc_add_notice( $e->getLocalizedMessage(), 'error' ); | ||||||||||||||||||||||||
WC_Stripe_Logger::log( 'Error: ' . $e->getMessage() ); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return $payment_method_to_display; | ||||||||||||||||||||||||
return __( 'N/A', 'woocommerce-gateway-stripe' ); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Helper function to get and temporarily cache the payment method details for a customer and payment method ID. | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional: we can move this helper method to WC_Stripe_Subscriptions_Helper. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is also a good suggestion. Done in 2920871. |
||||||||||||||||||||||||
* Note that we use the Stripe /v1/customers/:customer_id/payment_methods/:payment_method_id endpoint to get the payment method details. | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
* @see https://docs.stripe.com/api/payment_methods/customer | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
* @param string $stripe_customer_id The Stripe customer ID. | ||||||||||||||||||||||||
* @param string $payment_method_id The Stripe payment method ID. | ||||||||||||||||||||||||
* @return object|null The payment method details or null if the payment method is not found. | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
private function get_subscription_payment_method_details( string $stripe_customer_id, string $payment_method_id ): ?object { | ||||||||||||||||||||||||
static $cached_payment_methods = []; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if ( empty( $stripe_customer_id ) || empty( $payment_method_id ) ) { | ||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if ( isset( $cached_payment_methods[ $stripe_customer_id ][ $payment_method_id ] ) ) { | ||||||||||||||||||||||||
return $cached_payment_methods[ $stripe_customer_id ][ $payment_method_id ]; | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a static variable for caching in a web environment can lead to data leakage between requests or users. Consider using WordPress transients or object cache instead for safer caching that respects request boundaries.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For this use case, I think it's OK to use a static variable, due to us using the customer ID and payment method ID in the path, but I'd be happy to switch to a transient. |
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
$encoded_customer_id = rawurlencode( $stripe_customer_id ); | ||||||||||||||||||||||||
$encoded_payment_method_id = rawurlencode( $payment_method_id ); | ||||||||||||||||||||||||
$api_path = "customers/{$encoded_customer_id}/payment_methods/{$encoded_payment_method_id}"; | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As an alternate approach which would cover this related to the open question as well,
we can retrieve the payment method or source using the get_payment_method method and check if AFAIK, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call. I've switched that over in 3aecb6c. |
||||||||||||||||||||||||
|
||||||||||||||||||||||||
$saved_payment_method = WC_Stripe_API::retrieve( $api_path ); | ||||||||||||||||||||||||
daledupreez marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
if ( ! isset( $saved_payment_method->id ) ) { | ||||||||||||||||||||||||
$saved_payment_method = null; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
// Make sure we build the array tree. | ||||||||||||||||||||||||
if ( ! isset( $cached_payment_methods[ $stripe_customer_id ] ) ) { | ||||||||||||||||||||||||
$cached_payment_methods[ $stripe_customer_id ] = []; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
$cached_payment_methods[ $stripe_customer_id ][ $payment_method_id ] = $saved_payment_method; | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return $saved_payment_method; | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Helper function to get the descriptive text for a payment method or source. | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
* @param object $source The payment method or source object. | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional: My preference would be to call it a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call. Updated in 9e6ebdd |
||||||||||||||||||||||||
* @return string The descriptive text for the payment method or source. | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
protected function get_payment_method_to_display_for_payment_method( object $source ): string { | ||||||||||||||||||||||||
// Legacy handling for Stripe Card objects. ref: https://docs.stripe.com/api/cards/object | ||||||||||||||||||||||||
if ( isset( $source->object ) && WC_Stripe_Payment_Methods::CARD === $source->object ) { | ||||||||||||||||||||||||
return sprintf( | ||||||||||||||||||||||||
/* translators: 1) card brand 2) last 4 digits */ | ||||||||||||||||||||||||
__( 'Via %1$s card ending in %2$s', 'woocommerce-gateway-stripe' ), | ||||||||||||||||||||||||
( isset( $source->brand ) ? wc_get_credit_card_type_label( $source->brand ) : __( 'N/A', 'woocommerce-gateway-stripe' ) ), | ||||||||||||||||||||||||
$source->last4 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
switch ( $source->type ) { | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::CARD: | ||||||||||||||||||||||||
return sprintf( | ||||||||||||||||||||||||
/* translators: 1) card brand 2) last 4 digits */ | ||||||||||||||||||||||||
__( 'Via %1$s card ending in %2$s', 'woocommerce-gateway-stripe' ), | ||||||||||||||||||||||||
( isset( $source->card->brand ) ? wc_get_credit_card_type_label( $source->card->brand ) : __( 'N/A', 'woocommerce-gateway-stripe' ) ), | ||||||||||||||||||||||||
$source->card->last4 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::SEPA_DEBIT: | ||||||||||||||||||||||||
/* translators: 1) last 4 digits of SEPA Direct Debit */ | ||||||||||||||||||||||||
return sprintf( __( 'Via SEPA Direct Debit ending in %1$s', 'woocommerce-gateway-stripe' ), $source->sepa_debit->last4 ); | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::CASHAPP_PAY: | ||||||||||||||||||||||||
/* translators: 1) Cash App Cashtag */ | ||||||||||||||||||||||||
return sprintf( __( 'Via Cash App Pay (%1$s)', 'woocommerce-gateway-stripe' ), $source->cashapp->cashtag ); | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::LINK: | ||||||||||||||||||||||||
/* translators: 1) email address associated with the Stripe Link payment method */ | ||||||||||||||||||||||||
return sprintf( __( 'Via Stripe Link (%1$s)', 'woocommerce-gateway-stripe' ), $source->link->email ); | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::ACH: | ||||||||||||||||||||||||
return sprintf( | ||||||||||||||||||||||||
/* translators: 1) account type (checking, savings), 2) last 4 digits of account. */ | ||||||||||||||||||||||||
__( 'Via %1$s Account ending in %2$s', 'woocommerce-gateway-stripe' ), | ||||||||||||||||||||||||
ucfirst( $source->us_bank_account->account_type ), | ||||||||||||||||||||||||
$source->us_bank_account->last4 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::BECS_DEBIT: | ||||||||||||||||||||||||
return sprintf( | ||||||||||||||||||||||||
/* translators: last 4 digits of account. */ | ||||||||||||||||||||||||
__( 'BECS Direct Debit ending in %s', 'woocommerce-gateway-stripe' ), | ||||||||||||||||||||||||
$source->au_becs_debit->last4 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::ACSS_DEBIT: | ||||||||||||||||||||||||
return sprintf( | ||||||||||||||||||||||||
/* translators: 1) bank name, 2) last 4 digits of account. */ | ||||||||||||||||||||||||
__( 'Via %1$s ending in %2$s', 'woocommerce-gateway-stripe' ), | ||||||||||||||||||||||||
$source->acss_debit->bank_name, | ||||||||||||||||||||||||
$source->acss_debit->last4 | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::BACS_DEBIT: | ||||||||||||||||||||||||
/* translators: 1) the Bacs Direct Debit payment method's last 4 numbers */ | ||||||||||||||||||||||||
return sprintf( __( 'Via Bacs Direct Debit ending in (%1$s)', 'woocommerce-gateway-stripe' ), $source->bacs_debit->last4 ); | ||||||||||||||||||||||||
case WC_Stripe_Payment_Methods::AMAZON_PAY: | ||||||||||||||||||||||||
/* translators: 1) the Amazon Pay payment method's email */ | ||||||||||||||||||||||||
return sprintf( __( 'Via Amazon Pay (%1$s)', 'woocommerce-gateway-stripe' ), $source->billing_details->email ?? '' ); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
return __( 'N/A', 'woocommerce-gateway-stripe' ); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.