| Project: | Simplenews |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Berdir |
| Status: | closed (fixed) |
| Issue tags: | schedule |
Issue Summary
Hi guys,
I'm not too sure if you agree, but I think that when a node is not published, then it should not be sent (emailed.)
If that is agreeable, then there is a very easy fix to get simplenews to mail the node only when the user scheduled it to be sent. This is done with one since SQL change testing whether the node is published. If not yet published, nothing happens (the node is ignored). If published, then the node can be sent.
And if you use the Scheduler module to decide when the node should be published, you're done. No need for a specialized module (i.e. simplenews_scheduler which is not yet available for D6)
Anyway... if you agree on that, there is a patch that checks what is necessary. Speaking of which, the simplenews_get_spool() function accepts $nid and $vid as parameters but they are not being used. Is that a bug?
Thank you.
Alexis
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| simplenews-6.x-1.x-send_published_only.patch | 724 bytes | Idle | FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch simplenews-6.x-1.x-send_published_only.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots. | View details |
Comments
#1
I second this patch. This is exactly what I needed.
I can see that some people may want to send unpublished nodes, but an option could always be added to handle this. By default it should not send a node unless it is published.
#2
> I'm not too sure if you agree, but I think that when a node is not published, then it should not be sent (emailed.)
Yup, that seems logical.
But this patch creates a big flaw in the UI -- I can select 'send now' when I create my newsletter AND unpublish (without knowing what this means) and then simplenews appears to not be obeying me.
So at the very least we need a bit of description on the sending options (only if the user has publishing permissions).
#3
We could also add a validation check which says that "Send now" is not permissible with Published unchecked? That way, it won't feel like it does not do anything, it will tell you what's wrong and give you a chance to fix it. What do you think?
Thank you.
Alexis
#4
Sounds good to me.
#5
I think I agree that there is a problem. I did not check the published box, but still used the scheduler module to publish at a certain time. When the time came to publish the node, it published but never sent.
#6
It worked for me. Try the patch with the latest dev version of simplenews module.
Thanks AlexisWilke :-)
#7
Ouch, this hasn't been committed and is holding up my upgrade of simplenews. What needs to be done to get this into the module?
#8
In the current concept of simplenews, sending is not directly related to published/unpublished state.
I'd much more expect to see a clearer separation such as:
#781708: Move simplenews node actions into Tab
#9
Just bumping this issue, as it actually makes sense, changing the version (no new feature in 6.x-1.x).
#10
I might support the initial idea about "publishing".
However the patch is ugly and doesn't fix the issue in its source. This has not to be fixed in the spool but much more initially in the nodeapi.
If you publish delayed, the recipient list should be built on publish - not on "send" button hit.
Also i won't commit as long as we have the UI issue. Users shouldn't be able to hit "Send"... This would be much more confusing.
Also remember when contributing code to solve this issue that in case of limited node edit permissions, the publishing flag might be inaccessible by the UI.
There's much to do here.
#11
Closing #167237: Simplenews + sched_act as a duplicate of this one.
#12
Eagerly awaiting this feature. I just checked my email and found to my surprise that my newsletter was sent just after midnight (with an hourly cron run). When I go to check on the node, it is still unpublished and scheduled to go at 9am like I set it up last night. This is very counterintuitive. When someone is using Scheduler, they know that a Save will not immediately publish because on the next screen the message tells them it will not be published until [datetime]. I think it should work the same way with Simplenews. They can hit Save and Send and on the next screen it will warn them that the newsletter will indeed not be sent until the scheduled time.
Yes, I definitely see that some may want to Save and Send but never publish or publish later. See then let's just create another option in the list so people have a choice. Go ahead and default it to email unpublished if you like, I just need the ability to override that and only send when published.
#13
Why not just use Rules and the Rules Scheduler to trigger sending the newsletters? This would support your published / unpublished logic along with many other scenarios.
#14
It's 3 clicks to use Scheduler. Click in Date Box. Click Date from Calendar Popup. Click in Time Box. Type Time.
Don't worry, though, SimpleNews is not the only module that is not yet compatible with Scheduler. The Twitter module will auto-tweet your nodes when published, but not if they are published through Scheduler.
Why add more complexity, more modules, when I all need a simple tweak? Anyway, I'll take a try at adapting the code to allow this option once I'm back from vacation in a few weeks. As I said, I need to get Twitter to respect Scheduler as well.
#15
+1 subscribe
#16
Possibly we should add this feature too...
Doesn't sound like a huge amount of work.
It's unlikely that we add a backport to 6.x.
#17
#18
Ok, this patch is a real kitten killer..
The patch implements this feature by adding a new "Send newsletter on publish" radio button to the Newsletter tab. When saving with that, a new state is written in simplenews_newsletter and that is checked in hook_node_update() and if set to send_publish and the node is published, the newsletter is sent.
Additionally, the patch also:
- Refactors some code to re-use it for the send publish
- Fixes a bunch of bugs, including but not limited to:
- proper saving/updating of the simplenews_newsletter table (updates actually never worked)
- Updating the state of directly sent newsletters (no cron) immediatly to sent once done.
- Adds a number of newsletter sending/creating/update tests which test sending with and without cron, with cron throttling, the new save on publish feature and also updating the simplenews_newsletter table. Note that this currently only tests the amount of mails sent and the recipients, not the actual content.
#19
Closed #1336208: Newsletter term not updated in database table if changed... as a duplicate.
#20
Can you please also state how the future of a module like Simplenews Scheduler will look like?
drupal.org/project/simplenews_scheduler
How will we cover the need of directly adding a publishing time in newsletter tab in future?
Another question is, as long as a newsletter is not published yet: Should we disallow people to choose "Send" on the newsletter tab?
Reviewing the patch asap.
#21
Had a quick look at that module, as I understand it that one allows to send regular, automatically created newsletters. I don't think this conflicts or even just overlaps much with this issue.
About the not published thing. Not sure, could make sense.
#22
Updating title
#23
Back-referencing #1169990: Improve usability and displayed information on the newsletter tab for UX and general improvements to the newsletter tab.
#24
#18: send_publish_bugfxes_tests.patch queued for re-testing.
#25
Updated to only show the send newsletter option when a node is published. The page will probably be completely redesigned in the linked issue above, so not spending more time on that right now.
#26
Ok, commited.
#27
Cool! Thank you guys. 8-)
#28
Automatically closed -- issue fixed for 2 weeks with no activity.
#29
This issue seems well covered with tests to me.