Closed (fixed)
Project:
Notifications
Version:
5.x-1.0-beta3
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
4 Jul 2008 at 12:42 UTC
Updated:
13 Aug 2008 at 17:52 UTC
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
Comment #1
David Goode commentedHello,
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
Comment #2
David Goode commentedHello,
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
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
Comment #3
mywyb2 commentedThanks, David, I will try out your patch. Your help is highly appreciated!
Comment #4
jose reyero commentedThanks David, good catch and good patch.
Comment #5
Anonymous (not verified) commentedAutomatically closed -- issue fixed for two weeks with no activity.