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

Update deprecated WooCommerce Blocks hook #54

Merged
merged 4 commits into from
Dec 15, 2021
Merged

Conversation

opr
Copy link
Contributor

@opr opr commented Nov 30, 2021

Description

This change is needed because recently WooCommerce Blocks graduated some of its experimental hooks (woocommerce/woocommerce-blocks#5017 and woocommerce/woocommerce-blocks#5014) to stable, so the __experimental prefix was dropped. This PR updates the use of those hooks to the stable version.

Without this PR, deprecation warnings are shown in the Checkout Block.

This code can break if the user does not update their version of WooCommerce Blocks to the latest version that includes the graduated hooks.

Tips/tricks on how to mitigate this would be much appreciated, because I'm really unsure on how we can cater for this situation.

How to test this PR

Place an order that includes subscription items and ensure the Checkout page does not show any warnings. Ensure the order in WP Admin is correct and has all the expected information.

Product impact

  • Added changelog entry (or does not apply)
  • Will this PR affect WooCommerce Subscriptions? yes/no/tbc, add issue ref
  • Will this PR affect WooCommerce Payments? yes/no/tbc, add issue ref
    ^ I'm not 100% confident on the above, whether WC Payments would call these methods, I think it won't but could someone who knows more 👍🏼 this one?

@opr opr added pr: needs review type: task The issue is an internally driven task (e.g. from another A8c team). size: small The issue is sized small. labels Nov 30, 2021
@opr opr requested a review from mattallan November 30, 2021 10:07
@mattallan mattallan self-assigned this Dec 1, 2021
Copy link
Contributor

@mattallan mattallan left a comment

Choose a reason for hiding this comment

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

Thanks @opr for working on this for us! 🙏

Tips/tricks on how to mitigate this would be much appreciated, because I'm really unsure on how we can cater for this situation.

I've left a comment with a suggestion on how we could work with old and new version of blocks. Let me know what your thoughts are on this approach!

Thanks!

@@ -24,7 +24,7 @@ public static function init() {
add_action( 'woocommerce_checkout_order_processed', array( __CLASS__, 'process_checkout' ), 100, 2 );

// Same as above, but this is for the Checkout block.
add_action( '__experimental_woocommerce_blocks_checkout_order_processed', array( __CLASS__, 'process_checkout' ), 100, 1 );
add_action( 'woocommerce_blocks_checkout_order_processed', array( __CLASS__, 'process_checkout' ), 100, 1 );
Copy link
Contributor

Choose a reason for hiding this comment

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

@opr to maintain backwards compatibility, what are your thoughts on surrounding these with version checks (see suggestion below)?

Suggested change
add_action( 'woocommerce_blocks_checkout_order_processed', array( __CLASS__, 'process_checkout' ), 100, 1 );
if ( class_exists( 'Automattic\WooCommerce\Blocks\Package' ) && version_compare( \Automattic\WooCommerce\Blocks\Package::get_version(), '6.3.0', '>=' ) ) {
add_action( 'woocommerce_blocks_checkout_order_processed', array( __CLASS__, 'process_checkout' ), 100, 1 );
} else {
add_action( '__experimental_woocommerce_blocks_checkout_order_processed', array( __CLASS__, 'process_checkout' ), 100, 1 );
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for that suggestion, seems super obvious now 🤦🏼 I also wrapped it in a is_experimental_build so we don't call the experimental hooks if we're in a dev version of the plugin. It seems version_compare doesn't like the -dev suffix we use.

Copy link
Contributor

@mattallan mattallan left a comment

Choose a reason for hiding this comment

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

Thanks @opr!

I've tested these changes on WooCommerce Blocks 6.4 (latest) and 6.2 and everything appears to be working in order 👍

Marking that as good to merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: ready to merge size: small The issue is sized small. type: task The issue is an internally driven task (e.g. from another A8c team).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants