WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#12774 closed enhancement (fixed)

Don't fire wp_notify_postauthor() when the author moderated the comment

Reported by: nacin Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch commit
Focuses: Cc:

Description

In wp_notify_postauthor(), there is this bit of code with an inline comment:

// The author moderated a comment on his own post
if ( $comment->user_id == $post->post_author )
    return false;

The comment doesn't match the code. The code actually prevents the e-mail whenever the author *made* a comment on their own post.

That said, the comment should have code for it. If the author moderates a comment on their own post (a comment not written by them, that is), the author should not get a comment notification. Thoughts?

Attachments (2)

dont_notify_author.patch (941 bytes) - added by mrmist 4 years ago.
Don't notify the author when he moderated the comment
12774.diff (2.0 KB) - added by nacin 4 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 scribu4 years ago

Makes sense.

Also, an admin shouldn't get a comment notification after moderating any comment. (I have a plugin that deals with this: Filter Email Notifications).

comment:2 nacin4 years ago

In some way related to that: #11277 #11210 #11297.

comment:3 follow-up: scribu4 years ago

Seems like a notification overhaul would be in order.

comment:4 in reply to: ↑ 3 nacin4 years ago

Replying to scribu:

Seems like a notification overhaul would be in order.

I'd identify the issue being the site admin email not being tied to a single admin/owner account, particularly for #11210 and #11297. Deciding whether to enforce that would be a big change.

mrmist4 years ago

Don't notify the author when he moderated the comment

comment:5 mrmist4 years ago

Attached my attempt at solving the original ticket's issue. (Assumes the moderator is the current user)

comment:6 mrmist4 years ago

  • Keywords has-patch needs-testing added

comment:7 nacin4 years ago

  • Keywords commit added; needs-testing removed
  • Milestone changed from Awaiting Triage to 3.1

comment:8 scribu4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

comment:9 nacin4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Didn't notice an issue with the logic in the patch until now:

( $user->user_id == $moderating_uid )

but:

$moderating_uid = (int) $user->id;

This should be:

$moderating_uid = $moderating_user->id;

Or even better, just get_current_user_id(), and we can clean this up a little in terms of standards.

nacin4 years ago

comment:10 nacin4 years ago

New patch. Can someone double-check my logic?

I've renamed a local variable or two, which makes it really easy to follow now.

comment:11 mrmist4 years ago

New patch seems sensible to me. Not sure how it relates to [16223]

comment:12 ryan4 years ago

Looks good.

comment:13 nacin4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [16304]) Don't notify the author when they posted or moderated the comment. fixes #12774.

Note: See TracTickets for help on using tickets.