Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

#7826 closed enhancement (fixed)

Privacy: "Settings > Data" page

Reported by: r-a-y's profile r-a-y Owned by: boonebgorges's profile 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)

7826.01.patch (14.6 KB) - added by r-a-y 6 years ago.
7826.02.patch (16.8 KB) - added by r-a-y 6 years ago.
Fixes an issue with expired data export download files.
7826.03.patch (17.2 KB) - added by boonebgorges 6 years ago.
7826.04.patch (19.0 KB) - added by boonebgorges 6 years ago.
Screenshot_2018-07-11_22-14-24.png (38.2 KB) - added by boonebgorges 6 years ago.
Screenshot_2018-07-11_22-14-41.png (41.3 KB) - added by boonebgorges 6 years ago.
7826.05.patch (19.2 KB) - added by boonebgorges 6 years ago.

Download all attachments as: .zip

Change History (21)

@r-a-y
6 years ago

#1 @r-a-y
6 years ago

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:

https://i.imgur.com/XYb93xY.png

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 to User profile and WordPress Comments / WordPress Media is changed to Comments / 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:

https://i.imgur.com/sdAYQQS.png

This is shown when a user has requested a data export, but the site admin has not confirmed the request yet.


Completed:

https://i.imgur.com/wNg6WvD.png

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.

Last edited 6 years ago by r-a-y (previous) (diff)

@r-a-y
6 years ago

Fixes an issue with expired data export download files.

#2 @DJPaul
6 years ago

I haven't looked at the patch, but I like the idea very much. It'll be a huge feature improvement!

#3 @DJPaul
6 years ago

  • Milestone changed from 3.1 to 4.0

Milestone renamed

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 @boonebgorges
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 @r-a-y
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 @boonebgorges
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 @boonebgorges
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 @r-a-y
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 @boonebgorges
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 @boonebgorges
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 @DJPaul
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.

Last edited 6 years ago by DJPaul (previous) (diff)

#14 @boonebgorges
6 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 12272:

Privacy: Introduce 'Export Data' settings panel.

This new panel allows users to request a data export. It also has
information on deleting one's personal data, which in BuddyPress means
deleting one's account.

Props r-a-y.
Fixes #7826.

Note: See TracTickets for help on using tickets.