Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#55439 closed enhancement (fixed)

Introduce term_exists_args filter

Reported by: chouby's profile Chouby Owned by: spacedmonkey's profile spacedmonkey
Milestone: 6.0 Priority: normal
Severity: normal Version: 6.0
Component: Taxonomy Keywords: has-patch has-dev-note
Focuses: Cc:

Description

[52921], as each time get_terms() replaces a direct sql query, introduced a breaking changed.

get_terms() is widely used and can be filtered with terms_clauses. The issue is that this filter will now be applied to term_exists() when it was not applied prior to [52921].

Currently there is no way to determine if the call to get_terms() is done by term_exists() or not, so we cannot disable the filter accordingly. Unless using an ugly hack with debug_backtrace()...

A new filter term_exists_args would allow to work around the breaking chanege in an elegant way.

Related:
[40994] introduced the filter wp_get_object_term_args after wp_get_object_terms() switched to use get_terms() since [38667].
#42104 Same issue for transform_query()
#54534 Same issue for get_terms_by()

Change History (16)

This ticket was mentioned in PR #2444 on WordPress/wordpress-develop by spacedmonkey.


2 years ago
#1

  • Keywords has-patch added

#2 @spacedmonkey
2 years ago

  • Milestone changed from Awaiting Review to 6.0
  • Owner set to spacedmonkey
  • Status changed from new to assigned

#3 @spacedmonkey
2 years ago

Thanks for the ticket @Chouby.

There is a PR for your review at #2444

#4 @Chouby
2 years ago

It's perfect for me.

#5 @audrasjb
2 years ago

  • Keywords needs-refresh added

The PR looks great to me, I added a small nitpick concerning docblocks :)

#6 @spacedmonkey
2 years ago

@audrasjb Feedback fixed.

spacedmonkey commented on PR #2444:


2 years ago
#7

Why did you apply the filter to the default arguments rather that the calculated arguments?

Those default args and then used to generate args for 3 different get_terms calls. 1. for id, 2. for slug 3. for name. If I were to apply to all args, it would mean repeating the filter 3 times in the same function. Which feels weird and wrong. The value of the filter would be different as well.

The ask here, was to add a filter so that developers can easily identify term_exists queries. Can do this, by changing the default args, maybe change the cache_domain or something. If you want to change any other args, you can do that with a filter like this get_terms_arg.

peterwilsoncc commented on PR #2444:


2 years ago
#8

Those default args and then used to generate args for 3 different get_terms calls. 1. for id, 2. for slug 3. for name. If I were to apply to all args, it would mean repeating the filter 3 times in the same function. Which feels weird and wrong. The value of the filter would be different as well.

OK, that makes sense. Thanks.

Is it okay if we go with one last name change for the filter term_exists_default_query_args -- query args is a quite common suffix and I forgot to add it while editing yesterday.

#9 @peterwilsoncc
2 years ago

Per note on GitHub, I made a mistake with my naming suggestion yesterday but once we have that down I think this is good for commit. I'll hold off adding the keyword for now so it doesn't get committed prematurely.

#10 @spacedmonkey
2 years ago

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

In 52987:

Taxonomy: Introduce term_exists_default_query_args filter.

In [52921] term_exists was converted to use get_terms. This change adds a filter called term_exists_default_query_args
to the term_exists function to allow developers to modify the arguments passed to get_terms before the query is run.

Props Chouby, Spacedmonkey, peterwilsoncc.
Fixes #55439.

#11 @spacedmonkey
2 years ago

  • Keywords needs-dev-note added; needs-refresh removed

#12 @SergeyBiryukov
2 years ago

In 52991:

Docs: Adjust the term_exists_default_query_args filter docs per the documentation standards.

Follow-up to [52987].

See #55439.

#13 @milana_cap
2 years ago

  • Keywords needs-docs added

#14 @spacedmonkey
2 years ago

Dev note ready for review.

#15 @spacedmonkey
2 years ago

Dev note ready for review.

#16 @spacedmonkey
2 years ago

  • Keywords has-dev-note added; needs-dev-note needs-docs removed
Note: See TracTickets for help on using tickets.