includes/simplenews.admin.inc | 40 ++++++---- includes/simplenews.mail.inc | 152 +++++++++++++++++++++------------- includes/simplenews.subscription.inc | 8 +- simplenews.module | 22 +++-- tests/simplenews.test | 4 +- 5 files changed, 136 insertions(+), 90 deletions(-) diff --git a/includes/simplenews.admin.inc b/includes/simplenews.admin.inc index 1af20b7..16922ab 100644 --- a/includes/simplenews.admin.inc +++ b/includes/simplenews.admin.inc @@ -175,7 +175,7 @@ function simplenews_admin_issues() { foreach ($query->execute() as $issue) { $categories = simplenews_category_list(); $subscriber_count = simplenews_count_subscriptions($issue->tid); - $pending_count = simplenews_count_spool($issue->nid); + $pending_count = simplenews_count_spool(array('nid' => $issue->nid)); $send_status = $issue->sent_status == SIMPLENEWS_STATUS_SEND_PENDING ? $subscriber_count - $pending_count : theme('simplenews_status', array('source' => 'sent', 'status' => $issue->sent_status)); $options[$issue->nid] = array( @@ -257,8 +257,8 @@ function simplenews_admin_issues_submit($form, &$form_state) { * Callback to send newsletters. */ function simplenews_issue_send($nids) { + $sent_nodes = array(); foreach (node_load_multiple($nids) as $node) { - $newsletter = simplenews_newsletter_load($node->nid); if ($newsletter->status != SIMPLENEWS_STATUS_SEND_NOT) { continue; @@ -266,16 +266,23 @@ function simplenews_issue_send($nids) { if ($node->status == NODE_NOT_PUBLISHED) { simplenews_newsletter_update_sent_status($node, SIMPLENEWS_COMMAND_SEND_PUBLISH); - drupal_set_message(t('Newsletter %title is unpublished and will be sent on publish', array('%title' => $node->title))); + drupal_set_message(t('Newsletter %title is unpublished and will be sent on publish.', array('%title' => $node->title))); continue; } - $status = simplenews_add_node_to_spool($node); - if ($status == SIMPLENEWS_STATUS_SEND_READY) { - drupal_set_message(t('Newsletter %title sent.', array('%title' => $node->title))); + simplenews_add_node_to_spool($node); + $sent_nodes[$node->nid] = $node->title; + } + + // If there were any newsletters sent, display a message. + if (!empty($sent_nodes)) { + $conditions = array('nid' => array_keys($sent_nodes)); + // Attempt to send immediatly, if configured to do so. + if (simplenews_mail_attempt_immediate_send($conditions)) { + drupal_set_message(t('Sent the following newsletters: %titles.', array('%titles' => implode(', ', $sent_nodes)))); } else { - drupal_set_message(t('Newsletter %title pending.', array('%title' => $node->title))); + drupal_set_message(t('The following newsletter are now pending: %titles.', array('%titles' => implode(', ', $sent_nodes)))); } } } @@ -718,7 +725,7 @@ function simplenews_subscription_list_add($form, &$form_state) { '#tree' => TRUE, ); - foreach (simplenews_categories_load_multiple() as $list) { + foreach (simplenews_get_mailing_lists(TRUE) as $list) { $form['newsletters'][$list->tid] = array( '#type' => 'checkbox', '#title' => check_plain(_simplenews_newsletter_name($list)), @@ -951,11 +958,11 @@ function simplenews_subscription_list_remove($form, &$form_state) { '#tree' => TRUE, ); - foreach (simplenews_categories_load_multiple() as $category) { - $form['newsletters'][$category->tid] = array( + foreach (simplenews_get_mailing_lists(TRUE) as $list) { + $form['newsletters'][$list->tid] = array( '#type' => 'checkbox', - '#title' => check_plain(_simplenews_newsletter_name($category)), - '#description' => _simplenews_newsletter_description($category), + '#title' => check_plain(_simplenews_newsletter_name($list)), + '#description' => _simplenews_newsletter_description($list), ); } @@ -991,7 +998,7 @@ function simplenews_subscription_list_remove_submit($form, &$form_state) { $removed = implode(", ", $removed); drupal_set_message(t('The following addresses were unsubscribed: %removed.', array('%removed' => $removed))); - $lists = simplenews_categories_load_multiple(); + $lists = simplenews_get_mailing_lists(TRUE); $list_names = array(); foreach ($checked_lists as $tid) { $list_names[] = _simplenews_newsletter_name($lists[$tid]); @@ -1466,7 +1473,7 @@ function simplenews_subscription_filters() { 'all' => t('All newsletters'), ), ); - foreach (simplenews_categories_load_multiple() as $list) { + foreach (simplenews_get_mailing_lists(TRUE) as $list) { $filters['list']['options']['tid-' . $list->tid] = _simplenews_newsletter_name($list); } @@ -1868,8 +1875,9 @@ function simplenews_node_tab_send_form_submit($form, &$form_state) { // Send newsletter to all subscribers or send test newsletter module_load_include('inc', 'simplenews', 'includes/simplenews.mail'); if ($values['simplenews']['send'] == SIMPLENEWS_COMMAND_SEND_NOW) { - $status = simplenews_add_node_to_spool($node); - if ($status == SIMPLENEWS_STATUS_SEND_READY) { + simplenews_add_node_to_spool($node); + // Attempt to send immediatly, if configured to do so. + if (simplenews_mail_attempt_immediate_send(array('nid' => $node->nid))) { drupal_set_message(t('Newsletter %title sent.', array('%title' => $node->title))); } else { diff --git a/includes/simplenews.mail.inc b/includes/simplenews.mail.inc index 27194a4..8bbe107 100644 --- a/includes/simplenews.mail.inc +++ b/includes/simplenews.mail.inc @@ -10,9 +10,6 @@ /** * Add the newsletter node to the mail spool. * - * Depending on the configuration, the node will either be sent immediatly - * afterwards or during cron runs. - * * @param $node * The newsletter node to be sent. * @@ -40,19 +37,48 @@ function simplenews_add_node_to_spool($node) { // Update simplenews newsletter status to send pending. simplenews_newsletter_update_sent_status($node); +} - // When cron is not used the newsletter is send immediately to the emails - // in the spool. When cron is used newsletters are send to addresses in the - // spool during the next (and following) cron run. - if (variable_get('simplenews_use_cron', TRUE) == FALSE) { - simplenews_mail_spool(); - simplenews_clear_spool(); - simplenews_send_status_update(); - return SIMPLENEWS_STATUS_SEND_READY; +/** + * Send mail spool immediatly if cron should not be used. + * + * @param $conditions + * (Optional) Array of spool conditions which are applied to the query. + */ +function simplenews_mail_attempt_immediate_send(array $conditions = array(), $use_batch = TRUE) { + if (variable_get('simplenews_use_cron', TRUE)) { + return FALSE; + } + if ($use_batch) { + // Set up as many send operations as necessary to send all mails with the + // defined throttle amount. + $throttle = variable_get('simplenews_throttle', 20); + $spool_count = simplenews_count_spool($conditions); + $num_operations = ceil($spool_count / $throttle); + + $operations = array(); + for ($i = 0; $i < $num_operations; $i++) { + $operations[] = array('simplenews_mail_spool', array($throttle, $conditions)); + } + + // Add separate operations to clear the spool and updat the send status. + $operations[] = array('simplenews_clear_spool', array()); + $operations[] = array('simplenews_send_status_update', array()); + + $batch = array( + 'operations' => $operations, + 'title' => t('Sending mails'), + 'file' => drupal_get_path('module', 'simplenews') . '/includes/simplenews.mail.inc', + ); + batch_set($batch); } else { - return SIMPLENEWS_STATUS_SEND_PENDING; + // Send everything that matches the conditions immediatly. + simplenews_mail_spool(SIMPLENEWS_UNLIMITED, $conditions); + simplenews_clear_spool(); + simplenews_send_status_update(); } + return TRUE; } /** @@ -173,13 +199,20 @@ function simplenews_send_source(SimplenewsSourceInterface $source) { * * @todo: Redesign API to allow language counter in multilingual sends. * + * @param $limit + * (Optional) The maximum number of mails to send. Defaults to + * unlimited. + * @param $conditions + * (Optional) Array of spool conditions which are applied to the query. + * * @ingroup spool */ -function simplenews_mail_spool($limit = SIMPLENEWS_UNLIMITED) { +function simplenews_mail_spool($limit = SIMPLENEWS_UNLIMITED, array $conditions = array()) { $check_counter = 0; - // Send pending messages from database cache - if ($spool_list = simplenews_get_spool(array(SIMPLENEWS_SPOOL_PENDING, SIMPLENEWS_SPOOL_IN_PROGRESS), $limit)) { + // Send pending messages from database cache. + $spool_list = simplenews_get_spool($limit, $conditions); + if ($spool_list) { // Switch to the anonymous user. simplenews_impersonate_user(drupal_anonymous_user()); @@ -302,48 +335,49 @@ function simplenews_get_expiration_time() { * time has expired, the message id will be returned as a message which is not * allocated to another process. * - * @param string $status - * The status of data to be retrieved. Must be one of: - * - 0: hold - * - 1: pending - * - 2: send - * - 3: in progress - * @param integer $limit - * The maximum number of mails to load from the spool. + * @param $limit + * (Optional) The maximum number of mails to load from the spool. Defaults to + * unlimited. + * @param $conditions + * (Optional) Array of conditions which are applied to the query. If not set, + * status defaults to SIMPLENEWS_SPOOL_PENDING, SIMPLENEWS_SPOOL_IN_PROGRESS. * - * @return array + * @return * An array of message ids to be sent in the current run. * * @ingroup spool */ -function simplenews_get_spool($status, $limit = SIMPLENEWS_UNLIMITED) { +function simplenews_get_spool($limit = SIMPLENEWS_UNLIMITED, $conditions = array()) { $messages = array(); - $clauses = array(); - $params = array(); - if (!is_array($status)) { - $status = array($status); + // Add default status condition if not set. + if (!isset($conditions['status'])) { + $conditions['status'] = array(SIMPLENEWS_SPOOL_PENDING, SIMPLENEWS_SPOOL_IN_PROGRESS); } - foreach ($status as $s) { - if ($s == SIMPLENEWS_SPOOL_IN_PROGRESS) { - // Select messages which are allocated by another process, but whose - // maximum send time has expired. - $clauses[] = '(s.status = :statusinprogress AND s.timestamp < :expiration)'; - $params[':statusinprogress'] = $s; - $params[':expiration'] = simplenews_get_expiration_time(); - } - else { - $clauses[] = 's.status = :status'; - $params[':status'] = $s; + // Special case for the status condition, the in progress actually only + // includes spool items whose locking time has expired. So this need to build + // an OR condition for them. + $status_or = db_or(); + $statuses = is_array($conditions['status']) ? $conditions['status'] : array($conditions['status']); + foreach ($statuses as $status) { + $status_or->condition('status', $status); + if ($status == SIMPLENEWS_SPOOL_IN_PROGRESS) { + $status_or->condition('s.timestamp', simplenews_get_expiration_time(), '<'); } } + unset($conditions['status']); $query = db_select('simplenews_mail_spool', 's') ->fields('s') - ->where(implode(' OR ', $clauses), $params) + ->condition($status_or) ->orderBy('s.timestamp', 'ASC'); + // Add conditions. + foreach ($conditions as $field => $value) { + $query->condition($field, $value); + } + /* BEGIN CRITICAL SECTION */ // The semaphore ensures that multiple processes get different message ID's, // so that duplicate messages are not sent. @@ -411,32 +445,32 @@ function simplenews_update_spool($msids, $data) { /** * Count data in mail spool table. * - * @param integer $nid - * Newsletter node id. - * @param array $status - * Array of status flags. + * @param $conditions + * (Optional) Array of conditions which are applied to the query. Defaults * - * @return integer - * Count of mail spool elements which own the attributes passed in as params. + * @return + * Count of mail spool elements of the passed in arguments. * * @ingroup spool */ -function simplenews_count_spool($nid, $status = array(SIMPLENEWS_SPOOL_PENDING, SIMPLENEWS_SPOOL_IN_PROGRESS)) { - $clauses = array(); - $params = array(); +function simplenews_count_spool(array $conditions = array()) { - if (!is_array($status)) { - $status = array($status); + // Add default status condition if not set. + if (!isset($conditions['status'])) { + $conditions['status'] = array(SIMPLENEWS_SPOOL_PENDING, SIMPLENEWS_SPOOL_IN_PROGRESS); } - foreach ($status as $s) { - $clauses[] = "status = $s"; + $query = db_select('simplenews_mail_spool'); + // Add conditions. + foreach ($conditions as $field => $value) { + $query->condition($field, $value); } - $query = db_select('simplenews_mail_spool') - ->condition('nid', $nid) - ->where(implode(' OR ', $clauses)); - return $query->countQuery()->execute()->fetchField(); + $query->addExpression('COUNT(*)', 'count'); + + return $query + ->execute() + ->fetchField(); } /** @@ -504,7 +538,7 @@ function simplenews_send_status_update() { ->fields('n', array('tnid')) ->condition('s.status', SIMPLENEWS_STATUS_SEND_PENDING); foreach ($query->execute() as $newsletter) { - $counts[$newsletter->tnid][$newsletter->nid] = simplenews_count_spool($newsletter->nid); + $counts[$newsletter->tnid][$newsletter->nid] = simplenews_count_spool(array('nid' => $newsletter->nid)); } // Determine which nodes are send per translation group and per individual node. diff --git a/includes/simplenews.subscription.inc b/includes/simplenews.subscription.inc index 21867e1..9a3222a 100644 --- a/includes/simplenews.subscription.inc +++ b/includes/simplenews.subscription.inc @@ -55,7 +55,7 @@ function simplenews_subscriptions_account_form(&$form, &$form_state, $subscriber // Get newsletters for subscription form checkboxes. // Newsletters with opt-in/out method 'hidden' will not be listed. - foreach (simplenews_category_get_visible() as $newsletter) { + foreach (simplenews_get_mailing_lists() as $newsletter) { $options[$newsletter->tid] = check_plain(_simplenews_newsletter_name($newsletter)); $default_value[$newsletter->tid] = FALSE; } @@ -215,7 +215,7 @@ function simplenews_subscriptions_page_form($form, &$form_state) { // Get newsletters for subscription form checkboxes. // Newsletters with opt-in/out method 'hidden' will not be listed. - foreach (simplenews_category_get_visible() as $newsletter) { + foreach (simplenews_get_mailing_lists() as $newsletter) { $options[$newsletter->tid] = check_plain($newsletter->name); $default_value[$newsletter->tid] = FALSE; } @@ -374,7 +374,7 @@ function simplenews_subscriptions_multi_block_form($form, &$form_state) { // Get newsletters for subscription form checkboxes. // Newsletters with opt-in/out method 'hidden' will not be listed. - foreach (simplenews_category_get_visible() as $newsletter) { + foreach (simplenews_get_mailing_lists() as $newsletter) { $options[$newsletter->tid] = check_plain($newsletter->name); $default_value[$newsletter->tid] = FALSE; } @@ -610,7 +610,7 @@ function simplenews_subscriptions_admin_form($form, &$form_state, $snid) { // Get newsletters for subscription form checkboxes. // Newsletters with opt-in/out method 'hidden' will not be listed. - foreach (simplenews_category_get_visible() as $newsletter) { + foreach (simplenews_get_mailing_lists() as $newsletter) { $options[$newsletter->tid] = check_plain($newsletter->name); $default_value[$newsletter->tid] = FALSE; } diff --git a/simplenews.module b/simplenews.module index 49951de..8bd0682 100644 --- a/simplenews.module +++ b/simplenews.module @@ -837,7 +837,7 @@ function simplenews_form_user_register_form_alter(&$form, &$form_state) { // Determine the lists to which a user can choose to subscribe. // Determine to which other list a user is automatically subscribed. - foreach (simplenews_categories_load_multiple() as $list) { + foreach (simplenews_get_mailing_lists(TRUE) as $list) { $subscribe_new_account = $list->new_account; $opt_inout_method = $list->opt_inout; if (($subscribe_new_account == 'on' || $subscribe_new_account == 'off') && ($opt_inout_method == 'single' || $opt_inout_method == 'double')) { @@ -1037,7 +1037,7 @@ function simplenews_user_view($account, $build_mode) { ); // Collect newsletter to which the current user is subscribed. // 'hidden' newsletters are not listed. - $newsletters = simplenews_category_get_visible(); + $newsletters = simplenews_get_mailing_lists(); $subscription = simplenews_subscriber_load_by_mail($account->mail); foreach ($newsletters as $newsletter) { @@ -1880,19 +1880,23 @@ function simplenews_category_delete($category) { } /** - * Loads all visible newsletter categories. + * Fetch all newsletter mailing lists. * - * Does not include categories with the opt-out/opt-in setting set to hidden. + * @param $show_all + * FALSE = Don't show mailing lists which are marked 'hidden'. + * TRUE = Show all mailing lists. * * @ingroup newsletter */ -function simplenews_category_get_visible() { - $categories = &drupal_static(__FUNCTION__, NULL); +function simplenews_get_mailing_lists($show_all = FALSE) { + $lists = &drupal_static(__FUNCTION__, array()); + $all = $show_all ? 'all' : 'not_all'; - if (!isset($categories)) { - $categories = simplenews_categories_load_multiple(array(), array('show_all' => FALSE)); + if (!isset($lists[$all])) { + $categories = simplenews_categories_load_multiple(array(), array('show_all' => $show_all)); + $lists[$all] = $categories; } - return $categories; + return $lists[$all]; } /** diff --git a/tests/simplenews.test b/tests/simplenews.test index 6603381..4b8c1ce 100644 --- a/tests/simplenews.test +++ b/tests/simplenews.test @@ -1540,8 +1540,8 @@ class SimplenewsSendTestCase extends SimplenewsTestCase { ); $this->drupalPost('admin/content/simplenews', $edit, t('Update')); - $this->assertText(t('Newsletter @title sent', array('@title' => $first->title))); - $this->assertText(t('Newsletter @title is unpublished and will be sent on publish', array('@title' => $unpublished->title))); + $this->assertText(t('Sent the following newsletters: @title.', array('@title' => $first->title))); + $this->assertText(t('Newsletter @title is unpublished and will be sent on publish.', array('@title' => $unpublished->title))); // Verify states. $newsletter = simplenews_newsletter_load($first->nid);