Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#26570 closed defect (bug) (fixed)

wp_set_object_terms - bug

Reported by: stiofansisland's profile stiofansisland Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 4.0 Priority: normal
Severity: normal Version: 2.3
Component: Taxonomy Keywords: has-patch has-unit-tests commit
Focuses: docs Cc:

Description (last modified by DrewAPicture)

wp_set_object_terms claims to be able to use a term's ID or slug when in fact it will only use slug, trying to use a category ID will create a new numerical category and assign that.
http://codex.wordpress.org/Function_Reference/wp_set_object_terms

Attachments (3)

26570.diff (910 bytes) - added by DrewAPicture 10 years ago.
Docs update
26570.2.diff (4.2 KB) - added by DrewAPicture 10 years ago.
+ unit tests
26570.3.diff (4.7 KB) - added by SergeyBiryukov 10 years ago.

Download all attachments as: .zip

Change History (20)

#1 follow-ups: @helen
10 years ago

  • Component changed from Taxonomy to Inline Docs
  • Version changed from trunk to 2.3

Actually, hierarchical taxonomies need the ID, non-hierarchical expect the slug. Sounds like the inline docs need an update, though.

#2 in reply to: ↑ 1 ; follow-up: @stiofansisland
10 years ago

Replying to helen:

Actually, hierarchical taxonomies need the ID, non-hierarchical expect the slug. Sounds like the inline docs need an update, though.

OK i went and did some digging and found the root of the problem, function term_exists checks the term to see if it's an int or a slug, the problem is it uses "is_int" which being that the majority of people using this function in the wild will be posting the term value is_int sees any POST value as a string so it then moves on to query the ID as a term which breaks.

I propose to fix this we change taxonomy.php line: 1570

FROM:

if ( is_int($term) ) {

TO:

if ( is_numeric($term) ) {

Thanks,

Stiofan

#3 in reply to: ↑ 2 @SergeyBiryukov
10 years ago

Replying to stiofansisland:

I propose to fix this we change taxonomy.php line: 1570

See #18009.

#4 in reply to: ↑ 1 ; follow-up: @DrewAPicture
10 years ago

  • Keywords dev-feedback added

Replying to helen:

Actually, hierarchical taxonomies need the ID, non-hierarchical expect the slug. Sounds like the inline docs need an update, though.

Not sure I'm following which inline docs need to change -- do you mean to specify which type should be passed for hierarchical vs non-hierarchical?

Happy to patch whatever needs patching, just need a clarification.

#5 in reply to: ↑ 4 @helen
10 years ago

Replying to DrewAPicture:

Not sure I'm following which inline docs need to change -- do you mean to specify which type should be passed for hierarchical vs non-hierarchical?

Yeah, I think that's what I meant. Also the Codex, I suppose.

#6 @nacin
10 years ago

  • Component changed from Inline Docs to Taxonomy
  • Focuses docs added

#7 @DrewAPicture
10 years ago

  • Keywords needs-patch needs-codex added

See comment:5

#8 @wonderboymusic
10 years ago

  • Keywords dev-feedback removed
  • Milestone changed from Awaiting Review to Future Release

This probably needs docs fixes, but no one has done them yet

#9 @DrewAPicture
10 years ago

  • Keywords close added; needs-patch needs-codex removed

After reviewing this again, I don't think any changes are warranted here. It actually seems to be a pretty close duplicate to #18009.

#10 @wonderboymusic
10 years ago

  • Keywords needs-unit-tests added; close removed
  • Milestone changed from Future Release to 4.0

We need unit tests that have all of the different permutations represented

@DrewAPicture
10 years ago

Docs update

#11 @DrewAPicture
10 years ago

  • Keywords has-patch added

26570.diff updates the parameter description for $terms:

A single term slug (if the taxonomy is hierarchical), single term id, or array of either term slugs or ids. Will replace all existing related terms in this taxonomy.

#12 @DrewAPicture
10 years ago

  • Description modified (diff)

s/taxonomies/term's

@DrewAPicture
10 years ago

+ unit tests

#13 @DrewAPicture
10 years ago

  • Keywords has-unit-tests commit added; needs-unit-tests removed

26570.2.diff adds unit tests for inserting a term by ID, by slug, by an array of IDs, and by an array of slugs per comment:10.

#14 @SergeyBiryukov
10 years ago

The main point of this ticket was fixed in #17646.

A single term slug (if the taxonomy is hierarchical)

This appears to contradict comment:1. I don't think it actually matters if the taxonomy is hierarchical or not, both the term ID and the term slug should work regardless. I've expanded the unit tests to demonstrate that.

We already have tests for wp_set_object_terms, and there's some overlap between the old and new ones, but I guess adding more wouldn't hurt.

To summarize, there are no significant changes here, we just clarify the docs a bit and add some more unit tests.

#15 @SergeyBiryukov
10 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 28951:

Clarify the docs and add more unit tests for wp_set_object_terms().

props DrewAPicture.
fixes #26570.

#16 @DrewAPicture
10 years ago

In 28952:

Fix parameter description for $append in wp_set_object_terms() inline docs.

See #26570.

#17 @SergeyBiryukov
10 years ago

In 28953:

Reorder test functions in term.php for consistency. Add @ticket references.

see #26570.

Note: See TracTickets for help on using tickets.