Posted by rorymadden on November 1, 2009 at 10:23pm
| Project: | Simplenews |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
| Issue tags: | rules integration |
Issue Summary
Thanks for the great module. I'm not sure if this is a feature request or a support request. I've started using simplenews on one of my sites however I would like to be able to set up a time delayed rule for when people sign up to a newsletter - e.g. Send out a specified issue 4 days after they have signed up. I know how to do this with the Rules module however I do not know how I can do this with the Drupal default Triggers and Actions modules.
Is there any integration between Rules and Simplenews, similar to the Simplenews_actions module?
Thanks for your help.
Comments
#1
So, this patch is a start for simplenews rules support. It provides actions for subscribing/unsubscribing users and e-mail addresses.
It can handle the subscription of multiple e-mail addresses at once and even the subscription to newsletters by their Term IDs. As a consequence you could use token or php to subscribe an user to a newsletter given in some cck field for example. We are using this by taking the country of the user profile and subscribing user to the right newsletter (we have one per country) after registration if a special newsletter checkbox is checked.
Sending newsletter actions have to be done though.
#2
Re-rolling patch from #1 against 6.x-1.x-dev (2010-Jan-18). Moved form functions into separate file. Added an additional action that mimics the rules action send mail to roles, but sends mail to users that are subscribed to selected newsletters instead of roles.
#3
I just had a look at the code (without trying it). From the rules point of view it looks, but I noted:
<?php+ foreach ($settings['email'] as $key => $address) {
+ if (valid_email_address($address)) {
+ $settings['email'][$key] = str_replace(array("\r", "\n"), '', trim($address));
+ }
+ }
?>
This looks dangerous. As validation is skipped when the address is not valid. Also simplenews tids get \r and \n replaced, which seems unnecessary if it are really tids.
#4
#5
Rerolling patch with fix to action for emailing all users subscribed to a newsletter.
#6
subscribe
#7
subscribe
#8
Is this still 'needs work'? If so, what needs to be done?
#9
This seems to be working well for us in production. Would be really nice to get this committed.
#10
Hmmm.... I have an issue at: http://drupal.org/node/731922#comment-2912156 and I am wondering if this patch will help me.
If I have a rule that gets triggered to send a message to a user, can I compare that against a simplenews list and only send a message if the user is subscribed?
#11
I'm not sure that this rule would do what you are hoping, mcfilms. This makes the newsletter lists available to use for sending out emails to.
#12
subscribe.
Will it be committed soon ?
#13
designerbrent: Which patch from this thread works for you ?
I tested #5 with both 6.x-2.x-dev and 6.x-1.1. In Rules I have simplenews actions and I used 'Subscribe an User to a newsletter' but it doesnt subscribe acting user to the selected newsletter.
Also I see #5 patch should be compatible only with 6.x-1.1 because this line:
$message['body'] = _simplenews_subscription_confirmation_text('unsubscribe_unsubscribed', $context['account']->language, $variables);
can be found only in this version of Simplenews module.
diff -upN simplenews/simplenews.module simplenews2/simplenews.module--- simplenews/simplenews.module 2010-02-11 11:53:38.000000000 -0800
+++ simplenews2/simplenews.module 2010-02-11 11:44:43.000000000 -0800
@@ -1642,6 +1642,10 @@ function simplenews_mail($key, &$message
$message['body'] = _simplenews_subscription_confirmation_text('unsubscribe_unsubscribed', $context['account']->language, $variables);
}
break;
+ case 'mail_to_subscribed_users':
+ $message['subject'] .= str_replace(array("\r", "\n"), '', $params['subject']);
+ $message['body'][] = $params['message'];
+ break;
}
I didnt create 'simplenews2' directory , Just applied patch in original dir. Is that what can be wrong ?
I use Drupal 6.16 and Rules 6.x-1.2.
#14
subscribe
#15
New features will only be added to major revisions.
#16
Sutharsan: It means it will be added to 6.x-2.x-dev , right ?
#17
It will be added when I or a co-maintainer has the time to go through the code.
#18
Would really be great to get this into 1.2 - especially if 2.0 is not expected in the next couple of months?
Any ETA on 2.0 going Alpha/Beta?
Thanks for the great work!
#19
This is my policy:
minor releases (6.x-1.1, 6.x-1.2) are used for bugfixes and security releases
major releases (6.x-1.x, 6.x-2.x) are used for new features
ETA of 2.x totally depends on my available time and contributed code by others. My time is currently limited. I plan to release 2.0 not after the release of simplenews 7.x
#20
subscribing
#21
Looks pretty fine to me.
Note that there are some help texts that don't correspond the "simplenews" case but refer to the origin.
Simetimes help text ist about "role" instead of "newsletter", ...
Can someone please fix the doc and submit a resulting patch against latest 2.x?
I would like someone to test each of the rules. (a simpletest would almost be too amazing...)
Looking forward to integrate this into simplenews soon!
#22
Subscribing (to remind me to work on that one day ;)).
#23
subscribing
#24
Some more comments:
Why did you add the mail_to_subscribed_users mail into simplenews_mail?
Isn't this a simplenews_rule specific mail?
#25
@miro_dietiker Yeah, the mail_to_subscribed_users should be in the simplenews_rules module rather than simplenews itself.
#26
Is there a way to send a newsletter via rules?
#27
Attached is the same patch from #5 only that is should apply correctly to the latest 6.x-2.x-dev snapshot from 2011-Jan-23.
I tested the send mail to subscribed users functionality and it works.
@benone, regarding #13: it probably wouldn't work with the patch in #5 because that one was creating a subdirectory (simplenews) inside simplenews directory.
See if the attached patch, which is exactly the same code only that should apply correctly, work for you.
#28
andreiashu,
I'm open to commit this topic.
However note that simplenews_action is also such a separate module. We should follow this approach and make simplenews_rules a separate module.
See also
http://drupal.org/node/620498#comment-3707998
Don't forget to move the rules specific mail into this module.
Also please check if there's some overlap with simplenews_action.
Would you please provide a patch with separate module and test it?
I'll then add it to the project simplenews.
#29
miro, thanks for your input on this.
I'll provide a patch that adds this as a module. I'll look into the 'mail_to_subscribed_users' as well.
Regards,
Andrei
#30
@Andrei
i'm very new to this Rules module...but the 'mail_to_subscribed_users' feature is exactly what I need in our project.
may I ask in which version of simplenews would your simplenews_rules_1.patch work? thanks.
#31
subscribing
#32
bjalopez
As you can see in the issue state, this issue is about 6.x-2.x.
#33
That seems like a fair question since there is a:
6.x-2.0-alpha1 tar.gz (76.68 KB) | zip (90.67 KB) 2011-Jan-01
and a:
6.x-2.x-dev tar.gz (79.6 KB) | zip (93.93 KB) 2011-Jan-22
#34
They're almost identical.
This patch will apply to both with a 99% chance.
#35
@miro and @mcfilms: thanks.
#36
Sorry for the delay. In the attached patch I moved all the rules based stuff into simplenews_rules.
From the initial patch I also added token integration (simplenews-[un]subsribe-url goodies). The only problem with this is that I don't know how to make the tokens to appear in the action's config form under "Token replacement patterns". Any help on how to do this is welcomed.
@miro_dietiker: I couldn't find a way to get rid of the 'mail_to_subscribed_users' case. We are basically sending a custom text mail and not a node one.
simplenews_mailis built towards a node sending system. I would like to get your opinion regarding this.Also: the attached patch is against the latest CVS 6.x-2.x branch.
Cheers,
Andrei
#37
Fine - thank you!
Implement simplenews_rules_mail() -- hook_mail() for simplenews_rules.module.
And then call drupal_mail('simplenews_rules' ...) and it will work cleanly separated.
#38
Good point Miro, attached patch as per your suggestion.
#39
#40
I didn't have time to test the last patch...
This one should fixes 2 problems, one of them related to unsubscribed users. Tested and works as expected.
Please review/test.
#41
Hi, Thank you!
Can we really say that the action functions are real simplenews elements - to be implemented in simplenews_rules context - integrating native simplenews processes into rules...?
If YES, i'm fine with the simplenews_action named function definitions and calls.
If NO, we should even prefix them to simplenews_rules_action.
Opinions first appreciated.
#42
Hi Miro,
Sorry, not really sure how to answer those questions as I don't really understand what you mean...
#43
As I understand an action is just that, a functionality, in this case subscribe and unsubscribe, exposed via hook_action.
http://api.drupal.org/api/drupal/developer--hooks--core.php/function/hoo...
Lots of modules use actions, one of them is Rules. The other is VBO, etc etc.
Not just Rules uses these actions.
So the answer to #41 is YES IMHO
#44
post removed, sorry
#45
Note that this issue blocks #1126948: Does Drupal for FB and Simplenews play nice together?
#46
Related to D7 topic
#1361288: Action implementation highly broken
If someone does the remaining qualified quality review, this might be added to D6. However, our current focus is to implement rules support in D7 cleanly asap. Time is too limited to push 6.x branch further from our side.
#47
Doing a review of the patch, also in regards to a possible 7.x port, It would be awesome if someone could port it to 7.x. And yes, I personally know that porting rules integration from 6.x to 7.x is often quite complicated, I did it multiple times myself already..
As miro said, nobody is really adding new stuff to Simplenews 6.x, active development happens (again) on 7.x.
+++ simplenews_rules/simplenews_rules.info
+++ simplenews_rules/simplenews_rules.info
@@ -0,0 +1,7 @@
@@ -0,0 +1,7 @@
+; $Id$
$Id$ is from the old CVS days, not necessary anymore.
+++ simplenews_rules/simplenews_rules.module@@ -0,0 +1,37 @@
+/**
+ * Implementation of hook_mail().
+ *
+ * Send simplenews mails using drupal mail API
+ * @see drupal_mail()
+ *
+ * @param $key: node | test | subscribe | unsubscribe
+ * @param array $message message array
+ * [from]
+ * [headers][From]
+ * [language] : preferred message language
+ * @param array $params parameter array
+ * [context][node] : node object of message to be sent
+ * [context][snid] : used for $key = subscribe or unsubscribe
+ * [context][from_name] : name of mail sender or site name (optional)
+ * [context][account] : account details of recipient
+ * [from] : array('address' => 'noreply@example.org', 'formatted' => 'site name <noreply@example.org>')
+ * [newsletter] : newsletter object (tid, name)
+ * [tokens] : tokens for variable replacement. Defaults to: user_mail_tokens()
hook implementations usually don't need detailed parameter descriptions, e.g $key.
If kept, it should be formatted properly, e.g. as a list so that it's recognised correctly by api.module. This will result in a large, unreadable text blog. Check the apidoc guidelines on how to do correct listings.
Also, the type (array) shouldn't be in here.
+++ simplenews_rules/simplenews_rules.module@@ -0,0 +1,37 @@
+function simplenews_rules_mail($key, &$message, $params) {
+ module_load_include('inc', 'simplenews', 'includes/simplenews.mail');
+ $context = $params['context'];
Simplenews.mail.inc doesn't seem to be used here?
+++ simplenews_rules/simplenews_rules.module@@ -0,0 +1,37 @@
+ $subject = token_replace($params['subject'], 'all', $context);
+ $subject = str_replace(array("\r", "\n"), '', $subject);
+ $body = token_replace($params['message'], 'all', $context);
I guess token replacement can be done directly within rules in 7.x, not sure about 6.x
+++ simplenews_rules/simplenews_rules.module@@ -0,0 +1,37 @@
+ $message['subject'] .= $subject;
Is there a specific reason for appending the subject? Looks strange to me.
+++ simplenews_rules/simplenews_rules.rules.inc@@ -0,0 +1,228 @@
+/**
+ * Action Implementation: Subscribe an e-mail adress to a Simplenews newsletter
+ */
+function simplenews_action_subscribe_email($settings) {
FYI: Pretty sure this one can be dropped in a 7.x implementation, as you can select user:mail in the data selection form.
+++ simplenews_rules/simplenews_rules.rules.inc@@ -0,0 +1,228 @@
+ // Get email address of all accounts subscribed to this newsletter.
+ $result = db_query('SELECT s.mail, s.snid, s.uid FROM {simplenews_subscriptions} s INNER JOIN {simplenews_snid_tid} t ON s.snid = t.snid WHERE t.status = 1 AND s.activated = %d AND t.tid = %d', 1, $tid);
+ while (($account = db_fetch_object($result)) && $message['result']) {
There is an API function for getting all recipients of a newsletter (at least in 7.x).
Also, this approach doesn't scale. if you have, say, 10k subscribers, this will most likely explode.
Not sure if this should even be supported like this. What's your use case for this and could you imagine a different way of doing it that does scale?
E.g. in 7.x, I could imagine that you could automatically create a newsletter node and then send it. In this case, sending would actually mean adding to the mail spool and it would then be processed as configured through cron.
+++ simplenews_rules/simplenews_rules.rules.inc@@ -0,0 +1,228 @@
+/**
+ * Helper function: Load all available newsletters.
+ */
+function simplenews_rules_action_get_newsletters() {
+ $tree = taxonomy_get_tree(variable_get('simplenews_vid', ''));
+ $terms = array();
+ foreach ($tree as $newsletter) {
+ $terms[$newsletter->tid] = $newsletter->name;
+ }
+ return $terms;
+}
No idea about 6.x but 7.x has functions like simplenews_get_mailing_lists() which returns a list of categories (yeah, weird name, I know).
+++ simplenews_rules/simplenews_rules.rules.inc
index 0000000..515ec8c
--- /dev/null
--- /dev/null
+++ simplenews_rules/simplenews_rules.rules_forms.inc
+++ simplenews_rules/simplenews_rules.rules_forms.inc
+++ simplenews_rules/simplenews_rules.rules_forms.inc
@@ -0,0 +1,114 @@
@@ -0,0 +1,114 @@
+<?php
+// $Id$
+
+/**
+ * @file
+ * Rules forms for the Simplenews newsletter module.
+ *
+ * @addtogroup rules
+ * @{
This one can probably be dropped almost completely for 7.x, we maybe only need a few form alterations.
#48
Attaching an initial patch for a 7.x version.
Supported actions:
- Subscribe an email
- Unsubscribe an email
Supported events:
- Subscription
- Unsubscription
Many more things are possible, but this already covers most of the existing simplenews_action integration. Integrating things like subscriber objects would be very nice, but requires quite a bit of work.
#49
#50
Update:
- Support for sending a newsletter node
- Some refactoring for the simplenews_add_node_to_spool() method:
- this is an api function and shouldn't display a drupal_set_message(), instead, I added a return value and moved the drupal_set_message() out of it
- the function now also automatically updates the status of a newsletter when being added to the pool.
- (already in the previous patch) Refactoring hook_simplenews_subsribe/unsubscribe to also pass the $subscription object so that it's actually possible to get the category. Also added documentation for that hook.
The refactoring parts could be done in a separate issue as well...
#51
The last submitted patch, simplenews_rules2.patch, failed testing.
#52
Updated the test.
#53
Added the missing module_load_include() (api functions should imho not be in a file that you need to include manually, will probably change this later on).
Manually tested that it is with this for example possible to send a newsletter automatically once published.
#54
Ok, commited! While tests (both manually and automatically) would be great, we want this in the 7.x-1.0-alpha2 to have a proper replacement for the removed simplenews_actions integration. Note that userpoints.module has tests for rules integration, we can copy from there later on...
Moving the issue back to 6.x-2.x.
#55
Thanks for adding this - how do you make a rule that's triggered when someone signs up though - I can't see anything to do with Simplenews in the triggers? I'm using the D6 version.
#56
There currently is no Rules integration in 6.x version... I don't have any time to work on it, but I'm willing to commit something as soon as the patch is tested by the community.