WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 years ago

#33717 assigned feature request

Send Notification Email When a Comment is Approved From Moderation

Reported by: jeffr0 Owned by: swissspidy
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

Currently in WordPress, commenters have no idea their comment is approved unless they visit the page often. When a comment is held for moderation, WordPress should send the commenter an email notification when their comment is approved. I'm using the Comment Approved plugin to add this functionality to WordPress but I really think it should be a core feature.

Attachments (10)

33717.diff (4.4 KB) - added by swissspidy 3 years ago.
33717.2.diff (4.1 KB) - added by swissspidy 3 years ago.
33717.3.diff (4.5 KB) - added by mrahmadawais 3 years ago.
33717.4.diff (4.3 KB) - added by swissspidy 3 years ago.
33717.5.diff (4.2 KB) - added by wonderboymusic 3 years ago.
33717.6.diff (4.3 KB) - added by swissspidy 3 years ago.
33717.7.diff (4.7 KB) - added by swissspidy 3 years ago.
33717.8.diff (4.8 KB) - added by swissspidy 3 years ago.
33717.9.diff (6.3 KB) - added by swissspidy 3 years ago.
33717.10.diff (8.7 KB) - added by swissspidy 3 years ago.

Download all attachments as: .zip

Change History (63)

#1 @rachelbaker
3 years ago

  • Version 4.3 deleted

#2 @knutsp
3 years ago

  • Keywords needs-patch added

Yes!

I don't know how many times the last ten years I have left a comment on a WordPress site and got the message that my comment is held for moderation. Then I leave the site, never knowing my comments would ever be approved.

Last edited 3 years ago by knutsp (previous) (diff)

#3 @atomicjack
3 years ago

+1 this is definitely needed.

#4 @danielbachhuber
3 years ago

This would basically be the best feature in 4.4

#5 @michaelbeil
3 years ago

I would really appreciate being notified of my approved commentary. Seems like a fantastic addition to native comments.

#6 @boonebgorges
3 years ago

  • Keywords good-first-bug added

#7 @rachelbaker
3 years ago

  • Milestone changed from Awaiting Review to Future Release

Sounds like there is plenty of interest here. Anyone interested in submitting a patch to make @jeffr0's dreams come true?

Last edited 3 years ago by rachelbaker (previous) (diff)

#8 @jeffr0
3 years ago

Here's another reason why it should be a core feature. The plugin I use to add this feature to WordPress doesn't work because the Tavern uses Jetpack Comments and not the native comment form. Sigh. I hope that by adding it to core, Jetpack comments wouldn't need to be involved.

http://wptavern.com/help-me-add-comment-approval-notifications-to-wordpress#comment-72576

#9 follow-up: @jeffr0
3 years ago

What are your thoughts on whether a commenter should have to opt-in to receive the notification instead of automatically sending it?

#10 follow-ups: @krogsgard
3 years ago

Current notification functions are pluggable, and that's (I hear) not a go anymore. So if you keep the same naming convention, you'd probably utilize wp_notify_commentauthor inside of wp-includes/comment-functions.php that would operate similarly to wp_notify_postauthor.

wp_notify_postauthor is triggered from wp_set_comment_status for newly approved comments. You could call wp_notify_commentauthor in the same way.

Unfortunately that method would likely add a lot of duplicate code from the other comment notification emails. It'd be nice to have a more generic notification function.

#11 in reply to: ↑ 10 @kasparsd
3 years ago

Replying to krogsgard:

wp_notify_postauthor is triggered from wp_set_comment_status for newly approved comments. You could call wp_notify_commentauthor in the same way.

There is also the transition_comment_status action.

I think the "Comment Approved" plugin is a good of example of how it could be achieved.

#12 in reply to: ↑ 10 @kraftbj
3 years ago

Replying to krogsgard:

Unfortunately that method would likely add a lot of duplicate code from the other comment notification emails. It'd be nice to have a more generic notification function.

Yeah, we could split out a wp_notify for all of the functional bits, then use wp_notify_postauthor, wp_notify_moderator, wp_notify_commentauthor (for example) as a wrapper of that function.

Replying to jeffr0:

