Split filter ep_set_default_sort into another one: ep_set_sort

[rebeccahum]

Description of the Change

When using the filter ep_set_default_sort to change search results to sort by post_date instead of score, it causes a malformed query on non-search results. This happens at the parse_orderby() level:

ElasticPress/includes/classes/Indexable/Post/Post.php

Line 1915 in a796de7

protected function parse_orderby( $orderbys, $default_order, $args ) {

This is because $formatted_args['sort'] does not return a valid result since it's not expecting a nested array:

array (size=1)
  0 => 
    array (size=0)
      empty

For example, I'd expect a result like:

array (size=1)
  0 => 
    array (size=1)
      'post_date' => 
        array (size=1)
          'order' => string 'desc' (length=4)

Would it be possible to use a different hook, ep_set_default_orderby in the first filtering instance, as it is expecting a string input?

ElasticPress/includes/classes/Indexable/Post/Post.php

Lines 866 to 874 in a796de7

	/**
	* Filter default post query order by
	*
	* @hook ep_set_default_sort
	* @param {string} $sort Default sort
	* @param {string $order Order direction
	* @return {string} New default
	*/
	$args['orderby'] = apply_filters( 'ep_set_default_sort', 'date', $order );

...Versus in the second instance, where it's a nested array (this is where we are using it to hook for the result sorting):

ElasticPress/includes/classes/Indexable/Post/Post.php

Lines 882 to 898 in a796de7

	$default_sort = array(
	array(
	'_score' => array(
	'order' => $order,
	),
	),
	);
	/**
	* Filter default post query order by
	*
	* @hook ep_set_default_sort
	* @param {string} $sort Default sort
	* @param {string} $order Order direction
	* @return {string} New default
	*/
	$default_sort = apply_filters( 'ep_set_default_sort', $default_sort, $order );

Alternate Designs

parse_orderby() logic could be re-factored to account for a nested array input as well, but that might introduce some confusion since the filter is being used differently in two places.

Benefits

Passing in a nested array into ep_set_default_sort doesn't unexpectedly break parse_orderby() (i.e. we have a CLI that calls format_args())

Possible Drawbacks

Those already filtering with ep_set_default_sort using a string input may have to revise their code to account for the new filter

Verification Process

1. Apply the snippet ep_set_default_sort to sort search results by date:

add_filter( 'ep_set_default_sort', 'ep_sort_by_date', 20, 2 );
function ep_sort_by_date( $sort, $order ) {
	return array(
				array(
					'post_date' => array(
						'order' => $order,
						),
					),
			);
}

2. On a non-search request, see a misshapen query from ES
3. Apply patch
4. Do step 2 again and notice correct query sent to ES

Checklist:

• I have read the CONTRIBUTING document.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my change.
• All new and existing tests passed.

Changelog Entry

Added: Split filter ep_set_default_sort into additional one, ep_set_default_orderby

[felipeelia]

Hi @rebeccahum! First of all, thanks for submitting the PR.

As you said, merging it now would break EP for people using that filter with a string or a simple array as

add_filter( 'ep_set_default_sort', 'ep_sort_by_date', 20, 2 );
function ep_sort_by_date( $sort, $order ) {
	return array( 'post_date' => $order );
}

If I'm seeing it right, as is, the second ep_set_default_sort filter is never called, as the first call would set $args['orderby'] and the else in the following if/else is not reached.

With that said, perhaps we should change the name of the second instance of the filter, and make it ep_set_sort as, ultimately, it will not set the default sort but the entire sort clause of the query. That way it would not be a breaking change and the problem you have detected would be solved.

Thoughts?

[rebeccahum]

Hi @felipeelia!

If I'm seeing it right, as is, the second ep_set_default_sort filter is never called, as the first call would set $args['orderby'] and the else in the following if/else is not reached.

That is correct. Sure, changing it to ep_set_sort would make sense as well for backwards compatibility. I've updated the changes to reflect that.