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

[WordPress Trac]

#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):

 I actually really like deferring all the post author notification checks
 until `wp_notify_postauthor()`. I pursued that approach in
 [attachment:761.2.diff].

 The main benefit is that it centralizes all post author notification
 logic, which:

 *  Removes code duplication currently found in `wp_new_comment()` and
 `wp_set_comment_status()` (the only two core functions to call
 `wp_notify_postauthor()`; both functions perform similar checks prior to
 calling `wp_notify_postauthor()`)
 * Removes code/effort duplication with certain checks that occur in
 `wp_new_comment()`/`wp_set_comment_status()` that are repeated in
 `wp_notify_postauthor()`
 * Fixes inconsistency between `wp_new_comment()` and
 `wp_set_comment_status()` (as [comment:10 pointed out by @scribu], the
 latter didn't include a check to compare the post author with the
 commenter to prevent self-notifications, which turns out to be unnecessary
 because `wp_notify_postauthor()` performs that check anyhow)
 * Brings `wp_notify_postauthor()` into closer alignment with
 `wp_notify_moderator()` which checks its notification setting value
 (`get_option( 'moderation_notify' )`) itself.

 And of course, as per the original ticket, the patch introduces a
 'wp_notify_postauthor' filter to allow overriding the
 `get_option('comments_notify')` and comment_approved checks for any
 `$comment`.

 The only risk I see for breaking existing compatibility would be if
 someone was calling `wp_notify_postauthor()` directly and not expecting
 `get_option('comments_notify')` and comment_approved checks to be
 performed. They can bypass those using the newly introduced
 'wp_notify_postauthor' hook and do a `__return_true`.

 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()` acts on the comment, and also prevents the
 notification from being triggered if the update fails for some reason.

 Tangentially, the `$comment_type` argument to `wp_notify_postauthor()` is
 superfluous, but I'll open a separate ticket to address that.


 Another thought: remove the `wp_notify_postauthor()` call from both
 `wp_new_comment()` and `wp_set_comment_status()` and instead hook
 `wp_notify_postauthor()` to the 'comment_post' and 'wp_set_comment_status'
 (or 'transition_comment_status') actions respectively.

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