[wp-trac] [WordPress Trac] #36901: Removing wp_die() from wp_allow_comment()

[WordPress Trac]

#36901: Removing wp_die() from wp_allow_comment()
-------------------------------------------------+-------------------------
 Reporter:  websupporter                         |       Owner:
     Type:  enhancement                          |  websupporter
 Priority:  normal                               |      Status:  reviewing
Component:  Comments                             |   Milestone:  4.7
 Severity:  normal                               |     Version:  4.6
 Keywords:  has-patch needs-testing has-unit-    |  Resolution:
  tests needs-dev-note                           |     Focuses:
-------------------------------------------------+-------------------------

Comment (by boonebgorges):

 Very anecdotal, but I can't find a single reference on GitHub of
 `check_comment_flood_db()` being called directly:
 https://github.com/search?p=1&q=check_comment_flood_db&type=Code&utf8=%E2%9C%93
 I'll search a copy of the plugin repo when I'm near a computer that has a
 copy handy.

 That said, the trick in [attachment:36901.9.diff] seems pretty clever to
 me. Respecting `$allow_die` in `check_comment_flood_db()` doesn't seem
 totally necessary to me, though. How about something like this?

 {{{
 do_action(
     'check_comment_flood',
     $commentdata['comment_author_IP'],
     $commentdata['comment_author_email'],
     $commentdata['comment_date_gmt']
     $commentdata['comment_date_gmt'],
     $avoid_die,
     false
 );

 ...
 add_action( 'check_comment_flood',      'check_comment_flood_db',
 10, 5 );

 ...

 function check_comment_flood_db( $ip, $email, $date, $avoid_die,
 $check_flood_now = true ) {
         if ( $check_flood_now ) {
             // We run the comment flood check and die in case of a flood.
             wp_check_comment_flood( false, $ip, $email, $date, $avoid_die
 );
 ....

 }}}

 This'll make it possible to call `check_comment_flood_db()` as an alias
 for `wp_check_comment_flood()` even when `$allow_die` is *false*. I'm not
 sure there's a real use case for this (why not just call
 `wp_check_comment_flood()`?) but it seems a bit more organized to me.

 This is just a thought, though. I'm comfortable with either
 [attachment:36901.8.diff] or [attachment:36901.9.diff] as well.

 Rest of the patch reads OK to me. @rachelbaker when you're happy with the
 approach, you've got the go-ahead from me.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/36901#comment:34>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform