wp_insert_term() calls term_exists() with 'slug' where 'slug' gets compared to term 'name'

[Rob Walker]

This may not specifically be a bug, but the resulting behavior is not what is expected.

It appears that a new term slug gets checked against existing term names when wp_insert_term() calls term_exists($slug) on line 2430 of taxonomy.php.

term_exists(), line 1645 of taxonomy.php, processes this slug as though it were the term name when checking against the db.

The ​documentation reads:

"If 'slug' argument exists then the slug will be checked to see if it is not a valid term. If that check succeeds (it is not a valid term), then it is added and the term id is given."

Is that supposed to mean it checks slug against slug?

Scenario:

I have a taxonomy where the term names need to support duplicate strings therefore I am using the slugs as unique identifiers. To prevent collisions when inserting new terms I compare slugs and not names.

Specifics (an example of how this issue came to light):

I am working a taxonomy of student_names where the term name = the student's first name. The student's family name is handled in a separate taxonomy and is not appropriate to use as a unique identifier (the family name needs to be associated with the parents and possibly other students).

When a new student is added, I generate the slug and ensure that it will be unique in the db.

Normally the first student with a given name, say Craig, will be given the default slug "craig". I call wp_insert_term("Craig", "student_taxonomy", array($slug=>"craig")). It is the first term "Craig", wp_insert_term() finds no collision. Execution completes successfully.

Adding an second student named Craig will result in my custom methods detecting the collision and generating a new slug "craig1". I call wp_insert_term("Craig", "student_taxomony", array($slug=>"craig1")). wp_insert_term() calls term_exists() with the slug "craig1", line 2430. term_exists checks "craig1" against existing term names (Craig), finds no conflict, execution completes successfully.

A third Craig insertion generates the slug "craig2" which is compared against the existing term names (Craig, Craig), finds no conflict, execution completes successfully.

Now, if the first Craig with the slug "craig" is deleted, and a fourth Craig is added, my code will generate the slug "craig" because there is no longer a conflict with that slug. Eventually wp_insert_term compares the slug "craig" to the term names "Craig, Craig" and finds a conflict.

[boonebgorges]

Thanks for the report. This does sound like a bug, though I'm not sure how best to solve it. We can't stop checking against the 'name' column in term_exists() for backward compatibility reasons. I wonder if we might introduce another parameter that allows you to specify which columns you're checking against, and then in wp_insert_term(), if a 'slug' has been passed, we can tell term_exists() only to check the slug column.

Whatever the solution, we'll need to have a unit test that demonstrates the problem.

[MikeNGarrett]

This seems to have been fixed in trunk by forcing wp_unique_term_slug to run. This is an issue for 3.9.2 and 4.0, but should be fixed in 4.1.

[boonebgorges]

Thanks for having a look, MikeNGarrett - but I think you're wrong about this. The term_exists() check is still being run, and if it matches based on 'name' alone, then a WP_Error will be thrown. In 4.0, see https://core.trac.wordpress.org/browser/tags/4.0/src/wp-includes/taxonomy.php#L2481, especially line 2500; in 4.1, see https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/taxonomy.php#L2826. In 4.1, we never reach wp_unique_term_slug() in the case described by Rob Walker.

29255.test.diff​ is an attempt to write a unit test that describes the bug in as narrow a fashion as possible. It fails on 4.1 and 4.0.

The purpose of this section of wp_insert_term() is to ensure that in a hierarchical taxonomy, terms at the same level of the hierarchy do not share the same 'name'. So the term_exists() check should probably be swapped out with get_term_by( 'name' ), though it'll probably need more massaging than just this to make it work properly.

[MikeNGarrett]

Hey Boone,
You're right. I ran back through my tests and realized I was looking too narrowly at the test case. Is the ideal to allow for Name(slug) of Craig(craig), Craig(craig1), Craig(craig2), etc to exist or to allow only one Craig(craig) to exist per level in a hierarchical taxonomy?

I mean, should users be allowed to create a term with the name "Craig" if one already exists regardless of slug? or should slug always be unique, but name can be anything? Either way implies a bit of difficulty in this situation.

[MikeNGarrett]

Just making a note. This may have been fixed with ticket #30780. We should retest this scenario with the latest from trunk.

[MikeNGarrett]

Tested on 4.2. Term splitting solved this ticket.

[boonebgorges]

Replying to MikeNGarrett:

Tested on 4.2. Term splitting solved this ticket.

Confirmed. See 29255.test.2.diff​ for a cleanly applying patch. Thanks for following up.

[MikeNGarrett]

Thanks, Boone. I appreciate the insight into the patch.