Wrong "Sent" numbers on "Created issues" list [#875672]

Comment	File	Size	Author
#7	snapshot1.png	100.9 KB	fmjrey
#7	snapshot2.png	99.82 KB	fmjrey
#1	simplenews_875672_1.patch	9.03 KB	miro_dietiker

Comment #1

miro_dietiker

Switzerland

CreditAttribution: miro_dietiker commented 6 August 2010 at 13:33

Status:

Active

» Fixed

File	Size
simplenews_875672_1.patch	9.03 KB

The attached patch is already committed in Branch 6.2

Log in or register to post comments

Comment #2

miro_dietiker

Switzerland

CreditAttribution: miro_dietiker commented 6 August 2010 at 14:12

Version:	6.x-2.x-dev	» 7.x-1.x-dev
Status:	Fixed	» Patch (to be ported)

Some work todo ;-)

Log in or register to post comments

Comment #3

roball CreditAttribution: roball commented 8 August 2010 at 20:57

Version:	7.x-1.x-dev	» 6.x-2.x-dev
Priority:	Minor	» Critical

The Drupal database update (update.php) #6201 will fail if your tables use a prefix. Line 728 of "simplenews.install" originally reads

  db_query("UPDATE {simplenews_newsletters} sn SET sent_subscriber_count = (SELECT count(tid) FROM simplenews_snid_tid WHERE tid = sn.tid AND status = %d)", SIMPLENEWS_SUBSCRIPTION_STATUS_SUBSCRIBED);

but must be changed to

  db_query("UPDATE {simplenews_newsletters} sn SET sent_subscriber_count = (SELECT count(tid) FROM {simplenews_snid_tid} WHERE tid = sn.tid AND status = %d)", SIMPLENEWS_SUBSCRIPTION_STATUS_SUBSCRIBED);

Table names must ALWAYS be put within curly brackets!!!

Log in or register to post comments

Comment #4

roball CreditAttribution: roball commented 8 August 2010 at 21:01

Status:

Patch (to be ported)

» Needs work

Log in or register to post comments

Comment #5

miro_dietiker

Switzerland

CreditAttribution: miro_dietiker commented 9 August 2010 at 09:46

Version:	6.x-2.x-dev	» 7.x-1.x-dev
Status:	Needs work	» Patch (to be ported)

The issue is fixed with the patch from
http://drupal.org/node/823688#comment-3304620
and it's already committed to Branch 6.2

Log in or register to post comments

Comment #6

miro_dietiker

Switzerland

CreditAttribution: miro_dietiker commented 17 August 2010 at 04:46

I needed to move things around additionally
#884338: Spool duplication on node resave

Log in or register to post comments

Comment #7

fmjrey CreditAttribution: fmjrey commented 19 August 2010 at 12:45

File	Size
snapshot2.png	99.82 KB
snapshot1.png	100.9 KB

I think this needs to be worked a bit further in the case of i18n newsletters.
When a newsletter has translations, each translation is listed in the newsletter issues page (admin/content/simplenews/sent).
However, the number of subscribers and the count of mail sent is only reported against the version in english (in default language I suppose).
In the attached screenshots, issue 4 has been has been translated from english to french. One subscriber is subscribed to the english version, another to the french version. Instead of seeing against both (0/1) before cron has run, I saw (0/2) against the english version. After running cron manually, (2) was reported against the english version.

Edit: my post is about 6.x-2.x-dev, not D7

Log in or register to post comments

Comment #8

miro_dietiker

Switzerland

CreditAttribution: miro_dietiker commented 19 August 2010 at 15:08

Version:	7.x-1.x-dev	» 6.x-2.x-dev
Status:	Patch (to be ported)	» Needs work

Uff... i didn't consider the 1:n relationship for the translated nodes in this code...
I'm not happy with simplenews node spooler handler anyway.
Indeed when editing translations of a just pending/sending node it could lead to duplicates...

Currently the creation of the spooler recipients is also not satisfying. They get fetched from SQL into $mails and inserted back again one by one.

The creation of the recipient list should be clearly limited to a one-time action per node - where the counters are generated.
Additionally considering the creation of this in 1:n translations will make this complete.