WordPress.org
Get WordPress

Make WordPress Core

Opened 15 years ago

Closed 10 years ago

Last modified 10 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 15 years ago.
Move notifications
14078.patch​ (636 bytes) - added by NickDuncan 10 years ago.
Updated patch as the last patch was done 4 years ago

Download all attachments as: .zip

Change History (10)

@mrmist
15 years ago

Move notifications

#1 @ocean90
15 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
15 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
12 years ago

  • Component changed from General to Comments

@NickDuncan
10 years ago

Updated patch as the last patch was done 4 years ago

#4 @NickDuncan
10 years ago

  • Severity changed from normal to minor

#5 @johnbillion
10 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
10 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
10 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
10 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.