Hi,

I think there are two things that do not work as they should.

1) every time I edit the newsletter node (original node, not versions), the scheduler settings are misconfigured. Not seem to save the settings.

2) I have also seen that every time I edit the newsletter node (and change the scheduler settings), the server sends me many mails as often I edited the node. I have received the same three newsletters (just changing the version). Looking into the DB I saw that every time I save the newsletter node, inserts a new record in the table simplenews_scheduler, instead of updating the record that already exists or create a new but annulling the previous one.

I hope I have helped and thanks for this module!

PS: The module version is the last one dev: 6.x-1.x-dev (2010-May-01)

CommentFileSizeAuthor
#6 simplenews_scheduler.796160_02.patch17.03 KBsgabe
#2 simplenews_scheduler.796160_01.patch17.21 KBsgabe

Comments

sgabe’s picture

sgabe commented

Hmm...I see the problem. I will look further into this and dig up a solution. Thanks for pointing this out!

sgabe’s picture

sgabe commented
Status: Active » Needs review
StatusFileSize
new simplenews_scheduler.796160_01.patch17.21 KB

I am attaching a patch which I believe will fix these problems among others.
Please, test it and report back. I would like to commit these changes and roll a new release ASAP.

Note: There are some changes in the database schema, so don't forget to run update.php!

sfyn’s picture

sfyn commented

subbed

I am going to try out this patch right away. I will let you know how it goes.

sfyn’s picture

sfyn commented

What I meant by "right away" was two days later.

For starters the update generated lots of exciting sql errors: here's a dump of drush output:

ALTER TABLE {simplenews_scheduler} ADD `activated` TINYINT NOT NULL  [success]
DEFAULT 0
WD php: Duplicate entry '805' for key 1                              [error]
query: ALTER TABLE simplenews_scheduler CHANGE snid nid INT(11) NOT
NULL DEFAULT '0', ADD PRIMARY KEY (nid) in
/srv/aegir/prod-drupal-6.16-1.4/sites/adiq.site.koumbit.net/modules/simplenews_scheduler/simplenews_scheduler.install
on line 125.
ALTER TABLE {simplenews_scheduler} DROP sid                          [success]
ALTER TABLE {simplenews_scheduler} CHANGE snid nid INT(11) NOT NULL  [error]
DEFAULT '0', ADD PRIMARY KEY (nid)
ALTER TABLE {simplenews_scheduler_editions} CHANGE edition_snid eid  [success]
INT(11) NOT NULL DEFAULT '0'
ALTER TABLE {simplenews_scheduler_editions} CHANGE snid pid INT(11)  [success]
NOT NULL DEFAULT '0'
Finished performing updates.                                         [ok]
Duplicate entry '805' for key 1                            [error]
query: ALTER TABLE simplenews_scheduler CHANGE snid nid INT(11) NOT
NULL DEFAULT '0', ADD PRIMARY KEY (nid) in
/srv/aegir/prod-drupal-6.16-1.4/sites/adiq.site.koumbit.net/modules/simplenews_scheduler/simplenews_scheduler.install
on line 125.
An error occurred at function : drush_core_updatedb                  [error]

I deleted the problem record in simplenews_scheduler, and ran the failed sql queries directly. That said, you may want to adjust your update script to account for multiple records for the same node in simplenews_scheduler.

I'm going to try running with the patch now.

sgabe’s picture

sgabe commented
Status: Needs review » Needs work

Yeah, that's because there are multiple records for the same node due to the inappropriate behavior of the module which is addressed by this issue. I will modify the update procedure to remove those records first, I guess it will be fine than.

Thanks for your feedback!

sgabe’s picture

sgabe commented
Status: Needs work » Needs review
StatusFileSize
new simplenews_scheduler.796160_02.patch17.03 KB

I am attaching a new patch in which the update procedure deletes the duplicates and the insert/update operations are combined in one query.

sfyn’s picture

sfyn commented

Great - I have tested both patch in #2 with my sql hacks and patch in #6 without. Both are properly recurring daily sends.

+1 For the patchg in #6.

disparil’s picture

disparil commented

Congratulations, this works perfectly.

I tried the patch from # 6 and run update.php for schema changes, and nothing has broken :)

Now save the settings right. However, to leave it all working, after you apply the patch, you have to edit the newsletter node and re-save the scheduler options to make the newsletter scheduling as active.

Thanks!!

sgabe’s picture

sgabe commented
Status: Needs review » Fixed

Committed. Thanks for your help!

Status: Fixed » Closed (fixed)

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