Opened 6 years ago
Last modified 4 years ago
#46294 new defect (bug)
wp rest api fails to paginate page requests correctly when ordering on menu_order
Reported by: |
|
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:
- Create around 200 pages.
- Set menu_order != on some of the pages.
- edit any of the pages.
- 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 (22)
#3
@
6 years 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.
This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.
6 years ago
#6
follow-up:
↓ 7
@
6 years 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
@
6 years 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:
↓ 9
@
5 years 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
@
5 years 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?
#10
@
5 years 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.
#11
follow-up:
↓ 12
@
5 years 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:
↓ 13
@
5 years 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.
#13
in reply to:
↑ 12
@
5 years 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
@
5 years ago
I would like to confirm this is still an issue on New Installs and Upgraded Sites.
#15
@
5 years 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
follow-up:
↓ 19
@
4 years 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
@
4 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
- Severity changed from critical to normal
This ticket was mentioned in Slack in #core by clorith. View the logs.
4 years ago
#19
in reply to:
↑ 16
@
4 years ago
Replying to andreasgregor:
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?
The patch has indeed stopped working. For some reason we only get 100 pages now.. Which means the javascript cannot build the tree correctly and produces bad results.
Bumping posts_per_page doesn't work for me. Yes, the ajax response contains all the pages, but the parent page select tree does still not get built correctly.
I have created a new ticket: https://core.trac.wordpress.org/ticket/52626/
#20
@
4 years ago
There's a new workaround/hack/patch here: https://core.trac.wordpress.org/ticket/52626#comment:4
The old hack no longer works due to a new bug.
Here is a new workaround: https://core.trac.wordpress.org/ticket/52626#comment:4
This is the old hack, it no longer works, but it still demonstrates the issue with menu_order bug:
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.