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

Remove defunct maybe_restore_shipping_methods #794

Open
wants to merge 4 commits into
base: trunk
Choose a base branch
from

Conversation

c-shultz
Copy link

@c-shultz c-shultz commented Feb 24, 2025

Fixes #398

Description

Removes the private maybe_restore_shipping_methods method and all references from the class WC_Subscriptions_Cart. @james-allan has an excellent summary here. The expected nonce field has been renamed since WooCommerce 3.4 in 2018, so this code has been unused for about 7 years. It has no known purpose at this point and contain a call to a long deprecated core function (WC_Customer::calculated_shipping).

Also deprecates the public method maybe_restore_chosen_shipping_method since it is no longer reference internally.

How to test this PR

The code appears to have been related to fixing a corner case when the shipping method is updated on the cart page, but the nonce check has not been passing, so this shouldn't have any impact other than remove a logged PHP warning. I would encourage exploratory testing to double-check that nothing was missed.

At a minimum, I recommend:

  • Adding a shortcode cart page
  • Adding a subscription product to cart
  • Updating the address and shipping method on the cart page
image

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

@c-shultz c-shultz marked this pull request as ready for review February 24, 2025 21:05
@c-shultz c-shultz requested review from a team and barryhughes and removed request for a team February 24, 2025 22:40
@@ -2355,6 +2289,7 @@ public static function get_cart_shipping_method_full_label( $label, $method ) {
* method to a key that's not going to get wiped by WC's method, and then later restore it.
*/
public static function maybe_restore_chosen_shipping_method() {
wcs_deprecated_function( __METHOD__, 'subscriptions-core 8.1.0', 'The use of this function is no longer recommended and will be removed in a future version.' );
Copy link
Member

Choose a reason for hiding this comment

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

Minor detail (not essential we change anything, though, as this is a bit of a grey area), but might this be better as:

Suggested change
wcs_deprecated_function( __METHOD__, 'subscriptions-core 8.1.0', 'The use of this function is no longer recommended and will be removed in a future version.' );
wcs_deprecated_function( __METHOD__, '7.3.0', 'The use of this function is no longer recommended and will be removed in a future version.' );

Here's my thinking:

  • From a user perspective, the product they are using is WooCommerce Subscriptions and I believe we currently expect the next version to be 7.3.0.
  • I realize Subscriptions Core can be used elsewhere, not just in the Subscriptions product, but that feels like more of a temporary edge case (in fact, based on some recent discussion, we decided to generally use the version number from the parent plugin for things like @since and @version tags).

Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

I left a comment about a possible change, but beyond that this looks great!

  • Tests as per the provided instructions.
  • I experimented with a few different shipping provider choices, to test for any problems, and couldn't find any.
  • Verified this check will not be triggered outside of a shortcode-powered environment.

Approving but not merging so you can review my note re version numbers in deprecation notices, but definitely go ahead with a merge when you are ready.

Thanks!

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.

PHP Warning: Undefined array key "_wpnonce" in class-wc-subscriptions-cart.php
2 participants