WordPress.org

Make WordPress Core

Opened 13 months ago

Last modified 3 months ago

#44133 new enhancement

Should the Data Export indicate when we have no information on the user

Reported by: garrett-eclipse Owned by:
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-screenshots has-patch needs-testing 2nd-opinion
Focuses: administration Cc:

Description

Hello,

If a data export is done for a non-existent user should we indicate in the .html file provided that we have no information on the subject? Currently the file is provided and just the initial table provided. If there's nothing else should a message be there to indicate that we currently have nothing stored on them?

Thanks

Attachments (15)

Screen Shot 2018-05-17 at 1.42.03 PM.png (73.6 KB) - added by garrett-eclipse 13 months ago.
.html export when we have no data on subject
no-personal-data-found-version-1.jpg (27.0 KB) - added by birgire 13 months ago.
no-personal-data-found-version-2.jpg (26.2 KB) - added by birgire 13 months ago.
no-personal-data-found-version-3.jpg (27.0 KB) - added by birgire 13 months ago.
44133.diff (563 bytes) - added by birgire 13 months ago.
no-personal-data-found-in-email.jpg (26.9 KB) - added by birgire 13 months ago.
44133.2.diff (4.2 KB) - added by birgire 13 months ago.
44133.3.diff (5.4 KB) - added by birgire 13 months ago.
44133.4.diff (5.1 KB) - added by desrosj 13 months ago.
Minor docblock fixes.
44133.5.diff (6.9 KB) - added by desrosj 12 months ago.
44133.6.diff (7.3 KB) - added by desrosj 12 months ago.
44133.7.diff (7.2 KB) - added by desrosj 12 months ago.
44133.8.diff (6.6 KB) - added by xkon 3 months ago.
refresh of 44133.7
44133.9.diff (7.8 KB) - added by xkon 3 months ago.
Inform the user within mail and export file that there are no personal data found.
44133.10.diff (12.8 KB) - added by garrett-eclipse 3 months ago.
Additional improvements and fixed unit tests

Download all attachments as: .zip

Change History (67)

@garrett-eclipse
13 months ago

.html export when we have no data on subject

#1 @desrosj
13 months ago

  • Version changed from trunk to 4.9.6

Marking Privacy change as introduced in 4.9.6.

@birgire
13 months ago

#2 @birgire
13 months ago

  • Keywords has-patch has-screenshots added

I posted some ideas in

and attached a patch for the third version in 44133.diff.

It adds a Result row to the About group, with two possible values:

  • No personal data found.
  • Personal data found.

Another approach might be to only generate an export file, when personal data is found, and mention that in the email instead.

So instead of sending an export link to an "empty" export file, the email would say No personal data found.

Last edited 13 months ago by birgire (previous) (diff)

#3 @garrett-eclipse
13 months ago

Thanks @birgire that's awesome. In my opinion I think it would make the most sense to not generate an export and instead have the email indicate there was no data. Even with the result field of no data found in the .html you have to download a zip, open the .html to find there's nothing. Just seems like extra unnecessary steps. Cheers

@birgire
13 months ago

#4 @birgire
13 months ago

@garrett-eclipse yes it's probably better user experience to not have to download a zip file and open it, and to discover that no personal data was found :).

The patch in 44133.2.diff is just a PoC patch.

The screenshot in no-personal-data-found-in-email.jpg is from Gmail.

#5 @garrett-eclipse
13 months ago

Niiice @birgire I like that alot thank you. And I agree it feels nicer to remove the extra steps when they're not needed. Cheers

This ticket was mentioned in Slack in #gdpr-compliance by desrosj. View the logs.


13 months ago

#7 @desrosj
13 months ago

  • Milestone changed from Awaiting Review to 4.9.7

#8 @desrosj
13 months ago

  • Keywords needs-refresh added

+1 for not generating and sending a ZIP when no data is found for a user.

@birgire the patch looks good overall and just needs a refresh. Here is some general feedback.

  • Can we change the wp_privacy_send_personal_data_export_email() documentation to make note that an email is sent to a user if no data is found.
  • wp_privacy_personal_data_email_content bool type for the new parameter is misaligned.
  • A @since 4.9.7 tag should be added to the wp_privacy_send_personal_data_export_email() function and wp_privacy_personal_data_email_content filter noting the introduction of the new $has_data_export parameter.

Also, is there any reason a plugin would want to override the value of $has_data_export? If so, a filter could be added there.

@birgire
13 months ago

@desrosj
13 months ago

Minor docblock fixes.

#9 @desrosj
13 months ago

  • Keywords needs-refresh removed

44133.4.diff makes some minor improvements:

  • @param and @return tags shouldn't be aligned together.
  • The @since 4.9.7 should be added and the @since 4.9.6 should remain. The 4.9.7 comment should describe that the parameter was added.

#10 @birgire
13 months ago

Thanks for good suggestions @desrosj

I got some unexpected interruption, earlier today, so I couldn't finish adding the patch and the writeup :)

Looks like you covered it in the latest patch, thanks.

The patch also includes a missing comment for translators.

Regarding a special filter for $has_data_export, I don't see a reason for it, at the moment. It might be considered later, if requested with a good use-case example. In the meanwhile there are workarounds, with current filters, if needed.

I now wonder if $has_export_data sounds better than $has_data_export ?

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


13 months ago

#12 @allendav
13 months ago

This (e.g. no data found) is a good first candidate for a row notice for export (like what erase does today). See also #43974

Last edited 13 months ago by allendav (previous) (diff)

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


12 months ago

#14 @desrosj
12 months ago

  • Keywords needs-patch added; has-patch removed

Patch is no longer applying cleanly.

#15 @desrosj
12 months ago

  • Keywords has-patch needs-refresh added; needs-patch removed

@desrosj
12 months ago

#16 @desrosj
12 months ago

  • Keywords needs-testing added; needs-refresh removed

44133.5.diff is a refresh with the following changes:

  • Only run the wp_privacy_personal_data_email_content filter on the email content sent with a successful export file link.
  • Introduce the wp_privacy_personal_data_email_content_no_data filter for filtering the email text sent when no data is found.
  • Only fire the wp_privacy_personal_data_export_file action when data is found for exporting.
  • Add the $request variable as a parameter for the wp_privacy_personal_data_export_file action.
  • Introduce the wp_privacy_personal_data_export_no_data action that fires when no data is found for the request. This will allow plugins to perform custom code when a request data has been gathered and determined to be empty.

#17 @ocean90
12 months ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

This ticket was mentioned in Slack in #core by pbiron. View the logs.


12 months ago

#19 @JoshuaWold
12 months ago

I prefer the suggestion of NOT giving a download link when the export has nothing to share.

Two quick points:

  1. Are we going to surface in the admin a message to the user?
  2. With the email that gets sent out can we write a slightly different message?

Instead of saying:

“Howdy, Your request for an export of personal data has been completed. No personal data was found.”

What if we said:

“Howdy, Your request for an export of personal data has been completed. [Site name] has identified no personal data for the user associated with [email].”

#20 @pbiron
12 months ago

The general feeling on today's bug scrub is that this should land in 4.9.8, assuming we can get agreement on the wording of the email quickly.

Beta for 4.9.8 is scheduled for Tuesday July 17, 2018. Does everyone thing we can get that agreement by then? If not, then we'll have to punt to 4.9.9.

Last edited 12 months ago by pbiron (previous) (diff)

#21 @pbiron
12 months ago

Also, if it's going to land in 4.9.8 the patch needs a refresh for @since 4.9.8.

#22 @desrosj
12 months ago

  • Keywords needs-refresh added

@desrosj
12 months ago

#23 @desrosj
12 months ago

  • Keywords needs-refresh removed

In 44133.6.diff:

  • Update @since tags to 4.9.8.
  • Add @since tag to wp_privacy_send_personal_data_export_email().
  • Update wording for email. I went with "Your request for an export of personal data has been completed. [Sitename] has identified no personal data associated with this email address." @JoshuaWold does this work?
  • Removed the ###EMAIL### placeholder replacement as it is no longer used in email content.

#24 @JoshuaWold
12 months ago

@desrosj

Update wording for email. I went with "Your request for an export of personal data has been completed. [Sitename] has identified no personal data associated with this email address." @JoshuaWold does this work?

I like that change, and taking the unique email out of it is better!

#25 @desrosj
12 months ago

@JoshuaWold Great! Also, I forgot to include my thoughts on the admin message. When a request to export a user's data is performed and no data is found, I think that showing a notice is a good idea. The "Download Personal Data" action link that appears on hover should also be hidden once it is determined there is no personal data. Let's open a new ticket for those improvements.

#26 @JoshuaWold
12 months ago

@desrosj awesome! If someone else is able to open the new ticket I'm happy to weigh in with any UX related feedback.

#27 @desrosj
12 months ago

Related: #44539.

#28 @desrosj
12 months ago

  • Keywords needs-refresh added

This needs a refresh after r43435.

@desrosj
12 months ago

#29 @desrosj
12 months ago

  • Keywords needs-refresh removed

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


12 months ago

#31 @birgire
12 months ago

Good improvement on the wording.

Few thoughts related to the ticket:

  • Within the _wp_privacy_send_erasure_fulfillment_notification() function, we have a single user_confirmed_action_email_content filter (src) for both empty and non-empty privacy policy url. Should we use the same structure here for the wp_privacy_personal_data_email_content filter, in the case of available export data and no-data, for consistency?
  • I think we could skip the $has_export_data filter arguments, if two filters are used. It's needed though in the one filter approach.
  • We should consider adding a unit test, e.g. Tests_Privacy_WpPrivacySendPersonalDataExportEmail::test_function_should_send_email_to_requester_without_export_link_when_no_export_data_found(). I can look into that.
  • If the export data, defined by plugins, is e.g. missing 'group_id', 'group_label', 'item_id' or 'items', then it seems that we can get PHP notices. Also it seems possible for empty items to get registered into the _export_data_grouped post meta, that is used to generate export files. I'm considering creating a new ticket for this.
  • Just thinking out aloud, regarding the use of email placeholders in the email content: I guess it could be useful for users that have more than one email address, where all their email is collected by a single one. Those users might need to send a privacy request for each email address. (e.g. they might have used different email addresses when commenting on a site over the years).

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


11 months ago

This ticket was mentioned in Slack in #core by pbiron. View the logs.


11 months ago

This ticket was mentioned in Slack in #core by pbiron. View the logs.


11 months ago

#35 @desrosj
11 months ago

  • Milestone changed from 4.9.8 to 4.9.9

One scenario I came across that has not been discussed here. If a user data request is initiated and the administrator clicks the "Download Personal Data" link, the file shown by @garrett-eclipse originally above is generated. Thinking that should be changed to not generate a file and display a notice to the user.

Also, punting because 4.9.8 RC has been released.

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


11 months ago

#37 @garrett-eclipse
11 months ago

A thought I mentioned in slack but posting here so it's not overlooked is if we should staunch the flow early even before a confirmation email is sent. So when you put a username or email into the form and click Send Request maybe have it check the data at that point and if there’s none stop the entire workflow and just present a notice. Then the request won’t appear in the list, no confirm email is sent, they don’t get mislead into thinking there’s data and in fact they’d never be notified by the system in this case. Just thinking maybe that we could avoid having to update each step in the process to stop the file generation and link and updating other emails and verbiage to indicate no data and instead just halt the process upon the initial submission when no data is present.

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


11 months ago

#39 @desrosj
11 months ago

  • Keywords needs-refresh added

I think skipping the email altogether is the right path here. Similar to when a user's email changes, I think the user should be told how to contact an administrator. This way, if the user was not the one that asked for the requested action they have a way to reach out to the site for more information.

