WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#36302 closed enhancement (fixed)

wp_update_comment needs a filter

Reported by: frankiet Owned by: rachelbaker
Milestone: 4.7 Priority: normal
Severity: normal Version: 4.4.2
Component: Comments Keywords: has-patch dev-feedback 4.7-early
Focuses: Cc:

Description

wp_update_comment needs a filter
Prospective Update: Here's what the filters might look like:

<?php
/**
 *  To be able to add additional checks
 *  such as (empty comment_content, same content...etc)
 */
$commentdata = apply_filters( 'wp_update_comment', $commentdata );
if ( ! $commentdata || is_wp_error( $commentdata ) ) {
        $message = apply_filters( 
                'wp_update_comment_error',
                __('We apologize, but an error has occurred while processing your request'));
        do_action( 'wp_update_comment_error',$commentdata);
        if ( defined('DOING_AJAX') ){
                die($message );
        }
        wp_die( $message, 429 );
} 

WP 4.4.2 wp_update_comment

<?php

function wp_update_comment($commentarr) {
        global $wpdb;

        // First, get all of the original fields
        $comment = get_comment($commentarr['comment_ID'], ARRAY_A);
        if ( empty( $comment ) ) {
                return 0;
        }

        // Make sure that the comment post ID is valid (if specified).
        if ( ! empty( $commentarr['comment_post_ID'] ) && ! get_post( $commentarr['comment_post_ID'] ) ) {
                return 0;
        }
        
        /**
         *  To be able to add additional checks
         *  such as (empty comment_content, same content...etc)
         */
        $commentdata = apply_filters( 'wp_update_comment', $commentdata );
        if ( ! $commentdata || is_wp_error( $commentdata ) ) {
                $message = apply_filters( 
                        'wp_update_comment_error',
                        __('We apologize, but an error has occurred while processing your request'));
                do_action( 'wp_update_comment_error',$commentdata);
                if ( defined('DOING_AJAX') ){
                        die($message );
                }
                wp_die( $message, 429 );
        } 


        // Escape data pulled from DB.
        $comment = wp_slash($comment);

        $old_status = $comment['comment_approved'];

        // Merge old and new fields with new fields overwriting old ones.
        $commentarr = array_merge($comment, $commentarr);

        $commentarr = wp_filter_comment( $commentarr );

        // Now extract the merged array.
        $data = wp_unslash( $commentarr );

        /**
         * Filter the comment content before it is updated in the database.
         *
         * @since 1.5.0
         *
         * @param string $comment_content The comment data.
         */
        $data['comment_content'] = apply_filters( 'comment_save_pre', $data['comment_content'] );

        $data['comment_date_gmt'] = get_gmt_from_date( $data['comment_date'] );

        if ( ! isset( $data['comment_approved'] ) ) {
                $data['comment_approved'] = 1;
        } elseif ( 'hold' == $data['comment_approved'] ) {
                $data['comment_approved'] = 0;
        } elseif ( 'approve' == $data['comment_approved'] ) {
                $data['comment_approved'] = 1;
        }

        $comment_ID = $data['comment_ID'];
        $comment_post_ID = $data['comment_post_ID'];
        $keys = array( 'comment_post_ID', 'comment_content', 'comment_author', 'comment_author_email', 'comment_approved', 'comment_karma', 'comment_author_url', 'comment_date', 'comment_date_gmt', 'comment_type', 'comment_parent', 'user_id', 'comment_agent', 'comment_author_IP' );
        $data = wp_array_slice_assoc( $data, $keys );
        
        $rval = $wpdb->update( $wpdb->comments, $data, compact( 'comment_ID' ) );

        clean_comment_cache( $comment_ID );
        wp_update_comment_count( $comment_post_ID );
        /**
         * Fires immediately after a comment is updated in the database.
         *
         * The hook also fires immediately before comment status transition hooks are fired.
         *
         * @since 1.2.0
         *
         * @param int $comment_ID The comment ID.
         */
        do_action( 'edit_comment', $comment_ID );
        $comment = get_comment($comment_ID);
        wp_transition_comment_status($comment->comment_approved, $old_status, $comment);
        return $rval;
}

Attachments (3)

36302.diff (1.1 KB) - added by dshanske 3 years ago.
Initial Patch
36302.2.diff (1.1 KB) - added by DrewAPicture 3 years ago.
36302.3.diff (1.1 KB) - added by rachelbaker 3 years ago.
Corrected filter parameter types in filter docblock

Download all attachments as: .zip

Change History (28)

#1 @frankiet
3 years ago

sorry for $commentdata instead of $commentarr

#2 @dshanske
3 years ago

  • Keywords needs-patch added

What about replacing:


// Merge old and new fields with new fields overwriting old ones.
    $commentarr = array_merge($comment, $commentarr);

with:

    /**
     * Filter the comment before it is updated in the database.
     * By Default, merge the old and new fields with new overwriting old.
     *
     * @since x.x.x
     *
     * @param string $comment The old comment data
     * @param string $commentarr The new comment data
     */
    $commentarr = apply_filter( 'pre_edit_comment', array_merge($comment, $commentarr), $comment, $commentarr);

That would basically allow the new comment array to be edited.

This ticket was mentioned in Slack in #core by dshanske. View the logs.


3 years ago

This ticket was mentioned in Slack in #core by dshanske. View the logs.


3 years ago

#5 follow-up: @boonebgorges
3 years ago

Hi @frankiet - Welcome to WordPress Trac!

A filter somewhere in here seems like an OK idea, but I wonder whether you've chosen the best place for it. It seems to me that we would want the filter to appear *after* all pre-processing has taken place (eg, setting 'comment_approved' based on context), but before any database-related sanitization takes place (eg, wp_unslash()). See, for example, the way that the similar 'wp_insert_post_data' filter works in wp_insert_post().

Related - let's come up with a better filter name. 'wp_update_comment' sounds to me (given our naming conventions) like an action that would fire at a certain point during the comment update process. Something like 'wp_update_comment_data' seems more appropriate, and it parallels other filter names better.

Last edited 3 years ago by boonebgorges (previous) (diff)

#6 in reply to: ↑ 5 @dshanske
3 years ago

Would the location I suggested, at the array merge work better?

Replying to boonebgorges:

Hi @frankiet - Welcome to WordPress Trac!

A filter somewhere in here seems like an OK idea, but I wonder whether you've chosen the best place for it. It seems to me that we would want the filter to appear *after* all pre-processing has taken place (eg, setting 'comment_approved' based on context), but before any database-related sanitization takes place (eg, wp_unslash()). See, for example, the way that the similar 'wp_insert_post_data' filter works in wp_insert_post().

Related - let's come up with a better filter name. 'wp_update_comment' sounds to me (given our naming conventions) like an action that would fire at a certain point during the comment update process. Something like 'wp_update_comment_data' seems more appropriate, and it parallels other filter names better.

#7 follow-up: @boonebgorges
3 years ago

Would the location I suggested, at the array merge work better?

@dshanske It's better, but not ideal, because then it's impossible to override any of the 'comment_approved' logic, 'comment_date_gmt', the filtered $commentarr, or the results of 'comment_save_pre'. The best place for this kind of filter is right before the $rval line. But some juggling may need to be done with wp_slash() and wp_unslash(), to make sure that content (especially the 'comment_content') is unslashed properly before it gets into the DB; alternatively, you could write in the documentation that the data being passed to the filter is already unslashed.

As a side note, is there a reason why the individual filters applied in wp_filter_comment() won't work? Why are we introducing a single filter on the whole array, when each value is already filtered separately? What value is the 'filtered' flag that's set in wp_filter_comment() if we are applying an overriding filter elsewhere?

#8 in reply to: ↑ 7 @dshanske
3 years ago

That may be the best point...but it was more a companion to preprocess_comment. You can't add preprocess comment to edit_comment at this point, because of backcompat. It is specifically if you want to run something only on update, but not on new.

Replying to boonebgorges:

Would the location I suggested, at the array merge work better?

@dshanske It's better, but not ideal, because then it's impossible to override any of the 'comment_approved' logic, 'comment_date_gmt', the filtered $commentarr, or the results of 'comment_save_pre'. The best place for this kind of filter is right before the $rval line. But some juggling may need to be done with wp_slash() and wp_unslash(), to make sure that content (especially the 'comment_content') is unslashed properly before it gets into the DB; alternatively, you could write in the documentation that the data being passed to the filter is already unslashed.

As a side note, is there a reason why the individual filters applied in wp_filter_comment() won't work? Why are we introducing a single filter on the whole array, when each value is already filtered separately? What value is the 'filtered' flag that's set in wp_filter_comment() if we are applying an overriding filter elsewhere?

#9 @frankiet
3 years ago

