Skip to content
This repository was archived by the owner on May 21, 2025. It is now read-only.

Remove unnecessary renewal order paid date update #792

Merged

Conversation

james-allan
Copy link
Contributor

@james-allan james-allan commented Feb 21, 2025

Fixes #791

Description

Back when we were working on WC 2.7 (which later became WC 3.0) we added some code to the WC_Subscriptions_Renewal_Order::maybe_record_subscription_payment() function which would set the renewal order's paid date to the current time.

Based on the original commit message (515c8e2) this was necessary because the paid date was set inconsistently as it required the gateways to call the payment_complete() function.

This is no longer necessary as WooCommerce core sets the paid date on status transition. See WC_Order::set_status()

Given the maybe_record_subscription_payment() function is hooked onto a status transition hook, it's really not necessary for us to set the paid date.

This PR:

  1. Removes WC version compatibility code that is no longer needed (pre 3.0).
  2. Removes the $order->set_date_paid( $current_time ) call when a renewal order status is changed.

Given we're no longer explicitly setting the paid date on a renewal order I did a review of the places where we use the paid date.

  • There are some in the switch calculations where it gets the subscription's last order paid date.
    • This wouldn't be affected given the paid date is still set, it just won't be overridden if an order is reset back to unpaid and than paid.
  • You can call $subscription->get_date( 'last_order_date_paid' ) and it will return the paid date of the most recent order, however I wasn't able to find any cases where we do that.

How to test this PR

  1. Purchase a subscription.
  2. From the edit subscription admin screen process a renewal from the actions dropdown.
  3. Edit the order and make a note of the paid date.

Screenshot 2025-02-21 at 5 45 20 pm
Renewal order was paid @ 5:45 pm

  1. Mark the order as on-hold
    • On the issue this is described as the order was disputed.
  2. Mark the order as processing or completed.
  3. On trunk the renewal order paid date would be updated.

Screenshot 2025-02-21 at 5 51 03 pm
Renewal order was paid @ 5:50 pm

  1. On this branch the order paid date should not be updated.

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

WooCommerce core now handles setting the order's paid date when the
order status is set. Because the maybe_record_subscription_payment()
function is hooked onto the order status transition, it's not
necessary for us to set this date too.
@james-allan james-allan changed the title Issue/791 only set paid date on renewal when needed Remove unnecessary renewal order paid date update Feb 24, 2025
@james-allan james-allan requested review from a team and mattallan and removed request for a team March 4, 2025 23:44
Copy link
Contributor

@mattallan mattallan left a comment

Choose a reason for hiding this comment

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

Thanks @james-allan for working on this.

From code review I spotted a couple of small issues that need to be addressed before merging.

Thinking about the changes, not updating the paid_date() on every payment completed status transition makes complete sense to me, especially given the context of disputed payments. If a payment is disputed and then marked back as processing, I don't necessarily think the paid date should be reset as that effectively makes switch calculations act as it was paid later than it was.

I tested the changes and confirmed that the paid date was not updated on the renewal order when updating it from on-hold to processing. LGTM

@james-allan james-allan merged commit 86e24e6 into trunk Mar 19, 2025
9 checks passed
@james-allan james-allan deleted the issue/791-only-set-paid-date-on-renewal-when-needed branch March 19, 2025 06:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch use of WC_Order::set_date_paid for WC_Order::maybe_set_date_paid in maybe_record_subscription_payment()
2 participants