WordPress.org
Get WordPress

Make WordPress Core

Opened 13 years ago

Closed 8 years ago

Last modified 8 years ago

#14078 closed enhancement (fixed)

Don't send notifications for comments too early

Reported by: mrmist's profile mrmist Owned by: boonebgorges's profile boonebgorges
Milestone: 4.4 Priority: normal
Severity: minor Version: 3.0
Component: Comments Keywords:
Focuses: Cc:

Description

Not sure if this classes as a bug or enhancement, will call it an enhancement for now.

In wp_set_comment_status the notifications are currently sent out before the work is done, and there is potential for the subsequent work to fail (due to database issue etc.)

Attached patch (attempts to) move the notification so it is only sent after the work is done.

Attachments (2)

send_notification_later.patch​ (743 bytes) - added by mrmist 13 years ago.
Move notifications
14078.patch​ (636 bytes) - added by NickDuncan 9 years ago.
Updated patch as the last patch was done 4 years ago

Download all attachments as: .zip

Change History (10)

@mrmist
13 years ago

Move notifications

#1 @ocean90
13 years ago

  • Milestone changed from Unassigned to Future Release

I think there is a typo in your patch, you have commented $comment out.

#2 @mrmist
13 years ago

Think that's ok.. $comment is already set by that stage. Not sure why I left the comment in... Rush job.

#3 @iseulde
10 years ago

  • Component changed from General to Comments

@NickDuncan
9 years ago

Updated patch as the last patch was done 4 years ago

#4 @NickDuncan
9 years ago

  • Severity changed from normal to minor

#5 @johnbillion
9 years ago

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

This patch changes the conditions under which the notification can be sent. Note the multiple possible values for the condition in the switch statement.

#6 @boonebgorges
8 years ago

  • Keywords needs-patch removed
  • Milestone changed from Future Release to 4.4
  • Owner set to boonebgorges
  • Status changed from new to assigned

We can do this with a hook. We have the technology.

#7 @boonebgorges
8 years ago

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

In 34537:

Send comment approval notification to post author via hook.

This is part of the project of #33587.

Moving this particular message to the
'wp_set_comment_status' action has the added bonus that the notification is
sent after the comment status has been updated in the database. Previously, a
database error could lead to a case where an approval notification is sent,
but the comment status change fails for some reason.

Props mrmist, NickDuncan.
Fixes #14078.

#8 @boonebgorges
8 years ago

In 34545:

Improve status checks when sending comment post author notification.

[34537] hooked wp_new_comment_notify_postauthor() to the
'wp_set_comment_status' when a comment had been approved. When performing
multiple actions on a comment in the same request (as happens in
Tests_Ajax_DeleteComment::test_ajax_trash_double_action(), and may happen
sometimes in real life too), and when one of those actions involves deleting
the comment, the $comment_ID passed to wp_new_comment_notify_postauthor()
can correspond to an already-deleted comment. The comment_status check should
account for this possibility.

See #14078.

Note: See TracTickets for help on using tickets.