Opened 6 years ago
Closed 6 years ago
#7826 closed enhancement (fixed)
Privacy: "Settings > Data" page
Reported by: | r-a-y | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 4.0 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Settings | Keywords: | has-patch 2nd-opinion |
Cc: |
Description
Parent ticket: #7698
Specifically, see https://buddypress.trac.wordpress.org/ticket/7698#comment:12.
I've tested how WordPress does the data export and this functionality is restricted to the site admin. Meaning only the site admin can initialize a data export or erase for a user under the "Tools" admin dashboard page.
For our purposes, would we we want to let users manage their own data requests on a new BP frontend page like "Settings > Data"?
Attachments (7)
Change History (21)
#2
@
6 years ago
I haven't looked at the patch, but I like the idea very much. It'll be a huge feature improvement!
This ticket was mentioned in Slack in #buddypress by boone. View the logs.
6 years ago
This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.
6 years ago
#6
@
6 years ago
Thanks for working on this, @r-a-y !
7826.03.patch changes some @since
annotations and does a bit of spelling/coding standards cleanup.
A couple thoughts/questions:
- I think that "data erase" is probably functionally equivalent to "account deletion", at least if it's meant to be a *bulk* data erasure. Perhaps we could add to the Delete Account page some language to the effect that deleting your account will also trigger the deletion of all your identifiable data, and perhaps the "Erase Data" section on this Data panel could just point to the account deletion page. When user account deletion is disabled, perhaps this section should have a note about contacting the site administrator? In any case, I don't think we need a separate action for it.
- I'd wager that, in many communities, the administrator will not want to have to approve these requests. What do you think of having a filter that allows them to be auto-approved? (It's not totally clear how hard this would be technically.)
#7
@
6 years ago
Thanks for your feedback, @boonebgorges.
I think that "data erase" is probably functionally equivalent to "account deletion", at least if it's meant to be a *bulk* data erasure.
You're probably right that WP's "data erase" is the same as our "account deletion". I haven't actually taken a look at what WP is doing for data erasing, but I wanted to compare what they did.
I haven't looked, but do we notify the site admin if a user deletes an account in BuddyPress? If not, maybe this is something we should do?
Perhaps we could add to the Delete Account page some language to the effect that deleting your account will also trigger the deletion of all your identifiable data, and perhaps the "Erase Data" section on this Data panel could just point to the account deletion page.
.
When user account deletion is disabled, perhaps this section should have a note about contacting the site administrator? In any case, I don't think we need a separate action for it.
Good idea. We should definitely add both of these notes on our current "Delete Account" page.
I'd wager that, in many communities, the administrator will not want to have to approve these requests. What do you think of having a filter that allows them to be auto-approved? (It's not totally clear how hard this would be technically.)
I wanted to keep this functionality the same so the administrator would be aware and is able to know how many data export requests are received. If this is all done transparently, the admin would have no idea.
#8
@
6 years ago
I haven't looked, but do we notify the site admin if a user deletes an account in BuddyPress? If not, maybe this is something we should do?
I don't think we do, but I agree it's a good idea. Could probably use its own ticket.
Good idea. We should definitely add both of these notes on our current "Delete Account" page.
I stared at the 'Delete Account' messaging for a while and couldn't come up with a good way to change it to reflect this focus (ie, the focus on data erasure vs account deletion). However, in 7826.04.patch I added the Data Erase section to the Data Management panel, which is sensitive to whether the admin allows self-deletions. See screenshots.
I wanted to keep this functionality the same so the administrator would be aware and is able to know how many data export requests are received. If this is all done transparently, the admin would have no idea.
I assume that by "the same" you mean "the same as WordPress". I guess this is fair, certainly for the default behavior. That said, BP sites are different from typical WP sites, and I guarantee that many admins would want these to be auto-confirmed. 7826.04.patch contains a hook at the end of the request process that would allow a plugin to do this. Something like:
add_action( 'bp_user_data_export_requested', function( $request_id, $success ) { if ( ! $success ) { return; } // Do something to trigger the creation of the export. Not sure what :) $message = __( 'Your export is being generated. You will receive an email when it\'s ready to download.', 'my-plugin' ); bp_core_add_message( $message ); bp_core_redirect( bp_get_requested_uri() ); }, 10, 2 );
There might come a day when WP itself offers the ability to set an auto-generate flag for these requests, at which point we could just inherit it. Until then, I think the hook is a low-overhead way to enable plugins to do it themselves. What do you say to this compromise?
#9
@
6 years ago
- Keywords dev-feedback added
Hi @r-a-y - If you've got a minute, would you mind having a look at my comment above and the screenshots, and let me know whether you think we have a good enough compromise here for 4.0?
#10
@
6 years ago
@boonebgorges - The 'bp_user_data_export_requested'
hook sounds sensible to me and the new "Data Erase" section explains things well enough.
I guess I don't like the separation between the "Data Management" and "Delete Account" pages. Is "Data Management" a good title for the new page? The user isn't really managing any data. Perhaps "Export Data" might be better?
We should probably create a new ticket for whether an admin should be notified if a user deletes their own account. I think that would be a useful data point for sites running BuddyPress.
#11
@
6 years ago
- Keywords dev-feedback removed
Thanks very much, @r-a-y. Agreed about "Data Management" - "Export Data" sounds more straightforward. I'll go with that.
I've opened #7992 to track your idea about account deletion.
#12
@
6 years ago
- Keywords 2nd-opinion added
7826.05.patch is an update with the following changes:
- Use "Export Data" instead of "Data Management" on tab name
- Move new functions into bp-settings-functions.php rather than the /screens/ file
- Remove the modifications to 'bp_settings_data_exporter_name'. I get the motivation for this - as you note, the "friendly names" are intended to be administrator-facing. But I think it's hopeless to try to parse these labels and make them friendly for regular users. For one thing, there's no way we can do it reliably across languages. I'd vote that either we leave them as is (which is what the patch does) or remove the list altogether, replacing it with some generic text.
#13
@
6 years ago
I've tested this and it works well!
Looking at the patch, only thing I have to comment on is bp-settings/actions/data.php. I don't think that request handler should be hooked with an anonymous function -- all the other similar registrations in BuddyPress use named functions, and we might as well keep it consistent at this point.
I think this is ready @boonebgorges.
Attached is a first pass at a "Settings > Data" page.
This requires testing on WP 4.9.6 or latest WP trunk.
This is what the patch looks like:
Request:
This is shown when a user has never requested a data export before.
For the listing of items, this is done dynamically and is pulled from the list of registered WP data exporters. Once we register BuddyPress as a data exporter, their items will be displayed here as well.
I had to filter the items because they are meant to be seen in the WP admin dashboard. For example,
WordPress User
is changed toUser profile
andWordPress Comments
/WordPress Media
is changed toComments
/Media
. The reason I did this is for whitelisting purposes and that users do not know (or care) their content is from WordPress. However, this might not conform with GDPR if this needs to be spelled out explicitly. If that is the case, then I'll remove this part of the code in a later patch.When the user clicks on this button, we auto-confirm the request. This differs from what WordPress does as a site admin has to create the initial request. Next, an email is sent to the user so they can confirm this data export request. We don't have to worry about this since the user is initiating the data export request themselves.
Pending:
This is shown when a user has requested a data export, but the site admin has not confirmed the request yet.
Completed:
This is shown when the data export is ready to download. The copy is mostly stolen from the WP email that is sent to the user.
Luckily, I didn't have to write too much code for this. We might have to tweak the template parts for Nouveau, but I think this is a good first pass.
We might also want to add a "Data Erase" section on this page as well to mirror the other portion of what WordPress has done for GDPR. Related: #7825.