Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Notifier commenters of replies via email #511

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

Send email to reply or post people,instead of ADMINS.
I think it will more useful.

@renwofei423 renwofei423 Update mezzanine/generic/views.py
send email to reply or post people,instead of ADMINS
2d03712
Owner

stephenmcd commented Dec 22, 2012

Hey there, thanks a lot for submitting this.

There's quite a bit more work that would be involved in implementing it properly:

  • The code references blog posts explicitly, but comments can be associated with any model. We'd need a generic way to look up the model, and it'd also need to check a Boolean field on that model (called say email_replies, defaulting to False) to see if the author of the blog post even wants comments email to them, since they might not!
  • No option is given to the commenter (or even the blog post author as I mentioned) to let them control whether or not they want emails sent to them, so we don't even have their permission to do so. We'd need another new Boolean field on the comment model (also defaulting to False) for the commenter to flag that they actually want email sent to them. Also with that in place, the email would need an unsubscribe link so that the commenter could choose to stop receiving emails. This would need to be a separate email send by the way, since the unsubscribe link would be specific to the commenter, not the blog post author.
  • The way it's implemented, if it finds emails to send to, it ends up ignoring the COMMENTS_NOTIFICATION_EMAILS setting entirely. They should still get the same email, along with the blog author if the setup above is implemented.
  • There's an empty except clause - what error is being trapped? There's probably no need for this with all of the above implemented more granularly.

Thanks again for kicking this off - it'll be a great addition with the above implemented.

@stephenmcd stephenmcd closed this Dec 22, 2012

@stephenmcd stephenmcd reopened this Dec 22, 2012

Owner

stephenmcd commented Dec 22, 2012

Sorry, no need to close this really. Let's leave it open.

Sorry! The application scene is in our cmpany,which might be mandatory in some way. I didn't think that much.

Time permitted,I will try to improve.

@stephenmcd stephenmcd closed this Mar 4, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment