[wp-trac] [WordPress Trac] #761: Add hook to conditionally disable comment notifications [w/ patch]

WordPress Trac wp-trac at lists.automattic.com
Thu Sep 27 06:32:12 UTC 2012


#761: Add hook to conditionally disable comment notifications [w/ patch]
-------------------------------------+-----------------------
 Reporter:  coffee2code              |       Owner:  matt
     Type:  enhancement              |      Status:  reopened
 Priority:  normal                   |   Milestone:  3.5
Component:  Comments                 |     Version:  1.2.2
 Severity:  normal                   |  Resolution:
 Keywords:  has-patch needs-testing  |
-------------------------------------+-----------------------

Comment (by coffee2code):

 As @nacin [https://irclogs.wordpress.org/chanlog.php?channel=wordpress-
 dev&day=2012-09-26&sort=asc#m465647 pointed out in IRC], the problem with
 pushing the logic into a pluggable function is that any "plugged" versions
 of this function out there can't be guaranteed to have the proper checks
 in them. The existing checks in `wp_new_comment()` and
 `wp_set_comment_status()` may be currently preventing emails from being
 sent that would otherwise get sent if these custom-defined but non-
 checking versions of `wp_notify_postauthor()` are called. It's unfortunate
 that pluggable functions limit us in what we can implement.

 Therefore, [attachment:761.3.diff] is attached as a change-minimizing and
 more backwards compatible approach, though less centralized and de-
 duplicative of effort as previously put forth.

 Other than introducing the 'wp_notify_post_author' filter, this patch does
 not provide for any of the benefits I [comment:13 mentioned in my previous
 comment].

 However, in addition to the new filter, I carried over one related change
 from the previous patch:

 > Additionally, the patch does change `wp_set_comment_status()` such that
 the call to `wp_notify_postauthor()` happens after the `$wpdb->update()`.
 This ensures that the comment is in the approved state prior to
 `wp_notify_postauthor()` acting on it, and also prevents the notification
 from being triggered if the update fails for some reason (prior to this
 patch, the notification gets sent before the update is attempted).

 I still favor [attachment:761.2.diff], but [attachment:761.3.diff] is
 sufficient to satisfy this ticket.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/761#comment:14>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software


More information about the wp-trac mailing list