I notice there is a TODO for this hook. I need access to this hook for my simplenews SMS add-on.

Patch to follow.

CommentFileSizeAuthor
#13 hook_simplenews_send-1560084-12.patch943 bytesrurri
#10 hook_simplenews_send-1560084-9.patch883 bytesrurri
#8 hook_simplenews_send-1560084-7.patch881 bytesrurri
#5 hook_simplenews_send-1560084-4.patch878 bytesrurri
#1 hook_simplenews_send-1560084.patch5 KBrurri

Comments

rurri’s picture

rurri commented
Status: Active » Needs review
StatusFileSize
new hook_simplenews_send-1560084.patch5 KB

Here is the patch to make the hook happen.

rurri’s picture

rurri commented

Basically this patch just replaces

simplenews_add_node_to_spool($node);

with

module_invoke_all('simplenews_send', $node);

adds documentation for the hook, and then creates a single method that implements the hook and calls the spool

/**
 * Implements hook_simplenews_send
 */
function simplenews_simplenews_send($node) {
  simplenews_add_node_to_spool($node);
}
rurri’s picture

rurri commented
Category: task » bug
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
+++ b/includes/simplenews.admin.incundefined
@@ -267,7 +267,7 @@ function simplenews_issue_send($nids) {
-    simplenews_add_node_to_spool($node);
+    module_invoke_all('simplenews_send', $node);

Adding a hook sound useful but it's not necessary to change the API for this. Simply place the module_invoke_all() call inside simplenews_add_node_to_spool().

Then your patch is simplified to an additional line + maybe a comment and the api.php changes.

+++ b/simplenews.api.phpundefined
@@ -50,10 +50,15 @@
 /**
- * @defgroup spool Mail spool
+ *  Hook is called a newsletter is sent.
  *
- * @todo
+ *  Simplenews uses this hook to add the newsletter to the outgoing mail spool
+ *  @param stdObject $node The newsletter being sent
+ *  ¶
+ *  @return All return values are ignored
  */
+function hook_simplenews_send($node) {

Looks like you've overriden one of the placeholder defgroups. We do want to keep that..

rurri’s picture

rurri commented
StatusFileSize
new hook_simplenews_send-1560084-4.patch878 bytes

Simplified patch

rurri’s picture

rurri commented
Status: Needs work » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
+++ b/includes/simplenews.mail.incundefined
@@ -37,6 +37,9 @@ function simplenews_add_node_to_spool($node) {
+  ¶
+  //Notify other modules that a newsletter was just sent

Trailing space on the first line.

Comments should have a space after the // and end with a .

Looks good after this, much simpler now :)

rurri’s picture

rurri commented
Status: Needs work » Needs review
StatusFileSize
new hook_simplenews_send-1560084-7.patch881 bytes

Removed space and fixed comment.

Status: Needs review » Needs work

The last submitted patch, hook_simplenews_send-1560084-7.patch, failed testing.

rurri’s picture

rurri commented
Status: Needs work » Needs review
StatusFileSize
new hook_simplenews_send-1560084-9.patch883 bytes

Not sure how change the comment caused that one to fail.. but in any case, changed the variable name in the api.php to be $newsletter in this one.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

#8: hook_simplenews_send-1560084-7.patch queued for re-testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

This is a problem with the test, that one fails sometimes. Triggered a re-test.

Actually, let's just name the variable @node and also add a @param to the docblock explaining that this is the sent node.

It's not the newsletter/category that we are passing to that function.

rurri’s picture

rurri commented
StatusFileSize
new hook_simplenews_send-1560084-12.patch943 bytes

Minor changes to docbloc in api

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Looks good, commited, thanks!

Status: Fixed » Closed (fixed)

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