Thank you all for the replies
What do you think of this version?

New

/**
 * Filter a comment's data before it is sanitized and updated into the database.
 *
 * @since x.x.x
 *
 * @param array $commentarr data.
 */
$commentarr = apply_filters( 'wp_update_comment_data', $commentarr);

// If an error occurred
if ( ! $commentarr || is_wp_error( $commentarr ) ) {
    $error_message = __('We apologize, but an error has occurred while processing your request');
    if( is_wp_error( $commentarr ) ){
        $error_message = $commentarr->get_error_message();
    }

     /**
     * Fires after an error occurs
     *
     * @since  x.x.x
     *
     * @param string $error_message
     * @param array $commentarr data.
     */
    do_action( 'wp_update_comment_data_error', $error_message, $commentarr);

    if ( defined('DOING_AJAX') ){
        die($error_message );
    }

    wp_die( $error_message, 429 );
}

New do_action( 'edit_comment',$comment_ID, $comment, $old_comment);

Full version:

<?php
/**
 * Updates an existing comment in the database.
 *
 * Filters the comment and makes sure certain fields are valid before updating.
 *
 * @since 2.0.0
 *
 * @global wpdb $wpdb WordPress database abstraction object.
 *
 * @param array $commentarr Contains information on the comment.
 * @return int Comment was updated if value is 1, or was not updated if value is 0.
 */
function wp_update_comment($commentarr) {
    global $wpdb;

    // First, get all of the original fields
    $comment = get_comment($commentarr['comment_ID'], ARRAY_A);
    
    $old_comment = $comment;

    if ( empty( $comment ) ) {
        return 0;
    }

    // Make sure that the comment post ID is valid (if specified).
    if ( ! empty( $commentarr['comment_post_ID'] ) && ! get_post( $commentarr['comment_post_ID'] ) ) {
        return 0;
    }

    /**
     * Filter a comment's data before it is sanitized and updated into the database.
     *
     * @since x.x.x
     *
     * @param array $commentarr data.
     */
    $commentarr = apply_filters( 'wp_update_comment_data', $commentarr);

    // If an error occurred
    if ( ! $commentarr || is_wp_error( $commentarr ) ) {
        $error_message = __('We apologize, but an error has occurred while processing your request');
        if( is_wp_error( $commentarr ) ){
            $error_message = $commentarr->get_error_message();
        }

         /**
         * Fires after an error occurs
         *
         * @since x.x.x
         *
         * @param string $error_message
         * @param array $commentarr data.
         */
        do_action( 'wp_update_comment_data_error', $error_message, $commentarr);

        if ( defined('DOING_AJAX') ){
            die($error_message );
        }

        wp_die( $error_message, 429 );
    } 

    // Escape data pulled from DB.
    $comment = wp_slash($comment);

    $old_status = $comment['comment_approved'];

    // Merge old and new fields with new fields overwriting old ones.
    $commentarr = array_merge($comment, $commentarr);

    $commentarr = wp_filter_comment( $commentarr );

    // Now extract the merged array.
    $data = wp_unslash( $commentarr );

    /**
     * Filter the comment content before it is updated in the database.
     *
     * @since 1.5.0
     *
     * @param string $comment_content The comment data.
     */
    $data['comment_content'] = apply_filters( 'comment_save_pre', $data['comment_content'] );

    $data['comment_date_gmt'] = get_gmt_from_date( $data['comment_date'] );

    if ( ! isset( $data['comment_approved'] ) ) {
        $data['comment_approved'] = 1;
    } elseif ( 'hold' == $data['comment_approved'] ) {
        $data['comment_approved'] = 0;
    } elseif ( 'approve' == $data['comment_approved'] ) {
        $data['comment_approved'] = 1;
    }

    $comment_ID = $data['comment_ID'];
    $comment_post_ID = $data['comment_post_ID'];
    $keys = array( 'comment_post_ID', 'comment_content', 'comment_author', 'comment_author_email', 'comment_approved', 'comment_karma', 'comment_author_url', 'comment_date', 'comment_date_gmt', 'comment_type', 'comment_parent', 'user_id', 'comment_agent', 'comment_author_IP' );
    $data = wp_array_slice_assoc( $data, $keys );
    $rval = $wpdb->update( $wpdb->comments, $data, compact( 'comment_ID' ) );

    clean_comment_cache( $comment_ID );
    wp_update_comment_count( $comment_post_ID );
    
    /**
     * Fires immediately after a comment is updated in the database.
     *
     * The hook also fires immediately before comment status transition hooks are fired.
     *
     * @since 1.2.0
     * @since x.x.x The `$comment` and `$old_comment` parameters were added.
     * @param int $comment_ID The comment ID.
     * @param object $comment The comment.
     * @param array $old_comment Old comment.
     */
    $comment = get_comment($comment_ID);
    do_action( 'edit_comment',$comment_ID, $comment, $old_comment); 
    wp_transition_comment_status($comment->comment_approved, $old_status, $comment);
    return $rval;
}

