Offer skip condition

As discussed, this has nothing to do with subscriber skipping, it happens way, way earlier.

We already have hook_simplenews_scheduler_edition_node_alter(), we could add something there that allows to skip?

I've been looking for this.

My first instinct is that if the body field is empty (or renders to empty), it should not add an edition. But I realize that simplenews passes the whole node for rendering to the theming system and there might be other fields doing the rendering. I think this is best left to custom code but simplenews_scheduler should allow for a way to support it. The obvious hook that comes to mind is hook_simplenews_scheduler_edition_node_alter.

Right now, that wouldn't work because the very next line says $edition_node->save() and there is no handler for an exception. Should we make it so that if the alter hook sets a property, e.g. simplenews_scheduler_skip_edition to TRUE, it should just exit?

Here is a patch with the idea.

Yeah, something like that.

What if we pass that as a separate variable to the alter hook, then we can explicitly document it and it is very obvious to anyone who implements this hook how to do this if they want to?

I thought it was not worth adding another parameter to the alter call, which had only one left. But then I decided that documenting it is more important. Here are the changes, with some other improvements as well. One of the things I fixed from the previous patch is that the scheduler record is update to the next run time even if the mail is not sent.

As I noted in another issue, the failures seem to be unrelated to the patch.

A tiny change in the .api.php documentation. I realize interdiff would be useful but I couldn't create it easily due to other patches. I hope the patch is not too long to review again. The only difference between #8 and this one is that this one contains &$context in .api.php.

Setting it back to Needs Review for unrelated failing tests.

Your code is missing the use statement, that's not a fault of this patch. Doesn't matter if it's Node or NodeInterface, as long as it has the corresponding use or full namespace.

I would like to suggest an addition. It would be useful to have the scheduler object in the hook, e.g. to get the interval length. Because of that I added a line of code to the patch #11 to store the scheduler object under the key 'scheduler' in the context array. My suggested patch is #12

I updated my patch because it failed to apply

Here is another version of the 'offer skip condition' patch because since the very first patch there was a problem with skipped editions, which nobody noticed yet. Skipped editions appear like the template under 'content' but without any additional fields changed. This is because the editions get saved on cloning. I added a 'delete' when the edition is skipped to solve this.

I rerolled the patch for 2.0.x version.

Ignore the patch in #21. I have forgotten how to create patches without a difftool. :D

I created a MR instead. Please review.

I created patch by #22

This feature is fundamental.

This patch needs a re-roll.

I rerolled the patch for 4.0.0-beta1

Should this be a feature in the next beta version? I'll have to rebase the merge request source branch, so anybody could do a review.

I restarted the issue fork branch based on current 4.x and applied the latest patch. The merge request is ready for a review again.

Is it possible to simply skip sending when the created newsletter body field is empty? This seems like it would be automatic, but evidently not.

You can write your own skip condition in hook_simplenews_scheduler_edition_node_alter. Here is an example:

function your_module_simplenews_scheduler_edition_node_alter(NodeInterface $edition_node, NodeInterface $scheduler_node, array &$context) {
  if ($your_condition) {
    $context['skip_edition'] = TRUE;
  }
}

#26 patch work fine for me.