Inconsistent parameters for edited_term_taxonomy action

[ipm-frommen]

There are three edited_term_taxonomy action hooks:

• in the wp_update_term function;
• in the _update_post_term_count function;
• in the _update_generic_term_count function.

The first one provides as second parameter $taxonomy the taxonomy slug, while the others provide the taxonomy object.

[valendesigns]

It appears that edit_term_taxonomy is also inconsistent in what is being passed around.

[valendesigns]

The offending code was added in changesets 13625 & 19035. Though it is a valid bug and should be discussed and patched, keep in mind it's been a bug for 5 years and could have backwards compatibility concerns. So I'll assume this ticket is going to need unit tests before a patch will even be considered.

[boonebgorges]

valendesigns - Thanks for tracking down the changesets.

I'm not currently near my computer that has a checkout of the full plugin repo, but having a look at the repo would be a good next step. If it turns out that, say, the vast majority of plugins are expecting the taxonomy object, it'd make it easier to decide how to solve this problem. (Unit tests wouldn't hurt, but are not critical in this case - the main issue will be to inform devs of the change.)

[toscho]

We could limit the damage if we would turn the $taxonomy object into a real data type, i.e. a class WP_Taxonomy, with a __toString() method.

[ipm-frommen]

Replying to boonebgorges:

I'm not currently near my computer that has a checkout of the full plugin repo, but having a look at the repo would be a good next step. If it turns out that, say, the vast majority of plugins are expecting the taxonomy object, it'd make it easier to decide how to solve this problem.

Did you have a look at the full plugin repo? What does the vast majority of the plugins hooking to the edited_term_taxonomy action expect?

---

Replying to toscho:

We could limit the damage if we would turn the $taxonomy object into a real data type, i.e. a class WP_Taxonomy, with a __toString() method.

I really like the idea (not just for taxonomies, BTW). This would, of course, be a substantial change and goes beyond the scope of this ticket. But anyway, we could start by using the new class only in the three functions related to this issue. This would be a minimal change as this, right?

class WP_Taxonomy {

    public function __construct( stdClass $taxonomy ) {

        foreach ( get_object_vars( $taxonomy ) as $key => $value ) {
            $this->{ $key } = $value;
        }
    }

    public function __toString() {

        return $this->name;
    }

}

In the wp_update_term function, all we'd have to do is this:

$taxonomy_object = new WP_Taxonomy( get_taxonomy( $taxonomy ) );
do_action( 'edited_term_taxonomy', $tt_id, $taxonomy_object );

Same for edit_term_taxonomy, as mentioned by valendesigns.

[SergeyBiryukov]

Searching for edited_term_taxonomy in the plugin directory yields 29 results:

011-ps-custom-taxonomy/ps-custom-taxonomy.php
012-ps-multi-languages/includes/ps-multilingual-taxonomy.php
achievements/includes/achievements/functions.php
anspress/includes/anspress-ranks.php
anspress-question-answer/includes/anspress-ranks.php
awesome-support/includes/functions-custom-fields.php
co-authors-plus/co-authors-plus.php
connections/includes/class.terms.php
content-audit/content-audit-fields.php
easysocial/inc/ranks.php
file-gallery/file-gallery.php
getmecooking-recipe-template/recipe-template-functions.php
intellectual-property-basic/taxonomies/company/setup.php
mark-posts/public/class-mark-posts.php
maven-algolia/admin/controllers/indexer.php
media-features/media-features.php
organize-series/orgSeries-utility.php
pay2post/lib/framework/includes/term-counts.php
piklist/includes/class-piklist-taxonomy.php
polylang/include/model.php
rtbiz/app/lib/rt-attributes/class-rt-attributes.php
rtbiz/app/lib/rt-offerings/class-rt-offerings.php
shopp/core/model/Collection.php
simple-groups/simple-groups.php
smsify/modules/usergroups/UserGroups.php
user-groups/user-groups.php
user-tags/UserTags.php
user-taxonomies/user-taxonomies.php
wp-autoloader/lib/WPExtend/BasicWPActions.php

2 of those plugins treat $taxonomy as a string:

• Co-Authors Plus
• GetMeCooking Recipe Template

5 plugins treat $taxonomy as an object:

• Maven Algolia
• Organize Series
• Pay2Post
• Polylang
• rtBiz

The rest appear to be unaffected by the change of type.

[ipm-frommen]

Replying to SergeyBiryukov:

Searching for edited_term_taxonomy in the plugin directory yields 29 results:
[...]

2 of those plugins treat $taxonomy as a string:

• Co-Authors Plus
• GetMeCooking Recipe Template

5 plugins treat $taxonomy as an object:

• Maven Algolia
• Organize Series
• Pay2Post
• Polylang
• rtBiz

The rest appear to be unaffected by the change of type.

Thanks for checking this out, Sergey.

Could you please do this for edit_term_taxonmy as well?

So far, we have 2:1 for taxonomy objects wrt. action hooks, and 5:2 wrt. plugin expectations.

[boonebgorges]

SergeyBiryukov - Thanks so much for looking this up. I did a similar search on 'edit_term_taxonomy' and only found one plugin that uses the $taxonomy parameter: user-series. It expects a string.

My inclination is to settle on passing a string (the taxonomy slug). Elsewhere in taxonomy.php (and WP) we generally pass names to filters, not the corresponding objects.

If others are OK with this, I'll reach out to the authors of the 5 affected plugins with suggestions for a fix.

[ipm-frommen]

Pass taxonomy name instead of object

[boonebgorges]

In 31525:

Pass taxonomy name, not object, to 'edit_term_taxonomy' and 'edited_term_taxonomy' actions.

These actions are fired in a number of different places, and in some cases
the tax name is passed, while in others the taxonomy object is passed. This
inconsistency made it difficult for plugins to use the $taxonomy value.

Props ipm-frommen.
Fixes #30999.

[boonebgorges]

I'll be reaching out individually to the plugin authors listed above to warn them about the change in [31525]. I'll also put a note on make/core once #18828 has been settled one way or another.

[Chouby]

Replying to boonebgorges:

I'll be reaching out individually to the plugin authors listed above to warn them about the change in [31525].

Thanks a lot for this. Polylang will be ready for WP 4.2 :)