fix(esp-sync): get inactive subscriptions with failed or pending orders

[dkoo]

All Submissions:

• Have you followed the Newspack Contributing guideline?
• Does your code follow the WordPress' coding standards and VIP Go coding standards?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes proposed in this Pull Request:

Fixes an issue where Newspack\Reader_Activation\Sync\WooCommerce::get_most_recent_cancelled_or_expired_subscription( $user_id ) fails to get subscriptions whose most recent order has a failed or pending status. This is because $subscription->get_date( 'last_order_date_completed' ) will only return a value if the subscription's most recent order has a completed status.

How to test the changes in this Pull Request:

1. As a new reader, purchase a subscription.
2. As an admin, edit the subscription and use the "Order actions" menu to create a pending renewal order for the subscription (this will put the subscription into "on hold" status"):

3. On release, using wp shell, run \Newspack\Reader_Activation\Sync\WooCommerce::get_contact_from_customer( $user_id ); to get the contact data for the order. Observe that the contact data does not contain any info about the on-hold subscription:

> \Newspack\Reader_Activation\Sync\WooCommerce::get_contact_from_customer( 127 );

= [
    "email" => "[email protected]",
    "metadata" => [
      "account" => 127,
      "registration_date" => "2025-01-07 22:40:54",
      "total_paid" => "",
      "membership_status" => "",
      "payment_page" => "",
      "payment_page_utm" => "",
      "sub_start_date" => "",
      "sub_end_date" => "",
      "cancellation_reason" => "",
      "billing_cycle" => "",
      "recurring_payment" => "",
      "last_payment_date" => "",
      "last_payment_amount" => "",
      "product_name" => "",
      "next_payment_date" => "",
    ],
    "name" => "Test Contact",
  ]

4. Check out this branch, start a new wp shell session, and confirm that \Newspack\Reader_Activation\Sync\WooCommerce::get_contact_from_customer( $user_id ); now contains info about the on-hold subscription:

> \Newspack\Reader_Activation\Sync\WooCommerce::get_contact_from_customer( 127 );
= [
    "email" => "[email protected]",
    "metadata" => [
      "payment_page" => "https://localhost/support/?utm_campaign=Newspack%202025&utm_content=testing",
      "payment_page_utm_campaign" => "Newspack 2025",
      "payment_page_utm_content" => "testing",
      "membership_status" => "on-hold",
      "sub_start_date" => "2025-01-07 22:40:55",
      "sub_end_date" => "",
      "billing_cycle" => "month",
      "recurring_payment" => "25.00",
      "last_payment_amount" => "25.00",
      "last_payment_date" => "2025-01-08 16:47:45",
      "next_payment_date" => "2025-02-07 22:40:55",
      "product_name" => "All Access - Monthly",
      "payment_page_utm_source" => "",
      "payment_page_utm_medium" => "",
      "payment_page_utm_term" => "",
      "cancellation_reason" => "",
      "total_paid" => "75.00",
      "account" => 127,
      "registration_date" => "2025-01-07 22:40:54",
    ],
    "name" => "Test Contact",
  ]

Other information:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes, as applicable?
• Have you successfully ran tests with your changes locally?

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1209109673133239

[leogermani]

It works.

I have pushed one commit. I noticed that if a subscription is set to on-hold, the Membership status would not get the "Ex-" prefix.

I have a small performance feedback:

While looping through the related orders, if we find one completed order, that would be enough. In the current implementation we'll always loop through all the orders, and call wc_get_order to all of them even if it's not necessary. It would be better to exit the loop when we find the first completed order.

And a nitpick. You don't need array_keys there, as get_related_orders also return the IDs in the values. example:

array(1) {
  [4788]=>
  int(4788)
}

[dkoo]

@leogermani 82f889a does a little refactoring to address your feedback—thanks!

[leogermani]

Now the change to add the all param is making woocommerce-subscriptions call wc_get_order for every order. So the gain in performance we had here was nulled by the fact that we are calling wc_get_order for all orders anyways. (see https://github.com/woocommerce/-unused-woocommerce-subscriptions/blob/bff84d85400f964c4ddfc048381db9bf7be400d7/includes/class-wc-subscription.php#L1836 )

We can just get the ids as before, and only fetch the order details as needed

[dkoo]

But now that we need to check the status of each order, we'd need to call wc_get_order() ourselves anyway if we don't use the all argument. Unless there's a way to get an order's status by order ID without fetching the whole object that I don't know about?

[dkoo]

Ah, I think I found a better way to do it. 882a2e6 uses WC_Subscription::get_payment_count() to check the number of completed orders for the subscription instead of fetching and iterating through all orders.

[dkoo]

b921675 should be safest

[dkoo]

@leogermani let me know if you have any other improvements to suggest!

[matticbot]

🎉 This PR is included in version 5.10.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