Skip to content

Use comment status and type to set $maybe_notify default #10489

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Use comment status and type to set $maybe_notify default #10489

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4d99d9
Use comment status and type to set $maybe_notify default
adamsilverstein Nov 7, 2025
bbb6e05
Update filter doc block
adamsilverstein Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions src/wp-includes/comment.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2429,9 +2429,17 @@ function wp_new_comment_notify_postauthor( $comment_id ) {

$maybe_notify = $is_note ? get_option( 'wp_notes_notify' ) : get_option( 'comments_notify' );

// By default, only notify for approved comments and notes.
if (
! isset( $comment->comment_approved ) ||
( '1' !== $comment->comment_approved && ! $is_note ) ) {
$maybe_notify = false;
}
Comment on lines 2430 to +2437
Copy link
Member

@westonruter westonruter Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this refactor into a single if/elseif/else statement is easier to reason about?

Suggested change
$maybe_notify = $is_note ? get_option( 'wp_notes_notify' ) : get_option( 'comments_notify' );
// By default, only notify for approved comments and notes.
if (
! isset( $comment->comment_approved ) ||
( '1' !== $comment->comment_approved && ! $is_note ) ) {
$maybe_notify = false;
}
// By default, only notify for approved comments and notes.
if (
! isset( $comment->comment_approved ) ||
( '1' !== $comment->comment_approved && ! $is_note )
) {
$maybe_notify = false;
} else if ( $is_note ) {
$maybe_notify = get_option( 'wp_notes_notify' );
} else {
$maybe_notify = get_option( 'comments_notify' )
}

Copy link
Member

@westonruter westonruter Nov 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside: Will $comment->comment_approved ever be not-set or null? It's default value is '1' and it is typed as a string. Doesn't hurt, but just curious. If so, then ! isset( $comment->comment_approved ) could be removed.

Copy link
Member Author

@adamsilverstein adamsilverstein Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will change the logic slightly. If get_option( 'comments_notify' ) is true and '1' !== $comment->comment_approved and the type is a comment, the default should be false. This would make it true. I will try to make the logic clearer, maybe remove the trinary or add inline docs.


/**
* Filters whether to send the post author new comment notification emails,
* overriding the site setting.
* Filters whether to send the post author new comment and note notification emails,
* overriding the site settings and defaults. By default, notifications are sent for
* all notes and for approved comments.
*
* @since 4.4.0
*
Expand All @@ -2448,13 +2456,6 @@ function wp_new_comment_notify_postauthor( $comment_id ) {
return false;
}

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

return wp_notify_postauthor( $comment_id );
}

Expand Down
Loading