REST API: Audit `orderby` params and add any others

[jnylen0]

Migrated from ​https://github.com/WP-API/WP-API/issues/2906. It looks like at some point we dropped the capability to order by author and taxonomy term, probably when we introduced mandatory enum validation. What other orderby parameters can we easily support?

Joining the discussion as I'd love to see some of the orderby options, like rand, being brought back to the API (​background).

If some of the options can't be added back, I think it'd be nice if we could add options back on a case-by-base basis, thanks to a filter.

As discussed in github, I think the useful mising 'orderby' parameters are: 'none', 'modified', 'parent', 'rand' and 'meta_value_num'. In addition, it might be helpful to introduce a filter so developers could alter which fields were accepted.

The "rest_{$this->post_type}_collection_params" filter can be used to add in additional orderby values. To add random, you could hook in like this:

<?php
add_filter( 'rest_post_collection_params', 'my_prefix_add_rest_orderby_params', 10, 1 );

function my_prefix_add_rest_orderby_params( $params ) {
    $params['orderby']['enum'][] = 'rand';

    return $params;
}

If there is a consensus on what should be added as supported defaults, I can make a patch. The hook is already available to use.

The collection params filters support this use case really nicely - thanks for the example.

meta_value_num requires meta_key to be set in the query so we shouldn't add that one.

I can't think of any reason not to add all of the other parameters mentioned above ('none', 'modified', 'parent', 'rand').

Setting this back to "bug" as it's a regression from previously established behavior.

Oh, also author, from the GitHub issue linked in the OP.

So add in none, modified, parent, and rand, author? I don't know how to test for random sorting but I will add test cases for the others.

My only concern with this is rand as that's not idempotent, so this will lead to reverse-cache issues etc. Also, rand is pretty bad for performance too, so I'm a bit hesitant with that one.

Leaving rand as plugin territory and adding the others seems reasonable to me.

Patch for rest post orderby parameters.

I have to revise that patch one of the tests doesn't work right.

Fixed the test in this patch.

The second patch should be good.

Thanks for patching this, @ChopinBach. Looks good to me apart from a couple of minor notes.

A few of the new tests don't have strictly defined order:

• test_get_items_orderby_author_query
• test_get_items_orderby_none_query
• test_get_items_orderby_parent_query

For none it's fine to just assert that the response is a 200. For the others, you could either verify that the list of authors/parents is sorted correctly, or make sure each post has a unique author/parent.

Also, rather than copy/paste update_post_modified from tests/query/dateQuery.php, let's move it to the base WP_UnitTestCase class.

Adds orderby params author, parent, none, and modified to the posts endpoint for the REST API. Unit tests revised to not rely upon secondary sorting by SQL. Line endings should be fixed as well.

This one should actually work. Adds orderby params author, parent, none, and modified to the posts endpoint for the REST API. Unit tests revised to not rely upon secondary sorting by SQL. Line endings should be fixed as well.

Created by Linux not Windows so the Line endings should be fine now. Curse you WINDOWS! Adds orderby params author, parent, none, and modified to the posts endpoint for the REST API. Unit tests revised to not rely upon secondary sorting by SQL. Line endings should be fixed as well.

Avoid duplicating update_post_modified helper

Minor change in 38693.6.diff​: I removed the old copy of update_post_modified now that it lives in the base test class. This is good to commit.

Unfortunately I found another bug with orderby while testing this → #38985

Hmm reading through the patch why are we supporting none? That doesn't make any sense to me. Who would want that, and even if we did, wouldn't orderby => null be better?

Also, why are we trying to get this in 4.7 for an enhancement after RC1? Is that defo necessary?

This is not an enhancement - this is a regression from previously established behavior that at least 3 different people have asked about.

Removing commit for workflow reasons - need commit from one permanent committer and dev-reviewed from another, please.

Replying to jnylen0:

This is not an enhancement - this is a regression from previously established behavior that at least 3 different people have asked about.

@jnylen0 I disagree. This is an enhancement, and I am not comfortable with trying to squeeze this into 4.7.
What about adding a filter for the orderby enum params so sites can expand the supported orderby values for 4.7, and properly fixing in 4.8?

What about adding a filter for the orderby enum params so sites can expand the supported orderby values for 4.7, and properly fixing in 4.8?

Given the collection params are filterable, i don't think there's any benefit to adding 2-3 values to the enum that won't actually do anything. A plugin that wants to add support can do so via the filter.

This is not an enhancement - this is a regression from previously established behavior that at least 3 different people have asked about.

I don't agree this isn't an enhancement, we haven't had these order by values for some time. IMO now is the time to punt this to 4.8, it's fine that we add it then.

Okay, let's take a look at adding these via a filter in a separate ticket. Thank y'all for weighing in.

#39657 was marked as a duplicate.

note: I closed #39657 which was asking about adding menu_order to the accepted orderby options.

Replying to jnylen0:

Okay, let's take a look at adding these via a filter in a separate ticket. Thank y'all for weighing in.

Has this separate ticket been created? I'd be happy to see the random orderby reappear--but it was apparently decided that would be plugin territory. If that's so, would I need to code that myself?

Thanks!

Unless there are any objections, I'll plan to land these additional parameters in the next few days. 38693.6.diff​ is basically ready, but we can improve/simplify the tests considerably after #39079.

Replying to openhatch:

Has this separate ticket been created?

comment:5 looks pretty good as a code sample, so I'm not sure we need to do anything else regarding a filter, unless that doesn't work well for some reason.

Test ORDER BY clauses explicity; remove orderby=none

In 38693.7.diff​:

• Refresh against latest trunk
• Explicitly test ORDER BY clauses used by new orderby= methods
• Remove orderby=none (see comment:15; let's revisit this later if needed)
• Update wp-api.js test fixtures

In 40605:

REST API: Add author, modified, and parent sort order options for posts.

These (and a few others that can be revisited later if needed) were present in
beta versions of the WP REST API but were removed during the merge to WP 4.7.

Props ChopinBach, jnylen0.
Fixes #38693.