Clear post cache after updating menu order for products.

[barryhughes]

When Advanced Post Caching is in effect, changes made to product menu order appear not to take effect because the cached results (for the posts query) are not invalidated.

Closes #31389.

How to test the changes in this Pull Request:

Please also refer to the linked issue which contains a good screencast and some solid testing instructions.

1. Start by setting up an environment where you can replicate the problem described in the linked issue:
   • You can achieve this by testing on an Atomic site.
   • Or, to test locally, grab a copy of Advanced Post Cache as used on WP.com (internal conversation p1659562709451669/1659534670.204749-slack-C0E1AV8T0 contains a link where you can grab a copy) and set it up as a mu-plugin. You will also need persistent object caching (Redis/Memcached) to be in place.
2. Navigate to Products ▸ Categories and pick a category associated with 1 or more products. Click on the link in the Count column.
3. You should now find yourself looking at a list table of products belonging to that category. Open the Sorting view.
4. Try reordering the products, then refresh the page. The change should persist.

Demo

Here's how things work without the fix (note how after reordering the products I click on Sorting to refresh the page, and the order reverts to how it was at the start of the video):

withoutfix.mov

And here is how things work with the fix. This time, on refreshing, the new order persists:

withfix.mov

FOR PR REVIEWER ONLY:

• I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

[barryhughes]

Some notes on performance... ✍🏽

Switching from $wpdb->update() to wp_update_post() also solves this, because that takes care of invalidating various caches, etc, but with a large catalog the performance impact is notable (and I assume that is why we chose to use direct queries in the first place). For that reason, I kept the $wpdb->update() approach.

Similarly, to keep things snappy, I opted only to call clean_post_cache() if the post row was actually changed (again, especially with a large catalog, the majority of rows will not be updated so calling it for each product seemed wasteful).

[github-actions]

Test Results Summary

Commit SHA: c5001f2

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	115	0	0	2	0	117	0m 43s
E2E Tests	185	0	0	1	0	186	17m 25s

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.

[bazza]

Using direct SQL and bypassing core APIs is always going to cause problems. You mentioned it's done for performance reasons, but did you compare the performance of the two after adding clean_post_cache() ? You also mentioned there is a "noticeable" difference, but do you have any specific data to share? Maybe Core WordPress needs to provide a better (i.e. more performant) API to update multiple posts in bulk? If so, that would be a good thing to propose to the Core team.

[barryhughes]

Thanks for the question, @bazza.

Switching from $wpdb->update() to wp_update_post() also solves this

I should start by acknowledging that, by itself, this is a very naive approach (because, if we literally just replace the existing $wpdb->update() calls with wp_update_post() calls then, with for example a catalog of 500 products, it ensures there will be updates to 500 rows whether or not the menu_order actually changes—because post_modified etc will still be bumped).

So, a better strategy would be to check if the menu order even needs to be modified before updating (ie, perform a check along the lines of get_post_field( 'menu_order', $id ) !== $new_menu_order, which is the approach I'll use from here).

Do you have any specific data to share?

Just from local testing (with a catalog of 500 products, and where re-ordering triggers an update to 4 rows, and where I perform 5 runs per test-case to get an average):

Strategy	Caching	Time spent (ms)
wp_update_post()	Obj Cache + APC	1199
wp_update_post()	None	980
$wpdb->update()	Obj Cache + APC	101
$wpdb->update()	None	116

Did you compare the performance of the two after adding clean_post_cache() ?

Yes, it was a wash. $wpdb->update() by itself averages out at virtually the same as when we follow-up with a call to clean_post_cache() (so long as we only do it when rows were modified). Hence, I included just one set of results in the table above.

---

From there:

• We could be more thorough in our benchmarking, 5 runs on a catalog of the same size within my local dev env isn't conclusive. Though, I think it's a good enough indicator.
• The current algorithm examines every product in the catalog; perhaps it could be optimized—but I don't know if that's worth focusing on right now, if we have a ready-to-go fix that works. Though we could follow-up later if required.

[jorgeatorres]

This looks good to me.

While I agree that ideally we would have access to some WP functions to performantly update the menu order in bulk, the approach here not only fixes the issue but does so with minimal impact, as opposed to changing the code to use wp_update_post().

Maybe we can add an issue to review some of these places where we've deviated from the "official" way of doing something for performance (or other) reasons and either re-work the code then or propose new APIs to upstream.

Note: I don't currently see any good reason not to merge this as-is, but will hold off on doing so for a few days, just in case we want to discuss further.

[barryhughes]

@bazza let me know if any other questions/concerns on this one.

[beaulebens]

Maybe we can add an issue to review some of these places where we've deviated from the "official" way of doing something for performance (or other) reasons and either re-work the code then or propose new APIs to upstream.

Agreed -- can we please create an issue to review anywhere we're doing this sort of query, so that we can propose improvements to WordPress Core that would enable us to use official APIs vs direct queries?

[github-actions]

Hi @barryhughes, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

• Add the release: add testing instructions label