Performance improvements: rendering the Subscription Relationship column and checking if order is a renewal, switch etc. #732
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,21 +68,63 @@ function wcs_get_subscriptions_for_order( $order, $args = array() ) { | |
|
|
||
| $all_relation_types = WCS_Related_Order_Store::instance()->get_relation_types(); | ||
| $relation_types = $get_all ? $all_relation_types : array_intersect( $all_relation_types, $args['order_type'] ); | ||
| $subscription_ids = wcs_get_subscription_ids_for_order( $order, $relation_types ); | ||
|
|
||
| foreach ( $subscription_ids as $subscription_id ) { | ||
| if ( wcs_is_subscription( $subscription_id ) ) { | ||
| $subscriptions[ $subscription_id ] = wcs_get_subscription( $subscription_id ); | ||
| } | ||
| } | ||
|
|
||
| return $subscriptions; | ||
| } | ||
|
|
||
| /** | ||
| * Get the subscription IDs for an order. | ||
| * | ||
| * @param WC_Order $order The order to get the subscription IDs for. | ||
| * @param string|array $order_types The order types to get the subscription IDs for. | ||
| * | ||
| * @return array The subscription IDs. | ||
| */ | ||
| function wcs_get_subscription_ids_for_order( $order, $order_types = [] ) { | ||
| $subscription_ids = []; | ||
|
|
||
| if ( ! is_a( $order, 'WC_Abstract_Order' ) ) { | ||
| $order = wc_get_order( $order ); | ||
| } | ||
|
|
||
| if ( ! wcs_is_order( $order ) ) { | ||
| return $subscription_ids; | ||
| } | ||
|
|
||
| if ( ! is_array( $order_types ) ) { | ||
| $order_types = [ $order_types ]; | ||
| } | ||
|
|
||
| $valid_order_types = array_intersect( WCS_Related_Order_Store::instance()->get_relation_types(), $order_types ); | ||
|
|
||
| foreach ( $valid_order_types as $order_type ) { | ||
| $subscription_ids += WCS_Related_Order_Store::instance()->get_related_subscription_ids( $order, $order_type ); | ||
| } | ||
|
|
||
| // An order cannot be both a renewal, switch or resubscribe as well as a parent order, so only fetch subscription IDs if we didn't find any in the related order store. | ||
|
There was a problem hiding this comment. I don't think we can make this assumption. If I resubscribe to a cancelled subscription, that order is a parent to the new subscription and a resubscribe to the old subscription. wcs_get_subscription_ids_for_order( $resubscribe_order, [ 'resubscribe', 'parent' ] )This should return 2 results, the cancelled subscription and the new one. In it's current form, it would just return the resubscribe. I understand the intention is to avoid unnecessary expensive queries. I think callers if they were being super performance conscious, they would be be better off doing something like this: $related_subscriptions = wcs_get_subscription_ids_for_order( $order, [ 'resubscribe', 'renewal', 'switch' ] );
// If the subscription contains a subscription
if ( ! empty( $related_subscriptions ) || wcs_is_parent_order( $order ) {
... ie separate the related order relations from the parent check. There was a problem hiding this comment. Thanks for flagging this! I've fixed this in 5f237ab |
||
| if ( empty( $subscription_ids ) && in_array( 'parent', $order_types, true ) ) { | ||
| $subscription_ids = wc_get_orders( | ||
| [ | ||
| 'parent' => $order->get_id(), | ||
| 'type' => 'shop_subscription', | ||
| 'status' => 'any', | ||
| 'limit' => -1, | ||
| 'return' => 'ids', | ||
| 'orderby' => 'ID', | ||
| ] | ||
| ); | ||
| } | ||
|
|
||
| return $subscription_ids; | ||
| } | ||
|
|
||
| /** | ||
| * Copy the billing, shipping or all addresses from one order or subscription to another. | ||
| * | ||
|
|
@@ -365,10 +407,7 @@ function wcs_order_contains_subscription( $order, $order_type = array( 'parent', | |
| $contains_subscription = false; | ||
| $get_all = in_array( 'any', $order_type, true ); | ||
|
|
||
| if ( ( in_array( 'renewal', $order_type, true ) || $get_all ) && wcs_order_contains_renewal( $order ) ) { | ||
| $contains_subscription = true; | ||
|
|
||
| } elseif ( ( in_array( 'resubscribe', $order_type, true ) || $get_all ) && wcs_order_contains_resubscribe( $order ) ) { | ||
|
|
@@ -377,6 +416,8 @@ function wcs_order_contains_subscription( $order, $order_type = array( 'parent', | |
| } elseif ( ( in_array( 'switch', $order_type, true ) || $get_all ) && wcs_order_contains_switch( $order ) ) { | ||
| $contains_subscription = true; | ||
|
|
||
| } elseif ( ( in_array( 'parent', $order_type, true ) || $get_all ) && wcs_is_parent_order( $order ) ) { | ||
| $contains_subscription = true; | ||
| } | ||
|
|
||
| return $contains_subscription; | ||
|
|
@@ -1055,3 +1096,29 @@ function wcs_set_recurring_item_total( &$item ) { | |
| ] | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if an order is a Subscriptions parent/initial order. | ||
| * | ||
| * @param WC_Order|int $order The WC_Order object or ID of a WC_Order order. | ||
| */ | ||
| function wcs_is_parent_order( $order ) { | ||
| $order = ! is_object( $order ) ? wc_get_order( $order ) : $order; | ||
|
|
||
| if ( ! $order || ! wcs_is_order( $order ) ) { | ||
| return false; | ||
| } | ||
|
|
||
| // Check if the order ID is the parent of a subscription. | ||
| $is_parent_order = wc_get_orders( | ||
| [ | ||
| 'parent' => $order->get_id(), | ||
| 'type' => 'shop_subscription', | ||
| 'status' => 'any', | ||
| 'limit' => 1, | ||
| 'return' => 'ids', | ||
| ] | ||
| ); | ||
|
|
||
| return apply_filters( 'woocommerce_subscriptions_is_parent_order', ! empty( $is_parent_order ), $order ); | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,20 +21,8 @@ | |
| * @since 1.0.0 - Migrated from WooCommerce Subscriptions v2.0 | ||
| */ | ||
| function wcs_order_contains_resubscribe( $order ) { | ||
mattallan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| $related_subscription_ids = wcs_get_subscription_ids_for_order( $order, 'resubscribe' ); | ||
| return apply_filters( 'woocommerce_subscriptions_is_resubscribe_order', ! empty( $related_subscription_ids ), $order ); | ||
|
There was a problem hiding this comment. This applies to all the Unfortunately I think this means we will need to return to the if ( ! is_a( $order, 'WC_Abstract_Order' ) ) {
$order = wc_get_order( $order );
}There was a problem hiding this comment. Good catch @james-allan! There was a problem hiding this comment. Just pushed up d492796 to address this! Going to run through some more tests on these latest changes |
||
| } | ||
|
|
||
| /** | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the intention behind the empty
[]$order_typesparam?Passing an empty array returns no results. Is it supposed to be interpreted as all order types?
It's probably worth detailing this in the function block param comment too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @james-allan. I think it makes sense for us to return 'any' subscription IDs linked to the given order so I've just implemented the any param here cd7ac49.
I also added these default details to the docblock (see 4051a4c)