Update wp_privacy_send_personal_data_export_email to provide the same filters as _wp_privacy_send_erasure_fulfillment_notification

[garrett-eclipse]

Hello,

With the Erasure Complete/Fulfilllment email the subject is filtered using the user_erasure_complete_email_subject filter.
​https://github.com/WordPress/WordPress/blob/5.1/wp-includes/user.php#L3170-L3189

However with the Export Complete email the subject is unfiltered;
​https://github.com/WordPress/WordPress/blob/5.1/wp-admin/includes/file.php#L2250

We should be consistent and apply the same type of filter on the Export Complete email found in wp_privacy_send_personal_data_export_email.

Cheers

[thakkarhardik]

Filter added for the Export Complete email subject

[garrett-eclipse]

Thanks @thakkarhardik, greatly appreciate the patch, a good start here. I've got a few minor notes below for you.

1. Attempting to apply the patch I'm greeted with error, and have to manually correct the path;

   Running "patch:46303" (patch) task
   (Stripping trailing CRs from patch.)
   can't find file to patch at input line 5
   Perhaps you used the wrong -p or --strip option?
   The text leading up to this was:
   --------------------------
   |Index: includes/file.php 
   

This just means you created your patch from the wp-admin directory, please 'ensure that you are in the root directory created by your SVN checkout'.
Creating a Patch - ​https://make.wordpress.org/core/handbook/tutorials/working-with-patches/#creating-a-patch

2. Comparing this to the user_erasure_complete_email_subject filter I really like the position (prior to the email content setup, and the use of $email_data to provide the filter with the request, user_email, privacy_policy_url and other useful data.

Here's the snippet I'm speaking of - ​https://github.com/WordPress/WordPress/blob/5.1/wp-includes/user.php#L3156-L3189

Let me know if you have any questions and thanks again for the contribution.
Cheers

[thakkarhardik]

Hi @garrett-eclipse , thanks for pointing that out. Really appreciate that. Next time I will take care of it.

[thakkarhardik]

Here's the patch with updated root path

[garrett-eclipse]

Thanks for correcting the path @thakkarhardik. Please see below for my other feedback to take into account;

garrett-eclipse:

2. Comparing this to the user_erasure_complete_email_subject filter I really like the position (prior to the email content setup, and the use of $email_data to provide the filter with the request, user_email, privacy_policy_url and other useful data.

Here's the snippet I'm speaking of - ​https://github.com/WordPress/WordPress/blob/5.1/wp-includes/user.php#L3156-L3189

Let me know if you have any questions,
Cheers

[birgire]

Thanks for the patch @thakkarhardik

In addition to what @garrett-eclipse said, we will also need to add @since 5.x.y in the filter's doc block. See e.g. the other filter's doc block:

​https://github.com/WordPress/WordPress/blob/5.1/wp-includes/user.php#L3170-L3189

I also wonder about the site name, as it's also an available parameter to the other filter and used in the email subject.

[garrett-eclipse]

Updated patch to setup $email_data and include wp_privacy_personal_data_email_to and other updates

[garrett-eclipse]

Thanks for the refresh @thakkarhardik and the feedback @birgire while working through this and comparing with the _wp_privacy_send_erasure_fulfillment_notification function I found several inconsistencies and decided to address them in 46303.3.diff.

1. I've introduced the wp_privacy_personal_data_email_to filter to match user_erasure_fulfillment_email_to.
2. I've setup the $email_data array and used it in the new subject filter as well as the content filter.
3. Introducing the wp_privacy_personal_data_email_subject that @thakkarhardik worked on I added the $site_name and $email_data.

I've moved this to the 5.3.0 milestone and versioned as such. Please test when you have a chance and we'll work on unit tests.

[rconde]

Any info on when wp_privacy_personal_data_email_subject is going to be released?
Thanks

[garrett-eclipse]

Refreshed patch to apply cleanly in new privacy-tools.php, updated versions and CS issues. Introduced unit tests.

[garrett-eclipse]

Hi @rconde thanks for the ping, I'm hoping to land this in 5.3.0.

I've refreshed the patch in 46303.4.diff so it applies cleanly now that everything lives in privacy-tools.php. Updated version numbers and addressed some CS issues.

Also introduced unit tests;

• Tests_Privacy_WpPrivacySendPersonalDataExportEmail::test_email_address_of_recipient_should_be_filterable - Tests new wp_privacy_personal_data_email_to filter.
• Tests_Privacy_WpPrivacySendPersonalDataExportEmail::test_email_subject_should_be_filterable - Tests new wp_privacy_personal_data_email_subject filter.
• Tests_Privacy_WpPrivacySendPersonalDataExportEmail::test_email_content_should_be_filterable_using_email_data - Tests addition of $email_data to the wp_privacy_personal_data_email_content filter.
• These are all found in the wpPrivacySendPersonalDataExportEmail.php test suite.

@birgire would you mind reviewing my unit tests and review/test the patch in general.

@rconde / @thakkarhardik would appreciate any review/testing you're able to do on the patch.

Thank you
P.S. Marking with needs-dev-note as this patch introduces two new filters (wp_privacy_personal_data_email_to & wp_privacy_personal_data_email_subject) and adds $email_data to the wp_privacy_personal_data_email_content filter.

[pputzer]

Replying to garrett-eclipse:

Hi @rconde thanks for the ping, I'm hoping to land this in 5.3.0.

I've refreshed the patch in 46303.4.diff so it applies cleanly now that everything lives in privacy-tools.php. Updated version numbers and addressed some CS issues.

Patch looks good to me.

[garrett-eclipse]

Thanks @pputzer this still applied nicely to trunk with all unit tests passing, moving to commit.

[birgire]

I tested this successfully with a request from user1@example.com and then with a plugin:

<?php
Plugin Name: Test 46303

add_filter( 'wp_privacy_personal_data_email_to', function($user_email) {
	return 'user2@example.com';
} );

add_filter( 'wp_privacy_personal_data_email_subject',function($subject ) {
	return 'Test';
} );

Steps to test:

• A request is made for user1@example.com.

• A verification link is sent to user1@example.com.

• user1 clicks on verification link in email.

• Admin sends the export link (clicks the button).

• The export download link is sent to user2@example.com emailbox and not user1@example.com emailbox as expected.

The privacy test group runs successfully.

One should though use the email filter with great care, to avoid sending the data export link to wrong emails!

ps:

Just a minor regarding PHPdoc, maybe we should replace:

* @since 5.3.0 Introduced the $email_data array.

with

 * @since 5.3.0 Introduced the `$email_data` array.

for the changelog display on ​https://developer.wordpress.org/reference/

It took me few seconds though to notice in the test filter, that $site_url of $email_data are not value generated within the string as it's single quote but not double quote :-)

[garrett-eclipse]

Adjusted the phpdoc to codequote the $email_data variable.

[garrett-eclipse]

Thanks @birgire I appreciate the testing there. Looks good, I did make that tweak to the phpdoc as you suggested in 46303.5.diff

[SergeyBiryukov]

In 46265:

Privacy: Introduce wp_privacy_personal_data_email_to and wp_privacy_personal_data_email_subject filters.

Pass email data to the wp_privacy_personal_data_email_content filter.

Props garrett-eclipse, thakkarhardik, birgire.
Fixes #46303.

[desrosj]

As far as I can tell, this did not receive a developer note. Removing the needs-dev-note keyword.