Update amount of subscribers [#773196]

At this moment the amount of subscribers is only updated on cron. Please add this line to fix it :)

function simplenews_roles_form_simplenews_admin_types_form_alter(&$form, $form_state) {
  if (!empty($form['tid']['#value'])) {
    $form['#submit'][] = 'simplenews_roles_newsletter_submit';
    // THIS LINE
    $form['#submit'][] = 'simplenews_roles_cron';

Comment	File	Size	Author
#11	simplenews_roles.patch	677 bytes	muschpusch
#5	simplenews_roles.patch	676 bytes	muschpusch

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

joachim CreditAttribution: joachim commented 17 April 2010 at 09:49

Status:

Needs review

» Active

This module used to sync all the time; I changed it to just run on cron as it the syncing seemed rather excessive. Just haven't got round to examining where it's appropriate to sync.

A change of newsletter settings seems like a good time to do it, but I think calling our hook_cron is bad form. Also, there's no need to shove in an extra form submit handler: simplenews_roles_newsletter_submit is our function, so we can add a sync call there. And finally, it would be cool if we could sync just the newsletter that has just been edited rather than all of them. There's a TODO note in simplenews_roles_newsletter_submit() about just that.

Unfortunately I've not much free time to work on this module at the moment so I don't know when I can get round to looking at this. I will review a patch however :)

Comment #2

muschpusch CreditAttribution: muschpusch commented 19 April 2010 at 09:23

Hm... Maybe it was a lazy hack... I will commit a nicer patch in a couple of days...

Comment #3

muschpusch CreditAttribution: muschpusch commented 3 May 2010 at 13:25

Status:

Active

» Needs review

Just two more lines of code. I'm not really sure about the second synch call but it works...

/**
 * Forms API callback; additional submit handler for newsletter form.
 */
function simplenews_roles_newsletter_submit($form, &$form_state) {
  $role_newsletters = variable_get('simplenews_roles_tids_rids', array());
  
  $tid = intval($form_state['values']['tid']);
  $roles = $form_state['values']['roles'];
  
  $roleids = array();
  foreach($roles as $roleid => $checked) {
    if ($checked == $roleid) {
      $roleids[] = $roleid;
    }   
  }
  
  if (count($roleids) > 0) {
    $role_newsletters[$tid] = $roleids;
    // Newsletter has roles: request a sync.
    // TODO: sync just this NL.
    variable_set('simplenews_roles_need_sync', TRUE);
    simplenews_roles_update_subscriptions($tid, $role_newsletters[$tid]);

  }
  elseif (isset($role_newsletters[$tid])) {
    unset($role_newsletters[$tid]);
    simplenews_roles_update_subscriptions($tid, array(0));
  }
  
  variable_set('simplenews_roles_tids_rids', $role_newsletters);
}

Would be nice if you commit a new dev-release with this & #640740: Still using function simplenews_mail_tokens() instead of token module

Comment #4

joachim CreditAttribution: joachim commented 3 May 2010 at 16:12

Status:

Needs review

» Needs work

Re: 640740 -- Simplenews Template isn't one of my modules :)

If you're syncing manually, you don't need to do variable_set('simplenews_roles_need_sync', TRUE) -- that will set cron off!

Do you think you could post your change as a patch please?

Comment #5

muschpusch CreditAttribution: muschpusch commented 4 May 2010 at 09:53

Status:

Needs work

» Needs review

File	Size
simplenews_roles.patch	676 bytes

oh... computers confuse me... I will try the simplenews_template issue queue... Nevermind here the patch!

Comment #6

mr.andrey CreditAttribution: mr.andrey commented 24 June 2010 at 17:49

subscribing...

I just tried a little test (before patch). Create a newsletter that sends to all the users in role "test", set it to "send", so it gets queued up in cron, then add a user to role "test". The role synchronization occurs after the "sending", so that user never gets the email. Does this make sense? It would be better if the synchronization occurred before simplenews sends the emails via cron.

Thanks for a great module!
Andrey.

Comment #7

mr.andrey CreditAttribution: mr.andrey commented 27 June 2010 at 22:07

Patch in #5 fails on 2.x-dev... maybe I'm not doing something correctly?

# patch -p0 < simplenews_roles_0.patch 
patching file simplenews_roles.module
Hunk #1 FAILED at 103.
1 out of 1 hunk FAILED -- saving rejects to file simplenews_roles.module.rej

Best,
Andrey.