Also, I think that an "empty" request should still be listed with the rest of the requests for historical record. But, I am not sure that Completed is the correct status for requests where no data is removed or exists to be exported. Marking them complete would give the impression that an action was taken, when in fact no action was taken because there was no data to act on.

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


11 months ago

#41 @birgire
11 months ago

It was discussed during the weekly meeting on Slac, if the file should be generated, even if there is no data to export, if the requester needs to show what information different WP sites have of them.

A paragraph could then be added to the email, in a clear way, so the requester wouldn't have to open the file, just to discover it contains no personal data.

Also if the admin gets an export request, that happens to be without personal data, the admin would then most likely have to pass that information to the requester, so wouldn't it be useful to use the existing process for that?

So in short, we could recap with:

  1. Keep the email flow as is
  2. Keep the file generation as is
  3. Inject "No personal data" into the email data to the requester in a clear way
  4. Add "No personal data" into the file header
  5. Add info to hooks about "No personal data" found (if needed)
  6. Inform the admin (the question is how and when ?)

The last point needs further discussion.

Last edited 11 months ago by birgire (previous) (diff)

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


10 months ago

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


9 months ago

#44 @desrosj
9 months ago

  • Milestone changed from 4.9.9 to 5.0

Punting to 5.0 since it falls outside of the 4.9.9 focus.

#45 @pento
8 months ago

  • Milestone changed from 5.0 to 5.1

#46 @desrosj
5 months ago

  • Milestone changed from 5.1 to Future Release

This still needs further discussion and a refresh.

#47 @garrett-eclipse
3 months ago

#46468 was marked as a duplicate.

@xkon
3 months ago

refresh of 44133.7

#48 @xkon
3 months ago

44133.8.diff is just a refresh of the last iteration @desrosj made on 44133.7.diff so it can be applied and re-checked just in case of further discussion ( I'm personally in favor of this idea without having a file generated at all ).

There has been further discussion on this as seen on comment 41 so we'll work on that as well so we can compare both flows and see how to finally move forward with this.

@xkon
3 months ago

Inform the user within mail and export file that there are no personal data found.

#49 @xkon
3 months ago

  • Keywords needs-refresh removed

In 44133.9.diff the exports flow is the same but it splits the e-mails into 2 different types ( so they can be filtered also ) depending on if there are data or not.

The files continue to properly generate and in the case of no data a "No personal data found." message is added within the .html.

@birgire I'm still trying to figure out the last point ( Inform the admin (the question is how and when ?) ). But most likely there's no need to overthink of that at the moment since we want a pretty much 'identical' flow.

Any further ideas?

@desrosj If this is somewhat close to what you have been chatting on slack we could add this for 5.2 as well and see if we can make it? It's not major changes and it adds a nice extra touch to the exports.

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


3 months ago

#51 @garrett-eclipse
3 months ago

  • Milestone changed from Future Release to 5.3

@garrett-eclipse
3 months ago

Additional improvements and fixed unit tests

#52 @garrett-eclipse
3 months ago

  • Keywords 2nd-opinion added

Hi @xkon thanks for the refresh here. This worked nicely.

I uploaded 44133.10.diff to address some minor items (spelling, grammar and missed the ##LINK## phpdoc reference on the second filter) and milestone it for 5.3.0. It also updates the unit tests to account for the additional parameters. Which makes me wonder if we should make the extra two params optional with the = false for backward compatibility as I added a check if $request is false already. Thoughts?

I'm a little torn with the introduction of a second filter for almost the same email, especially since the boolean differentiating the two is supplied as an argument. I understand the email content differs by the single new line so there's that to take into account as well.

As to the indication to the admin user that @birgire mentioned I assume he's speaking of the in-table-row notice produced when erasing the content. See #44135 for the screens and existing work around this. I added a related there to tie back and indicate that changing flow there would also benefit here.

All the best,
Cheers

Last edited 3 months ago by garrett-eclipse (previous) (diff)
Note: See TracTickets for help on using tickets.