Sends notifcations with Unpublished comments

arlind - May 14, 2009 - 10:46
Project:Watcher
Version:6.x-1.1
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Description

Hi

Thanks for a great module :-)

On my site comments requires approval before they are published. Watcher currently sends notifications of unpublished comments to subscribers.

Watcher should only send notifications when comments are published.

:-) Bo

#1

danfinney - May 17, 2009 - 02:50

I have the same problem. When Spam is entered into the comments, my subscribers are receiving the Spam even though I moderate and delete these posts.

Need a fix for users that are moderating comments.

Thanks for a great module!

#2

solipsist - May 17, 2009 - 08:49

Thanks for reporting this. I'll get a fixed version up so you can test it. Keep an eye on this issue.

#3

solipsist - May 17, 2009 - 14:43
Status:active» needs review

Try the attached patch against release 1.1. It should sort the issue.

AttachmentSize
462430.patch 2.48 KB

#4

danfinney - May 22, 2009 - 16:24

Solipsist,

Thank you for your extremely speedy response to this issue!

One of the reasons I was using Watcher was to be notified when someone made a comment so I could go into the moderation que and approve or deny the comment. With this fix my subscribers are no longer receiving notification of unapproved comments, but now neither am I.

#5

solipsist - May 19, 2009 - 07:17

First of all, did the patch fix the issue? Did it have any undesired side effects?

Secondly, if you have other issues to report, please open a new thread. By reusing the same thread you just make it near impossible for me to keep track of what's outstanding and what's done. Re-post the feature request above as its own issue and I'll have a look at it. If you can, please edit the post above and remove the feature request entirely.

#6

danfinney - May 22, 2009 - 16:25

I receive this error now when trying to approve a comment:

Fatal error: Cannot use object of type stdClass as array in /public_html/blog/sites/all/modules/watcher/watcher.module on line 549

It apparently approves the comment for publication even though there is an error, but does not send the email notification.

#7

solipsist - May 21, 2009 - 09:50

danfinney: Please do not alter existing posts the way you did with #4. You radically modified #4 above which makes my comment inconsequential and the whole issue thread a lot harder to follow. I cannot respond to requests unless they're filed in a way that makes sense.

I appreciate you opening a new issue for the feature request. I'll get to responding to it in a bit. Regarding this, please restore post #4 above to what it was and post any further comments as new posts.

Thanks for understanding.

This is your original text for post #4:

Solipsist,

Thank you for your extremely speedy response to this issue!

Ok, now I have another issue which is more of a feature request, I
suppose. One of the reasons I was using Watcher was to be notified when
someone made a comment so I could go into the moderation que and approve
or deny the comment. With this fix my subscribers are no longer
receiving notification of unapproved comments, but now neither am I. I
would like an option to have different roles receive notifications based
on the status of the comment. For instance I would like to be able to
set Logged-In-Users and Anonymous-Users to not receive notification
until the Comment is approved, but anyone assigned as an Admin should
receive notifications if there is a Comment awaiting approval.

Is this doable?

#8

solipsist - May 21, 2009 - 21:57

I receive this error now when trying to approve a comment:

Fatal error: Cannot use object of type stdClass as array in /public_html/blog/sites/all/modules/watcher/watcher.module on line 549

Thanks for reporting. Try the attached patch and let me know if it fixes it.

How to patch:
http://drupal.org/node/60108

Also, please consider doing what I wrote in #7. I often follow up on issues months later and inconsequential comments make it really tricky to figure out what the issue was all about. It's like in Back to the Future when Marty undoes his own birth, it messes up the timeline which is really, really bad, as the Doc is never late to remind Marty of.

AttachmentSize
462430.patch 1.59 KB

#9

danfinney - May 22, 2009 - 16:25

You specifically asked me to do this:

If you can, please edit the post above and remove the feature request entirely.

What part would you like put back in? I made some edits. See if they suit your needs.

#10

solipsist - May 22, 2009 - 16:31

Well you did, which was very kind of you, but you also posted your response (i.e. the error message the patch caused), in the same post. That makes it extremely hard to make any sense of anything :)

Post #4 should be:

Solipsist,

Thank you for your extremely speedy response to this issue!

This:

Solipsist,

I receive this error now when trying to approve a comment:

Fatal error: Cannot use object of type stdClass as array in /public_html/blog/sites/all/modules/watcher/watcher.module on line 549

Should have gone in post #6.

If you read #4, #5 and #6 as they are now, does it make any sense? Nope. The natural order is what I described above plus what is now #6 being #7 instead. That's the actual order of events. Reason I am so strict about this is because I postpone some issues or need to trace code to an issue filed here, and that may happen months or even years later. My memory isn't good enough to remember chronological oddities like these :) Reminds me of trying to make sense of messy accounting...

Ok, enough meta discussion :). Did the second patch work? It should have fixed the "Cannot use object of type stdClass as array" error.

#11

danfinney - May 22, 2009 - 16:45

I don't know if the patch works or not. Terminal scares me. =)

#12

solipsist - May 22, 2009 - 17:32

I just realized that patch isn't in universal format. I'll have to repost it. I recommend learning how to patch, it makes everything Drupal a lot easier! :)

Rename the attached file and remove the .txt extension.

AttachmentSize
watcher.module.txt 102.17 KB

#13

danfinney - May 26, 2009 - 04:19

Ok, we seem to have a winner! I tested the commenting and approval mechanism and it works flawlessly. I implemented this on my live site. Will let you know if it runs into any real world problems, but I think you are good to go.

Thanks again for your speedy response time!

#14

danfinney - May 26, 2009 - 04:36

Bah, must have been because it was post # 13. If you approve the comment by going to: admin/content/comment, click the "approval que" tab, checking the checkbox for "publish the selected comments, and clicking update you have no problems.

If instead you go to the Story, click the "edit" button on the Unpublished comment, expand "Administration," mark the post as Published, and Save... you get this error:

* warning: array_fill() [function.array-fill]: Number of elements must be positive in /includes/database.inc on line 241.
* warning: implode() [function.implode]: Invalid arguments passed in /includes/database.inc on line 241.
* warning: array_keys() [function.array-keys]: The first argument should be an array in /modules/user/user.module on line 502.
* user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ')' at line 1 query: SELECT p.perm FROM role r INNER JOIN permission p ON p.rid = r.rid WHERE r.rid IN () in /modules/user/user.module on line 502.

I just went to attempt the second method of comment approval without Watcher enabled. I unchecked Watcher in the Modules admin, clicked Save Configuration and got this error:

warning: call_user_func_array() [function.call-user-func-array]: First argument is expected to be a valid callback, '_watcher_menu_access_binder_shortcut' was given in /includes/menu.inc on line 452.

FYI, I received no errors when approving the comment using the second method without Watcher enabled.

#15

solipsist - July 10, 2009 - 14:53

Figured ... user_load() doesn't take an integer as argument. That explains the first error message. The second one is due to your menu cache needing to be emptied.

#16

jaako - July 26, 2009 - 14:20

This is fantastic, thanks for this!

Until now, every anonymous user who watched comments on my blog unsubscribed within a week because of the amount of e-mails they were receiving due to spam on my blog. This is a much-much-much-overdue feature!

I trust you'll build this into the module admin page in future versions? I love watcher and I would like to see it more widely adopted because I think its a great module, but this fix was a looooong time coming.

Keep up the good work.
KJ

#17

solipsist - July 26, 2009 - 14:51

jaako: Please download the dev snapshot and post any bugs you find. I'll make a new release as soon as I'm certain it's stable.

#18

danfinney - July 27, 2009 - 03:04

Patch has been successfully working for me since installing. No issues to report.

#19

solipsist - July 27, 2009 - 07:30

I also want to stress the importance of using the Captcha or reCaptcha module. It will cut down on spam considerably. The README file for Watcher explains how to use captchas with anonymous notifications.

#20

jaako - July 27, 2009 - 19:33

Thanks. No bugs to report so far.

As for the issue of spam entries in the anonymous e-mail list.. a useful feature for the TOOLS section of the admin page would be a dump of the watcher table in the database. It would save me from having to open phpMyAdmin every time I want to see who's in the list? Even just a dump of the anonymous entries...

Just a thought.
KJ

#21

solipsist - July 27, 2009 - 20:19

Sure, make a feature request and I'll queue it for the next major release.

#22

solipsist - July 28, 2009 - 19:15
Status:needs review» fixed

I've rolled out the solution I implemented and which danfinney and jaako confirmed to be working in version 6.x-1.3.

#23

System Message - August 11, 2009 - 19:20
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.