| Project: | Simplenews |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | corvus_ch |
| Status: | closed (fixed) |
| Issue tags: | Needs tests |
Issue Summary
See title.
The value is always zero, looks like it's completely ignored. Beside initializing it to 0 is that column never referenced at all, so it's completely missing and not simply broken.
I think we do have to extend simplenews_mail_spool(), count the sent mails per nid and not global and execute an update query for each nid.
We can the extend the existing send tests and assert the correct amount a few times in some of the existing send tests, that should be enough. maybe add a test that sends two nodes to two different categories in parallel to make sure they are both updated correctly.
Additionally, we can extend it for the content translation integration and use the actually used nid. SimplenewsSpool already has support for this, see getProcessed().
Comments
#1
Patch attached, including tests.
#2
The last submitted patch, sent_subscriber_count.patch, failed testing.
#3
Yay testbot!
Looks like we found yet another column that is missing when upgrading from 6.x-1.x. This should fix this.
#4
The update query took 3,7s on a database with 100 newsletters and 10k subscribers. Will need to think of a way to allow that query to scale better. Probably do one query per category instead of a subquery.
#5
Ok, here is a patch with an update function that can handle much more newsletters. Got the execution time down on my test environment with 8k subscribers and 5,5k newsletters from 50s to 0,05s.
#6
The last submitted patch, sent_subscriber_count-1532116-5.patch, failed testing.
#7
This should fix the test failures.
#8
The last submitted patch, sent_subscriber_count-1532116-7.patch, failed testing.
#9
#7: sent_subscriber_count-1532116-7.patch queued for re-testing.
#10
Ok, passes, commited.
#11
Automatically closed -- issue fixed for 2 weeks with no activity.