Notify Admin of Subscribe/Unsubscribe
callison - December 11, 2008 - 05:14
| Project: | Simplenews |
| Version: | HEAD |
| Component: | Usability |
| Category: | feature request |
| Priority: | normal |
| Assigned: | callison |
| Status: | needs work |
Description
There is no easy way that I have found to notify admins of users subscribing or unsubscribing. It seems that this could be fairly easily undertaken as an action (i.e. provide 'Email Admin' action on subscribe/unsubscribe triggers) or as an option in the simplenews configuration. However it is implemented, I think this is an important aspect that is needed in this module.

#1
What is there at Simplenews triggers is a very basic implementation. I can't remember to what extend it has been tested.
What you describe is very usefull functionality and is exactly where actions and triggers are for. It probably needs some code to get it working. Patches are welcome.
#2
I'm working on a patch for this, but I'm having just a little bit of trouble.
I've got a new action defined and I added a case in the simplenews_mail() function to handle mailing to the recipient admin. My trouble, though, is figuring out how I get the tid of the newsletter that was just subscribed/unsubscribed to/from.
I know that the account data is passed into the action function I created, and there is an array $context['account']->tids, but the index of that array is the tid itself, which I don't know. So, how do I get the tid to reference the $context['account']->tids in the first place?
Here is the action function I wrote:
function simplenews_send_email_action($object, $context) {
$account = $context['account'];
//This will get the first tid from $account->tids but that assumes the user is only subscribed to one newsletter
//Also, this will only work if the user is unsubscribing - I don't know where to get the tid (of the current newsletter) when the user first subscribes
// The account array has a list of tids the user is subscribes to but which one is the 'current' one?
foreach( $account->tids as $tid ) {
if( !isset( $my_tid ) ) {
$my_tid = $tid;
}
}
$params['from'] = _simplenews_set_from();
$params['context']['account'] = $account;
$params['newsletter'] = taxonomy_get_term( $my_tid );
drupal_mail('simplenews', 'action_send_email', $context['recipient'], $account->language, $params, $params['from']['address']);
}
And here is the case in simplenews_mail():
case 'action_send_email':
$account = $context['account'];
$newsletter = $params['newsletter'];
$message['headers']['From'] = $params['from']['formatted'];
if (!simplenews_user_is_subscribed($account->mail, $params['newsletter']->tid)) {
$subject = 'User subcribed to ' . $newsletter->name . ' newsletter';
$body = $account->mail . ' just subscribed to ' . $newsletter->name;
}
else {
$subject = 'User unsubscribed from ' . $newsletter->name . ' newsletter';
$body = $account->mail . ' just unsubscribed from ' . $newsletter->name;
}
$message['subject'] = $subject;
$message['body'] = $body;
break;
I fee like there is an easy way to do this, but I sure can't figure it out. Any help would be appreciated. Many thanks.
#3
The tid sould come in into the action via the context or object, depending on which trigger you use. I guess the object if you use the subscription trigger.
Note: If you are planning to make a patch, make sure you follow drupal coding standards.
#4
Actually, I don't think the tid of the newsletter being subscribed to (or unsubscribed from) is passed in through either of those variables. If a user is already subscribed to any newsletter at the time of the subscription, an array of all the tids the user is subscribed to is passed in through $object but not the current one. Does that make sense?
I'm wondering if we could add a variable to the $context array in the simplenews_call_actions function. The current function looks like this:
function simplenews_call_actions($op, $subscription) {// Only call actions when the simplenews_action module is enabled.
if (!module_exists('simplenews_action')) {
return;
}
$aids = _trigger_get_hook_aids('simplenews', $op);
$context = array(
'hook' => 'simplenews',
'op' => $op,
'account' => $subscription,
);
foreach ($aids as $aid => $action_info) {
actions_do($aid, $subscription, $context);
}
}
Could we change it like this to include the $tid
function simplenews_call_actions($op, $subscription, $tid) {// Only call actions when the simplenews_action module is enabled.
if (!module_exists('simplenews_action')) {
return;
}
$aids = _trigger_get_hook_aids('simplenews', $op);
$context = array(
'hook' => 'simplenews',
'op' => $op,
'account' => $subscription,
'tid' => $tid,
);
foreach ($aids as $aid => $action_info) {
actions_do($aid, $subscription, $context);
}
}
#5
It has been some time since I worked on simplenews_actions module. Feel free to write the required code and post a patch here.
#6
I have attached two patches, one for simplenews.module and one for simplenews_action.module.
Here are the changes I made to simplenews.module:
Here are the changes I made to simplenews.module:
#7
I updated simplenews_action.module so that you can choose which newsletters you want to be notified about.
#8
I edited your simplenews.module patch adding some t() calls and some text adjustments. For latest 6.x-1.x-dev.
How can we merge our edits?
Regards
#9
Thanks for your review. How about you just upload your revised patch or send me the changes and I'll do it.
#10
I'm behind a Firewall now, I attach my simplenews module. you can view my edits using a diff SW.
Regards
#11
Thanks. I have made the changes and posted the updated patch.
#12
Maybe you should also add a t() call in
<?php//Anonymous users need a name
if (!$user->uid) {
$user->name = 'Anonymous User'; // should be t('Anonymous User'); or I'm wrong?
?>
#13
I re-rolled to 6.x-1.x-dev CVS version and edited (again) your patches:
- Changed Action name (more clear)
- Added t() to "Anonymous User"
All the edits are working fine and notify eMails are sent Ok.
Regards
#14
Looks good. Thanks for your changes.
I added a feature to allow entering a username as a recipient (with autocomplete).
I also fixed the logic for determing if this is a subscription or UNsubscription. The original if statement used simplenews_user_subscribed() which failed if the user used the "Subscribe/Unsubscribe" button on the simplenews block. I changed it to look at the $_POST['op'] variable which, I believe, is very consistent.
Lastly, I made the multiple newsletter select (in the action configuration form) a required field.
Thanks for your help!
#15
Thanks for the code so far!
The content of the email should be more flexible. Themable at least, or perhaps better like 'Send e-mail' trigger does it with a webform. The latter should than have a way of inserting the newsletter name, tid, etc. into it.
Can the (un)subscribe action be modified so 'Send e-mail' and other actions can use it?
The patch contains code style errors. Please use Coder Module to check.
#16
This is fun! I've never submitted code to Drupal before.
I updated the action configuration to include a Message Settings fieldset where there is a subject, message, and checkbox (to include link for user's account page).
I allowed for a %status ('Subscribe' or 'Unsubscribe') variable within the subject and/or body fields. This allows people to use the same subject and message for both subscriptions and unsubscriptions. For example, the subject could be
[%newsletter] %status Notificationwhich makes the subject[Newsletter Name] Subcribe Notification. Also, the message could containSomeone has %statusdwhich becomesSomeone has Subscribed. After playing around (making separate subscribe/unsubscribe message settings, etc.) this seemed to be the best way to proceed. Any thoughts would be appreciated.These changes checked out fine in the Coder Module. I did, however, notice some style errors (and even a potential SQL security issue) in simplenews.module not related to these changes.
Sutharsan, I wasn't sure what you meant by modifying the action so 'Send e-mail' can use it? Would you please describe how that could be done and what the purpose would be. Thanks.
#17
Hi Callison, well done! You know.. OpenSource projects can be fun if you contribute (.. and if the community is responsive) ;)
I'm wondering if a multiple-receivers feature should be implemented.. and if receivers can/must be registered user (selection with auto-complete field?)
Another question is: is the sent message translatable? I can't find any Drupal suggestion about user-input translation..
Regards
#18
Thanks!
As far as multiple receivers go, this should probably not be a feature of this patch because users can add as many actions as they want defining who to send notifications to. However, that said, it could be really useful to be able to, for example, notify all moderators when someone subscribes (and enable that with one action). I'm up for suggestions on if and how this could be implemented. I already have the auto-complete functionality for a single recipient. I would have to think about how to best implement that with multiple recipients. Again...suggestions.
I don't know about translatability (is that a word?). Doesn't drupal_mail() automatically translate the message into the recipient's default language?
I appreciate your help and suggestions. Thanks.
#19
I suggested to investigate the possibility of using the 'send email' because I'd rather see smart use of existing tools than introducing new ones. Currently the 'send email' action can not be triggered by (un)subscription because this trigger is defined to only trigger 'simplenews' actions. Either the trigger definition must be altered or the action definition must be extended (if possible). Next step would be to get the (un)subscription and newsletter status/tid/name into the email.
Code style errors: spaces after/before opening/closing parenthesis and escaped quotes in t-string.
$user = user_load( $account->uid );...
t('Include a link to this user\'s account page if they are registered at this site'),
The body of the email should at least be themable or better editable by the action admin as the drupal core action 'send email' does.
I would discourage the option to send the email to multiple recipients. This notification email is usefull if you have a small or medium size list. A large list has zillions of (un)subscriptions; individual notifications would drive me mad. This notification is i.m.o. only an intermediate solution to a better subscription management system with proper listings of (un)subscribers on date, name, address, source, etc.
#20
@sutharsan: Isn't the only way to edit a trigger definition in system.module itself (i.e. add
'simplenews' => array('subscribe', 'unsubscribe'),to the trigger definition) or is there a way to extend that in simplenews_action.module? I'm kind of new to this, so I don't understand what you mean by extending the action definition in a way that will be triggered by a system trigger ('send e-mail'). If you get a chance, please expound on this a little more.In the meantime, I re-rolled the patch fixing the code style errors you mentioned. If I'm understanding you correctly, the body of the email already is editable by the action admin (including the all of the token variables I mentioned above - plus an additional %to_from token in this re-rolled patch). Thanks.
#21
The (un)subscribe trigger definition (simplenews_hook_info) defines what kind of actions can be triggered. Currently only 'simplenews' actions can be triggered. Perhaps this can be extended with 'user' actions.
The body of the email is currently only editable by using the localization. This is not sufficient. All drupal output must be themable. Also consider editable output as the 'send email' action uses.
#22
I tried to apply this patch to the latest release (1.0-rc4), and I get this text after the "Hunk #5 succeeded" line:
patching file simplenews_action.module
patch unexpectedly ends in middle of line
Hunk #2 succeeded at 304 with fuzz 1.
Does that mean that the patch applied successfully, or not? Sorry, this is my first time applying a patch, and I would have trouble believing that "unexpectedly ends" means that it was a success.
Thanks for the help. I'd really like to get the Notify Admin feature working!
#23
How i can port these patches to drupal 5x, or, well
waht's more simple?, update dru 5x to 6, or port this patch to 5?
thank's
#24
With the title changed, people will have a hard time finding the valuable discussions in this thread.
I therefore restore the old title.
#25
subscribe
#26
I am posting a patch to system.module that will allow the send e-mail action to be used for Simplenews triggers. Following is a quick step-by-step guide to creating your actions and triggers for alerting admins on (un)subscriptions:
Naturally, this assumes you already have simplenews and simplenews_actions modules installed and working and that you have applied this patch to system.module
Hopefully this makes sense. This is good because it's so simple, but I think it could be better if you could choose the newsletter that you want to be notified of, include more variables specific to simplenews in the message, etc. I'll work on it some more - any ideas, comments, and suggestions are very welcome.
#27
I should have known better than to hack the system module. There's a hook for altering the action. So...I have now used that and applied it to simplenews_action.module. Much better! The above guidelines still work, just don't apply that patch, use THIS ONE.
This could still be better by adding tokens, etc. so I'm looking into it and will post more later.
#28
subscribe
#29
subscribe
#30
I actually achieved this using the Trigger Unlock module - http://drupal.org/project/triggerunlock
You can then set up an action (i.e. Send email...) and the Trigger Unlock module allows you to assign any Action to any Trigger. So on Home » Administer » Site building » Triggers, instead of "No available actions for this trigger." you will be able to assign any action to the 'Subscribe' or 'Unsubscribe' Trigger.
#31
Thank you for the tip, I also tried Triggerunlock and it works.
I used Tokenized email option, of all I tested only Global tokens work (FYI).