Opened 14 years ago
Closed 13 years ago
#16469 closed defect (bug) (invalid)
Cannot pass in ID for get_term_link() if using custom taxonomy
Reported by: | sparkweb | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.0.4 |
Component: | Taxonomy | Keywords: | close |
Focuses: | Cc: |
Description
Cannot pass in ID for get_term_link() if using custom taxonomy. Just throws errors. Additionally, if attempt to pass in a term name and that name legitimately includes a "/" the term link can't be found.
Attachments (1)
Change History (19)
#2
@
14 years ago
- Keywords reporter-feedback added
http://core.trac.wordpress.org/browser/trunk/wp-includes/taxonomy.php#L2820
Cannot pass in ID for get_term_link() if using custom taxonomy.
Are you passing an integer, or a string of the ID? Using (int)$term will retrieve a term by id, (string)$id will cause it to do a slug lookup instead, This is standard across the board for the taxonomy API. Can you confirm if this is the way you're calling get_term_link() ?
term name and that name legitimately includes a "/" the term link can't be found.
There's another ticket for this #16282, get_term_link() has never been designed to be passed a term name. Rather, a term slug. In 3.1 this is being caught a bit more, but it has always been a possibility through other versions.
#3
follow-up:
↓ 5
@
14 years ago
- Resolution set to invalid
- Status changed from new to closed
- Version set to 3.0.4
I was passing in $term->term_id. When I switched to (int)$term->term_id that worked. As did $term->slug which is much better than $term->name. Thank you and I'm sorry for the invalid ticket!
#5
in reply to:
↑ 3
@
14 years ago
- Resolution invalid deleted
- Status changed from closed to reopened
Replying to sparkweb:
I was passing in $term->term_id. When I switched to (int)$term->term_id that worked.
Thanks for the feedback, but I think $term->term_id
should have been already casted to int, so normally for ID values there should be no need at all to cast them into integers prior further use.
Can you say where it originated from? Was the $term
object returned by some API function? If yes please name it and provide some additional code so it's clear how you recieved it.
This still might be an issue with core.
#6
@
14 years ago
- Milestone set to Awaiting Review
Might be.
FYI, you can just pass the whole object: get_term_link( $term );
#8
@
14 years ago
- Cc mikeschinkel@… added
+1. I have stumbled onto this problem where it fails when $term->term_id = '1';
but not when $term->term_id = 1;
and I spent considerable time debugging to figure out why. It would IMO be nice it the term_id
was cast to (int)
inside the function.
#10
@
14 years ago
- Keywords has-patch added
If you pass in a numeric string, is_int()
still returns false, while is_numeric()
returns true. So it's safer to use is_numeric()
here.
I also found that is_int()
is used in several other functions. Should we look into those functions as well so that confusion like this won't happen again?
#11
follow-ups:
↓ 12
↓ 14
@
14 years ago
If you pass in a numeric string, is_int() still returns false, while is_numeric() returns true. So it's safer to use is_numeric() here.
is_int != is_numeric in the Term API. Theres the possibility of having Numeric slugs.
The correct fix here, is to verify that ALL term objects being returned return the term_id as a integer, that'll fix most of these casting problems (ie. It may return a string '1' instead of (int)1)
If someone passes a (string)'1' in as the term_id, they're doing it wrong, OR an API call is returning the data in the incorrect format.
#12
in reply to:
↑ 11
@
14 years ago
Replying to dd32:
is_int != is_numeric in the Term API. Theres the possibility of having Numeric slugs.
The correct fix here, is to verify that ALL term objects being returned return the term_id as a integer, that'll fix most of these casting problems (ie. It may return a string '1' instead of (int)1)
If someone passes a (string)'1' in as the term_id, they're doing it wrong, OR an API call is returning the data in the incorrect format.
That makes sense. I didn't know term slug could be numeric.
#14
in reply to:
↑ 11
;
follow-up:
↓ 15
@
14 years ago
Replying to dd32:
If someone passes a (string)'1' in as the term_id, they're doing it wrong, OR an API call is returning the data in the incorrect format.
For the record, my example showing '1'
was for simplicity of example; my use-cases have been being returned as a string from something else, like $_GET['term_id']
.
The correct fix here, is to verify that ALL term objects being returned return the term_id as a integer, that'll fix most of these casting problems (ie. It may return a string '1' instead of (int)1)
WordPress can't force $_GET['term_id']
to return a numeric. Sure the developer can cast it as (int)
, but they have to know that it needs to be cast; hence the crux of the problem.
#15
in reply to:
↑ 14
;
follow-up:
↓ 16
@
14 years ago
Replying to mikeschinkel:
Replying to dd32:
If someone passes a (string)'1' in as the term_id, they're doing it wrong, OR an API call is returning the data in the incorrect format.
For the record, my example showing
'1'
was for simplicity of example; my use-cases have been being returned as a string from something else, like$_GET['term_id']
.
The correct fix here, is to verify that ALL term objects being returned return the term_id as a integer, that'll fix most of these casting problems (ie. It may return a string '1' instead of (int)1)
WordPress can't force
$_GET['term_id']
to return a numeric. Sure the developer can cast it as(int)
, but they have to know that it needs to be cast; hence the crux of the problem.
Well no, Because that's not a term object is it? It's your data, as a plugin developer it's your responsibility to pass the correct parameters to functions. The documentation *may* (I havn't read it) may need to stress the fact it requires a integer value to be passed, not a numeric string.
Upon reading some of the documentation:
- get_term():
* @param int|object $term If integer, will get from database. If object will apply filters and return $term.
- get_term_link():
* @param object|int|string $term
In both cases you need to read the documentation, and then possibly read the first few lines of the function if you're not used to "int" meaning a literal integer..
Simple fact here, Is that the function cannot change to accept a numerical string for ID lookups, nor can any of the Taxonomy API's which accept multiple data types be changed to accept numerical strings (those which only take int's seem to all cast them appropriately). so ultimately, the only change needed here is better education, through more documentation or more forceful hints as to the datatypes.
#16
in reply to:
↑ 15
@
14 years ago
Replying to dd32:
Replying to mikeschinkel:
Well no, Because that's not a term object is it? It's your data, as a plugin developer it's your responsibility to pass the correct parameters to functions. The documentation *may* (I haven't read it) may need to stress the fact it requires a integer value to be passed, not a numeric string.
Code architecture really should strive to minimize accidental misuse. To say "Yeah, it's easy to make that common mistake but it was documented otherwise" should at best be considered a short term hack and at work considered a failure of the code architect.
Taken to an extreme assume that calling wp_delete_post($args)
with an empty $args
array would delete all posts but that it was documented to be that way. I doubt you'd argue that deleting all posts on a misuse of that function would be acceptable; why would you argue the same for a different function just because its result is less damaging?
In both cases you need to read the documentation, and then possibly read the first few lines of the function if you're not used to "int" meaning a literal integer..
I've read the documentation, I know what it says and how to use it. That's not the issue; the issue is that it doesn't make sense to have such a distinction that invites misuse and hence unexpected bugs when few if any other functions in WordPress behave that way; most will accept numbers as string as if they were numbers.
Simple fact here, Is that the function cannot change to accept a numerical string for ID lookups, nor can any of the Taxonomy API's which accept multiple data types be changed to accept numerical strings (those which only take int's seem to all cast them appropriately).
I must be missing something obvious. Why can't is_int()
be changed to 'is_numeric()` to solve this issue simply?
-Mike
#17
@
14 years ago
- Keywords close added
I must be missing something obvious. Why can't is_int() be changed to 'is_numeric()` to solve this issue simply?
As mentioned in above comments; Because the function accepts either an ID, or a Slug. A Slug may be numeric.
#18
@
13 years ago
- Milestone Awaiting Review deleted
- Resolution set to invalid
- Status changed from reopened to closed
re-closing as invalid.
get_term_link expects either
- An int of the term_id
- A string which is the term slug (and a slug cannot contain a /)
- The entire $term object
Other API functions which return the int fields as strings can be fixed elsewhere, Such as #17646
Version of WordPress?