Comments: Make wp_update_comment() return false instead of 0 for an invalid comment or post ID.

This addresses an inconsistency where 0 could mean one of the three scenarios:

* Invalid comment ID.
* Invalid comment post ID.
* No DB rows updated. This is not an error and should not be treated as one.

With this change, `wp_update_comment()` always returns either `false` or a `WP_Error` object on failure, depending on the value of the `$wp_error` parameter.

Follow-up to [48154], [48215], [48216], [48218], [48230].

Props dd32, jnylen0, enrico.sorcinelli.
Fixes #39732. See #38700, #39735.

git-svn-id: https://develop.svn.wordpress.org/trunk@48235 602fd350-edb4-49c9-b593-d223f7449a82
This commit is contained in:
Sergey Biryukov 2020-06-30 14:11:00 +00:00
parent 0445f4b95c
commit 90e840f9db
2 changed files with 39 additions and 6 deletions

View File

@ -2422,6 +2422,8 @@ function wp_set_comment_status( $comment_id, $comment_status, $wp_error = false
* @since 2.0.0 * @since 2.0.0
* @since 4.9.0 Add updating comment meta during comment update. * @since 4.9.0 Add updating comment meta during comment update.
* @since 5.5.0 The `$wp_error` parameter was added. * @since 5.5.0 The `$wp_error` parameter was added.
* @since 5.5.0 The return values for an invalid comment or post ID
* were changed to false instead of 0.
* *
* @global wpdb $wpdb WordPress database abstraction object. * @global wpdb $wpdb WordPress database abstraction object.
* *
@ -2439,7 +2441,7 @@ function wp_update_comment( $commentarr, $wp_error = false ) {
if ( $wp_error ) { if ( $wp_error ) {
return new WP_Error( 'invalid_comment_id', __( 'Invalid comment ID.' ) ); return new WP_Error( 'invalid_comment_id', __( 'Invalid comment ID.' ) );
} else { } else {
return 0; return false;
} }
} }
@ -2448,7 +2450,7 @@ function wp_update_comment( $commentarr, $wp_error = false ) {
if ( $wp_error ) { if ( $wp_error ) {
return new WP_Error( 'invalid_post_id', __( 'Invalid post ID.' ) ); return new WP_Error( 'invalid_post_id', __( 'Invalid post ID.' ) );
} else { } else {
return 0; return false;
} }
} }

View File

@ -43,7 +43,9 @@ class Tests_Comment extends WP_UnitTestCase {
'post_type' => 'post', 'post_type' => 'post',
) )
); );
$comments = self::factory()->comment->create_post_comments( $post->ID, 5 ); $comments = self::factory()->comment->create_post_comments( $post->ID, 5 );
$result = wp_update_comment( $result = wp_update_comment(
array( array(
'comment_ID' => $comments[0], 'comment_ID' => $comments[0],
@ -51,8 +53,10 @@ class Tests_Comment extends WP_UnitTestCase {
) )
); );
$this->assertEquals( 1, $result ); $this->assertEquals( 1, $result );
$comment = get_comment( $comments[0] ); $comment = get_comment( $comments[0] );
$this->assertEquals( $comments[1], $comment->comment_parent ); $this->assertEquals( $comments[1], $comment->comment_parent );
$result = wp_update_comment( $result = wp_update_comment(
array( array(
'comment_ID' => $comments[0], 'comment_ID' => $comments[0],
@ -60,12 +64,14 @@ class Tests_Comment extends WP_UnitTestCase {
) )
); );
$this->assertEquals( 0, $result ); $this->assertEquals( 0, $result );
$result = wp_update_comment( $result = wp_update_comment(
array( array(
'comment_ID' => $comments[0], 'comment_ID' => $comments[0],
'comment_post_ID' => $post2->ID, 'comment_post_ID' => $post2->ID,
) )
); );
$comment = get_comment( $comments[0] ); $comment = get_comment( $comments[0] );
$this->assertEquals( $post2->ID, $comment->comment_post_ID ); $this->assertEquals( $post2->ID, $comment->comment_post_ID );
} }
@ -92,6 +98,7 @@ class Tests_Comment extends WP_UnitTestCase {
*/ */
public function test_wp_update_comment_updates_comment_meta() { public function test_wp_update_comment_updates_comment_meta() {
$comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) ); $comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) );
wp_update_comment( wp_update_comment(
array( array(
'comment_ID' => $comment_id, 'comment_ID' => $comment_id,
@ -101,6 +108,7 @@ class Tests_Comment extends WP_UnitTestCase {
), ),
) )
); );
$this->assertEquals( 'fire', get_comment_meta( $comment_id, 'sauce', true ) ); $this->assertEquals( 'fire', get_comment_meta( $comment_id, 'sauce', true ) );
} }
@ -142,6 +150,29 @@ class Tests_Comment extends WP_UnitTestCase {
$this->assertEquals( $updated_comment_text, $comment->comment_content ); $this->assertEquals( $updated_comment_text, $comment->comment_content );
} }
/**
* @ticket 39732
*/
public function test_wp_update_comment_returns_false_for_invalid_comment_or_post_id() {
$comment_id = self::factory()->comment->create( array( 'comment_post_ID' => self::$post_id ) );
$update = wp_update_comment(
array(
'comment_ID' => -1,
'comment_post_ID' => self::$post_id,
)
);
$this->assertSame( false, $update );
$update = wp_update_comment(
array(
'comment_ID' => $comment_id,
'comment_post_ID' => -1,
)
);
$this->assertSame( false, $update );
}
/** /**
* @ticket 39732 * @ticket 39732
*/ */