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

Increase number of args supported by wcs_get_subscriptions() #811

Merged
merged 4 commits into from
Mar 27, 2025

Conversation

barryhughes
Copy link
Member

@barryhughes barryhughes commented Mar 11, 2025

To date, wcs_get_subscriptions() has supported only a limited set of arguments (some that are specific to Subscriptions queries, and others that map directly to their WC_Order_Query equivalents). In the linked issue, which this PR tries to satisfy, it was decided that we should pass any and all arguments across to WooCommerce. This should make it possible to:

  • Take better advantage of current and future order query capabilities provided by WooCommerce Core
  • Increase the potential to reuse code (reuse the same set of arguments)
  • Allow extensions to enhance query capabilities for both orders and subscriptions

Fixes #4678

⛑️ How can this code break?

Before now, passing something like date_created to wcs_get_subscriptions() would have had no effect by default, but it's not impossible that an extension/custom code implemented support. With this change, we are implicitly add support for date_created (and other order query args), and that could lead to a hard-to-predict conflict of some kind. My instinct is that this risk is low, however.

🧪 What are we doing to make sure this code doesn't break?

There is already a pretty large battery of unit tests covering different wcs_get_subscriptions() queries. Those should provide us with some confidence, and this PR also increases the total number of assertions.

How to test this PR

This is primarily a developer-facing change and so, beyond reviewing the change, some freestyle exploration (and comparison) of queries made via wcs_get_subscriptions(), with and without this change, will be valuable.

Beyond that, you may wish to consider running through flows that involve subscription queries made using wcs_get_subscriptions(). I've detailed two such scenarios below, to help verify that we are not introducing any new breakages, though please feel free to look into others:

🚏 Subscription Limitations

  • Create a variable subscription product.
    • Via the Advanced tab (within the product meta box), set the limit subscription field to limit to one active subscription.
  • As a customer, purchase a specific tier of that product.
  • Still logged in as the same customer, revisit the product page. You should see a warning that, "You have a subscription to this product. Choosing a new subscription will replace your existing subscription. "
  • Continue, selecting a different tier/variant and then checkout. As the message suggested, this should result in a switch to your existing subscription.

🎭 Anonymizing Ended Subscriptions

Tip

For this next test, enable HPOS but disable Compatibility Mode. You can also test without HPOS (though, you will need to modify the steps provided here), but again you should ensure Compatibility Mode (Sync) is disabled. This is to avoid some weird side-effects of the sync process impacting your tests.

  • Visit the WooCommerce ‣ Settings ‣ Accounts & Privacy admin screen.
  • Locate the personal data retention area, and set retain ended subscriptions to 1 year then save your changes.
  • Acting as a customer, purchase a new subscription.
  • Then, as the merchant, locate the same subscription and cancel it.
  • Via the database, update the end date (we want to set it to a past date, that makes it a valid target for the anonymization tool). You can use WP CLI to do this if you like. Example (assuming HPOS is enabled):
# Replace <SUB_ID> with the actual subscription ID
wp db query "UPDATE $(wp db prefix)wc_orders_meta SET meta_value = '2023-01-01 12:00:00' WHERE order_id = '<SUB_ID>' AND meta_key = '_schedule_end'"
  • Again working with WP CLI, run the anonymization tool (normally this would happen via a sequence of scheduled actions but, for expediency, we'll invoke it directly):
wp eval "(new WCS_Privacy_Background_Updater)->anonymize_ended_subscriptions();"
  • Still acting as the merchant, visit the subscription editor and verify that the subscription has been anonymized:
Anonymized subscription, as seen via the subscription editor screen

Product impact

@barryhughes barryhughes changed the title Accept and pass arbitrary query args across to WC_Order_Query. Increase number of args supported by wcs_get_subscriptions() Mar 11, 2025
@barryhughes barryhughes marked this pull request as ready for review March 11, 2025 22:21
@barryhughes barryhughes requested review from a team and mattallan and removed request for a team March 11, 2025 22:21
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.

Sorry @barryhughes for taking so long to get to this PR, but really nice work on this one! 💯

The code changes are straightforward and I don't see any issues being introduced here. Since this function is widely used I've checked/tested the following:

  • Ensure the new $working_args param sent to woocommerce_get_subscriptions_query_args filter doesn't break compatibility.
  • Ensure the new $working_args param sent to woocommerce_got_subscriptions filter doesn't break compatibility.
  • Smoke tested Subscriptions + some specific popular merchant flows that result in calling wcs_get_subscriptions()
    • Marking a parent order as on-hold, puts the related subscription on-hold.
    • Paying for manually created parent order linked to a subscription (searches for a linked subscription to delete it)
  • Checked Stripes use of wcs_get_subscriptions()
  • Check WooPayments use of wcs_get_subscriptions()
  • Check Gifting use of wcs_get_subscriptions()
  • Checked APFS use of wcs_get_subscriptions()
  • Checked PayPal Payments use of wcs_get_subscriptions()
  • Test/check uses of wcs_get_subscriptions_for_order() (this function is another common public function which can also lead to wcs_get_subscriptions())

Testing some of the new changes:

  • Tested using 'status' arg instead of 'subscription_status'
  • wcs_get_subscriptions( [ 'currency' => 'CAD' ] ); (previously returned all subscriptions)

How can this code break?

Great call out! I would be surprised if this negatively impacts anyone, but I have been surprised recently so... 🤷 I don't think it's worth mitigating against this as it seems pretty edge case.

@barryhughes
Copy link
Member Author

Nice, thank you!

@barryhughes barryhughes merged commit eaa119c into trunk Mar 27, 2025
9 checks passed
@barryhughes barryhughes deleted the fix/4678-query-args branch March 27, 2025 14:11
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.

2 participants