Would it be hard to include an option to select multiple newsletters when creating a new newsletter issue?
I believe this would come very handy.

Comments

sutharsan’s picture

Just set 'Multiple select' in the newsletter vocabulary.

wmostrey’s picture

Status: Active » Fixed

You are correct, many thanks!

wmostrey’s picture

Status: Fixed » Active

I'm afraid it doesn't work. Only one tid is stored in the simplenews_newsletters and in the simplenews_submit function the array_intersect does not take into account multi-dimensional arrays. Two solutions for this:

1/
Why store the tid in the simplenews_newsletters db? With taxonomy_node_get_terms you can get the terms from the nid, which is already stored.

2/

$arraycheck = array_intersect($tids, $taxes[0]);
if(empty($arraycheck)) $test = array_intersect($tids, $taxes);
return array_intersect($tids, $arraycheck);

Perhaps this check isn't even needed since you could use
<? taxonomy_node_get_terms_by_vocabulary($nid->nid, _simplenews_get_vid()); ?>

I am now implementing these changes (and then some) and will port the patches back.

wmostrey’s picture

StatusFileSize
new7.43 KB

Here you go. The simplenews.module misses the ?> at the end of the file btw, this is also fixed in this patch.

wmostrey’s picture

Ofcourse, vocabulary_node_get_terms() and vocabulary_node_get_terms_by_vocabulary() only works after the node has been created (by selecting "Don't send now").

One error in the patch, change

$result = db_query(db_rewrite_sql('SELECT n.nid n.created FROM {node} n INNER JOIN {simplenews_newsletters} s ON n.nid = s.nid WHERE s.s_status = %d ORDER BY n.created ASC'), 1);

into

$result = db_query(db_rewrite_sql('SELECT n.nid, n.created FROM {node} n INNER JOIN {simplenews_newsletters} s ON n.nid = s.nid WHERE s.s_status = %d ORDER BY n.created ASC'), 1);

sutharsan’s picture

Could you please explain why it is handy to have multiple terms related to one newsletter.

Note that the closing php code tag is not used. See http://drupal.org/node/545

wmostrey’s picture

Don't see it as multiple terms, see it as sending one newsletter issue to multiple newsletters. For instance if you have a newsletter 'Belgium' and one 'Netherlands', with this patch you can send a newsletter issue two both of them, instead of having to create two nodes for it.

wmostrey’s picture

Status: Active » Closed (fixed)
tormu’s picture

Version: 5.x-1.1 » 5.x-1.x-dev
Status: Closed (fixed) » Active

Ok, since this is not contributed to the module already though patch has been made, I don't think it should be closed..
I just found this post after making an issue of it myself, http://drupal.org/node/161414

I'll have to try implementing that patch in this thread myself too.

tormu’s picture

Version: 5.x-1.x-dev » 5.x-1.1
StatusFileSize
new89.29 KB

Changed the version back to 1.1, since the original patch was made for it. I really hope this get implemented to the newest version of Simplenews, though I think the stable 1.1 and the newest version are quite different from each other. The patch introduced was and is really buggy: after correcting the query like suggested on the comment it still didn't send the mails. It ended up giving 6 errors about array being sent instead of ID number, which was due to "$term = taxonomy_get_term($tid);" at the _simplenews_send(). I made some changes only to that function in order to get the patch working.

  1. On the while-loop it had foreach($terms as $tid), but $tid isn't just term ID, it's an object, so I added a line $tid = $tid->tid; first line inside the foreach-loop.
  2. Inside that foreach-loop there was $nid->tid mentioned in several places, which was changed to $tid.
  3. Inside the same loop removed line $term = taxonomy_get_term($tid);, since there is no references for $term there anyway.
  4. Added a line that would make the user not receive two mails if he's on two lists. End of the "If the mail is sent" -if clause -> db_query("UPDATE {simplenews_subscriptions} SET s_status = %d WHERE mail = '%s'", 1, $user_node->to);
  5. Moved the line db_query('UPDATE {simplenews_subscriptions} SET s_status = %d', 0); outside the foreach-loop, so the users are set back to the "not-sent"-status.

There are still things that should be addressed even if you get this implemented:

  • When creating a newsletter issue after the newsletter vocabulary is sent to multiple select, you can't send the issue at all - it doesn't know that the issue has the newsletter selected, no matter how many of those are selected. Saving the newsletter issue in "Don't send"-status makes it save it. You can only send the mail to one or many newsletter lists by editing the node again and then selecting the send option.
  • There should be selection not to show the term name in the subject. If the same user is on 4 lists and mail is sent to all of them, the user only gets the first one (which is ok), but the subject of that mail also has the name of the first letter, which can lead to confusions. I don't see any problem in setting the subject of the mail to simply $node->title, since you can modify the title as much as you want.
  • Almost same as the subject-thing: when user is on multiple lists, it shows the default footer on the mail based on the first sent newsletter. Therefore, if the user clicks that link and confirms, he will be taken off only one of those lists that he is on. The footer should be constructed in other way IF the mail is sent to multiple list simultaneously. Maybe something like "click here to to go subscriptions page" or so, haven't figured the best way take this into account..
  • On the newsletter issues admin page it only shows that the issue is on one newsletter, no matter if it's on multiple ones actually.

I attached my current simplenews.module (as .txt cos of file type limitations), which is the 1.1 version downloaded today + patch by wmostrey + my changes to make the patch actually work.

dgtlmoon’s picture

There is also the simplenews scheduler newsletter module, you could create a newsletter that loads the body of other newsletters?

1st-angel’s picture

Version: 5.x-1.1 » 6.x-1.0-beta3

Is this possible in the current version?

sutharsan’s picture

No, it is not possible with simplenews 6.x. Feel free to submit code for it.

sutharsan’s picture

Status: Active » Closed (won't fix)

This conflicts with the design of simplenews. It needs a major rewrite to make it happen.

lilott8’s picture

Status: Closed (won't fix) » Active
StatusFileSize
new105.29 KB

I have a working solution. I want other people to test this before I turn it live on my website. I will draw up the documents and make it easy to implement :) The working solution is for 5.x version only. You can take a gander on my current modified module, as I will attach it; this builds on the patch that was attached in this thread. Happy sending!

Jackinloadup’s picture

@lilott8 Thanks for posting your solution in both possible relevant threads.

Maybe we should rename the threads to reflect the drupal version we want a solution for. This thread seems like a good place to continue the D6 discussion.
#506076: Allow multiple taxonomy term selection with simplenews to send emails to multiple mailing list ? for D5?

@Sutharsan A feature such as this might be a long way out but it still seems like a desired feature that we should except patches to. Or is this a feature that is being planned for a later 6.x-2.0 release?
There are many use-cases where this would be a great feature. ^_^

lilott8’s picture

This is for 5.x Versions ONLY

Here is the code that needs to be added:

Look for: "function _simplenews_send($timer = FALSE) {" aprox line 1219
add to the function parameters: $preterms = NULL
Should look like: <?php function _simplenews_send($timer = FALSE, $preterms = NULL) { ?>
first line within the function add:
<?php global $multiple_lists; ?> aprox line 1220
in the "while($nid = db_fetch_object...)" loop
after <?php$node = simplenews_node_prepare($nid->nid, $nid->vid, $nid->tid);?> aprox line 1237
add:

<?php
    if (!$multiple_lists) {
      //this query is if you are only sending ONE newsletter
      $result2 = db_query('SELECT s.mail, s.snid FROM {simplenews_subscriptions} s INNER JOIN
          {simplenews_snid_tid} t ON s.snid = t.snid
          WHERE s.s_status = %d AND s.a_status = %d AND t.tid = %d ORDER BY s.snid ASC', 0, 1, $nid->tid);
    }//if
    //if we have multiple lists selected, then we need to work on our query.
    //so we will work accordingly to make sure we do what we want.
    else {
      $terms = array_values($preterms);
      $count = count($terms);
      //these are the status variables
      //that are needed in the regular,
      //non multiple lists query.
      //we just drop them in first.
      $values[0] = 0;
      $values[1] = 1;
      //this sets the numbering to 0
      //that way we can traverse the array
      //sequentially and numerically
      for ($x=0;$x<$count;$x++) {
        //we add 2 to values so we keep the
        //values above
        $values[$x+2] = $terms[$x];
      }//for
      //this is the base query.  We will be adding to it
      $select = "SELECT distinct s.mail, s.snid FROM {simplenews_subscriptions} s
        INNER JOIN {simplenews_snid_tid} t ON s.snid = t.snid
        WHERE s.s_status = %d AND s.a_status = %d AND ";
      //the closing "order by" statement
      $order = 'ORDER BY s.snid ASC';
      $placeholders = array_fill(0, $count, "%d");
      for ($x = 0;$x < $count;$x++) {
        if ($x != $count-1) {
          $ors .= "t.tid = {$placeholders[$x]} OR ";
        }//if
        else {
          $ors .= "t.tid = {$placeholders[$x]} ";
          //was having some wierd errors, so I just
          //want to be sure.  We are done, and I am making
          //sure we are done.
          break;
        }//else
      }//for
      //basic query with only the placeholders
      $query = $select . $ors . $order;
      //this is the query to end all queries...
      //this is the aforementioned basic query with
      //only the placeholders and we send the values
      //array to the db_rewrite as well.  We get what
      //we want.
      $result2 = db_query(db_rewrite_sql($query), $values);
    }//else
	?>

Now lets move to <?php function simplenews_nodeapi(&$node, $op, $a3 = NULL, $a4 = NULL) { ?> aprox line 456
we need to add a new line after the
<?php if ($node->send == TRUE) { ?> aprox line 454
<?php $pre_tax = array_values($node->taxonomy); ?>
then we need to edit the function call for _simplenews_send(). aprox line 455
<?php _simplenews_send(TRUE, $pre_tax[0]); ?>

So the entire function should read:

<?php
/**
 * Implementation of hook_nodeapi().
 */
function simplenews_nodeapi(&$node, $op, $a3 = NULL, $a4 = NULL) {
  if ($node->type == 'simplenews' && ($op == 'update' || $op == 'insert')) {
    if ($node->send == 1) {
      $pre_tax = array_values($node->taxonomy);
      _simplenews_send(TRUE, $pre_tax[0]);
      drupal_set_message(t('Newsletter %newsletter is being sent', array('%newsletter' => $node->title)));
    }
    elseif ($node->send == 2) {
      simplenews_send_test($node);
    }
  }
}
?>

We want function simplenews_validate($node) { aprox line 338
under global $valid_emails;
add the following...

<?php
 global $multiple_lists;
//we just want to remove the multiple nested
//arrays that are conjoined with
//$node->taxonomy
$pre_tax = array_values($node->taxonomy);
  if (count($pre_tax[0]) > 1) {
    $multiple_lists = TRUE;
  }
?>

This is it. It's not too terrible. Also note that this is AFTER the patch. If you do not use the patch first then I doubt this will work.

I don't know if this plays nicely with TAC. I haven't tested that.
[EDIT]
Tested with TAC, works perfectly. Just wanted to be sure.
[/EDIT]

Also important to note: That when you send multiple newsletters out, the subject line will read that of the topmost newsletters you select.

So if you had these newsletters:
Alpha
Beta
Theta

And you sent to Alpha and Beta the subject line would only show that of Alpha.
[EDIT]
When unsubscribing to a newsletter that is an aggregate, you will unsubscribe to the topmost newsletter you selected.
ie. In previous example you would only be able to unsubscribe from Alpha
[/EDIT]

ps. Please feel free to make enhancements/changes/etc to make this better :)

This is for 5.x Versions ONLY

Jackinloadup’s picture

@lilott8 this is for 5.x?

lilott8’s picture

Correct. I will edit my post accordingly. My apologies for not stating that!

Jackinloadup’s picture

Version: 6.x-1.0-beta3 » 5.x-1.5

Changed version to reflect lilott8's patch.

theshanergy’s picture

Any word on a 6.x patch? This would be extremely handy functionality!

ruloweb’s picture

Hi people!

I've made a patch for D6. As this issue is for D5 I created a new one. See #540432: Send a newsletter issue to multiple newsletters (D6 patch). Please test it out!

Thanks!

--
Jose Sanchez
www.deviancefactory.com

wmostrey’s picture

Status: Active » Closed (fixed)