Great module, absolutely needed and there is plenty of room for capable developers to add improvements... here are some features I think would be great.

Much of these just have to do with simply giving site designers more option to control comment notification.

- Allow some settings changes on a PER NODE basis. ie. instead of just allowing global settings and letting users choose their setting, you set a node and its comments to be OPT-IN(so default DON'T notify) while the rest of the site is OPT-OUT(default notify)

- it appears that if you submit a comment with the notify feature enabled, then any other comment to the node above, whether a DIRECT reply or another comment, is sent to the individual. While this is often useful, I could envision certain threads where you might want to limit notification to DIRECT replies.

-perhaps an easy interface to review the notification emails being sent by the site (I guess you can probably locate these in the log now? But a site might want someone other than the web designer to monitor the notifications going out)

-making sure that when comment notify is enabled, PAST comments are at least defaulted to NOT notify, since the user never agreed to be notified. This could be changed by the admin, but should at least be default off. I found when we enabled that everyone started getting default notification. I probably could make notification opt-in(ie unchecked) but that would not be ideal.

Just thought I'd try to share some thoughts after testing it out.

Thanks,

art

Comments

Mantichora’s picture

Mantichora commented

I agree. This module is a great idea but there is still plenty to improve.

Especially the last note: PLEASE do not put all past messages by default on notify.
So I suppose you should change the install-file and make of it 'ADD COLUMN `notify` tinyint(1) NOT NULL DEFAULT '0'' ?? There are of course other ways to do it.
Maybe it is preferable to just create a new table for the module so you don't need to mess up the comment-table at all?

Would it be a lot of work to extend the module and make it possible for visitors or members who didn't make any comments at all to have a follow up? I agree the form in which you send the comment is a great place to put the subscribe-checkbox but if you do it like this there are a lot of users who are excluded from being capable of subscribing to some node.

Oh btw is it really necessary to do that many form-checks for the e-mail address in the query in function _comment_notify_mailalert2 ? Shouldn't all e-mails have a good form when submitted by registered users? And where are the email addresses of visitors retrieved?? Don't seem to find this anywhere in the code.
Though I still think (referring to my first point) a new comment_notify - table would be a better idea. It would make a lot of things easier, no?

In function comment_notify_form_alter I found this 'drupal_set_message': ATTENTION: your comment is NOT YET posted - please click the post button to confirm your post.
I mean... I can understand you think it is important for users to know they still need to submit but this has nothing to do with this module!

There are still some things I could point out but these were the most important ones.
There is still much to do, so I would be happy to help you coding, testing or whatever...

Greets

ahneill’s picture

ahneill commented

great thoughts, try to make sure the maintainer of comment notify sees these directly, the module is great but does need some work, especially with things like users who have difficulty unsubscribing, and making it easy for admins on the drupal site to unsubscribe them...

thanks

art

Lloyd Standish’s picture

Lloyd Standish commented
Title: Some feature requests » Can't approve anonymous comment
Category: feature » bug

I installed comment_notify-5.x-1.x-dev on Drupal 5.2. I replaced comment.module with the module's version, activated the module, and made it obligatory for anonymous commenters to leave contact info. I posted a comment as anonymous user. Then I signed back in as administrator, and tried to approve the comment. The following errors appeared:

* warning: Invalid argument supplied for foreach() in /var/www/lloyd/drupal/modules/node/node.module on line 521.
* warning: implode() [function.implode]: Bad arguments. in /var/www/lloyd/drupal/modules/node/node.module on line 525.
* 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 n.nid, n.vid, n.type, n.status, n.created, n.changed, n.comment, n.promote, n.sticky, r.timestamp AS revision_timestamp, r.title, r.body, r.teaser, r.log, r.format, u.uid, u.name, u.picture, u.data FROM node n INNER JOIN users u ON u.uid = n.uid INNER JOIN node_revisions r ON r.vid = n.vid WHERE in /var/www/lloyd/drupal/includes/database.mysql.inc on line 172.

- Lloyd

Christoph C. Cemper’s picture

Christoph C. Cemper commented
Category: bug » feature

@mantichora

thanks - def. some good ideas here , tough I think some of them require A LOT of effort, which I cannot provide in terms of time & $ at the mo,
so your help in coding would be appreciated as you seem to have some good ideas (tough removing sanity checks is NOT a good one IMHO)

I saw you didn't contribute any modules or commit to any,
but comment_notify could be the first if you'd like to followow up on some suggestions via more detailled suggestions on where/how to implement them

please let me know, christoph

@art

great suggestions and I agree these might make sense for SOME - but instead of switching defaults to some other behaviour (WHICH would be a pain in the ass for people upgrading!!!) I would at least want to have all of that stuff configurable, which in turn needs more effort (see above)

@lloyd: you alr opened this as sep. bug request - switching back to

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Status: Active » Fixed

I'm going to mark this issue fixed. I believe that all of the ideas presented here are either already done, or have a different issue in the queue which is more focused on the subject, or are not entirely in keeping with the goals of this module.

While I appreciate that it is easier to submit all your ideas and problems in one issue, it is much easier for development purposes to have separate issues for each concept or problem. In the future please be sure to open separate issues for each idea so they can be discussed more specifically.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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