Hi all,

This may seem like a stupid question, but I have to ask it.

I am running both a production and a test server. On both servers, all of my users when they subscribe to an allowed node, receive two copies of each notification. Each notification is exactly the same and are sent at the same time.

Just for the sake of troubleshooting I have limited the Messaging mail type to HTML mail, using PHP mailer only.

On my test server I am running the following:

Drupal 6.6

Notifications 6.x-1.0
Messaging 6.x-1.0
CCK 6.x-2.1
FeedAPI 6.x-1.4
Token 6.x-1.11

Any help you could offer would be greatly appreciated.

Gary

CommentFileSizeAuthor
#2 notifications_content.dualnotifications.patch981 bytesryan_courtnage

Comments

ryan_courtnage’s picture

ryan_courtnage commented
Version: 6.x-1.0 » 6.x-1.x-dev
Category: support » bug

I can verify that this is happening with DRUPAL-6--1 as of this morning. I'm using Simple Mail as the sending method.

Notifications (cvs tag DRUPAL-6--1)
+Simple Mail

Messaging (cvs tag DRUPAL-6--1)
+Content Notifications
+Notifications Autosubscribe

ryan_courtnage’s picture

ryan_courtnage commented
Component: Miscellaneous » Code
Status: Active » Needs review
StatusFileSize
new notifications_content.dualnotifications.patch981 bytes

I've figured out what is happening here. 2 events are getting added to notifications_event whenever a comment is made on a node.

notifications_content.module is the culprit, where it implements hook_comment(). hook_comment gets called 3 times, with ops: "validate", "insert", and then "publish". The condition matches true 2 times:

... ($op == 'insert' || $op == 'update' || ($op == 'publish' && !user_access('administer comments'))) ...

If a user does not have the permission "post comments without approval", then hook_comments is not called with op=publish. Perhaps this is different than the way D5 worked?

Anyways, I think that the condition should only match "update" and "publish" ops. I see no reason it needs to check the user_access for anything. So the above would be:

... ($op == 'update' || $op == 'publish') ...

(do we even need "update"?)

Patch for notifications_content.module is attached.

geverest’s picture

geverest commented

Thanks rcourtna,

I installed the patch on both my test and production servers and everything seems to be working fine now.

Gary

pildwell’s picture

pildwell commented

thanks for good patch!
it works =)

jose reyero’s picture

jose reyero commented
Status: Needs review » Fixed

Ok, applied this one, though this other related thread is still open, #285226: Redefine notifications and node/comment approval workflow

Thanks

Status: Fixed » Closed (fixed)

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