Add the WooPay Direct Checkout flow to the classic mini cart widget #8903
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cesarcosta99
requested review from
a team and
rafaelzaleski
and removed request for
a team
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +20.2 kB (+2%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
cesarcosta99
commented
Jun 4, 2024
| // Enqueue the WCPay common config script only if it hasn't been enqueued yet. | ||
| // This may happen when Direct Checkout is being enqueued on pages that are not the cart page, | ||
| // such as the home and shop pages. | ||
| if ( did_filter( 'wcpay_payment_fields_js_config' ) === 0 ) { |
There was a problem hiding this comment.
If this filter (ref) isn't applied, wcpayConfig won't be registered in the frontend and will be evaluated to undefined. Therefore, we need to enqueue it so that the direct checkout flow doesn't break.
Comment on lines
+135
to
+137
| return $this->is_cart_page() | ||
| || did_action( 'woocommerce_blocks_cart_enqueue_data' ) > 0 | ||
| || ( wp_script_is( 'wc-cart-fragments', 'enqueued' ) && ! $this->is_checkout_page() ); |
There was a problem hiding this comment.
This entire condition could probably be turned into ! $this->is_checkout_page, however, I didn't want to leave it too loose because we don't know what kind of customizations the merchants are applying to their themes. I went for a safer approach and added a condition to check whether the wc-cart-fragments is enqueued (ref) and it's not the checkout page.
I also tried a similar approach used for the Blocks mini cart by doing did_action( 'woocommerce_widget_shopping_cart_buttons' ) > 0 (ref) but that always return 0 since that action is not fired before the wp_footer hook.
rafaelzaleski
requested changes
Jun 5, 2024
There was a problem hiding this comment.
✅ Regression test: Ensure the direct checkout flow works from classic and blocks carts
✅ Test: Ensure the direct checkout scripts are enqueued in the right pages
✅ Test: Ensure the direct checkout flow works from the mini cart
The implementation looks good, and the functionality is working as expected. However, I believe we should remove the code duplication to improve future maintainability.
Comment on lines
89
to
104
| wp_register_script( 'WCPAY_WOOPAY_COMMON_CONFIG', '', [], WCPAY_VERSION_NUMBER, false ); | ||
| wp_localize_script( | ||
| 'WCPAY_WOOPAY_COMMON_CONFIG', | ||
| 'wcpayConfig', | ||
| [ | ||
| 'woopayHost' => WooPay_Utilities::get_woopay_url(), | ||
| 'testMode' => $is_test_mode, | ||
| 'wcAjaxUrl' => WC_AJAX::get_endpoint( '%%endpoint%%' ), | ||
| 'woopaySessionNonce' => wp_create_nonce( 'woopay_session_nonce' ), | ||
| 'isWooPayDirectCheckoutEnabled' => WC_Payments_Features::is_woopay_direct_checkout_enabled(), | ||
| 'platformTrackerNonce' => wp_create_nonce( 'platform_tracks_nonce' ), | ||
| 'ajaxUrl' => admin_url( 'admin-ajax.php' ), | ||
| 'woopayMinimumSessionData' => WooPay_Session::get_woopay_minimum_session_data(), | ||
| ] | ||
| ); | ||
| wp_enqueue_script( 'WCPAY_WOOPAY_COMMON_CONFIG' ); |
There was a problem hiding this comment.
Since this section is a copy of existing code, can we extract it into a static method and use it in both places? This would help avoid duplication in the codebase.
There was a problem hiding this comment.
Good call! I applied this change in 8be13dc. Can you please re-test it?
cesarcosta99
commented
Jun 5, 2024
| * | ||
| * @return void | ||
| */ | ||
| public static function enqueue_woopay_common_config_script() { | ||
| public static function validate_and_enqueue_woopay_common_config_script() { |
There was a problem hiding this comment.
I don't like the name of this function much, however I was a bit out of options since maybe_enqueue_woopay_common_config_script is taken.
For clarity, it's not possible to move this check to the maybe_enqueue_woopay_common_config_script function because it generates a warning saying is_page was called incorrectly. That's why I needed to keep this logic separate.
rafaelzaleski
approved these changes
Jun 6, 2024
There was a problem hiding this comment.
✅ Regression test: Ensure the direct checkout flow works from classic and blocks carts
✅ Test: Ensure the direct checkout scripts are enqueued in the right pages
✅ Test: Ensure the direct checkout flow works from the mini cart
Everything is working and the code looks good. Thank you for implementing the suggested refactoring.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes https://github.com/Automattic/woopay/issues/2688
Changes proposed in this Pull Request
This PR adds the WooPay direct checkout flow to the classic mini cart widget.
It also enqueues the direct checkout scripts if the current page has the
wc-cart-fragmentsscript enqueued. I expand more on this in the comment below.Testing instructions
Setup
_wcpay_feature_woopay_direct_checkoutis set to1.npm run build:client.Regression test: Ensure the direct checkout flow works from classic and blocks carts
Test: Ensure the direct checkout scripts are enqueued in the right pages
WCPAY_WOOPAY_DIRECT_CHECKOUTscript is enqueued in the page source.Test: Ensure the direct checkout flow works from the mini cart
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge