Hi,
Maybe updating the subscriber list every time cron is run (typically every hour) is a bit too much. Besides, since simplenews is sending emails in several batches at cron time, it is a bit odd to modify the subscriber list while the mail is being sent.

So I suggest we remove the cron hook, and only update the subscription list when a newsletter is created. I am submitting a patch for this. The patch includes a install/update hook to make sure simplenews_roles hooks are run before simplenews ones.

Also in this patch, I added a query to remove users that are not subscribed to any newsletter from the subscription table. You may want to incorporate this even if you decide to keep the cron hook.

Patrick

CommentFileSizeAuthor
simplenews_roles.tar.gz1.22 KBpfournier
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim’s picture

Version: 5.x-1.x-dev » 6.x-1.x-dev
Status: Needs review » Needs work

I agree that every cron run is a bit excessive, especially given that the DB updating code is not that clean (it deletes everything and re-inserts all records)

> I added a query to remove users that are not subscribed to any newsletter from the subscription table
I think Simplenews itself does that now.

The question is now: which events should cause synchronization?
I reckon:
- on user change / add / delete
- newsletter creation (ie the taxonomy term)

Do we need:
- newsletter node creation?

Bumping this to D6 -- discussion welcome :)