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

Prevents a PHP fatal error that occurs when the cart contains a renewal order item that no longer exists #544

Merged
merged 3 commits into from
Nov 22, 2023

Conversation

mattallan
Copy link
Contributor

Fixes #537

Description

Inside subscriptions we have code that makes sure that if a cart contains a renewal item, we should make sure we're honoring the price of the subscription/renewal order and not the new price if it's been updated etc.

When the cart contains an item that no longer exists on the order, we get the following fatal errors that occur:

[20-Nov-2023 05:57:18 UTC] PHP Warning:  Undefined array key "line_subtotal" in /Users/matt/local/woo/wp-content/plugins/woocommerce-subscriptions-core/includes/class-wcs-cart-renewal.php on line 454
[20-Nov-2023 05:57:18 UTC] PHP Warning:  Undefined array key "taxes" in /Users/matt/local/woo/wp-content/plugins/woocommerce-subscriptions-core/includes/class-wcs-cart-renewal.php on line 469
[20-Nov-2023 05:57:18 UTC] PHP Warning:  Trying to access array offset on value of type null in /Users/matt/local/woo/wp-content/plugins/woocommerce-subscriptions-core/includes/class-wcs-cart-renewal.php on line 469
[20-Nov-2023 05:57:18 UTC] PHP Fatal error:  Uncaught TypeError: array_sum(): Argument #1 ($array) must be of type array, null given in /Users/matt/local/woo/wp-content/plugins/woocommerce-subscriptions-core/includes/class-wcs-cart-renewal.php:469
Stack trace:
#0 /Users/matt/local/woo/wp-content/plugins/woocommerce-subscriptions-core/includes/class-wcs-cart-renewal.php(469): array_sum(NULL)
#1 /Users/matt/local/woo/wp-includes/class-wp-hook.php(310): WCS_Cart_Renewal->get_cart_item_from_session(Array, Array, 'e4097d961260a5e...')
#2 /Users/matt/local/woo/wp-includes/plugin.php(205): WP_Hook->apply_filters(Array, Array)
#3 /Users/matt/local/woo/wp-content/plugins/woocommerce/includes/class-wc-cart-session.php(206): apply_filters('woocommerce_get...', Array, Array, 'e4097d961260a5e...')
#4 /Users/matt/local/woo/wp-includes/class-wp-hook.php(310): WC_Cart_Session->get_cart_from_session('')
#5 /Users/matt/local/woo/wp-includes/class-wp-hook.php(334): WP_Hook->apply_filters(NULL, Array)
#6 /Users/matt/local/woo/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#7 /Users/matt/local/woo/wp-settings.php(654): do_action('wp_loaded')
#8 /Users/matt/local/woo/wp-config.php(101): require_once('/Users/matt/loc...')
#9 /Users/matt/local/woo/wp-load.php(50): require_once('/Users/matt/loc...')
#10 /Users/matt/local/woo/wp-admin/admin-ajax.php(22): require_once('/Users/matt/loc...')
#11 /Users/matt/.composer/vendor/laravel/valet/server.php(110): require('/Users/matt/loc...')
#12 {main}

This PR fixes this issue by making sure we're only honoring subscription prices on cart items that still belong to the order/subscription. If the cart item cannot be found on the order, we simply return the default.

How to test this PR

  1. Purchase a subscription product to create a subscription
  2. Process a pending renewal order using the drop-down actions
  3. Go to My Account > Subscriptions and click on the Pay button next to the pending order to add it to you cart
  4. Go to the WP Admin > WC > Orders and find the new pending order
  5. Delete the order item and save the order.
  6. While on trunk if you refresh the page and you should see the above fatal errors

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 a team November 20, 2023 06:20
@diegocurbelo diegocurbelo requested review from diegocurbelo and removed request for a team November 20, 2023 15:34
Copy link
Member

@diegocurbelo diegocurbelo left a comment

Choose a reason for hiding this comment

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

Code looks good, and tests well, after removing the item from the order and refreshing the pay for order page no longer generates an error.

@mattallan I just want to confirm that the expected behavior is for the cart to still contain the original order product and the checkout to complete successfully, right?

@mattallan
Copy link
Contributor Author

I just want to confirm that the expected behavior is for the cart to still contain the original order product and the checkout to complete successfully, right?

Thanks @diegocurbelo for the review and also for asking this question!

Yes, this is correct. It's expected that the cart still contains the original order items and that the checkout completes successfully.

@mattallan mattallan merged commit b064d6e into trunk Nov 22, 2023
12 checks passed
@mattallan mattallan deleted the fix/537-empty-item-to-renew branch November 22, 2023 03:42
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.

Fatal error if the user has an already paid renewal left in the cart
2 participants