WordPress.org

Make WordPress Core

Opened 6 months ago

Last modified 4 weeks ago

#46303 assigned enhancement

Update wp_privacy_send_personal_data_export_email to provide the same filters as _wp_privacy_send_erasure_fulfillment_notification

Reported by: garrett-eclipse Owned by: garrett-eclipse
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch needs-testing needs-unit-tests
Focuses: Cc:

Description

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

Attachments (3)

46303.patch (1.1 KB) - added by thakkarhardik 6 months ago.
Filter added for the Export Complete email subject
46303.2.patch (1.1 KB) - added by thakkarhardik 6 months ago.
Here's the patch with updated root path
46303.3.diff (5.6 KB) - added by garrett-eclipse 5 months ago.
Updated patch to setup $email_data and include wp_privacy_personal_data_email_to and other updates

Download all attachments as: .zip

Change History (12)

@thakkarhardik
6 months ago

Filter added for the Export Complete email subject

#1 @thakkarhardik
6 months ago

  • Keywords has-patch added; needs-patch removed

#2 follow-up: @garrett-eclipse
6 months ago

  • Keywords needs-refresh added; good-first-bug has-patch removed

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

  1. 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

#3 @thakkarhardik
6 months ago

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

@thakkarhardik
6 months ago

Here's the patch with updated root path

#4 @thakkarhardik
6 months ago

  • Keywords has-patch added; needs-refresh removed

#5 in reply to: ↑ 2 @garrett-eclipse
6 months ago

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

garrett-eclipse:

  1. 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

#6 @garrett-eclipse
5 months ago

  • Summary changed from Create user_erasure_complete_email_subject to match user_erasure_complete_email_subject for privacy export emails to Create wp_privacy_personal_data_email_subject to match user_erasure_complete_email_subject for privacy export emails

#7 @birgire
5 months ago

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
5 months ago

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

#8 @garrett-eclipse
5 months ago

  • Component changed from Administration to Privacy
  • Focuses privacy removed
  • Keywords needs-testing needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.3
  • Owner set to garrett-eclipse
  • Status changed from new to assigned
  • Summary changed from Create wp_privacy_personal_data_email_subject to match user_erasure_complete_email_subject for privacy export emails to Update wp_privacy_send_personal_data_export_email to provide the same filters as _wp_privacy_send_erasure_fulfillment_notification

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.

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


4 weeks ago

Note: See TracTickets for help on using tickets.