Hello

I own a big blow, where can be more than 100 comments at the post. And it's totally bad idea to send notifications to all of this people.

The comment_subscribe module provides such functionality, but when I saw it's code it was full of garbage.

So, I decided to make a feature for this module that allows admin to disable notifications to whole list of commenters, but only for people who actually got response. I saw several issues with such requests too, I hope that guys will be happy :)

Comments

StatusFileSize
new3.51 KB

Revised version.

Category:task» feature
Status:Needs review» Needs work

So, the idea here is to only send notification if someone replies to your specific comment, not just to the node in general?

Does this work for two (or more) levels of replies? For example, if I post a comment and request notifications, someone response to me and I get a mail, someone responds to them - then what?

This is a common feature request so I would love to implement this.

Also, can you explain the $sent_to[] = $alert->mail bit? That seems like it should be removed, right?

Yes, you get it right. I implemented only one-level replies, but if you want, I can option for multilevel replies.

As for this code
$sent_to[] = $alert->mail

I saw initiated array $sent_to in this function, but it wasn't used at all, so I writed the code that I guessed was planned for this veriable. It just stops sending a lot of letters to the one recepient at once, if he had many replies, site sends only one letter. Now (after some sleep :) I think it should be option too.

Please, make your suggestions.

I split out the $sent_to into its own issue. You are right that it should be added to the module - thanks for pointing that out.

In my opinion, the ideal solution to this issue (whether replies are sent for the whole node, replies to the thread of comments, or only replies to the comment) should be up to the end user making the comment. That way when you comment you see a set of radio buttons:

"Notify me of all followup comments to this %node-type."
"Notify me of responses that are related to my comment."
"Notify me of responses to my comment only."

They could default to not being selected and then if one is selected it would create that type of subscription. This would require using some new notation in the way we use the database column like a 0 for not subscribed, 1 for the first case (since that is the present behavior), 2 for the next and 3 for the final style.

What do you think?

Status:Needs work» Needs review
StatusFileSize
new9.78 KB
new6.88 KB

"Notify me of all followup comments to this %node-type."
"Notify me of responses that are related to my comment."
"Notify me of responses to my comment only."

I decided only to tackle the first and last cases. Subscribing to replies to my comment and any comments down the chain of pids underneath it is a lot of code that would run pretty slowly and I don't want to do that...

I've also attached a screenshot. Where there used to be a checkbox (under comments, on the user edit screen, on the admin settings screen) it is now replaced by this dropdown which says:

'No notifications'
'For all comments on this post'
'Just for replies to my comment'

The "post" text is hardcoded, not dependent on the content type.

Version:5.x-1.x-dev» 5.x-2.x-dev
Assigned:Unassigned» greggles
Status:Needs review» Fixed

Committed to both 5.x-2.x and 6.x-1.x.

Thanks, Greg, nice patch.
(sorry, I've lost a track of this issue and remembered about it only when saw in tracker your comments)

I'm glad to hear it. There is always room for improvement, so if you see any ways to make this better please do let me know.

So how would all this work if you implemented the "Flat Comments" module, which forces comments on comments to actually be comments on the main post? I've implemented this module, as the nested comments-on-comments wasn't really helpful on my site... but i do use comment notify, and do want everyone involved in the 'conversation' to get updates by default when someone comments.

shawn

Status:Fixed» Needs work

@sdsheridan - great question. I guess that really this should be a checkbox and a dropdown for the admin so they can choose which are enabled and which is default. Then the selectbox only shows the style that is available for users. How does that sound?

Sounds reasonable to me. I totally appreciate all the different combinations and permutations that everyone needs in their varied circumstances. As a commercial software designer (former life... don't anybody be gettin' any ideas! ;-), I always tried to accommodate them all as much as possible to ensure all my clients could use the system. I'm delighted to see that approach here too. Great work!

StatusFileSize
new4.82 KB

For 5.x...

Status:Needs work» Fixed

And...hopefully fixed (along with a few other things).

@sdsheridan - You were a software developer? Come on, just make one patch. I promise...it won't be addicting ;)

@greggles: hehehe... i already have... check some of my posts (and no, they're not real patches, but i just can't be bothered with trying to figure out the unix-like patch-making software, which i realise is a sad excuse) ;-) i'm not a php programmer by a long shot, but i can still read code... ok, well, some code... the simple stuff... :-)

so in what version is all this stuff rolled together now? are we coming out with a point-release shortly?

@sdsheridan - it's in the 5.x-2.x-dev version of the code. And yes, I plan to make a new point release in the next few days if this one doesn't blow up too much...

Status:Fixed» Closed (fixed)

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