Feedback on the Proposed Design

[allendav]

First, thank you @dejliglama for pulling this together. I appreciate it is not easy to tackle such an audacious task as this. I have some concerns about this design that I'll do my best to communicate below, along with a proposed alternate design.

FWIW, to help inform my feedback, I wrote a simple plugin that handles sensitive user-identifying data here and then looked at your design to imagine how I would go about hooking it in.

I have included feedback here for both this repo and the GDPRWP-Plugin-Sample repo:

GDPRWP/GDPRWP-Plugin

• The intention is that this plugin is a feature plugin for eventual inclusion in core, correct? I am starting to get the impression this is not meant for core but for a product for Peytz & Co
• Assuming that is true, I recommend not prefixing things with Gdpr… but with something more generic, like WP_User_Privacy, since 1) it is intended for WordPress core and 2) there are other jurisdiction’s laws with similar requirements to GDPR that will benefit from standardized user data handling by plugins
• I’m having trouble following the encapsulation / data-flow in GDPRWP/GDPRWP-Plugin
• If I read it correctly, a single GdprToolbox is going to be passed to each plugin ala do_action('gdpr_set_userdata', new GdprToolbox( $email ) );
  • Doesn’t this however mean that plugins will clobber each others data in there as the action loop progresses for each hooked callback on that action? The GdprToolbox only seems to be able to support a single plugin and a single instance of GdprDataContainer
• In general, comments/PHPDoc on each method in each class would be very helpful.

GDPRWP/GDPRWP-Plugin-Sample

• I don’t think we are going to be able to use an action to retrieve all the personal data held by all the plugins on a site. Plugins that store a lot of data will easily cause a PHP timeout this way. A better design could be for plugins to register callbacks for core to call as part of something like a cron to assemble the data.
• Besides that scaling issue, passing a GdprToolbox instance to each plugin doesn’t follow any paradigms in WordPress today, does it? I would have expected instead something more along the way custom post types are registered and by having objects that process user data use that to expose callbacks to core, e.g. something like having (in core):

/**
 * Registers a processor of user data for the purpose of privacy management. Should be invoked through
 * the 'init' action.
 *
 * @param string $slug a unique identifier for the data processor, typically plugin slug (e.g. myplugin)
 * @param string $label a localized label describing the data processor (e.g. "My Plugin: User Order Data" )
 * @param Array $callbacks a set of callbacks that can be called to perform privacy operations
 * @return bool Returns false if the slug is already registered, true otherwise
 */
function register_user_data_processor( $slug, $label, $callbacks ) {
	// TODO
}

and then inside a plugin, that plugin’s user-data processing object(s) could register by something like this:

	protected function __construct() {
		add_action( 'init', array( $this, 'register_data_processing' ) );
	}

	/**
	 * Register this object's handling of personal data to aid in privacy compliance and management
	 */
	public function register_data_processing() {
		if ( ! function_exists( 'register_user_data_processor' ) ) {
			return;
		}

		register_user_data_processor(
			'wpuiph',
			__( 'WP User IP History', 'wpuiph' ),
			array(
				'get_user_data' => array( $this, 'get_ip_history_as_csv' ),
				'redact_user_data' => array( $this, 'clear_user_ip_history' ),
				'get_privacy_policy_section' => array( $this, 'get_privacy_policy_section' )
			)
	}

That way, after all the plugins have registered all their user-data processors in init, we could safely in a later hook do whatever we need to do, whether it is assembling user data for export, redacting user data from the database, or building a privacy policy using snippets provided by plugins.

I appreciate this is a radical departure from your proposed design. What do you think?

[jespervnielsen]

Hello @allendav

Assuming that is true, I recommend not prefixing things with Gdpr… but with something more generic, like WP_User_Privacy, since 1) it is intended for WordPress core and 2) there are other jurisdiction’s laws with similar requirements to GDPR that will benefit from standardized user data handling by plugins

This is a question on naming, and even throug naming is very important, this is an issue there can be taken care of later on, since a lot of people have an opinion about naming, and for our point of view, we focus on the functionality,

If I read it correctly, a single GdprToolbox is going to be passed to each plugin ala do_action('gdpr_set_userdata', new GdprToolbox( $email ) );
Doesn’t this however mean that plugins will clobber each others data in there as the action loop progresses for each hooked callback on that action? The GdprToolbox only seems to be able to support a single plugin and a single instance of GdprDataContainer

it is correct we send a $GdprToolbox object to the plugins developers. - when we use an action. A new instance of the toolbox will be created for each plugin.
When they use the provided functions such as set_field(), the set field() triggers an update to the GdprDataContainer. The container is a singleton, and the data will be stored in and array, containing the plugins key as array key. The container is the object we use, to collect all plugins data.
So in summary: each plugin get a instance of the toolbox, witch updates the container.
The container is the object we use to read all the plugin data.

In general, comments/PHPDoc on each method in each class would be very helpful.

The code will be commented, but for now, the focus is to make a code that works, (and smaller comments to note what the functions does) and then we will create better documentation.

GDPRWP/GDPRWP-Plugin-Sample

I don’t think we are going to be able to use an action to retrieve all the personal data held by all the plugins on a site. Plugins that store a lot of data will easily cause a PHP timeout this way. A better design could be for plugins to register callbacks for core to call as part of something like a cron to assemble the data.
We have listended and could follow the issue. Today we have adressed this issue, so instead of having seperate hooks, to collect seperate data, and functions, we now ask the plugin developers to assign callback functions. The example uses the "anonymize" function.
so if a plugin set a anonymize callback (or multiple) , they will be called using ajax, and each callback will get its own request. (for now the code is just plain php, but it is a @todo)

The plan is, that we have a settings page, where the data is presented for the admin, and the data is loaded using ajax. - The data of the fields, may also be processed using a callback (@todo)

Besides that scaling issue, passing a GdprToolbox instance to each plugin doesn’t follow any paradigms in WordPress today, does it? I would have expected instead something more along the way custom post types are registered and by having objects that process user data use that to expose callbacks to core, e.g. something like having (in core):
We have actually look in directions of WooCommerce, and CMB2, to do this.
Technically, we would like to follow the same methods as the cart object in WooCommerce, where other plugins can update the cart.

an example is the "woocommerce_checkout_create_order_line_item" action.
In this we get an array of objects, and more parameters.
when looping over the items, data is added using $item->add_meta_data( $meta['label'], $meta['value'], true );
Then the $item updates the corresponding data that needs to be updated.

I know it is not 100% the same, but i see many similarities.

That way, after all the plugins have registered all their user-data processors in init, we could safely in a later hook do whatever we need to do, whether it is assembling user data for export, redacting user data from the database, or building a privacy policy using snippets provided by plugins.

We have adapted some of this. - so instead of running on different hooks, we run on a init.

[dejliglama]

The intention is that this plugin is a feature plugin for eventual inclusion in core, correct? I am starting to get the impression this is not meant for core but for a product for Peytz & Co

The intention is to make an Open Source plugin available on the Repo. Hopefully, that will be implemented into core - in some capacity or other. It's NOT a product of Peytz & Co.

That said, we will, of course, be offering add-on plugins to add pro features when we see a need - but anybody can do that on the basis of this.