Opened 9 years ago
Closed 9 years ago
#36878 closed defect (bug) (fixed)
get_term_by returns random result with multiple terms
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.6 | Priority: | normal |
Severity: | normal | Version: | 4.5.2 |
Component: | Taxonomy | Keywords: | needs-docs |
Focuses: | docs | Cc: |
Description
current situation: Taxonomy term names are not unique. GET_TERM_BY allows a developer to add a query for name. If multiple names are present e.g.:
\legal\public\contract
\finance\leasing\contract
it will return a random one of the two because of LIMIT 1. A developer who creates code without the content yet in place, could create a bug by only realizing much later that multiple entries exist in the content or never even face the bug since sometimes it delivers the correct one and sometimes it does not. So in the end, the end-user will have to notify the developer something strange is going on...
proposal: the code should protect the developer from introducing bugs. So proposal to either
- return multiple (remove limit 1) (can't see why not) or
- return WP_ERROR if multiple so it is at least clear there is a bug
- or not allow names
Change History (6)
#1
@
9 years ago
- Focuses docs added
- Keywords needs-patch good-first-bug added
- Milestone changed from Awaiting Review to Future Release
#2
@
9 years ago
- Keywords needs-docs needs-codex added; needs-patch removed
I also don't see a way to improve this function without impacting reverse compatibility. There is thus nothing to fix except for improving the documentation.
#3
@
9 years ago
- Keywords needs-codex removed
I added a note to the Codex, and submitted the same to the Code Reference, but it's stuck in the moderation queue there.
#4
follow-up:
↓ 5
@
9 years ago
@bcworkz Your submission on DevHub was approved a couple of weeks ago.
@boonebgorges What's left here?
#5
in reply to:
↑ 4
@
9 years ago
- Keywords good-first-bug removed
- Milestone changed from Future Release to 4.6
Replying to DrewAPicture:
@bcworkz Your submission on DevHub was approved a couple of weeks ago.
@boonebgorges What's left here?
I'm going to add a note to the doc block.
Hi @cogmios - Thanks very much for the ticket and for the clear explanation of the issue.
A quick note about the history of this issue. Before WP 4.1 or thereabouts, this problem would only arise in cases where two terms were in different branches of a hierarchical taxonomy, because it wasn't possible (through WP's functions, anyway) to create two terms with the same names in any other way, at least not within the same taxonomy. And since
get_term_by()
always required a$taxonomy
argument, the problem would arise fairly rarely.Versions 4.1 through 4.3 eased some of these requirements - it's now possible for duplicate names, and even slugs, to exist in certain cases, both within and between taxonomies. And in WP 4.4, the
$taxonomy
param was made optional, meaning that the possibility for ambiguity arose even more - not just with names, but also slugs, since terms in different taxonomies can share a slug.I'm not sure that we can change the default return value of
get_term_by()
to be either aWP_Error
object or an array. Consider the following:This will break if we return a non-falsey value that is not a
WP_Term
. Even in core, we do this in many spots. Plugins probably do it in thousands of ways.We could potentially add another parameter (or another value of the
$output
parameter) that tellsget_term_by()
to return an array of terms. But in that case, we're really just reproducingget_terms()
, which people ought to be using anyway when they might be expecting multiple results.I think the best we can do here is a note in the documentation that warns people of this danger, and suggests that they use
get_terms()
(or at least to specify the$taxonomy
) when there's the potential for conflict.