I am pretty sure the Trigger module is not actually triggering Taxonomy events at all.
The reason is that it is trying to do this by implementing "hook_taxonomy()", which no longer exists in Drupal 7. See http://api.drupal.org/api/function/trigger_taxonomy/7
Instead, it needs a mechanism like what it is doing for node, user, and comment hooks. For instance, see http://api.drupal.org/api/function/_trigger_node/7 and the functions that call it.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 583805_taxonomytriggerjv3.patch | 9.27 KB | jvandyk |
| #8 | 583805_taxonomytriggerjv2.patch | 9.27 KB | jvandyk |
| #5 | 583805_taxonomytriggerjv.patch | 9.27 KB | jvandyk |
| #3 | taxonomytriggerjv.patch | 9.29 KB | jvandyk |
Comments
Comment #1
jhodgdonJust as a note, there are only test functions in trigger.test to test cron and node triggers. That is probably why taxonomy snuck through the cracks. User and comment triggers are also not tested that I can see.
Comment #2
jhodgdonI've just attached a patch to fix this at #525540: trigger.module and includes/actions.inc need overhaul
Comment #3
jvandyk commentedHere is a patch based on jhodgdon's work in #525540. The problem being solved is that trigger.module was never updated to the new taxonomy hook style, e.g., we now use hook_taxonomy_term_insert() instead of hook_taxonomy($op ...).
The patch adds the new hook implementation and tests. The test uses a generic action that just sets a variable. Tests for user and comment actions are added here, too, with the same generic action.
Comment #4
jhodgdonThe patch needs some cosmetic rewriting. The current standard short form doc for hook implementations is:
As far as functionality/suitability goes, maybe I shouldn't be the one to review it, since I wrote the patch this was largely taken from.
Comment #5
jvandyk commentedSearched 'n' replaced to get Implements hook_whatever().
Comment #6
jvandyk commentedComment #8
jvandyk commentedComment #9
jhodgdonImplements trigger_user_login() should probably be Implements hook_user_login()?
Comment #10
jvandyk commentedYes.
Comment #11
jhodgdonThis looks good now, and definitely needs to go in.
Comment #12
jhodgdonJust as a note, if you apply the patch in http://drupal.org/node/525540#comment-1982822 , it takes care of this issue as well. And makes a lot of other improvements. So I recommend that other patch instead.
Comment #13
webchickI'm going to mark this as a dupe. Let's get #525540: trigger.module and includes/actions.inc need overhaul in first.
Comment #14
jhodgdonThat other issue didn't make it in for code freeze. It's an API change. If it doesn't get added as a special case, then this issue still needs to be addressed.
Comment #15
jhodgdon