Add a filter to OrdersTableQuery to allow overriding of HPOS queries

[lsinger]

Submission Review Guidelines:

• I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have reviewed my code for security best practices.
• Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

This PR adds a filter to the OrdersTableQuery class to allow for the overriding of HPOS queries.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

1. Create four orders and note their IDs.
2. Add a filter snippet to, for example, woocommerce.php, so that an HPOS-based order query will only return those IDs. Example:

$callback = function( $result, $query_object, $sql ) {
	// an array of the order IDs to return, the number of orders, and the number of pages
	// for this test, simply replace the two order IDs with the ones you noted down
	return array( array( 15, 18 ), 2, 1 );
};
add_filter( 'woocommerce_hpos_pre_query', $callback, 10, 3 );

3. Go to the order list screen and verify that all four orders are displayed.
4. Go to Settings / Features and activate HPOS
5. Go to the order list screen and verify that only the two orders returned by the callback are displayed.

[github-actions]

Test Results Summary

Commit SHA: d8abb70

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	259	0	0	2	0	261	0m 58s
E2E Tests	195	0	0	15	0	210	17m 59s

---

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

[github-actions]

Hi @vedanshujain,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[rrennick]

@lsinger Nice work. I noted a few things and will leave the approval for @vedanshujain .

[vedanshujain]

Looks good, added few more comments along with what Ron suggested.

[barryhughes]

Left a couple of (very) minor notes about the filter docblock. Beyond that, I'm curious if we should provide a mechanism for query overrides to share the SQL they use? To illustrate:

use Automattic\WooCommerce\Internal\DataStores\Orders\OrdersTableQuery;

add_filter( 'woocommerce_hpos_pre_query', function ( $override ) {
	// Assume I do some custom SQL here, before returning an alternative data-set.
	return [
		'orders' => [ /* some orders */ ],
	];
} );

$query = new OrdersTableQuery;

// I get back the default SQL, not the SQL used by the callback.
echo $query->request;

This probably isn't essential (there is no @property-read declaration for request), but I thought I'd ask the question.

[lsinger]

I'm curious if we should provide a mechanism for query overrides to share the SQL they use?

@barryhughes we could add that as an optional field in the $order_data array. Did you have a specific use case in mind? I'd like to make sure we understand what kind of usage we want to support before adding this.

Since it would be an additional optional return value, I also wonder whether this should we a separate discussion and PR?

[barryhughes]

Did you have a specific use case in mind?

No, not really—I just noticed that we can retrieve the query SQL (ie, via $query->request) and realized it would be incorrect in these situations, and therefore might be confusing for anyone who is using this property to do some debugging.

On taking a second look, I don't think we use the magic request property ourselves and, since we're inside the internal namespace, third parties probably shouldn't be using it anyway: let's move on, we can ignore this.

[barryhughes]

Looks good to me, not sure if we want final sign-off by one of the other reviewers. Otherwise, may be a good candidate for needs: analysis.

[lsinger]

final sign-off by one of the other reviewers

I re-requested another review from @vedanshujain as the HPOS DRI just for an extra level of thoroughness.

[vedanshujain]

LGTM, added couple of super minor comments. Another note is, we require more detailed testing instructions than provided in the PR, can you please update them with an example of the filter than can be added to snippets? I think a nice testing instruction set could go like:

1. Create couple of orders, note their ID.
2. Add a filter snippet so that order query only return those IDs.
3. Go to order list screen, and verify that only those orders are displayed.