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

Miscellaneous tasks on ECE #3551

Merged
merged 9 commits into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelog.txt
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
* Tweak - Remove the subscription order notes added each time a source wasn't migrated.
* Tweak - Update ECE default button type.
* Fix - Fix position of ECE button on shortcode cart page.
* Fix - Call ECE specific 'paymentFailed' function only when payment request fails.
* Fix - Fix issue in purchasing subscriptions when the store has no shipping options.

= 8.8.2 - 2024-11-07 =
* Fix - Prevent marking renewal orders as processing/completed multiple times due to handling the Stripe webhook in parallel.
Expand Down
8 changes: 7 additions & 1 deletion client/blocks/express-checkout/express-checkout-container.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import React from 'react';
import { Elements } from '@stripe/react-stripe-js';
import ExpressCheckoutComponent from './express-checkout-component';
import { getPaymentMethodTypesForExpressMethod } from 'wcstripe/express-checkout/utils';
import {
getExpressCheckoutButtonAppearance,
getExpressCheckoutData,
getPaymentMethodTypesForExpressMethod,
} from 'wcstripe/express-checkout/utils';

export const ExpressCheckoutContainer = ( props ) => {
const { stripe, billing, expressPaymentMethod } = props;
Expand All @@ -13,6 +17,8 @@ export const ExpressCheckoutContainer = ( props ) => {
paymentMethodTypes: getPaymentMethodTypesForExpressMethod(
expressPaymentMethod
),
appearance: getExpressCheckoutButtonAppearance(),
locale: getExpressCheckoutData( 'stripe' )?.locale ?? 'en',
};

return (
Expand Down
6 changes: 4 additions & 2 deletions client/blocks/express-checkout/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ export const useExpressCheckout = ( {
window.location = redirectUrl;
};

const abortPayment = ( onConfirmEvent, message ) => {
onConfirmEvent.paymentFailed( { reason: 'fail' } );
const abortPayment = ( onConfirmEvent, message, isOrderError = false ) => {
if ( ! isOrderError ) {
onConfirmEvent.paymentFailed( { reason: 'fail' } );
}
setExpressPaymentError( message );
onAbortPaymentHandler( onConfirmEvent, message );
};
Expand Down
9 changes: 7 additions & 2 deletions client/entrypoints/express-checkout/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ jQuery( function ( $ ) {
currency: options.currency,
paymentMethodCreation: 'manual',
appearance: getExpressCheckoutButtonAppearance(),
locale: getExpressCheckoutData( 'stripe' )?.locale ?? 'en',
paymentMethodTypes: getExpressPaymentMethodTypes(),
} );

Expand Down Expand Up @@ -289,6 +290,7 @@ jQuery( function ( $ ) {
currency: getExpressCheckoutData( 'checkout' )
.currency_code,
appearance: getExpressCheckoutButtonAppearance(),
locale: getExpressCheckoutData( 'stripe' )?.locale ?? 'en',
displayItems,
order,
} );
Expand Down Expand Up @@ -466,9 +468,12 @@ jQuery( function ( $ ) {
*
* @param {PaymentResponse} payment Payment response instance.
* @param {string} message Error message to display.
* @param {boolean} isOrderError Whether the error is related to the order creation.
*/
abortPayment: ( payment, message ) => {
payment.paymentFailed( { reason: 'fail' } );
abortPayment: ( payment, message, isOrderError = false ) => {
if ( ! isOrderError ) {
payment.paymentFailed( { reason: 'fail' } );
}
onAbortPaymentHandler( payment, message );

displayExpressCheckoutNotice( message, 'error' );
Expand Down
6 changes: 4 additions & 2 deletions client/express-checkout/__tests__/event-handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,8 @@ describe( 'Express checkout event handlers', () => {
);
expect( abortPayment ).toHaveBeenCalledWith(
event,
'Order creation error'
'Order creation error',
true
);
expect( completePayment ).not.toHaveBeenCalled();
} );
Expand Down Expand Up @@ -467,7 +468,8 @@ describe( 'Express checkout event handlers', () => {
);
expect( abortPayment ).toHaveBeenCalledWith(
event,
'Order creation error'
'Order creation error',
true
);
expect( completePayment ).not.toHaveBeenCalled();
} );
Expand Down
3 changes: 2 additions & 1 deletion client/express-checkout/event-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ export const onConfirmHandler = async (
if ( orderResponse.result !== 'success' ) {
return abortPayment(
event,
getErrorMessageFromNotice( orderResponse.messages )
getErrorMessageFromNotice( orderResponse.messages ),
true
);
}

Expand Down
4 changes: 2 additions & 2 deletions includes/compat/trait-wc-stripe-subscriptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ public function add_subscription_payment_meta( $payment_meta, $subscription ) {
],
'_stripe_source_id' => [
'value' => $source_id,
'label' => 'Stripe Source ID',
'label' => 'Stripe Payment Method ID',
],
],
];
Expand Down Expand Up @@ -720,7 +720,7 @@ public function validate_subscription_payment_meta( $payment_method_id, $payment
&& 0 !== strpos( $payment_meta['post_meta']['_stripe_source_id']['value'], 'pm_' )
)
) {
throw new Exception( __( 'Invalid source ID. A valid source "Stripe Source ID" must begin with "src_", "pm_", or "card_".', 'woocommerce-gateway-stripe' ) );
throw new Exception( __( 'Invalid payment method ID. A valid "Stripe Payment Method ID" must begin with "src_", "pm_", or "card_".', 'woocommerce-gateway-stripe' ) );
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public function init() {
add_action( 'woocommerce_checkout_order_processed', [ $this, 'add_order_meta' ], 10, 2 );
add_filter( 'woocommerce_login_redirect', [ $this, 'get_login_redirect_url' ], 10, 3 );
add_filter( 'woocommerce_registration_redirect', [ $this, 'get_login_redirect_url' ], 10, 3 );
add_filter( 'woocommerce_cart_needs_shipping_address', [ $this, 'filter_cart_needs_shipping_address' ], 11, 1 );

add_action( 'before_woocommerce_pay_form', [ $this, 'localize_pay_for_order_page_scripts' ] );
}
Expand Down Expand Up @@ -415,4 +416,17 @@ public function display_express_checkout_button_separator_html() {
<p id="wc-stripe-express-checkout-button-separator" style="margin-top:1.5em;text-align:center;display:none;">&mdash; <?php esc_html_e( 'OR', 'woocommerce-gateway-stripe' ); ?> &mdash;</p>
<?php
}

/**
* Determine whether to filter the cart needs shipping address.
*
* @param boolean $needs_shipping_address Whether the cart needs a shipping address.
*/
public function filter_cart_needs_shipping_address( $needs_shipping_address ) {
if ( $this->express_checkout_helper->has_subscription_product() && wc_get_shipping_method_count( true, true ) === 0 ) {
return false;
}

return $needs_shipping_address;
}
Comment on lines +425 to +431
Copy link
Contributor

Choose a reason for hiding this comment

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

@Mayisha What behavior do we want from ECE when there are no defined shipping zones, but the cart is shippable? Should we let the order push through with just the billing address? Do you know if there any implications when a shippable order does not have a shipping address?

Asking also because I think I introduced a bug in #3560 for block cart and block checkout, when using ECE and there are no defined shipping zones. It should be an easy fix but I wanted to make sure it's the behavior we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is, if we are offering Stripe card payment method on the cart/checkout page and the displayed/charged amount on the Google/Apple Pay modal is correct, then we should offer Google/Apple Pay as well.
What are your thoughts on this @annemirasol ? 👀

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, this makes sense. If we are letting it happen via other payment methods, we should let it happen via express checkout 👍

}
2 changes: 2 additions & 0 deletions readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,5 +132,7 @@ If you get stuck, you can ask for help in the [Plugin Forum](https://wordpress.o
* Tweak - Remove the subscription order notes added each time a source wasn't migrated.
* Tweak - Update ECE default button type.
* Fix - Fix position of ECE button on shortcode cart page.
* Fix - Call ECE specific 'paymentFailed' function only when payment request fails.
* Fix - Fix issue in purchasing subscriptions when the store has no shipping options.

[See changelog for all versions](https://raw.githubusercontent.com/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt).
Loading