Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
The patch is for adding triggers for signup module.
Comment | File | Size | Author |
---|---|---|---|
#33 | signup-rules.patch | 9.34 KB | Magnus |
#20 | signup_rules.patch | 3.89 KB | Magnus |
#19 | signup-rules.patch | 3.99 KB | smk-ka |
#18 | signup-rules.patch | 3.98 KB | smk-ka |
#8 | signup.rules_.inc_.patch | 788 bytes | juicytoo |
Comments
Comment #1
dwwThanks 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
Comment #2
shenzhuxi CreditAttribution: shenzhuxi commentedHi 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?
Comment #3
juicytoo CreditAttribution: juicytoo commentedThanks 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
Comment #4
shenzhuxi CreditAttribution: shenzhuxi commentedYou 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.
Comment #5
juicytoo CreditAttribution: juicytoo commentedyes, 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.
Comment #6
juicytoo CreditAttribution: juicytoo commentedHi 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
Comment #7
mattis CreditAttribution: mattis commentedThanks 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?
Comment #8
juicytoo CreditAttribution: juicytoo commentedDo 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.
Comment #9
rc2020 CreditAttribution: rc2020 commentedNice, looks promising. Subscribing.
Comment #10
BenK CreditAttribution: BenK commentedMuch need feature! Subscribing....
Comment #11
OliverColeman CreditAttribution: OliverColeman commentedThanks for code, juicytoo,
I needed to change the inserted line(s) around 1089 to:
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.
Comment #12
gaurzilla CreditAttribution: gaurzilla commentedJust what I need! Subscribing ..
Comment #13
Kristina-2 CreditAttribution: Kristina-2 commentedHi.
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.
Comment #14
Magnus CreditAttribution: Magnus commentedSubcribe
Comment #15
jludwig CreditAttribution: jludwig commentedSubscribe!
Comment #16
smk-ka CreditAttribution: smk-ka commentedSubscribe, and tagging.
Comment #17
welly CreditAttribution: welly commentedSubscribing 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.
Comment #18
smk-ka CreditAttribution: smk-ka commentedHere 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.
Comment #19
smk-ka CreditAttribution: smk-ka commentedOnly minor changes.
Comment #20
Magnus CreditAttribution: Magnus commentedsmk-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.
Comment #21
gausarts CreditAttribution: gausarts commentedSubscribing. Thanks
Comment #22
smk-ka CreditAttribution: smk-ka commentedJust 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.
Comment #23
my-family CreditAttribution: my-family commented#20 works perfect, thank you!
Comment #24
ezra-g CreditAttribution: ezra-g commentedThis should probably be revised to say "to a node" instead of "to a content".
Comment #25
MasterChief CreditAttribution: MasterChief commentedI tested the patch of Magnus and it seems to work fine, a reason to not yet be commited ?
Comment #26
grub3 CreditAttribution: grub3 commentedI applied #20 without problem.
But I cannot see any signup event in /admin/rules/trigger
Comment #27
MasterChief CreditAttribution: MasterChief commentedHi 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.
Comment #28
thomasmuirhead CreditAttribution: thomasmuirhead commentedHi 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.
Comment #29
MasterChief CreditAttribution: MasterChief commentedI 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:
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!
Comment #30
Magnus CreditAttribution: Magnus commentedI 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.
Comment #31
MasterChief CreditAttribution: MasterChief commentedHi Magnus!
I will test your patch when you will have time to attach it here :)
Thanks in advance for your work.
Comment #32
MasterChief CreditAttribution: MasterChief commentedHi magnus!
Did you find time to sort your code ?
Comment #33
Magnus CreditAttribution: Magnus commentedFinally 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
Comment #34
Magnus CreditAttribution: Magnus commentedComment #35
MasterChief CreditAttribution: MasterChief commentedHi 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 ?
Comment #36
MasterChief CreditAttribution: MasterChief commentedHi 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 ?
Comment #37
MasterChief CreditAttribution: MasterChief commentedOk 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 ?
Comment #38
WinWin CreditAttribution: WinWin commentedI 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?
Comment #39
MasterChief CreditAttribution: MasterChief commentedHi 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.
Comment #40
joachim CreditAttribution: joachim commentedNeeds work, and also more reviewing by people who use Rules more than I do :)
Rules event machine names should all be namespaced with the module name.
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?
This.
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.
Comment #41
Magnus CreditAttribution: Magnus commentedMasterChief: 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.
Comment #42
MasterChief CreditAttribution: MasterChief commentedHi Magnus, is it possible to talk to you on #Drupal channel on irc, i am connected if you can :)
Comment #43
Magnus CreditAttribution: Magnus commentedMasterChief: Sorry, I have a job interview in about an hour I have to attend =)
Comment #44
MasterChief CreditAttribution: MasterChief commentedOk 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 :)
Comment #45
fagoin 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
Comment #46
michellezeedru CreditAttribution: michellezeedru commentedHi - 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!
Comment #47
Zemo CreditAttribution: Zemo commentedHi 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?
Comment #48
joachim CreditAttribution: joachim commentedWell it looks like the patch here has that:
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.
Comment #49
Zemo CreditAttribution: Zemo commentedCan 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.
Comment #50
meichr CreditAttribution: meichr commentedHi,
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.
Comment #51
kclarkson CreditAttribution: kclarkson commentedWith 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
Comment #52
meichr CreditAttribution: meichr commentedHi 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.
Comment #53
meichr CreditAttribution: meichr commentedThe 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.
Comment #54
MasterChief CreditAttribution: MasterChief commentedHi 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.
Comment #55
meichr CreditAttribution: meichr commentedHi 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.
Comment #56
MasterChief CreditAttribution: MasterChief commentedI will test when the tgz file will be available to download :)
Comment #57
meichr CreditAttribution: meichr commentedHi 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.
Comment #58
MasterChief CreditAttribution: MasterChief commentedI tested your module and I made a new issue.
Let me know if you can do something about it :)
Comment #59
meichr CreditAttribution: meichr commentedHi 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.
Comment #60
jordojuice CreditAttribution: jordojuice commentedThis 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?
Comment #61
Magnus CreditAttribution: Magnus commentedIt 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.
Comment #62
jordojuice CreditAttribution: jordojuice commentedYou 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.
Comment #63
jordojuice CreditAttribution: jordojuice commentedI'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.
Comment #64
jordojuice CreditAttribution: jordojuice commentedAlright, 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.
Comment #65
jordojuice CreditAttribution: jordojuice commentedI 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).
Comment #66
dwees CreditAttribution: dwees commentedRelated 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).