Notes unexpected do not notify post author (unlike comments) #10472

adamsilverstein · 2025-11-05T21:30:13Z

Fix an issue where new notes did not trigger a notification because they are submitted via the REST API. Ensures REST API submissions (for notes) trigger the post author notification. Leverage existing comment notification infrastructure.

Testing instructions

Create a post
Log in as a different user (notifications are only sent when a note is left by another user, not your own notes).
Leave a comment on the post
Check your email

Expected result

An email looks like this for a note "Leaving some feedback here." on a post entitled "Admin post":

Subject: [blogname] Note: "Admin post"

New note on your post "Admin post"
Author: Test User 002 (IP address: ::1, localhost)
Email: [email protected]
Note:
Leaving some feedback here.

You can see all notes on this post here:
https://wpdev.localhost/wp-admin/post.php?post=2167`

Trac ticket: https://core.trac.wordpress.org/ticket/64204

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

github-actions · 2025-11-05T21:30:22Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props adamsilverstein, mamaduka, peterwilsoncc, ellatrix, wildworks, mukesh27, desrosj, annezazu, jeffpaul.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

src/wp-includes/default-filters.php

github-actions · 2025-11-06T00:16:02Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

src/wp-includes/comment.php

t-hamano

Thanks for the PR!

I installed the WP Mail SMTP plugin in my local environment, configured it with my personal Gmail SMTP server settings, and tested whether emails were actually being received.

The biggest concern is that adding a note takes a significant amount of time before the email sending process is actually completed. In my local environment, it took several seconds from pressing the "Add note" button until the snackbar appeared:

2a451d15c104aaead49a39947dab4d9f.mp4

I think it might be necessary to display some kind of loading indicator on the screen, or perhaps process the email sending using a cron job.

Another issue is that emails are also sent when a ticket is resolved or reopened, but there are two problems with this.

It seems like I'm receiving multiple emails.
Presumably, a different email content is needed for the approval/reopening action, because the "Note:" section will always be empty.

src/wp-includes/pluggable.php

adamsilverstein · 2025-11-06T05:07:51Z

Maybe we can skip notifications when the note is empty?

Good point about the delay when submitting a note, I was testing without actually sending the emails. I'll try a cron approach and use your testing approach.

Note: requires cron be active for notifications to work

Co-authored-by: Aki Hamano <[email protected]>

adamsilverstein · 2025-11-06T05:48:12Z

Maybe we can skip notifications when the note is empty?

I did this for now in f089d47 - I wonder if we should send a notification for "Thread resolved" or "Thread reopened" events.

Good point about the delay when submitting a note, I was testing without actually sending the emails. I'll try a cron approach and use your testing approach.

I added a cron callback for the emails in cbfef88 - please give it another test @t-hamano

src/wp-includes/comment.php

Mamaduka · 2025-11-06T13:27:56Z

The biggest concern is that adding a note takes a significant amount of time before the email sending process is actually completed. In my local environment, it took several seconds from pressing the "Add note" button until the snackbar appeared:

@t-hamano, do you see performance difference with normal comments, when their notifications are sent? It might be an SMTP config that's adding those extra time.

Usually, large installations will queue similar jobs in background and avoid adding overhead to the actual events. I think 10up popularized one of those libraries, back in a days :)

P.S. I don't think core should tread wp_new_comment_via_rest_notify_postauthor differently from wp_new_comment_notify_postauthor.

adamsilverstein · 2025-11-06T13:51:30Z

I am fine reverting the cron approach, I was mostly exploring the idea. I also agree we shouldn't treat the code path differently than comments. Maybe this means comments should also use a cron approach, but that is a discussion for another day.

t-hamano · 2025-11-06T13:58:53Z

do you see performance difference with normal comments, when their notifications are sent?

There is no difference in performance. When sending normal comments, the browser loading time may be similarly long.

However, the editor is reactive, and it feels very unnatural to me that the submitted note seems to disappear or that the snackbar appears with a delay. If we are sending emails in a conventional way, I would at least like to have some kind of loading indicator on the client side.

adamsilverstein · 2025-11-06T14:10:36Z

