Opened 14 years ago
Last modified 20 months ago
#17374 assigned defect (bug)
get_pages() with child_of forgets sort
Reported by: | janfabry | Owned by: | DrewAPicture |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.1.2 |
Component: | Posts, Post Types | Keywords: | dev-feedback needs-testing needs-refresh |
Focuses: | Cc: |
Description
If you call get_pages()
with both the child_of
and sort_column
, the sorting is not applied.
child_of
makes it select all pages (sorted) and later applies a subselect via get_page_children()
. This subselect can mess up the sort order.
An example was reported on http://wordpress.stackexchange.com/questions/16921/get-pages-not-ordering-as-it-should
Related: #12821
Attachments (1)
Change History (10)
This ticket was mentioned in IRC in #wordpress-dev by jessepollak. View the logs.
11 years ago
#4
@
11 years ago
- Keywords needs-unit-tests added
- Milestone changed from Awaiting Review to Future Release
This would also strongly benefit from a unit test. It would go in here: https://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/post/getPages.php.
#5
follow-up:
↓ 6
@
11 years ago
I'll write this patch and unit test later today, any preference on whether I do the sort during merge or post merge?
#6
in reply to:
↑ 5
@
11 years ago
Replying to jessepollak:
I'll write this patch and unit test later today, any preference on whether I do the sort during merge or post merge?
I don't; I really have no idea what they'll look like.
#7
@
11 years ago
- Keywords has-patch dev-feedback added; needs-patch needs-unit-tests removed
OK, so it turned out that neither of my ideas were good ones. Both involved reimplementing parts of the MySQL search, which seemed like Bad Idea.
Instead, I decided that I'd rewrite the get_page_children
. In the current version, we do a top down recursive search to find all the children of child_of
. The problem with this is that we get all the children at the same time, so they all get added to the new list at the same time — this messes up the sort that we had previously.
In this patch, I reverse this so we do a bottom up search with a while loop to check whether a page has an ancestor with the ID child_of
. If it does, we add it to the new list of pages to return. This maintains the sorted order because we're iterating through in sorted order and adding one page at a time. To make this search faster, we add all the pages to a dictionary so we can do O(1) lookup when traversing up the tree.
I've also written a unit test that verifies the failed functionality and confirms the patch. I can write more, if we want.
Thanks for the bug report janfabry, sorry you never got a response!
I just confirmed that this bug is still around in trunk at revision 27076.
To recreate:
Expected:
Get Page 1a, Page 1b, Page 1aa in that order (because that's the order they were created in).
Actual
The sub pages are ordered Page 1a, Page 1aa, Page 1b.
Why
This happens because, as janfabry points out, the sort happens when the all of the pages are gotten. To get the children, we go through all the sorted pages and select the children and array_merge them into the page list. If a sub-page has a sub-page, this gets added right behind the sub-page, potentially messing up the ordering.
There are two possible ways to patch this:
I'd be happy to write up a patch with either of these options.
Thanks again janfabry!