What are your thoughts on whether a commenter should have to opt-in to receive the notification instead of automatically sending it?

Personally, automatic that can be disabled with a filter. When enabled, the "Your comment awaits moderation." changes to "Your comment awaits moderation. You will be notified if it is approved."

Personally, I'm of the mindset that we should not (at least by default) notify comment authors that are deleted/spammed, etc.

#13 @jeffr0
3 years ago

<blockquote>Personally, I'm of the mindset that we should not (at least by default) notify comment authors that are deleted/spammed, etc.</blockquote>

I agree. The notifications should be strictly limited to pending comments. If they are sent to spam or trashed, no notification is sent. It wouldn't make sense.

#14 @ubernaut
3 years ago

how would notifications work with twitter/fb commenters?

#15 @idogenealogy
3 years ago

That would be a great feature to add to core.

#16 @mrahmadawais
3 years ago

This seems to be a good feature. +1 @jeffr0.

I think we can hook it inside wp_transition_comment_status

#17 @swissspidy
3 years ago

  • Owner set to swissspidy
  • Status changed from new to assigned

@swissspidy
3 years ago

@swissspidy
3 years ago

#18 follow-up: @swissspidy
3 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

What 33717.2.diff does:

  • Adds a new function called wp_notify_commenter which is hooked to transition_comment_status
  • Includes two new filters:
    • comment_approval_notification_text
    • comment_approval_notification_subject
  • Changes the Your comment is awaiting moderation. text to Your comment is awaiting moderation. You will receive an email when it gets approved.

The email looks as follows:

Howdy Jane Doe,

Your comment on the post "Hello World" got approved.

View comment: http://src.wordpress-develop.dev/2015/09/06/hello-world/#comment-16

Changes to the first patch:

  • Email is only sent when comment gets approved, not when it got trashed etc.
  • No comment_notification_notify_commenter filter. Functionality can be easily disabled using remove_action( 'transition_comment_status', 'wp_notify_commenter' )

#19 @swissspidy
3 years ago

  • Milestone changed from Future Release to 4.4

@mrahmadawais
3 years ago

#20 in reply to: ↑ 18 @mrahmadawais
3 years ago

Replying to swissspidy:

Added a check for if in case The comment was left by the author & The author moderated a comment on their own post BTW we can use a better action i.e. comment_unapproved_to_approved action is defined in function wp_transition_comment_status 1286 comment-functions.php.

What do you think?

Last edited 3 years ago by mrahmadawais (previous) (diff)

@swissspidy
3 years ago

#21 follow-up: @swissspidy
3 years ago

Replying to mrahmadawais:

Added a check for if in case The comment was left by the author & The author moderated a comment on their own post

Thanks, good catch. However I noticed the following things with your patch:

  • Indentation is wrong throughout the function.
  • $author isn't used anywhere
  • $notify_author isn't defined anywhere, which would lead to an error.

33717.4.diff accommodates for that. Plus it fixes the one multi-line comment.

BTW we can use a better action i.e. comment_unapproved_to_approved action is defined in function wp_transition_comment_status 1286 comment-functions.php.

I'd say the transition_comment_status hook can be more easily found and debugged as the dynamic comment_{$old_status}_to_{$new_status} hook. Also, does this work when the comment was previously trashed / mistakenly marked as spam but then approved?

#22 in reply to: ↑ 21 @mrahmadawais
3 years ago

Replying to swissspidy:

Replying to mrahmadawais:

Added a check for if in case The comment was left by the author & The author moderated a comment on their own post

Thanks, good catch. However I noticed the following things with your patch:

  • Indentation is wrong throughout the function.
  • $author isn't used anywhere
  • $notify_author isn't defined anywhere, which would lead to an error.

Thanks for the fix. I had actually completed writing a function inside pluggable.php when you uploaded the patch. I think I just copied this check and forgot about the two variables, should have submitted it after second cup of coffee.

33717.4.diff accommodates for that. Plus it fixes the one multi-line comment.

BTW we can use a better action i.e. comment_unapproved_to_approved action is defined in function wp_transition_comment_status 1286 comment-functions.php.

