Issue #12795 - alter orderby for menu_order to ensure unique ordering

[bobbingwide]

Description

When populating the Parent page: drop down in the Page Attributes settings the editor may perform multiple REST requests. The requested sort sequence is menu_order asc. For sites where there are more than 100 posts to be returned the results can vary, depending on the values of the menu_order on the posts.

• Some users get to see hardly anything at all - buildTermsTree returns only 1 result #20430
• Others get a load of duplicate entries - Parent page duplicated on huge use with menu_order in dropdown #12795.

Additionally the sequence of pages actually display is nothing like the list found in Quick Edit or in the Classic Editor.

The explanation ( documented in TRAC 46294 - https://core.trac.wordpress.org/ticket/46294 ) is that there's a MySQL bug that needs to be worked around by passing a unique value in the sort sequence.

• The unique field is the post ID.
• The post_title is used to ensure the sequence matches what's seen in Quick Edit.

While the solution for TRAC 46294 is still under development a workaround for Gutenberg is to hook into the posts_orderby filter.

When it's a REST request and the requested sort order is 'menu_order' then it is modified to include the additional fields, with post title coming before post ID. The sort sequence is assumed to be asc for all of the order by fields.

How has this been tested?

I tested this in my development environment where I previously noted the problem.

See #23637 for some details of the content being handled and screen captures showing the incorrect results still being obtained with that attempted fix, and the expected sequence from Quick Edit.

I don't believe desc is actually used when the requested order is menu_order. Not by core anyway.
If support were to be needed then a minor modification would be to replace 'asc' by $query->query['order'].

Screenshots

New results obtained with the additional sorting.

Types of changes

I implemented the code in lib/compat.php since it should only be temporary. When WordPress TRAC 46294 is resolved then it should no longer be necessary.

I tried an alternative solution to change the orderby in the requests being sent but that required WordPress core changes in the REST API to accept the additional sort fields and sort sequences.

Users may notice that the new sort order matches what they see in Quick Edit.

This change is not dependent upon a working solution for #13618, but it certainly does benefit from it.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR.

[youknowriad]

@TimothyBJacobs Is this something we want to land in Gutenberg as a temporary measure or should we defer to the trac ticket?

[TimothyBJacobs]

I don't personally have an issue with making a fix in Gutenberg first. But I think ideally someone from the Query component would weigh in on the actual fix, @boonebgorges. Or maybe @swissspidy?

[bobbingwide]

It's been 2 months since I submitted this PR. Can something be done to chivvy this along?
I know that there are conflicts with lib/compat.php.
But that should not prevent a decision being made.

[swissspidy]

I think the actual fix looks OK, but a unit test would be valuable here. Then we could easily also get this into WordPress core at the same time.

[swissspidy]

FWIW, to get feedback it's usually easiest to also ask on Slack. Not everyone is receiving all the GitHub notifications :-)

[bobbingwide]

FWIW, to get feedback it's usually easiest to also ask on Slack. Not everyone is receiving all the GitHub notifications :-)

Slack had locked up on me. It's been doing that recently. I've written my own plugin as a workaround. I'll try to develop a PHP Unit Test Case.

[annezazu]

Is this PR still relevant @bobbingwide? Given how much time has passed, it might be better to close and start a new PR if needed.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: bobbingwide <bobbingwide@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: TimothyBJacobs <timothyblynjacobs@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>
Co-authored-by: annezazu <annezazu@git.wordpress.org>
Co-authored-by: fabiankaegy <fabiankaegy@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[fabiankaegy]

As a follow up from @annezazu question half a year ago I'm going to close this PR for now. Feel free to reopen at any point if you want to pick this back up:)