Automattic / woocommerce-subscriptions-core

Subscriptions core package for WooCommerce
Other
80 stars 29 forks source link

Prevent potential infinite loops and duplicate subscription notes when processing subscription status transitions #597

Closed james-allan closed 2 months ago

james-allan commented 2 months ago

Fixes https://github.com/woocommerce/woocommerce-subscriptions/issues/4608

Description

Because of the way the WC_Subscription status transition is written, if you save the subscription during any one of the hooks fired during the status transition, you will get duplicate order note entries.

Screenshot 2024-04-08 at 11 43 39 am

This PR fixes that by bringing WC_Subscription::status_transition() in line with WC_Order::status_transition() which sets the status transition value to false as one of the first things it does. This will prevent subsequent object saves to try save the status transition a second time.

How to test this PR

  1. In WooCommerce > Settings > Emails make sure the Cancelled Subscription email is enabled.
  2. Go to the Admin Subscription list table.
  3. Cancel any active subscription you have.
  4. View the Subscription's order notes and notice there are 2 notes recording that status change.
  5. On this branch there should be only one note.

If you're unable to replicate using those steps above, perhaps try using this code snippet. I suspect it has something to do with the actions hooked in during the status transition.

add_action( 'woocommerce_subscription_status_updated', 'dup_order_notes_test' );

function dup_order_notes_test( $subscription ) {
    // Remove the action otherwise it would be a infinite loop. 
    remove_action( 'woocommerce_subscription_status_updated', 'dup_order_notes_test' );
    $subscription->update_meta_data( '_dummy_meta', true );
    $subscription->save();
}

With this code active I get 3 status transition notes.

Note in the code snippet that I had to unhook the function, this PR also fixes that infinite loop potential.

Product impact