Add rules event for signup

Thanks for contributing a patch. A few problems:

A) Always roll patches for new features against the end of the current branch (in this case, DRUPAL-6--1), not against a specific (stale) release.

B) Your patch includes some debugging statements.

C) Your patch has some code-style problems (e.g. whitespace, etc).

Also, your patch changes the API (e.g. adding hook_signup with an operation argument instead of separate hooks for cancel, etc). There are two problems with this:

D) Core is moving away from monolithic hooks with an $op parameter. Is there a reason you're advocating this change? I'd rather just keep separate triggers for the separate hooks, instead of trying to cram it all into a single trigger via a single hook.

E) If you were going to convince me this API change was worth making, you'd have to change signup.api.php to document the change.

Upon closer inspection, your patch is only *adding* this duplicate hook invocation in a few spots, so you're not going to break a bunch of existing code. But, I don't see why that's necessary. Why can't you just have separate triggers for the existing hook invocations? In fact, there are a few new hooks I recently added (see #581734: Improve the signup API: add hook_signup_data_alter(), hook_signup_create(), and hook_signup_update()) that could potentially also be trigger points. Why aren't those exposed, too?

Also, note: when you post a patch you want a maintainer to consider, please set the status to "needs review". If there are problem, they'll set it back to "needs work" (like I just did). When you post a new patch that addresses the existing concerns, you should set it to "needs review" again.

Thanks!
-Derek

Hi Derek,

Thanks for your review.
Here is the patch for 6.x-1.0-rc6.

Although according http://api.drupal.org/api/function/module_invoke_all/6, it should be:
module_invoke_all('signup_cancel', $signup, $node);

I can just make the triggers work following http://drupal.org/node/375833:
module_invoke_all('signup', 'signup_cancel', $signup, $node);

Do you know why?

Thanks shenzhuxi for this,

does that mean I can create an action to send emails to event owners based on signup triggers?

is it really that simple now ;-)

how different would this be to doing it with rules?

Is the outcome the same?

thanks

You need to work with http://drupal.org/project/triggerunlock, because all the actions are locked and only available to modules like node, comment and etc.

Feel free to report bugs. Early patch always need more patchs. :)

Besides, signup module does have email notification options itself.

yes, thanks.

I have a CCK which has a cck_email field.

I need to send email out to that cck_email field upon signup on/off.

I believe the existing signup email notification probably will not accommodate this.

But action/triggers will.

Thanks.

Hi shenzhuxi ,

would it be possible to modify this patch in calls rules?

rules has scheduling, like send email out in 30 days for a reminder, where I don't think triggers allow for scheduling.

I might be wrong.

cheers
Ed

Thanks alot for that patch.

I also need it for email-notification, but to send a mail to a user assigned to a cck-userreference field. Therefore I need the node-information in the action. Is there any reason, why you commented that one out?

Do the following to add a rules event along side a trigger.

1.
rename the attached file to

[signup.rules.inc]

in the [modules/signup]

2.
and add to signup.module file

2a.
>> line 1089
module_invoke_all('signup', 'signup_cancel', $signup, $node);
+ rules_invoke_event('signup_cancel', $node, $user);
>>

2b.
>> line 1269
// Insert the signup into the {signup_log} table.
signup_save_signup($signup);
+ rules_invoke_event('signup_sign_up', $node, $user);
>>

I have NOT look at it anymore yet. But its a start.

Having the rules event allows rules actions to be created.

Hope it helps someone. Enjoy.