Refactor get_recurring_cart_key() so it's available for grouping products into subscriptions

[james-allan]

Description

This PR refactors the get_recurring_cart_key() function so the functionality which groups subscription products/cart items into subscriptions with the same billing terms can be utilised in grouping order items into subscriptions. This change is necessary for a new REST API endpoint that will create subscriptions from parent order line items. See https://github.com/woocommerce/woocommerce-subscriptions/issues/4562

How to test this PR

This PR should result in no direct change/impact.

1. Add various subscription products with different billing schedules to your cart. eg
   • Period - weekly, monthly, daily, yearly
   • Intervals - every 2nd x, every 4th y
   • Free trials
   • Lengths/end dates
   • Synced
2. Confirm that subscription products are grouped correctly on the cart and checkout pages into recurring carts.

Product impact

• [x] 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

[james-allan]

Regarding your feedback about there being 2 filters. That's basically by necessity. The existing hook (woocommerce_subscriptions_recurring_cart_key) passes a cart item. Callbacks are able to fetch product data and cart context from that cart item to assist with generating a unique cart item key.

The new hook introduced in this PR, doesn't have access to a cart so cannot provide that kind of context. With that in mind the, two filters are basically incompatible with each other - 1 cannot be used to replace both. The only solution to resolve the two filter issue would be to deprecate the old one but there wouldn't be a backwards compatible hook.

To give you an example using the two features you mentioned - Gifting and Syncing.

Gifting requires the cart context. A product on it's own doesn't contain enough context to enable Gifting to separate gifted products in a cart by recipient. Being passed the cart item, it can locate the recipient cart item information and append it to the recurring cart key.

Sycning is product related - it doesn't require any cart information at all to be able to generate a key unique for a product so synced products can be grouped by the same key.

Deprecating the old filter would break Gifting with no alternative - it needs the cart item context. Not introducing this new hook breaks this new functionality because we do want to be able to split order items into subscription groups based on the product without a cart. eg synced or not synced.

There's no way around that other than the two hooks being separate. ie the two hooks wouldn't run in series, the cart one would run on the cart and the product one would run for the REST API context. That would resolve the two other issues you mentioned about the order of the key changing, and the risk if folks were unhooking callbacks etc.

I'm happy to take this approach but you mentioned you weren't keen on the two function approach, is there a reason behind that?

I couldn't find any hints as to why this hook needed to be on the earliest priority but maybe you might know.

I can't imagine it was hooked on early for any specific reason. The hook is used to generate a unique key by appending info to the string. So unless someone is splitting the key and expecting the order of the key to be something specific, the order that functions are hook onto this filter is irrelevant.

[mattallan]

Thanks @james-allan for sharing your thoughts! Given there are some extensions like gifting that rely on cart item data and this new API endpoint requiring the product/order item, it looks like having just the one hook is never going to be possible (like I was hoping)

I'm happy to take this approach but you mentioned you weren't keen on the two-function approach, is there a reason behind that?

I don't recall exactly what I was thinking at the time of writing that comment but I think my original comment was an initial impression on having developers hook on twice for theoretically the same thing and I was probably hoping we could find a way to just have the 1 hook.

Taking into account the things you mention, I would say I'm leaning more towards having two separate hooks as that gives the most flexibility without functional downsides aside. I guess with two hooks, if the customisation only needs the product then they could get away with having one function that is hooked onto the two hooks.

[james-allan]

@mattallan following our conversations about this I've done the following:

1. I've restored the hook used for cart items so Gifting can still use the cart item data to filter the recurring cart key.
2. I've kept the filter in the helper function wcs_get_subscription_grouping_key()
3. I've introduced a new function that is currently not being used but will be used by the rest API to generate a key. This new function has a hook similar to the cart item one, but for order items. woocommerce_subscriptions_item_grouping_key.
4. I've updated the synchroniser class to make sure it uses the cart and order item hooks. The synchroniser could be changed to just use the product hook but because of the order of the key, this approach has been taken to maintain the existing order of the key attributes.

As it stands there are 3 filters with different use cases.

1. woocommerce_subscriptions_recurring_cart_key - this is the OG hook and is unchanged. It's designed to be used by plugins like Gifting to ensure cart items with different recipients are separated into different subscriptions.
2. woocommerce_subscriptions_item_grouping_key - this is the equivalent of the cart item one (above) but for order items. Its second param is an order item, rather than a cart item.
3. wcs_subscription_product_grouping_key - this is a new hook and sits at the base of the two filters mentioned above. Rather than having a cart item or order item as it's context, it passes a product as the second param. This will be useful for plugins or code which don't have a specific need to filter the specific cart or order item context as it only requires a product to determine/alter the recurring cart item/subscription grouping key. Syncing could use this for example but isn't for backwards compatibility reasons.