[wp-trac] [WordPress Trac] #44721: The privacy data erase fulfillment email should be in the user's language

[WordPress Trac]

#44721: The privacy data erase fulfillment email should be in the user's language
-------------------------------------------------+-------------------------
 Reporter:  desrosj                              |       Owner:  desrosj
     Type:  defect (bug)                         |      Status:  assigned
 Priority:  normal                               |   Milestone:  5.2
Component:  I18N                                 |     Version:  4.9.6
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests needs-      |     Focuses:  privacy
  testing                                        |
-------------------------------------------------+-------------------------

Comment (by garrett-eclipse):

 Nice catch @birgire you're correct that the supported
 privacy_action_request_type for erasure is `remove_personal_data` as
 defined in `_wp_privacy_action_request_types` here;
 https://github.com/WordPress/wordpress-develop/blob/5.1.1/src/wp-
 includes/user.php#L2842-L2855

 It does look like any call to `wp_create_user_request` doesn't take into
 account this check allowing the any-action-name scenario. It would
 probably make the most sense to move the check from
 `_wp_personal_data_handle_actions` into `wp_create_user_request` so as to
 check all user_request creation calls. The check can be found here;
 https://github.com/WordPress/wordpress-develop/blob/5.1.1/src/wp-
 admin/includes/user.php#L691-L698
 *This is the only instance of checking against
 `_wp_privacy_action_request_types` and throwing an error. There is one
 other check but it's just in an if conditional in
 `_wp_privacy_account_request_confirmed_message`.

 I agree this deviates from the issue this ticket seeks to address so
 opened another here to investigate further;
 #46536
 *There are also two other pre-existing unit tests which were using the
 invalid request action name, to get my patch on the above ticket passing
 tests I had to address them. I made a note on that ticket that it'll need
 a refresh once this ticket is merged or vice versa.

 That being said I uploaded [44721.4.diff] to update @desrosj unit tests to
 account for @birgire recommendations.
 * As I addressed the `erase_personal_data` cases in tests on #46536 I
 focused on only fixing the ones directly related to @desrosj patch.
 * I also updated the email to `erase-user-not-registered at example.com` to
 match what's found in `test_should_send_fulfillment_email_in_site_locale`
 as @birgire noted.

 I also found the tests were failing initially and struggled until I
 realized that's why you supplied the .mo files seperately. It's
 unfortunate that svn doesn't support binary files as I tried to include
 them in the patch to avoid issues with unit tests. @desrosj will those .mo
 files be committed to trunk in time or how do we ensure they're submitted
 with the patch to avoid unit test failures.

 Also I had to adjust
 `test_should_send_fulfillment_email_in_user_locale_when_both_have_different_locales_than_site`
 in order to get past the following error;
 {{{
 1)
 Tests_Privacy_WpPrivacySendErasureFulfillmentNotification::test_should_send_fulfillment_email_in_user_locale_when_both_have_different_locales_than_site
 Failed asserting that '[Test Blog] Solicitud de borrado completada'
 contains "Erasure Request Fulfilled".
 /Users/garretthyder/WordPress/44721-privacy-data-erase-fullfullment-email-
 language/tests/phpunit/tests/privacy/wpPrivacySendErasureFulfillmentNotification.php:385
 }}}

 @desrosj can you take a look at this failing test, I feel it's due to
 en_US not being a locale of WP but the default that's causing issues here
 but am unsure. Here's the original failing test case;
 {{{
 /**
  * The function should respect the user locale settings when the site is
 not en_US and both the
  * administrator and the user use different locales.
  *
  * @ticket 44721
  * @group l10n
  */
 public function
 test_should_send_fulfillment_email_in_user_locale_when_both_have_different_locales_than_site()
 {
         update_option( 'WPLANG', 'es_ES' );
         switch_to_locale( 'es_ES' );

         update_user_meta( self::$admin_user->ID, 'locale', 'de_DE' );
         update_user_meta( self::$request_user->ID, 'locale', 'en_US' );

         wp_set_current_user( self::$admin_user->ID );

         _wp_privacy_send_erasure_fulfillment_notification(
 self::$request_id );
         $mailer = tests_retrieve_phpmailer_instance();

         $this->assertContains( 'Erasure Request Fulfilled',
 $mailer->get_sent()->subject );
 }
 }}}
 *Manual test of the same situation there's no issue and you receive the
 en_US version so seems to just be the test case but would love second eyes
 to be sure.

 Thanks @birgire for the ever-vigilant eyes.  Also @desrosj thank you for
 the initial unit tests. Mind giving it another once over and we can move
 it back into 'commit'.

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