Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change term objects in format_hits_as_terms to WP_Term instead of stdClass #2913

Merged
merged 1 commit into from
Aug 2, 2022
Merged

Change term objects in format_hits_as_terms to WP_Term instead of stdClass #2913

merged 1 commit into from
Aug 2, 2022

Conversation

jonathanstegall
Copy link
Contributor

@jonathanstegall jonathanstegall commented Jul 28, 2022

Description of the Change

The description of the format_hits_as_terms method reads like this:

/**
 * Format the ES hits/results as term objects.
 *
 * @param  array $terms The terms that should be formatted.
 * @param  array $new_terms Array of terms from cache.
 * @param  array $query_vars Query variables.
 * @since  3.1
 * @return array
*/

This pull request makes the term objects into WP_Term objects instead of stdClass objects. This brings it in line with how WordPress uses this result in actions like post_tag_row_actions, which specify that the term parameter should be a WP_Term object.

I'm not aware of any drawbacks, but I do realize that this method has been returning stdClass objects, even if incorrectly, for some time. I think likely this is causing issues unless the code using it is not using standard WordPress methods, but that is worth noting.

An alternative would be to add a filter instead, allowing developers to use WP_Term on their own. Ex: $term = apply_filters( 'ep_wp_query_terms_object', $term );. I don't think that is ideal, but if it's necessary for backward compatibility I would understand that.

Closes #2912

How to test the Change

The way I found this was using The Events Calendar:

  1. Index post_tag, at least, with some post tags.
  2. Install The Events Calendar.
  3. Go to /wp-admin/edit-tags.php?taxonomy=post_tag&post_type=post and search for a value that will return some tags. Instead of giving a fatal error, it will return the same data but formatted as a WP_Term object. Verify by logging in these lines like this:
public function event_tag_actions( $actions, WP_Term $tag ) {
    error_log( 'tag is ' . print_r( $tag, true ) );
    return $this->container->make( Event_Tag::class )->event_tag_actions( $actions, $tag );
}

Changelog Entry

Fixed - Change term objects in format_hits_as_terms to use WP_Term instead of stdClass to match WordPress expectations.

Credits

Props @jonathanstegall

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests pass.

I don't see any documentation or relevant tests, but am willing to update those if they do apply.

@jonathanstegall jonathanstegall changed the title Change term objects in format_hits_as_terms to WP_Term instead of stdClass` Change term objects in format_hits_as_terms to WP_Term instead of stdClass Jul 28, 2022
@felipeelia felipeelia added this to the 4.3.0 milestone Jul 29, 2022
@felipeelia felipeelia self-assigned this Jul 29, 2022
@jonathanstegall
Copy link
Contributor Author

I'm noticing that one of the tests failed but it's not clear to me that it's related to the code. Is there anything I should do with this?

@felipeelia felipeelia merged commit 58d6488 into 10up:develop Aug 2, 2022
rebeccahum added a commit to Automattic/ElasticPress that referenced this pull request Aug 4, 2022
rebeccahum added a commit to Automattic/ElasticPress that referenced this pull request Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: the format_hits_as_terms returns a term as stdClass instead of WP_Term
2 participants