do you see performance difference with normal comments, when their notifications are sent?

There is no difference in performance. When sending normal comments, the browser loading time may be similarly long.

However, the editor is reactive, and it feels very unnatural to me that the submitted note seems to disappear or that the snackbar appears with a delay. If we are sending emails in a conventional way, I would at least like to have some kind of loading indicator on the client side.

I believe this also happens without the email if you are on a slow connection or have a very busy server - saving the note itself could take some time and currently there is no indicator of the process happening in the background. So to me this feels like an existing issue that is made more obvious by introducing a further processing delay (sending an email).

This reverts commit cbfef88.

jeffpaul · 2025-11-06T20:17:49Z

@desrosj note that the additional notification handling you've described is something I have stashed as a draft GitHub issue for 7.0, I'll make sure to tag you once I get that opened (frankly Monday earliest as I'm currently still sitting on the tarmac for last hour with travels this week 🥴 )

adamsilverstein · 2025-11-06T20:56:47Z

Could you explain this @adamsilverstein ? What is a scenario where a Note would be empty? Would that be when someone clicks Resolve and does not enter a comment? Technically the note is not empty, it's a status resolution.

Right - exactly. When a note is resolved, a comment is stored along with some comment meta, but the comment content is empty. You can also re-open a thread without leaving a comment (from the ... menu). Should we send a notification then that says "A Thread was resolved/reopened"?

I added this in 887974e

jeffpaul · 2025-11-07T04:38:12Z

In quick testing, the additional setting for Notes emails in the Settings > Discussions menu appears as expected. Unfortunately I'm having trouble with Mailpit so I'm unable to validate the emails are firing as expected and have the format as expected.

peterwilsoncc

I've a added a few comments inline, nothing major.

In the message emailed to authors, I'm wondering if we should refer to it as an internal note or similar to better distinguish between a comment as someone scans their email. Naming things is hard so it's not a blocker and we can re-evaluate that at any time in the future without much work.

src/wp-includes/comment.php

src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php

src/wp-admin/includes/schema.php

peterwilsoncc · 2025-11-07T06:08:08Z

src/wp-includes/pluggable.php

+					/* 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";


Is it possible to determine whether the status is resolved or reopened when empty? It's a little vague as is.

I think that should be doable based on comment meta or derived based on parent note status.

Meta is not yet saved at this hook, not sure the parent is helpful but I'll take a look.

Mamaduka · 2025-11-07T07:00:01Z

src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php

+	 *
+	 * @param WP_Comment $comment The comment object.
+	 */
+	public static function wp_new_comment_via_rest_notify_postauthor( $comment ) {


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?

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.

Why doesn't the comment_post action execute when comments are posted through the REST API?

Let’s move it before RC so we don’t introduce accidental public function that we have to deprecate in next release.

Why doesn't the comment_post action execute when comments are posted through the REST API?

@ellatrix comment_post is triggered in wp_new_comment, but the REST controller never calls that - it calls wp_insert_comment directly. This triggers wp_insert_comment so we could hook there, it also fires rest_insert_comment slightly later which I decided was appropriate since I'm explicitly targeting the REST insertion with this change.

Let’s move it before RC so we don’t introduce accidental public function that we have to deprecate in next release.

@Mamaduka I'm working on a follow up including moving the function here: #10488

src/wp-includes/pluggable.php

t-hamano · 2025-11-07T09:04:21Z

I confirmed that emails are only received when the ” Anyone posts a note” setting is enabled:

New note on your post "Hello world!"
Author: editor (IP address: 172.18.0.1, 172.18.0.1)
Email: [email protected]
Note: 
This is a test note.

You can see all notes on this post here:
http://localhost:8889/wp-admin/post.php?post=1&action=edit

ellatrix · 2025-11-07T12:53:54Z

How are we doing here? Code freeze for Beta 4 is in about an hour. Is this ready to go?

Co-authored-by: Peter Wilson <[email protected]>

adamsilverstein · 2025-11-07T13:48:21Z

How are we doing here? Code freeze for Beta 4 is in about an hour. Is this ready to go?

I was waiting for feedback, I think we could commit this now and refine the few pieces of feedback later.

t-hamano

I tested it again, and it's working as expected. This PR seems essential for Beta4, so let's commit it, and let's address any remaining feedback by RC1 if necessary.

Mamaduka · 2025-11-07T13:58:45Z

src/wp-includes/rest-api/endpoints/class-wp-rest-comments-controller.php

+	 *
+	 * @param WP_Comment $comment The comment object.
+	 */
+	public static function wp_new_comment_via_rest_notify_postauthor( $comment ) {


Let’s move it before RC so we don’t introduce accidental public function that we have to deprecate in next release.

ellatrix · 2025-11-07T13:58:53Z

src/wp-includes/comment.php

+	// Send notifications for approved comments and all notes.
+	if (
+		! isset( $comment->comment_approved ) ||
+		( '1' !== $comment->comment_approved && ! $is_note ) ) {


In the future, it could be nice to have filters for this instead of hardcoded behavior for notes

👍🏼 Yea, its a bit odd to have this logic here. There is a filter just above notify_post_author that 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.

@ellatrix maybe something like this: 1f16c40

I can open a separate Trac ticket for this.

https://core.trac.wordpress.org/ticket/64217

Fix an issue where new notes did not trigger a notification because they are submitted via the REST API. Ensures REST API submissions (for notes) trigger the post author notification. Leverage existing comment notification infrastructure. Developed in #10472. Fixes #64204. Props adamsilverstein, mamaduka, peterwilsoncc, ellatrix, wildworks, mukesh27, desrosj, annezazu, jeffpaul. git-svn-id: https://develop.svn.wordpress.org/trunk@61179 602fd350-edb4-49c9-b593-d223f7449a82

github-actions · 2025-11-07T14:06:15Z

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 61179
GitHub commit: 6e2130c

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.

Fix an issue where new notes did not trigger a notification because they are submitted via the REST API. Ensures REST API submissions (for notes) trigger the post author notification. Leverage existing comment notification infrastructure. Developed in WordPress/wordpress-develop#10472. Fixes #64204. Props adamsilverstein, mamaduka, peterwilsoncc, ellatrix, wildworks, mukesh27, desrosj, annezazu, jeffpaul. Built from https://develop.svn.wordpress.org/trunk@61179 git-svn-id: http://core.svn.wordpress.org/trunk@60515 1a063a9b-81f0-0310-95a4-ce76da25c4cd

Fix an issue where new notes did not trigger a notification because they are submitted via the REST API. Ensures REST API submissions (for notes) trigger the post author notification. Leverage existing comment notification infrastructure. Developed in WordPress/wordpress-develop#10472. Fixes #64204. Props adamsilverstein, mamaduka, peterwilsoncc, ellatrix, wildworks, mukesh27, desrosj, annezazu, jeffpaul. Built from https://develop.svn.wordpress.org/trunk@61179 git-svn-id: https://core.svn.wordpress.org/trunk@60515 1a063a9b-81f0-0310-95a4-ce76da25c4cd

Send a notification to the post author when a note is added

8ec209c

phpcbf

6799a1d

adamsilverstein added the enhancement label Nov 5, 2025

adamsilverstein requested review from Mamaduka and desrosj November 5, 2025 21:35

Mamaduka reviewed Nov 5, 2025

View reviewed changes

src/wp-includes/default-filters.php Outdated Show resolved Hide resolved

adamsilverstein added 3 commits November 5, 2025 16:47

remove notes conditional

6faacfe

Only notify on notes REST insert

b3487af

update docblock

0daba4c

ellatrix reviewed Nov 6, 2025

View reviewed changes

src/wp-includes/comment.php Outdated Show resolved Hide resolved

t-hamano reviewed Nov 6, 2025

View reviewed changes

src/wp-includes/pluggable.php Outdated Show resolved Hide resolved

src/wp-includes/pluggable.php Outdated Show resolved Hide resolved

adamsilverstein and others added 3 commits November 5, 2025 22:40

Queue note notifications with cron

cbfef88

Note: requires cron be active for notifications to work

Skip notifications for empty notes

f089d47

Update src/wp-includes/pluggable.php

79fd7e5

Co-authored-by: Aki Hamano <[email protected]>

adamsilverstein added 2 commits November 5, 2025 22:50

Whitespace

2aa7c96

adjust since

f9d4686

mukeshpanchal27 reviewed Nov 6, 2025

View reviewed changes

src/wp-includes/comment.php Outdated Show resolved Hide resolved

Revert "Queue note notifications with cron"

79aa49d

This reverts commit cbfef88.

adamsilverstein changed the title ~~Send a notification to the post author when a note is added~~ Notes unexpected do not notify post author (unlike comments) Nov 6, 2025

adamsilverstein requested review from Mamaduka, ellatrix and mukeshpanchal27 November 6, 2025 15:37

adamsilverstein added 3 commits November 6, 2025 13:39

Move notification hook so it is always available when action triggered

099de2f

Empty notes are resolution/reopen events

887974e

Fix notification logic

62ed6c3

adamsilverstein added 3 commits November 6, 2025 13:58

phpcbf

25f0b56

Check comment object returned before using

f3bee16

Merge branch 'trunk' into add/notes-notifications

0f6fff0

peterwilsoncc reviewed Nov 7, 2025

View reviewed changes

Mamaduka reviewed Nov 7, 2025

View reviewed changes

mukeshpanchal27 reviewed Nov 7, 2025

View reviewed changes

src/wp-includes/pluggable.php Show resolved Hide resolved

adamsilverstein and others added 2 commits November 7, 2025 06:23

Update comment.php

3b168e3

Co-authored-by: Peter Wilson <[email protected]>

Update class-wp-rest-comments-controller.php

6fd6294

Co-authored-by: Peter Wilson <[email protected]>

t-hamano mentioned this pull request Nov 7, 2025

Notes: Send email notifications to post author WordPress/gutenberg#73030

Closed

6 tasks

adamsilverstein added 3 commits November 7, 2025 06:40

PHP not react

93b9bd1

rename notes_notify -> wp_ notes_notify

7a1ed8e

Move wp_ notes_notify to bottom and add comment

2f4c927

t-hamano approved these changes Nov 7, 2025

View reviewed changes

Mamaduka approved these changes Nov 7, 2025

View reviewed changes

ellatrix reviewed Nov 7, 2025

View reviewed changes

github-actions bot closed this Nov 7, 2025

adamsilverstein mentioned this pull request Nov 7, 2025

Improve notes notifications implementation #10488

Open

Notes unexpected do not notify post author (unlike comments) #10472

Notes unexpected do not notify post author (unlike comments) #10472

Uh oh!

Conversation

adamsilverstein commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing instructions

Expected result

Uh oh!

github-actions bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 6, 2025

Test using WordPress Playground

Some things to be aware of

Uh oh!

Uh oh!

t-hamano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adamsilverstein commented Nov 6, 2025

Uh oh!

adamsilverstein commented Nov 6, 2025

Uh oh!

Uh oh!

Mamaduka commented Nov 6, 2025

Uh oh!

adamsilverstein commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

t-hamano commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamsilverstein commented Nov 6, 2025

Uh oh!

jeffpaul commented Nov 6, 2025

Uh oh!

adamsilverstein commented Nov 6, 2025

Uh oh!

jeffpaul commented Nov 7, 2025

Uh oh!

peterwilsoncc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsilverstein Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamsilverstein Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

t-hamano commented Nov 7, 2025

Uh oh!

ellatrix commented Nov 7, 2025

Uh oh!

adamsilverstein commented Nov 7, 2025

Uh oh!

t-hamano left a comment

adamsilverstein commented Nov 5, 2025 •

edited

Loading

github-actions bot commented Nov 5, 2025 •

edited

Loading

adamsilverstein commented Nov 6, 2025 •

edited

Loading

t-hamano commented Nov 6, 2025 •

edited

Loading

adamsilverstein Nov 7, 2025 •

edited

Loading

adamsilverstein Nov 7, 2025 •

edited

Loading