I am using Notifications together with Messaging Mime Mail 5.x-1.0-beta2. Notifications are set to be sent immediately. Messaging is configured to send the token [node-body] upon node creation and updates.

When a node is newly created, an email with the body of the new node is correctly sent to subscribed users. However, when a node is *updated*, an email containing the body of the *old* node is sent, and the subject line contains the *old* title of the node.

The correct behaviour would be to send the updated data of the node.

Comments

David Goode’s picture

Hello,

Thank you very much for the catch. This is an issue with the immediate sending functionality. Basically what is happening is that notifications_content_nodeapi('update') is called after the node has been updated in the database, and it subsequently goes through the whole notifications sending stack until it node_load()s the node and fills in the message and tokens and etc. However, because the node was loaded earlier on the page, the node_load has it cached, and thus loads the OLD version. I have now fixed it for notifications, having it clear the cache manually in notifications_content, and I will submit a patch shortly after consulting with Jose. However, I am also going to query the Drupal 7 developers about whether this is something that should be fixed in that release.

David

David Goode’s picture

Status: Active » Needs review

Hello,

I'm back, with a patch to boot! Basically, as long as you call node_load(0, NULL, TRUE) somewhere between the save and the send, it'll be fine. The cleanest fix for medium-term inclusion is within notifications_content.module, as theoretically notifications doesn't have to deal with nodes, and also users or something could also send notifications on update, and if they were cached when loaded then the notifications_user.module should handle it, rather than stuffing it all in notifications.module.

Patch for notifications_content.module

@@ -347,6 +347,8 @@
       if ($node->status == '0') { // unpublished
         break; // don't notify
       }
+      //need to reset the node cache--doh
+      node_load(0, NULL, TRUE);
       /*
       // prevent already published edits from sending notifications
       // @ TODO send nodes that have been updated could be controlled here

To achieve the same result, you could also patch notifications.module's immediate sending call, or even drupal core's node.module if you felt so inclined. Also, as previously stated, I have submitted a query to the developer mailing list for Drupal 7 regarding placing this cache-clearing in node.module, so maybe in a few years we'll be able to eliminate this single line of code! And you were hoping for a flying car :-)

Also, in case people were wondering whether there would be some race condition with cron.php if immediate sending were off, you can sleep easy. Because the node_load caches statically within the function on one page, and cron will run on another, there won't be a problem. Also, poormanscron users may have intermittently had the same problem before, but that will be fixed by this patch as well.

David

mywyb2’s picture

Thanks, David, I will try out your patch. Your help is highly appreciated!

jose reyero’s picture

Status: Needs review » Fixed

Thanks David, good catch and good patch.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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