I'd say the transition_comment_status hook can be more easily found and debugged as the dynamic comment_{$old_status}_to_{$new_status} hook. Also, does this work when the comment was previously trashed / mistakenly marked as spam but then approved?

I haven't tested that, but in theory, it should work whenever a comment goes from unapproved to approved status.

#23 @wonderboymusic
3 years ago

  • Keywords needs-refresh added; good-first-bug needs-testing removed

This looks great, but we need a more robust way of knowing where the comment is transitioning from. Right now, an email is getting sent from Trash Undo, which is not good. We should aim to do something that results in the email being sent as few times as is possible.

#24 @jeffr0
3 years ago

I agree with Scott, the email should only be sent when a comment is approved from the moderation queue and not under any other circumstances.

@swissspidy
3 years ago

#25 follow-ups: @swissspidy
3 years ago

  • Keywords needs-refresh removed

As suggested initially by @mrahmadawais, 33717.6.diff uses the comment_unapproved_to_approved hook to only send a notification when the comment status changes from unapproved to approved.

#26 in reply to: ↑ 25 @mrahmadawais
3 years ago

Replying to swissspidy:

As suggested initially by @mrahmadawais, 33717.6.diff uses the comment_unapproved_to_approved hook to only send a notification when the comment status changes from unapproved to approved.

Yup, all good now. I have had actually tried this hook before. Working fine at my end now.

Last edited 3 years ago by mrahmadawais (previous) (diff)

#27 in reply to: ↑ 25 @jdgrimes
3 years ago

  • Keywords needs-refresh added

Replying to swissspidy:

As suggested initially by @mrahmadawais, 33717.6.diff uses the comment_unapproved_to_approved hook to only send a notification when the comment status changes from unapproved to approved.

This will still cause an email to be sent multiple times if the comment is approved and then unapproved and then approved again. As a result, the email will also be sent for old comments if the comment status is toggled in this way.

I would have suggested setting a meta flag on the comment once the email was sent, and then checking for that to prevent multiple emails. However, I think maybe we need to find a more robust way to determine whether an approval is the initial approval, or else the emails would still be fired off for old comments that were received before this code existed, if the comment status was toggled (possibly that is just an edge case, though).

#28 @knutsp
3 years ago

Also not send if comments are closed on the post. Toggling the status for old comments is not a problem, or too much of an edge case for core. Let plugins (a filter) handle that.

#29 @helen
3 years ago

What happens with custom comment statuses if using that hook? :)

#30 @dshanske
3 years ago

I just realized that #33735 is related to this in that what I am proposing re duplication and more flexibility can be resolved while adding this new notification.

Specifically since parts of all comment notification can come from a common source.

Last edited 3 years ago by dshanske (previous) (diff)

@swissspidy
3 years ago

#31 follow-up: @swissspidy
3 years ago

  • Keywords needs-refresh removed

We should aim to do something that results in the email being sent as few times as is possible.

33717.7.diff sets a flag using comment meta to prevent sending emails more than once when the status changes back and forth.

Also not send if comments are closed on the post.

I disagree here. If comments are closed and a comment gets approved, it will still be shown on the website and the user should be notified about that.

What happens with custom comment statuses if using that hook?

In my opinion, when someone adds new comment statuses, chances are that sending an email is not desired. If it is, the comment_{$old_status}_to_{$new_status} filter can easily be used to add this.

#32 in reply to: ↑ 31 ; follow-up: @obenland
3 years ago

Notify a comment author when his comment gets approved. - Let's use a gender-neutral their. We usually don't add an empty line between @param and @return (unfortunately). The meta key _wp_notification_sent seems a bit generic. I also wonder if we should accept both a comment ID and a WP_Comment object to be more in line with wp_notify_postauthor().

@swissspidy
3 years ago

#33 in reply to: ↑ 32 @swissspidy
3 years ago

Replying to obenland:

Notify a comment author when his comment gets approved. - Let's use a gender-neutral their.

Done in the latest patch.

We usually don't add an empty line between @param and @return (unfortunately).

I know, but my IDE unfortunately always adds an empty line, even though I configured all sniffs :/

The meta key _wp_notification_sent seems a bit generic.

Renamed it to _wp_commenter_notification_sent.

I also wonder if we should accept both a comment ID and a WP_Comment object to be more in line with wp_notify_postauthor().

Good idea. That way the function can also be hooked to other actions where only an ID is passed. Added now.

#34 @quicoto
3 years ago

+1 For this feature.

#35 in reply to: ↑ 9 @Monika
3 years ago

Replying to jeffr0:

What are your thoughts on whether a commenter should have to opt-in to receive the notification instead of automatically sending it?

The data privacy act at Austria and Germany doesn't allow any automatically e-mail without double opt in. So we need the possibility to ask the user or to disable this automatically notification or a way to inform the user before he commented.

#36 @rmccue
3 years ago

Just to note on the wording, "Your comment got approved" sounds a bit strange (though I can't remember the technical reason linguistically). "Your comment has been approved" would sound a bit better here. Even better if we can add a touch of pizzazz along the lines of "howdy" and keep it a bit more informal.

Last edited 3 years ago by rmccue (previous) (diff)

@swissspidy
3 years ago

#37 @swissspidy
3 years ago

  • Keywords has-unit-tests added

33717.9.diff makes it easier to remove the the functionality.

If you remove the action, e.g. using remove_action( 'comment_unapproved_to_approved', 'wp_new_comment_notify_commenter' ), the "You will receive an email when it gets approved." message won't be displayed.

The patch also fixes translator comments in the email and adds a unit test.

Oh, and yes, "Your comment has been approved" is definitely better than "Your comment got approved".

#38 @wonderboymusic
3 years ago

  • Milestone changed from 4.4 to Future Release

This ticket was mentioned in Slack in #core by helen. View the logs.


3 years ago

@swissspidy
3 years ago

#40 @swissspidy
3 years ago

  • Milestone changed from Future Release to 4.5

33717.10.diff is a new version of the patch that applies cleanly against trunk, as I'd really like to see this in 4.5.

Differences to the previous patch:

  • New function name: wp_new_comment_notify_comment_author
  • Some refined wording, e.g. do not use "Howdy %s," when comment author name is empty
  • Do not send email when comment author email is empty
  • A bunch of new unit tests to test for every possible scenario

This ticket was mentioned in Slack in #core by rachelbaker. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


2 years ago

#43 @swissspidy
2 years ago

  • Milestone changed from 4.5 to Future Release

#44 @voldemortensen
2 years ago

  • Milestone changed from Future Release to 4.6

Patch still applies cleanly.

#45 @rachelbaker
2 years ago

@swissspidy How are you feeling about this?

#46 @swissspidy
2 years ago

@rachelbaker I'd still love to get this into core!

There was a lack of feedback/interest during the last cycle, but if the consensus is "let's do it" now, by all means let's do it :)

#47 follow-up: @rachelbaker
2 years ago

@swissspidy I agree this would be a useful feature, and you have captured my interest.

I am concerned that emails to comments authors will be triggered when a moderator accidentally "approves" - and then realizing their mistake - quickly undoes the approval action. I don't know how often a scenario like that occurs to users other than me.

This ticket was mentioned in Slack in #core by rachelbaker. View the logs.


2 years ago

#49 in reply to: ↑ 47 @jeffr0
2 years ago

Replying to rachelbaker:

@swissspidy I agree this would be a useful feature, and you have captured my interest.

I am concerned that emails to comments authors will be triggered when a moderator accidentally "approves" - and then realizing their mistake - quickly undoes the approval action. I don't know how often a scenario like that occurs to users other than me.

This is a valid concern and something I've done a few times. Perhaps a delay of 5 minutes or so could be added to the action to give people more than enough time to correct a mistake without sending the email.

#50 @swissspidy
2 years ago

Perhaps a delay of 5 minutes or so could be added to the action to give people more than enough time to correct a mistake without sending the email.

That sounds like we'd need to leverage the cron API for this, similar to how scheduling posts works. Now that I think about it, it could be a great use case for a potential notifications API.

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


2 years ago

#52 @chriscct7
2 years ago

  • Milestone changed from 4.6 to Future Release

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


2 years ago

Note: See TracTickets for help on using tickets.