The patch is for adding triggers for signup module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww’s picture

Version: 6.x-1.0-rc5 » 6.x-1.x-dev
Status: Active » Needs work

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

shenzhuxi’s picture

FileSize
2.38 KB

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?

juicytoo’s picture

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

shenzhuxi’s picture

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.

juicytoo’s picture

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.

juicytoo’s picture

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

mattis’s picture

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?

juicytoo’s picture

Title: Add trigger for signup » Add rules event for signup
FileSize
788 bytes

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.

rc2020’s picture

Nice, looks promising. Subscribing.

BenK’s picture

Much need feature! Subscribing....

OliverColeman’s picture

Thanks for code, juicytoo,

I needed to change the inserted line(s) around 1089 to:

global $user;
rules_invoke_event('signup_cancel', $node, $user);

and the inserted line around 1269 to:
rules_invoke_event('signup_sign_up', $node, $account);

And the .inc file could go in signup/includes/ to be more consistent.

It'd be nice to see at least this included in this module. Is this code the right way of going about what we're trying to achieve, dww? If it is then I'll roll it as a patch against head.

gaurzilla’s picture

Just what I need! Subscribing ..

Kristina-2’s picture

Hi.
I was just thinking of adding an triggered rule in order auto e-mail users who confirm as attending (I have sign up status installed) on events with certain taxonomy terms selected with details for the location as those events are in private homes.

I think adding the ability to create rules for sign up related actions would be fantastic.

Magnus’s picture

Subcribe

jludwig’s picture

Subscribe!

smk-ka’s picture

Issue tags: +rules integration

Subscribe, and tagging.

welly’s picture

Subscribing too. It would be great if the signup data could be passed to the rule as well but this is definitely a start. Hopefully it can be improved and included in the core module.

smk-ka’s picture

Status: Needs work » Needs review
FileSize
3.98 KB

Here you go: this patch adds support for the signup object, which gives you access to all tokens. It is different from the one in #8 in that I chose to split the signup_sign_up event into two: signup_insert and signup_update. That should make it easier to react to a specific one, i.e. first time signups and updates thereof.

smk-ka’s picture

FileSize
3.99 KB

Only minor changes.

Magnus’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
3.89 KB

smk-ka: I use Windows and SmartCVS and could't apply your patch (but that is probably due to the programs I use), so I applied it manually. I put the signup.rules.inc in signup/includes and then I made three separate rules and sent an email with the three new rule events. Everything worked as expected. Thanks a lot!

What I miss now is a rules event to add users. That would be awesome to use with a VBO view to be able to mass add users to an event. Especially if you could choose either the signup core autocomplete widget, or this VBO view to add users to a signup. Does anyone have a solution for this?

In case anyone else have problems to apply the patch, I have attached a new one.

gausarts’s picture

Subscribing. Thanks

smk-ka’s picture

Just FYI, there is a new module, Signup Application Form, which provide printable copies of signup data and relies on rules support (ie., this patch) to be actually useful.

my-family’s picture

#20 works perfect, thank you!

ezra-g’s picture

This should probably be revised to say "to a node" instead of "to a content".

MasterChief’s picture

I tested the patch of Magnus and it seems to work fine, a reason to not yet be commited ?

grub3’s picture

I applied #20 without problem.
But I cannot see any signup event in /admin/rules/trigger

MasterChief’s picture

Hi everybody, i apply the patch in #20, all seems to be fine, but i would like help.

Here my problem, i made rules, and i would like if a contidition is not true to stop the user signup or if it's not possible to cancel it.

I appreciate any help on this.

thomasmuirhead’s picture

Hi there,

I just wanted to say I applied the patch in #20 and it seems to have worked. It's made signup useable on our site, as without it all signups to all events get sent to the same email address - this patch makes it possible to send different event signups to different addresses entered on event creation - really good!

So, thank you.

@grub3 - what it does is create events at /admin/rules/trigger/add in the Event drop down, you'll see signup options. If you then choose one you can then use tokens from the signup, from the node the signup refers to and from the user who has signed up. I hope that helps.

MasterChief’s picture

I applied the patch of Magnus and now I am trying to make a patch to do an action call "Cancel User Signup" here my code:


/**
 * Default arguments to signup rules events.
 */
function signup_rules_events_signup_arguments($have_node = FALSE) {
  return array(
    'signup' => array(
      'type' => 'signup',
      'label' => t('signup object'),
    ),
    'node' => array(
      'type' => 'node',
      'label' => t('node to which the user signed up'),
      'handler' => $have_node ? '' : 'signup_rules_events_argument_signup_node',
    ),
    'account' => array(
      'type' => 'user',
      'label' => t('acting user'),
      'handler' => 'signup_rules_events_argument_signup_user',
    ),
  );
}

/**
* Implementation of hook_rules_action_info().
* @ingroup rules
*/
function signup_rules_action_info() {
  return array(
    'signup_action_cancel_signup' => array(
      'label' => t('Cancel user signup'),
	  'module' => 'Signup',
      'arguments' => signup_rules_events_signup_arguments(TRUE),     
    ),
  );
}

/**
* Cancel the Signup
*/
function signup_action_cancel_signup($signup,$node,$user) {
  signup_cancel_signup($signup);
}

This code doesn't work, it's the first time i try to make an action rules so if somebody could help, it would be great!

Magnus’s picture

I do have an action to cancel users, but I haven't had the time to attach it here. I made some improvements to be able to bulk add users with a Views Bulk Operation view. I just have to sort out the code.

MasterChief’s picture

Hi Magnus!

I will test your patch when you will have time to attach it here :)

Thanks in advance for your work.

MasterChief’s picture

Hi magnus!

Did you find time to sort your code ?

Magnus’s picture

Assigned: Unassigned » Magnus
Status: Reviewed & tested by the community » Needs review
FileSize
9.34 KB

Finally I found some time to sort this out. The action is maybe not as you wanted it MasterChief. The action calls a Views Bulk Operation view.
I made this (with some help) to be able to bulk add users to a signup and mark diferent statuses for the users.

Please also review:
#972254: Allow administrators to add users after signup have closed
#242720: Mass/bulk user signup

Magnus’s picture

Version: 6.x-1.x-dev » 6.x-2.x-dev
MasterChief’s picture

Hi Magnus, i am happy you found time for patchs :)

I applied the patch, rules events appear but i have a problem with action to add:

=> Views Bulk Operations : Use signup object to execute a VBO programmatically

I choose the view : signup_status_user_vbo_admin

I choose the operation : Cancel Signup

Did i must enter something in View exposed input or in View arguments ?

I seen in VBO issues queue, a problem with Rules, in effect you can add an action but you can't edit it, it's a bug which will fix soon, so could you give the right action configuration which work for you to cancel an user signup ?

MasterChief’s picture

Hi again Magnus,

the bug of VBO with rules don't let me save the cancel action in a triggered rule, could you give the way you use to make an action which use VBO ?

MasterChief’s picture

Ok the maintainer of VBO make a new release, now i can save VBO action, but nothing happen for me, did you make rules signup with action VBO which work for you Magnus ?

WinWin’s picture

I installed this patch (#33) today and it seems to work perfectly. However, for it to be truly useful to me the rules have to affect the person that signed up NOT the person triggering the action. For instance, if I mark a signed up user as Attended, I want him to get points, not me. Is it possible to expand these rules to give that kind of result? Or is it already possible, somehow?

MasterChief’s picture

Hi Winwin,

Did you try to execute a VBO action in a triggered rule which use an event "User signup to a node" ?

For me, it works fine when the action isn't VBO.

joachim’s picture

Status: Needs review » Needs work

Needs work, and also more reviewing by people who use Rules more than I do :)

+++ includes/signup.rules.inc	Tue Jun 08 21:20:08 CEST 2010
@@ -0,0 +1,133 @@
+    'signup_insert' => array(
+      'label' => t('User signed up to a node'),
+      'module' => 'Signup',
+      'arguments' => signup_rules_events_signup_arguments(FALSE),
+    ),
+    'signup_update' => array(
+      'label' => t('User updated signup to a node'),
+      'module' => 'Signup',
+      'arguments' => signup_rules_events_signup_arguments(FALSE),
+    ),
...
+    'user_add' => array(
+      'label' => t('User was added to a signup'),
+      'module' => 'Signup',
+      'arguments' => signup_rules_events_signup_arguments(FALSE),
+    ),
+    'user_attended' => array(
+      'label' => t('User attended'),
+      'module' => 'Signup',
+      'arguments' => signup_rules_events_signup_arguments(FALSE),
+    ),

+++ signup.module	Sun Nov 14 07:05:43 CET 2010
@@ -875,9 +875,47 @@
       'description' => t('Mark signup did not attend'),

Rules event machine names should all be namespaced with the module name.

+++ includes/signup.rules.inc	Tue Jun 08 21:20:08 CEST 2010
@@ -0,0 +1,133 @@
+    $account = new stdClass();
+    $account->uid = 0;
+    $account->name = '';
+    $account->mail = $signup->anon_mail;
+    $account->roles = array(DRUPAL_ANONYMOUS_RID => 'anonymous user');

I don't like the look of this. Where does this fake user go? Who will be using it? Will they get what they expect?

+++ includes/signup.rules.inc	Tue Jun 08 21:20:08 CEST 2010
@@ -0,0 +1,133 @@
\ No newline at end of file

This.

+++ signup.module	Sun Nov 14 07:05:43 CET 2010
@@ -875,9 +875,47 @@
+  if ((arg(0) == 'node') && (is_numeric(arg(1)))) {

This is an action callback as far as I can tell... no guarantee of what path we're on, surely?

@#33 -- are those other patches you've linked related to this? Any dependencies?

Powered by Dreditor.

Magnus’s picture

MasterChief: I use a modifed view signup_status_user_vbo_admin and included a relation to Content Profile to be able to add the content fields field_first_name and field_last_name which holds my users first and last name. In the style I choose actions to perform and the last one in the list calls a function to alter signup status.

WinWin: I agree. I would need it as well, but this is not implemented yet with the patch.

Joachim: Thanks for the review, I will try to make some more work on it. I'm just a beginner with PHP and if someone wants to help it would be awesome. The other patches I mentioned are not required to be able to implement Rules in Signup, but the other patches require this patch to be able to mass signup users with a VBO view or alter signup status.

MasterChief’s picture

Assigned: Magnus » Unassigned

Hi Magnus, is it possible to talk to you on #Drupal channel on irc, i am connected if you can :)

Magnus’s picture

MasterChief: Sorry, I have a job interview in about an hour I have to attend =)

MasterChief’s picture

Ok no problem Magnus, my pseudo on irc is MasterChief2, when you will have time to go on irc, it will be great to talk, don't hesitate to contact me if you see me connected :)

fago’s picture

in addition to #40,
>+ 'label' => t('signup object'),
is too technically, users don't care about objects ;)

>+ 'label' => t('Node to which the user signed up')
Node is usually called 'content' in the rules ui

michellezeedru’s picture

Hi - I'm trying to understand how to make this work, and am having trouble following the thread. I upgraded to 6.x-1.0. I would like to use Rules to trigger an email to attendees when an admin marks them as attended. Do I need to use 6.x-2.x-dev, and apply all 3 patches from #33 for this functionality? If so, how stable is 2.x-dev to enable on a production site?

******
Update: Now I see the Signup Events in Rules - excellent, don't know why I didn't see them before. So I created a new rule on the event "User attended". No conditions. Action is "Send a mail to an arbitrary mail address". No email is sent when I edit a Signup and mark the person as attended. I first had a tokenized email address in the Recipient field, but tried removing that to a hard-coded email - still no email is sent.

Any ideas? If this question doesn't belong in this thread, just let me know and I'll create a new issue. Thanks!

Zemo’s picture

Hi all.
First i have to say sorry for my bad english.

That`s my problem. I want to give user who have assign as attended userpoints. With which patch is it possible to make the requested trigger? Or is that unpossible at this moment?

joachim’s picture

Well it looks like the patch here has that:

+    'signup_insert' => array(
+      'label' => t('User signed up to a node'),
+      'module' => 'Signup',
+      'arguments' => signup_rules_events_signup_arguments(FALSE),
+    ),

But I don't claim to understand exactly what is needed in conjunction with the other issues.

Unfortunately, I don't have the free time to look into this, and it doesn't look like the other maintainers do either. It's up to the people who want this feature to work together on this issue and get the patch working well enough to be committed -- or hire a developer to do the work.

Zemo’s picture

Can someone tell me with wich post patch i have to begin? I have signup 6x.1.0 and post #33 the text that is in includes/node_admin.inc befor the new lines isn`t im my file. Please give me a list how i have to work. Example first you have to patch post #XX than #YY.

Thanks for help.

meichr’s picture

Hi,
I've sucessfully used the patch in #20 (there are newer versions below) as a base for adding rules support to signup. I also needed a rules action for disabling signup on certain conditions so I added such functionality (not perfect yet, but working).
When working through the code I found that most of the functionality could also be outsourced into another module working with the "Signup API", though the API for disabling signups for a node was not detailed enough for my case, I needed more context, from where the hook is called (signup form or signup tab menu to be disabled).
So, I suggest to create another module for Signup Rules support, only needing a small Signup API change (patch or change with next version).
What do you think?
dww, would you agree, help with the API change necessary in my opinion?

Thanks,
Christian.

kclarkson’s picture

With these new patches is it possible to redirect someone to a different page "after" signup?

Have these patches been added to any of the versions?

Thanks

meichr’s picture

Hi kclarkson,
yes this is possible. Try the following:
1) Download a new module "signup_rules", still in approval phase at the moment, at this issue "http://drupal.org/node/1068348", attachment of comment #1.
2) Untar the tgz file at /sites/all/modules and activate the module "Signup Rules Integration" together with the signup and rules modules at the Administer->Site Building->Modules page.
3) Set up a triggered rule "User signed up to a node" using the rules admin menu and add the rules action "Page redirect" including the destination page.
This allows you to redirect to the destination page after the user signed up to a node. You can refine the rule by adding conditions, e.g. the content type to be signed up for which the page redirection should occur, etc.
Please let me know whether the module works for your needs.
Thanks,
Christian.

meichr’s picture

The proposed signup rules module now includes rules elements to allow suppressing the signup form based on rules conditions and a new condition to check whether a user is signed up to content.
If you are interested in the new version, please have a look at the project's sandbox site:
http://drupal.org/sandbox/meichr/1077134.
It still needs reviewers to be able to become a full project.
Thanks,
Christian.

MasterChief’s picture

Hi meichr!

Very good idea, i like what you add to Magnus patch.

I don't know about sandbox with drupal, it would be very interesting to become a real module.

I would like to test it.

meichr’s picture

Hi MasterChief,
thanks for your offer to review the module. A sandbox project usually is for developers, who can use git to access the project for review.
You are welcome to review the current version of the module. I'll create a tgz file out of the current project version, later (need about 12 hours to be on my dev computer again), and let you know here.
Christian.

MasterChief’s picture

I will test when the tgz file will be available to download :)

meichr’s picture

Hi MasterChief,
the tgz and zip files of the current signup rules module are now available for download at the signup rules project application issue.
Thanks for your review.
Christian.

MasterChief’s picture

I tested your module and I made a new issue.

Let me know if you can do something about it :)

meichr’s picture

Hi MasterChief,
I've added a new condition "Is signup status open for content" which you can use in a triggered rule with the event "Display of signup form is being inquired". Please have a look at your issue at the signup rules sandbox project for the solution.
Christian.

jordojuice’s picture

This Signup rules integration was submitted as a new project application in the project application queue, but it now appears the application was abandoned. I would just like some clarification on this before I move forward. The maintainers of Signup are indeed willing to commit a proper rules integration patch, correct? If so I think this would be a great addition to the module itself rather than starting another project. I'm willing to complete the rules integration by providing a patch instead of this separate module if that is preferred. Is it?

Magnus’s picture

It would be great to have the remaining issues cleared out with this patch. I have helped as much as I can to bring this forward, but I don't know how to resolve the remaining issues.

The last work I made was the patch in comment #33.
Joachim made a review in comment #40 and #48, and fago in comment #45
I don't know if meichr made some changes to the code in his sandbox site: http://drupal.org/sandbox/meichr/1077134, but he added some more features. I guess that would be a good start to bring a complete patch to Rules module.

jordojuice’s picture

You certainly have done some work! Thanks for the info, I'm going to work on this this weekend.

EDIT: but I'm still interested to know whether maintainers feel this could be it's own project (as Meichr was trying to do) - i.e. whether I should develop it as a full project or as a patch. I am willing to take on either circumstance, but I'm going to work on a patch in the mean time.

jordojuice’s picture

I've closed Meichr's project application issue which he hasn't responded to in well over three months and am taking on the Signup Rules project for myself. I'll be porting the module to Drupal 7 to work with the development release (because I need it!), and I've been working on a Signup Status Rules module as well.

jordojuice’s picture

Alright, well, I did a lot of work on both of these modules (Signup Rules and Signup Status Rules), and after doing a lot of work I reluctantly promoted them to full projects. The maintainers have seemed very busy and haven't objected to meichr's attempts to move it to another project, I tried to contact meichr months ago, and fortunately, I have all the time in the world for another project! And I really needed exactly this for a project that I need to finish this week!

http://drupal.org/project/signup_rules
http://drupal.org/project/signup_status_rules

I created alpha releases of both and have tested them as much as one man can. I changed the code quite a bit from what meichr and everyone that helped left as well. There are more conditions and events for managing signups, including canceling all signups for a user or node, mark users as attended/unattended, etc.

jordojuice’s picture

I also have completed most of the port to D7, but data types in Rules 2 have changed quite a bit, so that still needs work to be able to manipulate the signup itself (as opposed to signed up user or node).

dwees’s picture

Related to this issue: Signup module does not invoke Rules integration when doing Views Bulk operations, and instead operates directly on the database.

There are two choices, either the Signup Views Bulk actions should use a standard hookable method of updating the database (instead of updating it directly) to reflect the 'attended' or 'not attended' status, or it should include the hook for Rules in this location. Option 2 looks like the following (I'm not actually precisely sure what option 1 looks like).

/**
 * Action callback to mark a given signup that the user attended the node.
 *
 * @param $signup
 *   Reference to a fully-loaded signup object to record attendance on.
 *
 * @return
 *   Nothing: $signup object is modified by reference and the {signup_log}
 *   table is directly UPDATE'ed in the database.
 */
function signup_mark_attended_action(&$signup) {
  // only do this action if the signup changed
  if (module_exists('signup_rules')) {
    $signupstatus = db_result(db_query("SELECT attended FROM {signup_log} WHERE sid = %d",  $signup->sid));
    if ($signupstatus != 1) {
      signup_rules_invoke_attended('attended', $signup);
    }
  }
  db_query("UPDATE {signup_log} SET attended = %d WHERE sid = %d", TRUE, $signup->sid);
  $signup->attended = TRUE;
  watchdog('action', 'Marked signup @signup_id attended.', array('@signup_id' => $signup->sid));
}

/**
 * Action callback to mark a given signup that the user didn't attend the node.
 *
 * @param $signup
 *   Reference to a fully-loaded signup object to record attendance on.
 *
 * @return
 *   Nothing: $signup object is modified by reference and the {signup_log}
 *   table is directly UPDATE'ed in the database.
 */
function signup_mark_not_attended_action(&$signup) {
  if (module_exists('signup_rules')) {
    $signupstatus = db_result(db_query("SELECT attended FROM {signup_log} WHERE sid = %d",  $signup->sid));
    if ($signupstatus != 2) {
      signup_rules_invoke_attended('not_attended', $signup);
    }
  }  db_query("UPDATE {signup_log} SET attended = %d WHERE sid = %d", FALSE, $signup->sid);
  $signup->attended = FALSE;
  watchdog('action', 'Marked signup @signup_id did not attend.', array('@signup_id' => $signup->sid));
}