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().

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Active » Needs review
FileSize
7.15 KB

Patch attached, including tests.

Status: Needs review » Needs work

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

Berdir’s picture

Status: Needs work » Needs review
FileSize
7.6 KB

Yay testbot!

Looks like we found yet another column that is missing when upgrading from 6.x-1.x. This should fix this.

Berdir’s picture

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.

Berdir’s picture

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.

Status: Needs review » Needs work

The last submitted patch, sent_subscriber_count-1532116-5.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
FileSize
7.34 KB

This should fix the test failures.

Status: Needs review » Needs work
Issue tags: -Needs tests

The last submitted patch, sent_subscriber_count-1532116-7.patch, failed testing.

Berdir’s picture

Status: Needs work » Needs review
Issue tags: +Needs tests
Berdir’s picture

Status: Needs review » Fixed

Ok, passes, commited.

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