WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#26570 closed defect (bug) (fixed)

wp_set_object_terms - bug

Reported by: stiofansisland Owned by: 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 6 years ago.
Docs update
26570.2.diff (4.2 KB) - added by DrewAPicture 6 years ago.
+ unit tests
26570.3.diff (4.7 KB) - added by SergeyBiryukov 6 years ago.

Download all attachments as: .zip

Change History (20)

#1 follow-ups: @helen
7 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
7 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
7 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
7 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
7 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
7 years ago

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

#7 @DrewAPicture
7 years ago

  • Keywords needs-patch needs-codex added

See comment:5

#8 @wonderboymusic
6 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
6 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
6 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
6 years ago

Docs update

#11 @DrewAPicture
6 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
6 years ago

  • Description modified (diff)

s/taxonomies/term's

@DrewAPicture
6 years ago

+ unit tests

#13 @DrewAPicture
6 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
6 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
6 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
6 years ago

In 28952:

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

See #26570.

#17 @SergeyBiryukov
6 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.