An unsubscription can clash with a current sending process. This can cause that a user receives a newsletter after he unsubscribed (becuse he was pending recipient in a long-running cron sending task)

Thus when unsubscribing, the mail spool should be cleanup (removing the current user as a recipient).

Affects function simplenews_unsubscribe_user()

I'm considering this a bug because we're not following proper standards (508, spam)

Comments

berdir’s picture

Status: Active » Needs review
StatusFileSize
new3.11 KB

Attached patch adds this including simple test coverage.

Note that I renamed simplenews_clear_spool_from_node($node) to simplenews_delete_spool(array $conditions) to a) allow different conditions than just a node object and b) differentiate the function from simplenews_clear_spool(), which just deletes *old* records which are not used anymore.

Status: Needs review » Needs work

The last submitted patch, clear_spool_on_unsubscribe.patch, failed testing.

berdir’s picture

Status: Needs work » Needs review

#1: clear_spool_on_unsubscribe.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, clear_spool_on_unsubscribe.patch, failed testing.

berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new3.18 KB

I hate API functions in optional include files :) We really need to move stuff that can be used as API functions by other modules and ourself. Right now, "module_load_include('inc', 'simplenews', 'includes/simplenews.mail');" is part of the API if you want to send a newsletter, that's just wrong. We currently have that line 12x in our code, all over the place.

miro_dietiker’s picture

Status: Needs review » Reviewed & tested by the community

Almost perfect ... except the comment "Make sure that 100 mails have been sent."

Please change it to 99 and commit.

berdir’s picture

Status: Reviewed & tested by the community » Fixed

Commited.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.