SpoolStorage::countMails() currently has special processing for STATUS_IN_PROGRESS matching getMails(). However this is wrong and shouldn't be present.

1. It's not specified in the interface

2. It's not a good idea in practice. Mailer::updateSendStatus calls countMails() which means that if some entries are locked by another process (perhaps one that timed out), they won't be counted. The issue will be incorrectly set to SIMPLENEWS_STATUS_SEND_READY.

CommentFileSizeAuthor
#4 simplenews.in-progress.3106874-interdiff-3-4.txt598 bytesadamps
#4 simplenews.in-progress.3106874-4.patch3.06 KBadamps
#3 simplenews.in-progress.3106874-interdiff-2-3.txt1.42 KBadamps
#3 simplenews.in-progress.3106874-3.patch3.06 KBadamps
#2 simplenews.in-progress.3106874-2.patch1.54 KBadamps

Comments

AdamPS created an issue. See original summary.

adamps’s picture

adamps commented
Status: Active » Needs review
StatusFileSize
new simplenews.in-progress.3106874-2.patch1.54 KB
adamps’s picture

adamps commented
Issue summary: View changes
Issue tags: +Plan to commit
StatusFileSize
new simplenews.in-progress.3106874-3.patch3.06 KB
new simplenews.in-progress.3106874-interdiff-2-3.txt1.42 KB

OK so the tests are backing up the way the code currently works.

It's true that custom code on a site somewhere could expect countMails() to match the count of the array returned by getMails(). However this is speculative whereas the 2 points in the IS are concrete. Therefore I still think this patch is correct and I will update it to alter the tests to match.

adamps’s picture

adamps commented
StatusFileSize
new simplenews.in-progress.3106874-4.patch3.06 KB
new simplenews.in-progress.3106874-interdiff-3-4.txt598 bytes

  • AdamPS committed b46d78e on 8.x-2.x 
    Issue #3106874 by AdamPS: "In progress" spool entries are not counted...
adamps’s picture

adamps commented
Status: Needs review » Fixed
Issue tags: -Plan to commit

Status: Fixed » Closed (fixed)

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