[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