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 the checkout blocks hook that's attached to the process checkout flow to use the latest hook #523

Merged
merged 2 commits into from
Oct 13, 2023

Conversation

mattallan
Copy link
Contributor

Related issue: https://github.com/woocommerce/woocommerce-subscriptions/issues/4540

Description

There is an issue where processing a Switch request on a block checkout results in a new subscription being created instead of the existing subscription being updated. This issue was caused by our WC_Subscriptions_Checkout class using a deprecated checkout block hook while our WC_Subscriptions_Switcher is using the latest store API hook.

This was causing problems because WooCommerce calls the deprecated hooks first before the latest hook resulting in WC_Subscriptions_Checkout::process_checkout running before WC_Subscriptions_Switcher::process_checkout which was creating the extra subscription correctly.

It's important that the switcher class processes the checkout first so that it can remove any items from the recurring cart to prevent subscriptions from being created on the switch.

Note

This PR removes the version_compare on \Automattic\WooCommerce\Blocks\Package::get_version() because Subscriptions supports WooCommerce 7.7 at a minimum which already comes bundled with Checkout Blocks v10.0.2.
Therefore I've removed all of the deprecated hooks and conditions checking for older versions of checkout blocks that we no longer support.

I'm not sure if there are any concerns with stores installing an older version of checkout blocks as a standalone plugin but I don't think we should cater to that.
Open to discuss this :)

How to test this PR

Important

To follow these testing instructions you'll need to have the main Woo Subscriptions plugin active to access switching.
There's also another bug that prevents you from submitting a $0 switch checkout when using the block checkout. This will be addressed in a PR to the main repository.

  1. Make sure you have a block checkout page available to test
  2. Enable switching between variations in the WC > Settings > Subscriptions
  3. Purchase a variable subscription product
  4. Go to My Account > My Subscription
  5. Click on the Upgrade or Downgrade button
  6. Select a different variation and complete the switch checkout
  7. On trunk you'll notice you now have two active subscriptions for the same variable product
    image
  8. On this branch, you will only have 1 subscription.
    In order to be able to bipass the checkout erroring with "No valid payment method" I had to stub WC_Subscriptions_Order::order_needs_payment() to return false on the first line.

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
  • Added deprecated functions, hooks or classes to the spreadsheet

@mattallan mattallan requested a review from james-allan October 11, 2023 03:45
Copy link
Contributor

@james-allan james-allan left a comment

Choose a reason for hiding this comment

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

Testing in conjunction with https://github.com/woocommerce/woocommerce-subscriptions/pull/4567.

The new hooks used in this PR were added to WooCommerce core in v6.4 (woocommerce/woocommerce#32075) and given we only support versions of WC back to 7.7, this update is safe.

@mattallan mattallan merged commit 8c8ccce into trunk Oct 13, 2023
12 checks passed
@mattallan mattallan deleted the fix/add-checkout-blocks-hooks branch October 13, 2023 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants