After this current round of hacking on signup.module and issue triage, I'm starting to get the feeling that:
A) The email functionality in this module needs an overhaul to be a little more sane and consistent.
B) It should be easy to turn on/off various kinds of email functionality, since not all of it makes sense on every site. In fact, I don't personally use any of it, except broadcasts.
So, my rough proposal would be to split out the code into the following 4 modules:
signup_email_broadcast (email to everyone signed up for a node).
signup_email_notify (notification emails to admins/event coords when people signup/cancel)
signup_email_confirm (emails to people as they signup)
signup_email_remind (emails to people N days/hours/minutes before an event is going to start)
Each module could work its magic via some combo of the following hooks:
hook_form_alter()
hook_cron()
hook_signup_sign_up()
hook_signup_cancel()
...
We'd have to give a little more thought to how this should interact with the date backend stuff, since 1/2 of that is for signup_email_remind, and at first consideration, it'd be a little lame to split apart the backend stuff in 2 parts (separate files you have to consider for each backend, etc).
But, I think this would be a good move:
- it'd hopefully simplify the core signup.module
- it'd make it easy for sites that don't want this stuff to not have to pay for it in resources
- it'd give us a chance to make all of this more sane as we refactor it
Thoughts?
| Comment | File | Size | Author |
|---|---|---|---|
| #38 | signup-messaging-290305-38.patch | 17.23 KB | scottrigby |
| #37 | signup-messaging-290305-37.patch | 17.18 KB | scottrigby |
| #30 | signup-messaging.tar_.gz | 177.51 KB | stborchert |
| #14 | 2009-02-04-signup_mail_module.patch | 38.15 KB | mikl |
Comments
Comment #1
dwwAfter more time in the issue queue and in the code, this is definitely needed. Some inter-related problems this would give us a chance to address:
#237986: Default/Per Node Confirmation
#286979: Global Signup Settings for All Nodes
#295865: Default Emails based on Content Type
#295542: Add user tokens to those able to be substituted in Signup emails (Raises the specter of needing separate email templates for auth vs. anon users)
#118794: Using Event author for "send signups to" address (Talks about needing to specify who *gets* the notification email that someone signed up on a site-wide vs. per-node basis)
...
Not to mention all the other various email-related feature requests floating around in the queue:
#72824: Ability to edit "From" field (or name) in notifications
#103567: Multiple/configurable coordinators
#130953: Allow admins to customize the notification email
#158044: Reminder X Hours Before Event?
#291875: Automated email of signed up participates upon sign-up period end
#297274: Send emails to cck email field, rather than to node author
...
I think we should use this issue to hash out how all these various settings and features *should* work, and then work on a big cleanup patch. We're going to need schema changes, UI changes, and some serious code reorganization. Generally, I hate big patches that do too many things, but in this case, it seems like things are so spread out and yet inter-woven, and need such major changes, I think it's going to be even more difficult to try to clean it up with incremental small patches. That's part of why things are in the shape they're in now, in the first place (these features were all added at different times without any real thought for the overall structure of how they should fit together). At this point, I'd rather burn down the house and rebuild it from scratch how it should be. ;)
That said, I don't think all the features need to be implemented at the same time. Fundamentally, I'm talking about:
A) Identify all the spots when this module should be able to generate an email. Currently:
Other possible times:
B) Make it possible to have a global default, a per-node-type default, or a per-node override for the templates for each of those (and by template, I mean not just the existing body templates, but also subjects, and where appropriate, from and to addresses).
C) Identify the various permissions we need to control who has access to change these settings at various levels. I'd like to make it possible for admins to restrict how far "down the chain" things are allowed to go (from no customization at all -- everything uses the global site-wide defaults, to complete flexibility for the (therefore trusted) node authors).
D) Reorganizing the code to maximize code reuse, and to move most of this out of the "core" signup module. It's not yet clear to me if each spot from (A) should go into its own submodule, or what. Probably I could see signup_broadcast, signup_reminder (depends on a date solution, either CCK or event), and signup_notify (for everything else). I'm also not sure if the shared code should just live in signup.module itself, or if we should have a signup_email module to hold the common stuff that would be a dependency for all these others.
Phew, that's a lot. ;)
At this point, it's way too much to do for 5.x-2.5. I think all of this (and anything mentioned here) should wait for the following:
#86462: add backend support for cck date fields
#293005: Create a signup 5.x-2.5 release
Yes, I know the CCK patch sort of touches related code, but that's already 95% done, and is really one of the top features I've been aiming for in the 5.x-2.5 release, so I'd like to just wrap that up as-is, and then we can rip it back apart again as necessitated by this effort. So, even though implementation in here is blocked on the above, discussion should start now. Since so many things are going to be blocked on this, I'm bumping the priority to critical.
Thoughts?
Comment #2
dwwOh, I forgot #131445: Signup cancelation notification which is another good spot for a possible email notification: whenever a signup is canceled. Just like the initial signup notification, this could be potentially two recipients: the user whose signup is now canceled, and the "event" admin/coordinator(s).
Comment #3
stborchertJust some thoughts...
B): What do we need for it to work?
Currently the global template is stored in the {signup} table with
nid = 0.Does it make sense to have an additional table (called {signup_templates}) to store the default template and node-type specific templates?
This wouldn't be a exact copy of {signup} because
nidshould be replaced by a string value (content type name or an empty string for the global template) and subject and addresses are stored additionally.{signup} then only stores signup data and no templates.
C): "modify global templates" and "modify templates per node type" would be two permissions to start.
Do you think it would be useful to have one permission per node type? There could be a need for admins to grant the "modify template" permission for one node type to a user but not for other types. How to handle this (could become a very long list of permissions)?
Ah, and of course "modify template per signup enabled node".
Comment #4
socialnicheguru commentedsubscribing
could we also add a "thank you for coming" email?
Comment #5
Anonymous (not verified) commented+1
Comment #6
geodaniel commentedIt would be good if these sub modules could work with signup status module so that emails could be sent out only to those who have certain statuses (such as those who have attended that they will attend). Some of this could be done by just overriding or replacing the functionality in signup status, but it'd be nice if we could reuse as much as possible, perhaps using hooks to filter the recipient list.
Moving this on to 6.x-2.
Comment #7
socialnicheguru commentedalso, would it be worthwhile to have it work with messaging framework and notification. OG is moving in that direction. That way you could take advantage of the various features that are available in those modules including sending via SMS.
Comment #8
miklI think #7s suggestion is a really good idea. I've dabbled a bit with notfications/messaging before, and although the API is a bit intimidating at first, it's a very nice abstraction to not have to deal with message delivery etc., and it's a real nice way to go, since it gives a lot more control to community sites.
I'd like to take a stab at it, if you don't mind.
Comment #9
dwwIf you want to integrate with notifications/messaging, the way I'd like to see that work is:
A) signup itself just invokes a hook at all the interesting points when something might need to be triggered.
B) signup_notifications.module implements those hooks and invokes the right notifications/messaging API calls to generate the notifications.
We could still move the existing email code out into submodules that also implement the appropriate hooks, e.g. signup_reminder.module, etc, for folks that don't want all the complexity (and power) of the entire notifications framework.
If you want to take a stab at this, great, please do!
Thanks,
-Derek
Comment #10
3duardo commentedSubscribing +1
Comment #11
sziggle commented+1 for integration with messaging and notifications frameworks. seems great to let those modules do what they are designed to do and let this module concentrate on core functionality of user signups.
Comment #12
dwwProblem is that those are big and complicated to setup, and it's something of a barrier to entry for folks who just want reminder emails. ;) As I said above, I'm not opposed to providing integration, I just want to make it a plugable option, not a hard dependency.
Comment #13
miklHmm, I've gotten some work done, and I've come across a bit of a conundrum.
If we want to separate the signup_mail stuff from the main signup module, it follows that we should remove the e-mail-related columns from the signup table. But before they are removed, we should make sure to migrate the data away into the new signup_mail table. But how do we make sure that has happened before we drop the columns, since the migration should probably happen in signup_mail.install and the dropping in signup.install…
To my knowledge, there's no way to guarantee that the upgrade hooks run in any specific order.
Comment #14
miklHere's the work I've done so far. The data migration works, but the code for dropping the old columns have been commented out until we can find a way to drop them safely.
I've also started moving the code out of the signup module, but that is far from done.
Comment #15
giordy commentedSorry for my asking, does the patch work with 5.x-2.7?
Thanks
Comment #16
eoneillPPH commented+1 for #4: You have this nice "attended" feature, but there not a single blessed thing you can do with that information, that I've been able to discover. Am I wrong about that? What's it for? How can you use it? Follow-up emails would be the next obvious step, and relates to this thread/issue.
Even better, though I daresay a completely new feature request, would be the ability to select all those who attended and assign them a new role.
Comment #17
OliverColeman commentedSubscribing. I'm very interested in a solution that works with notifications/messaging. Will probably test and help with module(s) for this. Is the patch in #14 for the D6 version?
Comment #18
michellezeedru commentedSubscribing
Comment #19
firstlut commentedI tried the patch, which failed, and then I realized it was against the aborted 6.x-2.x-dev version.
But I very, very much want this.
Not having cancellation notification is a deal-breaker for my users. Signup Status doesn't help, because it only takes one site-wide set of addresses.
I tried the hacks: Rules, Form Rules. Didn't help.
Guess I need to sit down and learn PHP, because I don't understand why signup notification is automatic, but cancellation notification is a big hairy deal. And I could help. Also.
Comment #20
xsean commentedhi,
is signup module allow user/member to set their own X number of days before event start?
Comment #21
firstlut commentedNo, those are set by the administrator/author of the event and apply across the board to all signups.
Comment #22
BenK commentedSubscribing....
Comment #23
emdalton commentedI still need this one: #118794: Using Event author for "send signups to" address
So if that's been deferred to this effort, is this effort still live?
Comment #24
fsto commentedSubscribing for e-mail attachments like pdf files
Comment #25
macpharmer commentedsubscribing
Comment #26
minus commentedsubscribing
Comment #27
tevih commentedYikes! Two years old...
subscribing.
Comment #28
49digital commentedsubscribing.
Comment #29
miklComment #30
stborchertSome month ago I started rewriting signup to use http://drupal.org/project/messaging instead of sending the mails on its own.
The attached file is a rough start and just a reminder for me to work on this again in the near future.
Comment #31
stenjo commentedI was looking for a way if attaching an application form to the signup confirmation and ended up here.
Seems like a good idea to split the e-mailing out in other modules.
A'm an able programmer and currently co-maintains FAQ_Ask.
How can I contribute and where to start?
Comment #32
stborchert@stenjo:
If you have lots of time you could try to redo my previous work and use http://drupal.org/project/messaging for sending mails from signup.
Building a patch for this would be a great step to remove the email functionality from "core" signup module.
Comment #33
ambereyes commentedsubscribing
Comment #34
jpcwebb commentedDefinitely would like to see this - also, would it be possible to have notifications of signup based on roles rather than specific users?
Comment #35
scarvajal commentedSubscribing
Comment #36
pribeh commentedsubscribe. I highly suggest someone take stBorchert's work in #30 and expand on integration with the messaging framework. With 18018 sites using the messaging framework signup would likely only see a boost in usage as a result. Personally, my company is not building a single site without the messaging and notifications modules.
The official signup module thread for messaging and notifications handling of outgoing mail/messages is here: http://drupal.org/node/369637#comment-4086116
Comment #37
scottrigbySo - this was a while ago - and fully aware the 2x branch is no longer active.
However, to get the ball rolling again, I merged and made a patch from #30 -- against the snapshot stBorchert was working on (took a bit to find which one! For my ref, it was git reset --hard 17832bd3f95a0e2ddf6edaa0f0a4e52865ab88df).
There's no way this is going to apply cleanly to 1.x yet, but it's a start
Comment #38
scottrigbystraight git-formatted patch against 1.x
I wonder what this will break...