Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#34479 closed defect (bug) (fixed)

New get_orderby() method of class-wp-posts-list-table.class doesn't allow query orderby var to be an array

Reported by: szaqal21's profile szaqal21 Owned by: drewapicture's profile DrewAPicture
Milestone: 4.4 Priority: normal
Severity: normal Version: 4.4
Component: Posts, Post Types Keywords: has-patch needs-testing needs-refresh
Focuses: administration Cc:

Description

I've altered the default orderby of pages list table like so:

function my_pre_get_posts($query)
{
	if(!$query->is_main_query() && $query->query['post_type'] != 'page')
		return;
	
	$query->set('orderby', array('menu_order' => 'asc', 'title' => 'desc'));
}


add_action('pre_get_posts', 'my_pre_get_posts');

and I get a PHP Notice caused by strtolower() in get_orderby() method which tries to lower an array in my case.

It worked fine up to 4.3.x.

Attachments (1)

34479.0.patch (1.5 KB) - added by bordoni 9 years ago.

Download all attachments as: .zip

Change History (11)

#1 follow-up: @swissspidy
9 years ago

  • Component changed from Administration to Posts, Post Types

Probably the same reason as in #34473

#2 in reply to: ↑ 1 @szaqal21
9 years ago

Replying to swissspidy:

Probably the same reason as in #34473

I've did a quick test and removed:

...
else {
	$orderby = 'date';
}
...

from wp_edit_posts_query() so orderby would be set as prior 4.4 and same notice is shown.

Last edited 9 years ago by szaqal21 (previous) (diff)

#3 @bradyvercher
9 years ago

I just ran into a similar issue today where WP_Posts_List_Table::get_orderby() method doesn't work when doing $wp_query->set( 'orderby', 'meta_value' ). The list is sorted correctly when clicking a column header the first time, but reversing the direction doesn't work since it looks like the orderby query var needs to match the column key here: https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-list-table.php?rev=35489#L1112

#4 @DrewAPicture
9 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.4

#5 @bordoni
9 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

I've created a patch that will resolve the issue, but I'm not 100% sure that if that cover all the bases for the ordering on the Administration Table, on my testing there were no issues when you tried to overwrite the OrderBy.

The patch N0 above will work for both associative and numeric arrays on OrderBy

<?php

$query->set( 'orderby', array( 'menu_order', 'title' ) );
// OR
$query->set( 'orderby', array( 'menu_order' => 'asc', 'title' => 'desc' ) );

@bordoni
9 years ago

#6 @Thomas Vitale
9 years ago

I tested the 34479.0 patch and didn't find any issues when overwriting the OrderBy.
For testing I used several pages with different names and 'order' attributes. Both simple and nested pages are in the right order as defined by the my_pre_get_posts() function.

Version 0, edited 9 years ago by Thomas Vitale (next)

#7 @wonderboymusic
9 years ago

  • Keywords needs-refresh added

nice patch - but if the array is associative, then order was probably passed for the values, which your patch ignores

#8 @bordoni
9 years ago

@wonderboymusic on the get_order method I've set a conditional for if the array on orderby is associative, and then setting order as an array with the values. Unless I've mistaken what you said on comment N7, please let me know.

Last edited 9 years ago by bordoni (previous) (diff)

#9 @DrewAPicture
9 years ago

  • Owner set to DrewAPicture
  • Status changed from new to reviewing

#10 @wonderboymusic
9 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

Reverted the method overrides in [35682]

Note: See TracTickets for help on using tickets.