WordPress.org

Make WordPress Core

Opened 20 months ago

Last modified 3 weeks ago

#46294 new defect (bug)

wp rest api fails to paginate page requests correctly when ordering on menu_order

Reported by: hobzhobz Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.4.1
Component: Query Keywords: needs-patch
Focuses: rest-api Cc:

Description

Describe the bug
Gutenberg and wordpress fails to render the parent page dropdown correctly when there are more than 100 pages and the pages have various menu_order values set.

The problem is that gutenberg calls the wordpress rest api like this, getting 100 items at a time.

http://server/wp-json/wp/v2/pages?per_page=100&exclude=890&parent_exclude=890&orderby=menu_order&order=asc&context=edit&_locale=user
http://server/wp-json/wp/v2/pages?per_page=100&exclude%5B0%5D=890&parent_exclude%5B0%5D=890&orderby=menu_order&order=asc&context=edit&_locale=user&page=2

This results in the following 2 sql queries:
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND wp_posts.ID NOT IN (890) AND wp_posts.post_parent NOT IN (890) AND wp_posts.post_type = 'page' AND ((wp_posts.post_status = 'publish')) ORDER BY wp_posts.menu_order ASC LIMIT 0, 100
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND wp_posts.ID NOT IN (890) AND wp_posts.post_parent NOT IN (890) AND wp_posts.post_type = 'page' AND ((wp_posts.post_status = 'publish')) ORDER BY wp_posts.menu_order ASC LIMIT 100, 100

When ordering on a non unique field like menu_order you need to also add a second unique ordering field like ID See https://bugs.mysql.com/bug.php?id=72076 See the answer from Tor Didriksen.

This is very critical bug because it means that if the current pages parent is missing in the parent page dropdown and you click update it means that the menu order for that page will be reset and this means i cant use gutenberg.

To Reproduce
Steps to reproduce the behavior:

  1. Create around 200 pages.
  2. Set menu_order != on some of the pages.
  3. edit any of the pages.
  4. the parent page dropdown is missing pages

Expected behavior
the rest api should add some unique order by field to prevent the same items from coming on page 1 and 2 of the requests.

Change History (17)

#1 @mukesh27
20 months ago

  • Component changed from General to REST API
  • Focuses rest-api added

#2 @hobzhobz
20 months ago

  • Component changed from REST API to General
  • Focuses rest-api removed

Here is a hack to fix the issue:

In the file /wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php:290

around line 290.
change from:

$posts_query  = new WP_Query();
$query_result = $posts_query->query( $query_args );

to

$posts_query  = new WP_Query();
if (isset($query_args['orderby']) && is_string($query_args['orderby']) && $query_args['orderby'] == 'menu_order') {
    $query_args['orderby'] = ['menu_order' => 'ASC', 'ID' => 'ASC'];
}
$query_result = $posts_query->query( $query_args );

now the query will be deterministic between pages.

#3 @hobzhobz
20 months ago

Note, this is hard to reproduce unless you set menu_order != 0 on many pages.

If menu_order == 0 on all pages this issue doesnt seem to happen, but i bet it has to do with mysql internals as its "undefined behavior" how mysql sorts when sorting only on a non unique field.

#4 @swissspidy
20 months ago

  • Component changed from General to REST API

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


16 months ago

#6 follow-up: @TimothyBlynJacobs
16 months ago

  • Component changed from REST API to Query
  • Focuses rest-api added

This seems like a bug in the query component. I’d think paginating over a new WP_Query([ 'orderby' => 'menu_order' ]) should work as expected.

#7 in reply to: ↑ 6 @hobzhobz
16 months ago

Replying to TimothyBlynJacobs:

This seems like a bug in the query component. I’d think paginating over a new WP_Query([ 'orderby' => 'menu_order' ]) should work as expected.

I agree. Wp_query should automatically append order by id asc or something when the original order by clause contains only non-unique fields

#8 follow-up: @danpw
11 months ago

Good afternoon, any updates on this issue? We're noticing it on some of our client sites as well.

#9 in reply to: ↑ 8 @secretsurfer
11 months ago

Replying to danpw:

Good afternoon, any updates on this issue? We're noticing it on some of our client sites as well.

I see this issue in one of my WooCommerce sites, too. I realized that it fails when activating Barn2Media Product Table or WooCommerce Blocks (from Automattic!). I think that the problem appeared since WP 5.3.

Any help?

Last edited 11 months ago by secretsurfer (previous) (diff)

#10 @hobzhobz
11 months ago

Bump, this issue is still live and still easy to fix and still causing problems for ALL sites with gutenberg and more than 100 pages.

Last edited 11 months ago by hobzhobz (previous) (diff)

#11 follow-up: @jdorner
5 months ago

  • Severity changed from normal to major
  • Version set to 5.4.1

I'm experiencing problems as well. Is there a fix for this issue?

#12 in reply to: ↑ 11 ; follow-up: @hobzhobz
5 months ago

  • Severity changed from major to critical

Replying to jdorner:

I'm experiencing problems as well. Is there a fix for this issue?

There's a hacky patch in reply no2.

You need to apply it after every WordPress upgrade.

Last edited 5 months ago by hobzhobz (previous) (diff)

#13 in reply to: ↑ 12 @photosjob
5 months ago

Experiencing the same issue on a staging site with a CPT and 180+ articles.
Applied the hacky patch above for now (as more content needs to be added). Would be nice if a permanent solution could be applied soon...

Thanks in advance!!

Kind regards,
Job

Replying to hobzhobz:

Replying to jdorner:

I'm experiencing problems as well. Is there a fix for this issue?

There's a hacky patch in reply no2.

You need to apply it after every WordPress upgrade.

#14 @Colter
3 months ago

I would like to confirm this is still an issue on New Installs and Upgraded Sites.

#15 @emrikol
3 months ago

IF the patch in reply 2 works for you, I believe you can do it in a filter, rather than hacking core after every update:

<?php
add_filter( 'rest_page_query', function( $args, $request ) {
        if (
                        isset( $args['orderby'] ) &&
                        is_string( $args['orderby'] ) &&
                        'menu_order' === $args['orderby']
        ) {
                $args['orderby'] = [ 'menu_order' => 'ASC', 'ID' => 'ASC' ];
        }
        return $args;
}, 10, 2 );

In my testing when I ran into this issue, I also needed to bump up posts_per_page as well since it was still very low compared to the number of pages on the site experiencing the issue.

#16 @andreasgregor
5 weeks ago

I'm still having this issue on a 5.5.1 site with 200+ pages.

The patch in reply 2 is not working for me, neither is the filter variant.
Any other ideas?

#17 @swissspidy
3 weeks ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Severity changed from critical to normal
Note: See TracTickets for help on using tickets.