search

Automattic / woocommerce-subscriptions-core

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

woocommerce_subscription_renewal_payment_failed is called from BOTH woocommerce-payments and woocommerce-subscriptions plugins #628

Open rlmajcom opened 1 month ago

rlmajcom commented 1 month ago

Describe the bug

The woocommerce_subscription_renewal_payment_failed is called from BOTH woocommerce-payments and woocommerce-subscriptions plugins

To Reproduce

Use Woo Payments and Woo Subscriptions Use default Automatic payment retry system for Woo Subcriptions Order notes will always show: Retry rule applied: Order status changed from Failed to Pending payment FOLLOWED IN THE SAME SECOND BY Retry rule reapplied: Order status changed from Failed to Pending payment.

class-wc-retry-manager.php shows: add_action( 'woocommerce_subscription_renewal_payment_failed', array( CLASS, 'maybe_apply_retry_rule' ), 10, 2 ); add_action( 'woocommerce_subscription_renewal_payment_failed', array( CLASS, 'maybe_reapply_last_retry_rule' ), 15, 2 );

Logging within those functions shows the following unexpected results: maybe_apply_retry_rule is Exiting Early because is_scheduled_payment_attempt() is false and then maybe_reapply_last_retry_rule is Running Unconditionally: As a consequence of maybe_apply_retry_rule exiting early, maybe_reapply_last_retry_rule is running without a valid last retry

Note that the payment retry system DOES ultimately seem to work but it is causing this double-firing 100% of the time with all Scheduled Automatic Payment retries.

Expected behavior

Payment retry should not be triggered twice in the same second by both plugins

Actual behavior

Look at the nearly identical code in public function payment_failed( $new_status = 'on-hold' ) found in both plugins: wp-content/plugins/woocommerce-payments/vendor/woocommerce/subscriptions-core/includes/class-wc-subscription.php wp-content/plugins/woocommerce-subscriptions/vendor/woocommerce/subscriptions-core/includes/class-wc-subscription.php

Hypothesis: It may be that the payment_failed() function in the woocommerce-payments plugin is redundant in the context of WooCommerce Subscriptions Automatic Payment Retry System

Product impact

woocommerce-payments and woocommerce-subscriptions plugins

Additional context

james-allan commented 1 month ago

@rlmajcom your theory about WooPayments and WooCommerce Subscriptions both firing that hook is is unlikely in my opinion. In your issue description you mentioned that woocommerce-payments/vendor/woocommerce/subscriptions-core and /woocommerce-subscriptions/vendor/woocommerce/subscriptions-core are identical, and that is true and expected.

That code in WooPayments is a subset of Subscriptions functionality and was packaged in for what is now a sunset feature of WooPayments. That code won't be running when the WooCommerce Subscription plugin is active. If it were, you'd be seeing a lot of "class already exist" kind of errors.

So to go back to the original issue, you're essentially reporting that there are double order notes and double retry attempts for each payment failure. Is that correct?

The function which adds that order note and schedules the rety job is hooked onto the woocommerce_subscription_renewal_payment_failed hook so what I suspect is happening is the order transition from pending -> failed is actually running twice (potentially on 2 different instances of the same order), and that is causing this function to also run twice.

We'll need to confirm that so I've added the bug-needs-confirmation label.

rlmajcom commented 1 month ago

@james-allan I had put some error_log messages in the maybe_reapply_last_retry_rule to see what the values were when a scheduled Automatic Payment Retry occured (see below)

In case it helps: Note that the maybe_reapply_last_retry_rule does NOT run if I manually retry a renewal payment from within admin, it only does this double-firing when the Automatic Payment Retry system triggers a payment attempt.

Here is what was reported each time by the maybe_reapply_last_retry_rule in the my error_logs: --- Entering maybe_reapply_last_retry_rule --- is_scheduled_payment_attempt(): false Last Retry Status: N/A Last Retry Rule: N/A Object Type: order New Status: pending Object Has Status: false Updating object status to: pending Object Type: subscription New Status: on-hold Object Has Status: true --- Exiting maybe_reapply_last_retry_rule ---;