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.

Sign up for GitHub

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

Register Expresss Checkout block only when enabled in the settings #9646

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Register Expresss Checkout block only when enabled in the settings #9646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f344ee
Register Expresss Checkout block only when enabled in the settings
gpressutto5 Oct 30, 2024
d2af736
Merge branch 'develop' into fix/express-checkout-block-registering
gpressutto5 Oct 30, 2024
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
4 changes: 4 additions & 0 deletions changelog/fix-express-checkout-block-registering
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: fix

Register Expresss Checkout block only when enabled in the settings
20 changes: 11 additions & 9 deletions client/checkout/blocks/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,17 @@ if ( getUPEConfig( 'isWooPayEnabled' ) ) {
}
}

if ( getUPEConfig( 'isTokenizedCartPrbEnabled' ) ) {
registerExpressPaymentMethod(
tokenizedCartPaymentRequestPaymentMethod( api )
);
} else if ( getUPEConfig( 'isExpressCheckoutElementEnabled' ) ) {
registerExpressPaymentMethod( expressCheckoutElementApplePay( api ) );
registerExpressPaymentMethod( expressCheckoutElementGooglePay( api ) );
} else {
registerExpressPaymentMethod( paymentRequestPaymentMethod( api ) );
if ( getUPEConfig( 'isPaymentRequestEnabled' ) ) {
if ( getUPEConfig( 'isTokenizedCartPrbEnabled' ) ) {
registerExpressPaymentMethod(
tokenizedCartPaymentRequestPaymentMethod( api )
);
} else if ( getUPEConfig( 'isExpressCheckoutElementEnabled' ) ) {
registerExpressPaymentMethod( expressCheckoutElementApplePay( api ) );
registerExpressPaymentMethod( expressCheckoutElementGooglePay( api ) );
} else {
registerExpressPaymentMethod( paymentRequestPaymentMethod( api ) );
}
}
Comment on lines +160 to 171
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a matter of preference, but I started to find the below approach more readable after some time in our back-end PHP:

Suggested change
if ( getUPEConfig( 'isPaymentRequestEnabled' ) ) {
if ( getUPEConfig( 'isTokenizedCartPrbEnabled' ) ) {
registerExpressPaymentMethod(
tokenizedCartPaymentRequestPaymentMethod( api )
);
} else if ( getUPEConfig( 'isExpressCheckoutElementEnabled' ) ) {
registerExpressPaymentMethod( expressCheckoutElementApplePay( api ) );
registerExpressPaymentMethod( expressCheckoutElementGooglePay( api ) );
} else {
registerExpressPaymentMethod( paymentRequestPaymentMethod( api ) );
}
}
if (!getUPEConfig('isPaymentRequestEnabled')) {
return;
}
if (getUPEConfig('isTokenizedCartPrbEnabled')) {
registerExpressPaymentMethod(tokenizedCartPaymentRequestPaymentMethod(api));
return;
}
if (getUPEConfig('isExpressCheckoutElementEnabled')) {
registerExpressPaymentMethod(expressCheckoutElementApplePay(api));
registerExpressPaymentMethod(expressCheckoutElementGooglePay(api));
return;
}
registerExpressPaymentMethod(paymentRequestPaymentMethod(api));

feel free to ignore it if the change does not seem convincing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is indeed more readable, but we have more code after that we want to execute regardless of PRB/ECE settings. If we want to early return we need to create a new function for this so it doesn't affect the rest of the code. But then I think it starts to get less readable, what do you think?

Suggested change
if ( getUPEConfig( 'isPaymentRequestEnabled' ) ) {
if ( getUPEConfig( 'isTokenizedCartPrbEnabled' ) ) {
registerExpressPaymentMethod(
tokenizedCartPaymentRequestPaymentMethod( api )
);
} else if ( getUPEConfig( 'isExpressCheckoutElementEnabled' ) ) {
registerExpressPaymentMethod( expressCheckoutElementApplePay( api ) );
registerExpressPaymentMethod( expressCheckoutElementGooglePay( api ) );
} else {
registerExpressPaymentMethod( paymentRequestPaymentMethod( api ) );
}
}
const maybeRegisterExpressCheckoutMethod = () => {
if (!getUPEConfig('isPaymentRequestEnabled')) {
return;
}
if (getUPEConfig('isTokenizedCartPrbEnabled')) {
registerExpressPaymentMethod(tokenizedCartPaymentRequestPaymentMethod(api));
return;
}
if (getUPEConfig('isExpressCheckoutElementEnabled')) {
registerExpressPaymentMethod(expressCheckoutElementApplePay(api));
registerExpressPaymentMethod(expressCheckoutElementGooglePay(api));
return;
}
registerExpressPaymentMethod(paymentRequestPaymentMethod(api));
}
maybeRegisterExpressCheckoutMethod();

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, agreed, I'd keep it as it is then without changing your final version 👍 🚢

window.addEventListener( 'load', () => {
enqueueFraudScripts( getUPEConfig( 'fraudServices' ) );
Expand Down
9 changes: 9 additions & 0 deletions includes/class-wc-payment-gateway-wcpay.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -894,6 +894,15 @@ public function is_saved_cards_enabled() {
return 'yes' === $this->get_option( 'saved_cards' );
}

/**
* Checks if the setting to show the payment request buttons is enabled.
*
* @return bool Whether the setting to show the payment request buttons is enabled or not.
*/
public function is_payment_request_enabled() {
return 'yes' === $this->get_option( 'payment_request' );
}

/**
* Check if account is eligible for card present.
*
Expand Down
1 change: 1 addition & 0 deletions includes/class-wc-payments-checkout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ public function get_payment_fields_js_config() {
'isPreview' => is_preview(),
'isSavedCardsEnabled' => $this->gateway->is_saved_cards_enabled(),
'isExpressCheckoutElementEnabled' => WC_Payments_Features::is_stripe_ece_enabled(),
'isPaymentRequestEnabled' => $this->gateway->is_payment_request_enabled(),
'isTokenizedCartPrbEnabled' => WC_Payments_Features::is_tokenized_cart_prb_enabled(),
'isWooPayEnabled' => $this->woopay_util->should_enable_woopay( $this->gateway ) && $this->woopay_util->should_enable_woopay_on_cart_or_checkout(),
'isWoopayExpressCheckoutEnabled' => $this->woopay_util->is_woopay_express_checkout_enabled(),
Expand Down
Loading