Posted by jhodgdon on August 2, 2009 at 3:24pm
4 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | trigger.module |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (duplicate) |
Issue Summary
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.
Comments
#1
Just 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.
#2
I've just attached a patch to fix this at #525540: trigger.module and includes/actions.inc need overhaul
#3
Here 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.
#4
The patch needs some cosmetic rewriting. The current standard short form doc for hook implementations is:
/*** Implements hook_whatever().
*/
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.
#5
Searched 'n' replaced to get Implements hook_whatever().
#6
#7
The last submitted patch failed testing.
#8
#9
Implements trigger_user_login() should probably be Implements hook_user_login()?
#10
Yes.
#11
This looks good now, and definitely needs to go in.
#12
Just 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.
#13
I'm going to mark this as a dupe. Let's get #525540: trigger.module and includes/actions.inc need overhaul in first.
#14
That 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.
#15