#10 follow-up: @rachelbaker
3 years ago

@frankiet The best way communicate the exact code changes you are proposing is to submit a patch: https://make.wordpress.org/core/handbook/tutorials/trac/submitting-a-patch/ Would you be able to do that?

#11 in reply to: ↑ 10 @frankiet
3 years ago

Replying to rachelbaker:

@frankiet The best way communicate the exact code changes you are proposing is to submit a patch: https://make.wordpress.org/core/handbook/tutorials/trac/submitting-a-patch/ Would you be able to do that?

Hi rachelbaker, thanks for your reply.
Sorry, I'm not very familiar with svn
Maybe someone else can do it for me?
Thank you, see you soon!

https://gist.github.com/frankiet/6b9d1ebb95877d87f944625fd7c78e15

#12 follow-up: @rachelbaker
3 years ago

  • Keywords reporter-feedback added

@frankiet Can you share your use-case? I am not against adding another filter in wp_update_comment, but it would be useful to understand why the existing filters in wp_filter_comment and comment_save_pre are not meeting your requirements.

#13 in reply to: ↑ 12 @frankiet
3 years ago

Replying to rachelbaker:

@frankiet Can you share your use-case? I am not against adding another filter in wp_update_comment, but it would be useful to understand why the existing filters in wp_filter_comment and comment_save_pre are not meeting your requirements.

Hi rachelbaker , i'm sorry for delayed response.
I need to:

  • Add more control over the data
  • Store a revision of the comment when a comment is updated.

...

  • comment_save_pre is only for comment_content (It does not include other data).
  • wp_filter_comment does not say if in fact I'm posting a new comment or updating an existing comment...

#14 @pfefferle
3 years ago

@rachelbaker a filter on the unvalidated/unverified (before wp_filter_comment) comment-array would enable changes based on temporary fields, like @dshanske proposed for the pingback receiver (#34141): remote_source_original and remote_source.

I'am working on a Webmention plugin and (in contrast to the pingback core functionality) it also supports updates, so it would be nice to have kind of the same possibilities like with preprocess_comment for updates.

@dshanske
3 years ago

Initial Patch

#15 @dshanske
3 years ago

  • Keywords has-patch dev-feedback added; needs-patch reporter-feedback removed

#16 @dshanske
3 years ago

Hopefully this will make everyone happy.

This ticket was mentioned in Slack in #core-comments by dshanske. View the logs.


3 years ago

#18 @rachelbaker
3 years ago

  • Keywords 4.7-early added

Looks like a candidate for 4.7

#19 @rachelbaker
3 years ago

  • Milestone changed from Awaiting Review to Future Release

This ticket was mentioned in Slack in #core by helen. View the logs.


3 years ago

#21 @helen
3 years ago

  • Milestone changed from Future Release to 4.7

Seems like this should be named wp_update_comment_data to be consistent with similar hooks (posts, terms) and needs a little docs formatting cleanup (I've pinged @DrewAPicture for a look), but otherwise looks good to me.

@DrewAPicture
3 years ago

#22 @DrewAPicture
3 years ago

36302.2.diff updates the hook name and adjusts the documentation, placing emphasis on processed vs raw data in the available parameters.

#23 @jorbin
3 years ago

@rachelbaker Is this still on your radar for 4.7?

@rachelbaker
3 years ago

Corrected filter parameter types in filter docblock

#24 @rachelbaker
3 years ago

36302.3.diff corrects the filter parameter types from 36302.2.diff.

#25 @rachelbaker
3 years ago

  • Owner set to rachelbaker
  • Resolution set to fixed
  • Status changed from new to closed

In 38674:

Comments: Introduce the wp_update_comment_data filter.

Filters the comment data immediately before it is updated in the database.

Props frankiet, dshanske, DrewAPicture.
Fixes #36302.

Note: See TracTickets for help on using tickets.