-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Notes unexpected do not notify post author (unlike comments) #10472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8ec209c
6799a1d
6faacfe
b3487af
0daba4c
cbfef88
f089d47
79fd7e5
2aa7c96
f9d4686
79aa49d
8da27bc
98c2873
099de2f
887974e
62ed6c3
25f0b56
f3bee16
0f6fff0
3b168e3
6fd6294
93b9bd1
7a1ed8e
2f4c927
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1894,6 +1894,20 @@ function wp_notify_postauthor( $comment_id, $deprecated = null ) { | |
| $subject = sprintf( __( '[%1$s] Pingback: "%2$s"' ), $blogname, $post->post_title ); | ||
| break; | ||
|
|
||
| case 'note': | ||
| /* translators: %s: Post title. */ | ||
| $notify_message = sprintf( __( 'New note on your post "%s"' ), $post->post_title ) . "\r\n"; | ||
| /* translators: 1: Note author's name, 2: Note author's IP address, 3: Note author's hostname. */ | ||
| $notify_message .= sprintf( __( 'Author: %1$s (IP address: %2$s, %3$s)' ), $comment->comment_author, $comment->comment_author_IP, $comment_author_domain ) . "\r\n"; | ||
| /* translators: %s: Note author email. */ | ||
| $notify_message .= sprintf( __( 'Email: %s' ), $comment->comment_author_email ) . "\r\n"; | ||
| /* translators: %s: Note text. */ | ||
| $notify_message .= sprintf( __( 'Note: %s' ), "\r\n" . ( empty( $comment_content ) ? __( 'resolved/reopened' ) : $comment_content ) ) . "\r\n\r\n"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to determine whether the status is resolved or reopened when empty? It's a little vague as is.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that should be doable based on comment meta or derived based on parent note status.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Meta is not yet saved at this hook, not sure the parent is helpful but I'll take a look. |
||
| $notify_message .= __( 'You can see all notes on this post here:' ) . "\r\n"; | ||
| /* translators: Note notification email subject. 1: Site title, 2: Post title. */ | ||
| $subject = sprintf( __( '[%1$s] Note: "%2$s"' ), $blogname, $post->post_title ); | ||
| break; | ||
|
|
||
| default: // Comments. | ||
| /* translators: %s: Post title. */ | ||
| $notify_message = sprintf( __( 'New comment on your post "%s"' ), $post->post_title ) . "\r\n"; | ||
|
|
@@ -1917,11 +1931,15 @@ function wp_notify_postauthor( $comment_id, $deprecated = null ) { | |
| break; | ||
| } | ||
|
|
||
| $notify_message .= get_permalink( $comment->comment_post_ID ) . "#comments\r\n\r\n"; | ||
| /* translators: %s: Comment URL. */ | ||
adamsilverstein marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| $notify_message .= sprintf( __( 'Permalink: %s' ), get_comment_link( $comment ) ) . "\r\n"; | ||
| if ( 'note' === $comment->comment_type ) { | ||
| $notify_message .= get_edit_post_link( $comment->comment_post_ID, 'url' ) . "\r\n"; | ||
| } else { | ||
| $notify_message .= get_permalink( $comment->comment_post_ID ) . "#comments\r\n\r\n"; | ||
| $notify_message .= sprintf( __( 'Permalink: %s' ), get_comment_link( $comment ) ) . "\r\n"; | ||
| } | ||
|
|
||
| if ( user_can( $post->post_author, 'edit_comment', $comment->comment_ID ) ) { | ||
| if ( 'note' !== $comment->comment_type && user_can( $post->post_author, 'edit_comment', $comment->comment_ID ) ) { | ||
| if ( EMPTY_TRASH_DAYS ) { | ||
| /* translators: Comment moderation. %s: Comment action URL. */ | ||
| $notify_message .= sprintf( __( 'Trash it: %s' ), admin_url( "comment.php?action=trash&c={$comment->comment_ID}#wpbody-content" ) ) . "\r\n"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,18 @@ public function __construct() { | |
| $this->meta = new WP_REST_Comment_Meta_Fields(); | ||
| } | ||
|
|
||
| /** | ||
| * Send a notification to the post author when a new note is added via the REST API. | ||
| * | ||
| * @since 6.9.0 | ||
| * | ||
| * @param WP_Comment $comment The comment object. | ||
| */ | ||
| public static function wp_new_comment_via_rest_notify_postauthor( $comment ) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I wasn't able to find reasoning for moving this method here. So what was it? IMO, seems odd and unrelated to have action callback in REST controller. It kind of goes against the idea of separation concerns. @TimothyBJacobs, what do you think?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had it in comments.php previously, but noticed some unit test failure because the hook was being fired in the test with a null comment. I can move it back and add a guard for this instead. I wasn't really sure about the best location for this code.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why doesn't the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let’s move it before RC so we don’t introduce accidental public function that we have to deprecate in next release.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@ellatrix
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| if ( 'note' === $comment->comment_type ) { | ||
| wp_new_comment_notify_postauthor( $comment->comment_ID ); | ||
| } | ||
| } | ||
| /** | ||
| * Registers the routes for comments. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, it could be nice to have filters for this instead of hardcoded behavior for notes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏼 Yea, its a bit odd to have this logic here. There is a filter just above
notify_post_authorthat can be used to opt out of notifications. The status and type should probably be used to set the default that is run thru that filter instead of being checked later.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ellatrix maybe something like this: 1f16c40
I can open a separate Trac ticket for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://core.trac.wordpress.org/ticket/64217