From 87b81f220ce9f060460c5fa0ce7e5f30439fdccd Mon Sep 17 00:00:00 2001 From: Andrew Ozz Date: Tue, 1 May 2018 19:26:53 +0000 Subject: [PATCH] Privacy: improve `wp_privacy_erase_personal_data()`, return boolean values. Props ericdaams. See #43602. git-svn-id: https://develop.svn.wordpress.org/trunk@43061 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-admin/includes/ajax-actions.php | 20 ++++++------- src/wp-admin/js/xfn.js | 18 ++++++------ src/wp-includes/comment.php | 30 +++++++++++-------- tests/phpunit/tests/comment.php | 40 +++++++++++++------------- 4 files changed, 57 insertions(+), 51 deletions(-) diff --git a/src/wp-admin/includes/ajax-actions.php b/src/wp-admin/includes/ajax-actions.php index 3ac60c1eae..1a52e3e0fb 100644 --- a/src/wp-admin/includes/ajax-actions.php +++ b/src/wp-admin/includes/ajax-actions.php @@ -4564,8 +4564,8 @@ function wp_ajax_wp_privacy_erase_personal_data() { * Array of personal data exporters. * * @type string $callback Callable eraser that accepts an email address and - * a page and returns an array with the number of items - * removed, the number of items retained and any messages + * a page and returns an array with boolean values for + * whether items were removed or retained and any messages * from the eraser, as well as if additional pages are * available. * @type string $exporter_friendly_name Translated user facing friendly name for the eraser. @@ -4632,22 +4632,22 @@ function wp_ajax_wp_privacy_erase_personal_data() { ); } - if ( ! array_key_exists( 'num_items_removed', $response ) ) { + if ( ! array_key_exists( 'items_removed', $response ) ) { wp_send_json_error( sprintf( /* translators: %1$s: eraser friendly name, %2$d: array index */ - __( 'Expected num_items_removed key in response array from %1$s eraser (index %2$d).' ), + __( 'Expected items_removed key in response array from %1$s eraser (index %2$d).' ), esc_html( $eraser_friendly_name ), $eraser_index ) ); } - if ( ! array_key_exists( 'num_items_retained', $response ) ) { + if ( ! array_key_exists( 'items_retained', $response ) ) { wp_send_json_error( sprintf( /* translators: %1$s: eraser friendly name, %2$d: array index */ - __( 'Expected num_items_retained key in response array from %1$s eraser (index %2$d).' ), + __( 'Expected items_retained key in response array from %1$s eraser (index %2$d).' ), esc_html( $eraser_friendly_name ), $eraser_index ) @@ -4689,10 +4689,10 @@ function wp_ajax_wp_privacy_erase_personal_data() { } else { // No erasers, so we're done. $response = array( - 'num_items_removed' => 0, - 'num_items_retained' => 0, - 'messages' => array(), - 'done' => true, + 'items_removed' => false, + 'items_retained' => false, + 'messages' => array(), + 'done' => true, ); } diff --git a/src/wp-admin/js/xfn.js b/src/wp-admin/js/xfn.js index cd43d6e1e8..b0051b2c2f 100644 --- a/src/wp-admin/js/xfn.js +++ b/src/wp-admin/js/xfn.js @@ -141,8 +141,8 @@ jQuery( document ).ready( function( $ ) { var nonce = $action.data( 'nonce' ); var erasersCount = $action.data( 'erasers-count' ); - var removedCount = 0; - var retainedCount = 0; + var hasRemoved = false; + var hasRetained = false; var messages = []; $action.blur(); @@ -152,15 +152,15 @@ jQuery( document ).ready( function( $ ) { set_action_state( $action, 'remove_personal_data_idle' ); var summaryMessage = strings.noDataFound; var classes = 'notice-success'; - if ( 0 === removedCount ) { - if ( 0 === retainedCount ) { + if ( false === hasRemoved ) { + if ( false === hasRetained ) { summaryMessage = strings.noDataFound; } else { summaryMessage = strings.noneRemoved; classes = 'notice-warning'; } } else { - if ( 0 === retainedCount ) { + if ( false === hasRetained ) { summaryMessage = strings.foundAndRemoved; } else { summaryMessage = strings.someNotRemoved; @@ -192,11 +192,11 @@ jQuery( document ).ready( function( $ ) { return; } var responseData = response.data; - if ( responseData.num_items_removed ) { - removedCount += responseData.num_items_removed; + if ( responseData.items_removed ) { + hasRemoved = hasRemoved || responseData.items_removed; } - if ( responseData.num_items_retained ) { - retainedCount += responseData.num_items_removed; + if ( responseData.items_retained ) { + hasRetained = hasRetained || responseData.items_retained; } if ( responseData.messages ) { messages = messages.concat( responseData.messages ); diff --git a/src/wp-includes/comment.php b/src/wp-includes/comment.php index b9b714da28..884487d665 100644 --- a/src/wp-includes/comment.php +++ b/src/wp-includes/comment.php @@ -3412,17 +3412,18 @@ function wp_comments_personal_data_eraser( $email_address, $page = 1 ) { if ( empty( $email_address ) ) { return array( - 'num_items_removed' => 0, - 'num_items_retained' => 0, - 'messages' => array(), - 'done' => true, + 'items_removed' => false, + 'items_retained' => false, + 'messages' => array(), + 'done' => true, ); } // Limit us to 500 comments at a time to avoid timing out. - $number = 500; - $page = (int) $page; - $num_items_removed = 0; + $number = 500; + $page = (int) $page; + $items_removed = false; + $items_retained = false; $comments = get_comments( array( @@ -3469,6 +3470,8 @@ function wp_comments_personal_data_eraser( $email_address, $page = 1 ) { $messages[] = sprintf( __( 'Comment %d contains personal data but could not be anonymized.' ), $comment_id ); } + $items_retained = true; + continue; } @@ -3479,17 +3482,20 @@ function wp_comments_personal_data_eraser( $email_address, $page = 1 ) { $updated = $wpdb->update( $wpdb->comments, $anonymized_comment, $args ); if ( $updated ) { - $num_items_removed++; + $items_removed = true; clean_comment_cache( $comment_id ); + } else { + $items_retained = true; } } $done = count( $comments ) < $number; return array( - 'num_items_removed' => $num_items_removed, - 'num_items_retained' => count( $comments ) - $num_items_removed, - 'messages' => $messages, - 'done' => $done, + 'items_removed' => $items_removed, + 'items_retained' => $items_retained, + 'messages' => $messages, + 'done' => $done, ); } + diff --git a/tests/phpunit/tests/comment.php b/tests/phpunit/tests/comment.php index bb7ed60d39..7f63c62912 100644 --- a/tests/phpunit/tests/comment.php +++ b/tests/phpunit/tests/comment.php @@ -877,10 +877,10 @@ class Tests_Comment extends WP_UnitTestCase { $actual = wp_comments_personal_data_eraser( 'nocommentsfound@local.host' ); $expected = array( - 'num_items_removed' => 0, - 'num_items_retained' => 0, - 'messages' => array(), - 'done' => true, + 'items_removed' => false, + 'items_retained' => false, + 'messages' => array(), + 'done' => true, ); $this->assertSame( $expected, $actual ); @@ -908,10 +908,10 @@ class Tests_Comment extends WP_UnitTestCase { $actual = wp_comments_personal_data_eraser( $args['comment_author_email'] ); $expected = array( - 'num_items_removed' => 1, - 'num_items_retained' => 0, - 'messages' => array(), - 'done' => true, + 'items_removed' => true, + 'items_retained' => false, + 'messages' => array(), + 'done' => true, ); $this->assertSame( $expected, $actual ); @@ -939,10 +939,10 @@ class Tests_Comment extends WP_UnitTestCase { $actual = wp_comments_personal_data_eraser( $args['comment_author_email'], 2 ); $expected = array( - 'num_items_removed' => 0, - 'num_items_retained' => 0, - 'messages' => array(), - 'done' => true, + 'items_removed' => false, + 'items_retained' => false, + 'messages' => array(), + 'done' => true, ); $this->assertSame( $expected, $actual ); @@ -975,10 +975,10 @@ class Tests_Comment extends WP_UnitTestCase { $message = sprintf( 'Comment %d contains personal data but could not be anonymized.', $comment_id ); $expected = array( - 'num_items_removed' => 0, - 'num_items_retained' => 1, - 'messages' => array( $message ), - 'done' => true, + 'items_removed' => false, + 'items_retained' => true, + 'messages' => array( $message ), + 'done' => true, ); $this->assertSame( $expected, $actual ); @@ -1011,10 +1011,10 @@ class Tests_Comment extends WP_UnitTestCase { $message = sprintf( 'Some custom message for comment %d.', $comment_id ); $expected = array( - 'num_items_removed' => 0, - 'num_items_retained' => 1, - 'messages' => array( $message ), - 'done' => true, + 'items_removed' => false, + 'items_retained' => true, + 'messages' => array( $message ), + 'done' => true, ); $this->assertSame( $expected, $actual );