Can't process renewal order at checkout if cart contains out of stock product

[Lipppitt]

Describe the bug

When a customer is attempting to pay for a failed or pending renewal order from their account page, if the renewal order contains a product which is out of stock, the system prevents the checkout from loading and displays a 'This item is out of stock' error message on the cart page.

To Reproduce

1. Create a new product with the qty set to 1
2. Create a manual subscription with that product and parent order
3. Send customer link to pay for order
4. Customer pays and product qty is set to 0
5. Create a renewal order manually or automatically and set it to failed or pending
6. Send customer link to pay for renewal order
7. Checkout will fail and cart will be emptied due to product being out of stock

Expected behavior

1. Customer payment link and is redirected to checkout page to pay

Actual behavior

1. Checkout fails and cart is be emptied

I've identified the issue is coming from the WCS_Renewal_Cart_Stock_Manager class where you the methods responsible for getting the renewal order are calling the wrong methods in the class. This is due to those methods being called with static::, rather than the class where the correct method lives.

For example, the method cart_contains_renewal_to_product attempts to get the order from cart for the specific product and does this by calling static::get_order_from_cart() or static::get_order_from_query_vars(). This issue here is there are multiple static methods with that name in the codebase and it's resulting in the incorrect method being called.

To fix this you need to update both to WCS_Renewal_Cart_Stock_Manager::get_order_from_cart(); and WCS_Renewal_Cart_Stock_Manager::get_order_from_query_vars(); and change the same methods in the maybe_adjust_stock_checkout to point to self:: instead of static::. This ensures the order is being generated from the correct renewal class manager instead of the initial cart manager.

With these changes made, i can confirm the cart and checkout overrides the product stock status and allows the customer to pay with for their order.

public static function maybe_adjust_stock_checkout() {
    $renewal_order = self::get_order_from_cart();

    // Get the order from query vars if the cart isn't loaded yet.
    if ( ! $renewal_order ) {
        $renewal_order = self::get_order_from_query_vars();
    }

    if ( $renewal_order ) {
           self::maybe_attach_stock_filters( $renewal_order );
    }
}

protected static function cart_contains_renewal_to_product( $product ) {
       $cart_contains_renewal_to_product = false;
       $renewal_order = WCS_Renewal_Cart_Stock_Manager::get_order_from_cart();

       if ( ! $renewal_order ) {
           $renewal_order = WCS_Renewal_Cart_Stock_Manager::get_order_from_query_vars();
       }

       if ( $renewal_order && wcs_order_contains_product( $renewal_order, $product ) ) {
           $cart_contains_renewal_to_product = true;
       }

       return $cart_contains_renewal_to_product;
}

[Lipppitt]

Actually ignore me! I failed to see the filter hook woocommerce_subscriptions_disable_manual_renewal_stock_validation which by the looks of initialises the correct renewal class to override the stock status. Not sure how I managed to get it working in my example above.

I haven't tested the hook yet but it should work by the sounds of it. Out of curiosity, is there a reason why this is set to false by default?

[james-allan]

Hi @Lipppitt. 👋

Given your latest message I presume this isn't an issue any more but to respond to your questions and comments:

For example, the method cart_contains_renewal_to_product attempts to get the order from cart for the specific product and does this by calling static::get_order_from_cart() or static::get_order_from_query_vars(). This issue here is there are multiple static methods with that name in the codebase and it's resulting in the incorrect method being called.

I don't believe that is how self:: vs static:: works. Calling a function with self and static is almost indicical in how it works. static::get_order_from_cart() won't call any statically defined function of that name. How they differ is only when the class is extended.

If for example someone extended WCS_Renewal_Cart_Stock_Manager, because we're using static it means we ensure that if the child class overrides any function, we will call the child version of that function. If we were to use self, it would always call the parent class would always use the parent defined function and child classes could never override those function.

Out of curiosity, is there a reason why this is set to false by default?

I don't recall the decision behind that with the greatest detail but I assume that we intended bypassing stock checks when paying for renewal orders via the cart to be something folks opt into rather than being the default behaviour. Stock checks being part of manual renewals has been the way it has worked since inception and unless we consider it to be a bug we don't lightly change existing behaviour on our users.

I haven't tested the hook yet but it should work by the sounds of it.

I just tested this flow using the hook and there does appear to be a bug in it somewhere. It works up until the payment step. When I attempt to pay for a checkout, it fails with a "{product}" is out of stock and cannot be purchased. error message. From what I've gathered, there's a gap in our code that still allows WC to validate the stock right after the order has been processed and that halts the checkout.

I'm having to move onto other issues now, but for completeness this is the stack trace that leads to the error message:

#0 /wp-content/plugins/woocommerce/includes/wc-stock-functions.php(365): Automattic\WooCommerce\Checkout\Helpers\ReserveStock->reserve_stock_for_order(Object(Automattic\WooCommerce\Admin\Overrides\Order))
#1 /wp-includes/class-wp-hook.php(308): wc_reserve_stock_for_order(Object(Automattic\WooCommerce\Admin\Overrides\Order))
#2 /wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters('', Array)
#3 /wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#4 /wp-content/plugins/woocommerce/includes/class-wc-checkout.php(463): do_action('woocommerce_che...', Object(Automattic\WooCommerce\Admin\Overrides\Order))
#5 /wp-content/plugins/woocommerce/includes/class-wc-checkout.php(1256): WC_Checkout->create_order(Array)
#6 /wp-content/plugins/woocommerce/includes/class-wc-ajax.php(508): WC_Checkout->process_checkout()
#7 /wp-includes/class-wp-hook.php(308): WC_AJAX::checkout('')
#8 /wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters('', Array)
#9 /wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#10 /wp-content/plugins/woocommerce/includes/class-wc-ajax.php(96): do_action('wc_ajax_checkou...')
#11 /wp-includes/class-wp-hook.php(308): WC_AJAX::do_wc_ajax('')
#12 /wp-includes/class-wp-hook.php(332): WP_Hook->apply_filters(false, Array)
#13 /wp-includes/plugin.php(517): WP_Hook->do_action(Array)
#14 /wp-includes/template-loader.php(13): do_action('template_redire...')
#15 /wp-blog-header.php(19): require_once('/Users/jamesall...')
#16 /index.php(17): require('/Users/jamesall...')
#17 {main}

From what I can gather, WC is hooked onto woocommerce_checkout_order_created at priority 10 and reserving the stock. It's probably worth looking into using a new hook woocommerce_hold_stock_for_checkout to bypass WC core's checkout stock validation while paying for the renewal order.

eg

add_filter( 'woocommerce_hold_stock_for_checkout', array( get_called_class(), 'bypass_stock_validation_checkout' ) );

public static function bypass_stock_validation_checkout( $managing_stock ) {
    if ( ! $managing_stock ) {
        return $managing_stock;
    }

    $renewal_order = static::get_order_from_cart();

    return $renewal_order ? false : $managing_stock;
}

[Lipppitt]

Hi @james-allan 👋

Thanks for getting back to me!

I don't believe that is how self:: vs static:: works. Calling a function with self and static is almost indicical in how it works. static::get_order_from_cart() won't call any statically defined function of that name. How they differ is only when the class is extended.

You're right, that was due to my lack of knowledge around Late Static Bindings :) You learn something new everyday!

I just tested this flow using the hook and there does appear to be a bug in it somewhere. It works up until the payment step. When I attempt to pay for a checkout, it fails with a "{product}" is out of stock and cannot be purchased. error message. From what I've gathered, there's a gap in our code that still allows WC to validate the stock right after the order has been processed and that halts the checkout.

I've just tested this flow too and can confirm I had the same issue. However, adding the custom hook woocommerce_hold_stock_for_checkout like you suggested to the WCS_Renewal_Cart_Stock_Manager class successfully bypasses this.

I have now gone through the flow and can pay for the renewal order even if that item has a stock quantity of 0. The reason this sprung up was due to a customers card expiring and the subscription failing. When the customer added a new payment method via their account page and set it to 'default', it didn't update the subscription. Therefore, I thought the customer needed to manually pay for the renewal order using the new card for it to update on the subscription. However, after going through this process now it seems the subscription is still using the old expired card ,even though their latest renewal order was paid for using the new card.

How can the customer set the new card on their subscription for all future payments? I can see the new card is registered in Stripe and is set to default both in Stripe and on the website. I thought there might be a flag in the subscription wp_postmeta table where i can override the payment_token ID but all i can see are the _stripe_customer_id and _stripe_source_id flags.

Any help would be much appreciated!

[james-allan]

When the customer added a new payment method via their account page and set it to 'default', it didn't update the subscription.

There is a way customers can select a default and it update their existing subscriptions. After you click the "Make default" button it should pop up with this option to update their existing subscriptions.

Selecting 'Yes' here should have updated their subscription.

Therefore, I thought the customer needed to manually pay for the renewal order using the new card for it to update on the subscription. However, after going through this process now it seems the subscription is still using the old expired card ,even though their latest renewal order was paid for using the new card.

Paying for a failed renewal using a new payment method should update the subscription. IIRC this might not work for some payment gateways. By the sounds of it you're using Stripe and I'd expect it would work for Stripe.

For reference this is the function that should handle that: WC_Subscriptions_Change_Payment_Gateway::change_failing_payment_method()

The stripe plugin has a function called WC_Stripe_Subscriptions_Trait::update_failing_payment_method()