Make WordPress Core

Opened 6 years ago

Closed 4 years ago

#43974 closed defect (bug) (fixed)

Both personal data request processes should follow the same convention

Reported by: garrett-eclipse's profile garrett-eclipse Owned by: garrett-eclipse's profile garrett-eclipse
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch commit has-screenshots
Focuses: ui, javascript, administration, ui-copy Cc:

Description

Hello,

I noticed with the GDPR requests process that the export requests workflow only affects the Next Steps buttons, while the removal request instead injects an alert. Can the two processes be consistent?

Button Flow for Sending Email;
https://gyazo.com/241ea304afd342419b585697aa4ac9a3

Button Flow for Removing Data;
https://gyazo.com/3b72b3622b3e38b1c9868f8971eb6ee7

Was thinking the button should go away with 'Data Removed' or something.

But that being said I do like the alert as it does give more info if data was already removed but maybe not in the table and instead in the standard wp alert location. And maybe have an alert for the email send.

Thanks

Attachments (11)

Screen Shot 2018-05-04 at 1.32.43 PM.png (51.7 KB) - added by garrett-eclipse 6 years ago.
Alert Injected
43974.diff (2.7 KB) - added by Kerfred 5 years ago.
Screen Shot 2019-06-21 at 11.07.24 PM.png (461.5 KB) - added by garrett-eclipse 5 years ago.
Testing results for the patch, works nicely and is now consistent.
43974.2.diff (2.8 KB) - added by garrett-eclipse 5 years ago.
Minor tweak to 'Email sent' verbiage as there's now more space available have expanded on verbiage to make it clear
43974.3.diff (2.7 KB) - added by garrett-eclipse 5 years ago.
Updated Verbiage
f9157cd6e3a0f9e62e7e6fce27632ddd.gif (473.1 KB) - added by garrett-eclipse 5 years ago.
Send Export Link workflow
fa0efe9b5e9ac5dfc1598f65ff9bef85.gif (412.7 KB) - added by garrett-eclipse 5 years ago.
Erase Personal Data workflow
after.jpg (52.2 KB) - added by birgire 4 years ago.
43974.4.diff (2.7 KB) - added by garrett-eclipse 4 years ago.
Refresh to avoid the hunk on applying the latest patch
43974.5.diff (1.7 KB) - added by garrett-eclipse 4 years ago.
Restore Email Sent message
Screen Shot 2019-10-05 at 8.42.41 PM.png (57.4 KB) - added by garrett-eclipse 4 years ago.
Email Sent restored on success action

Download all attachments as: .zip

Change History (40)

#1 @xkon
6 years ago

  • Keywords gdpr added

#2 @desrosj
6 years ago

  • Component changed from General to Privacy

Moving to the new Privacy component.

#3 @desrosj
6 years ago

  • Version changed from trunk to 4.9.6

Marking Privacy change as introduced in 4.9.6.

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


6 years ago

#5 @desrosj
6 years ago

  • Keywords ui-feedback added
  • Milestone changed from Awaiting Review to Future Release

#6 @desrosj
6 years ago

  • Summary changed from Both request processes should follow the same convention - GDPR to Both personal data request processes should follow the same convention

#7 @desrosj
6 years ago

  • Keywords gdpr removed

Removing the GDPR keyword. This has been replaced by the new Privacy component and privacy focuses in Trac.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


5 years ago

#9 @kjellr
5 years ago

  • Keywords ui-feedback removed

👋 We discussed this ticket in the weekly #design triage chat:
https://wordpress.slack.com/archives/C02S78ZAL/p1558975417034500

These two messages do seem like they should be consistent. Since "No personal data found for this user" is likely too long to include in place of the button, I suggest we use the alert-style treatment in both places. Also, in both cases, the button should disappear when the alert is visible, since re-pressing should have no real effect.

Thanks!

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


5 years ago

#11 @garrett-eclipse
5 years ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Future Release to 5.3

Thanks @kjellr and #design for the review. Now that we have some direction going to move into 5.3 and label good-first-bug for wceu contributor day.

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


5 years ago

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


5 years ago

#14 @Kerfred
5 years ago

I'm working on it.

@Kerfred
5 years ago

#15 @Kerfred
5 years ago

  • Keywords has-patch added; needs-patch removed

I have hidden the "Erase" button upon success.
And I have applied the same style for the "Export" action.

This ticket was mentioned in Slack in #core-php by kerfred. View the logs.


5 years ago

@garrett-eclipse
5 years ago

Testing results for the patch, works nicely and is now consistent.

@garrett-eclipse
5 years ago

Minor tweak to 'Email sent' verbiage as there's now more space available have expanded on verbiage to make it clear

#17 @garrett-eclipse
5 years ago

  • Keywords needs-testing needs-copy-review added; good-first-bug removed

Thanks @Kerfred I appreciate the contribution.

Testing went successfully, see screenshot.

I'm adding needs-copy-review as I feel we can provide more context in the message now that there's more space available for 'Email sent'.

My initial thought on verbiage would be;
'The personal data exported for this user was sent successfully.'

I've put this in 43974.2.diff and will wait on some copy feedback before we move this forward.

All the best

#18 @birgire
5 years ago

I'm adding needs-copy-review as I feel we can provide more context in the message now that there's more
space available for 'Email sent'.

Good suggestion @garrett-eclipse

My initial thought on verbiage would be;
'The personal data exported for this user was sent successfully.'

This sounds good and I think this an improvement to the current "Email sent.".

I would suggest a minor change, to reflect the text changes from #44822

were we changed the "Email Data" text to "Send Export Link".

Perhaps:

'The export link for this user was sent successfully.'

or

'The export link was successfully sent to this user.'

My first thought was to add "link" to your suggestion, but I'm not so sure about the long phrase: "personal data export link" :-) But there might be ways to rearrange it, to avoid such cases, e.g. with: "The link to the personal data export was successfully sent to this user." or something similar.

#19 @garrett-eclipse
5 years ago

Thanks @birgire I appreciate the feedback and good point on it being an export link.

For options I'm looking at;

  1. "The link to the personal data export was successfully sent to this user."
  2. "The link to the personal data export was successfully sent."
  3. "The personal data export link was successfully sent."

I'm leaning towards #2 but #3 is a bit more concise and defines it as 'personal data export link' which is what is is, not sure if that's too much of a mouthful or hard to translate.

Going to see if @marybaum could take a look.

#20 @marybaum
5 years ago

Thanks @birgire I appreciate the feedback and good point on it being an export link.
For options I'm looking at;
"The link to the personal data export was successfully sent to this user."
"The link to the personal data export was successfully sent."
"The personal data export link was successfully sent."

Welp. How about

"Success! Personal data export link sent to mailto:user@domain.com."

Slightly less redundant and more energetic than 'was-successfully-sent' and gives user a look at the email address the data went to -- or swear under their breath and try again with another.

If this were copy instead of a string, I'd be advocating for 'the link has gone to' or some more active construction, but in a string that's of course getting too far ahead of ourselves in the process.

@garrett-eclipse
5 years ago

Updated Verbiage

#21 @garrett-eclipse
5 years ago

Thanks @marybaum I appreciate the feedback.

Sadly the email isn't readily available at the time of the string construction as it's localized before the javascript is loaded and then used in response to the email action. The email could be appended to the end in the javascript but this would break RTL language localizations.

I also see what you're saying with the 'Success!' being more energetic but am wondering if that's needed as it's already in a success notice. We could even do without the word completely.

The other strings we're trying to be consistent with here use 'the personal data' and 'for this user' such as the relevant one we're trying to match for Erasure;
'foundAndRemoved' => __( 'All of the personal data found for this user was erased.' ),

With that in mind I've added 43974.3.diff which goes with;
'emailSent' => __( 'The personal data export link for this user was sent.' ),
*By saying 'sent' it already indicates success, that and it's in a success notice.

Appreciate your thoughts @birgire / @marybaum / @Kerfred

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


5 years ago

@garrett-eclipse
5 years ago

Send Export Link workflow

@garrett-eclipse
5 years ago

Erase Personal Data workflow

#23 follow-up: @garrett-eclipse
5 years ago

  • Focuses ui javascript ui-copy added
  • Keywords commit added; needs-testing needs-copy-review removed
  • Owner set to garrett-eclipse
  • Status changed from new to accepted

Thank you, everyone, for the assistance on this ticket.

The latest patch still applies cleanly and passes unit tests.

Running the two workflows side by side they're now identical and provide the same experience which was the goal of this bug.

Moving forward to commit for final review.

Testing note: You'll need to push a request through the workflow and confirm the request email in order to have the Send Export Link and Erase Personal Data actions to present themselves.

@birgire
4 years ago

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


4 years ago

#25 @birgire
4 years ago

  • Keywords has-screenshots added

Thank you @garrett-eclipse, this tests well.

There was a little hunk when applying to trunk, but this tests well.

There seems to be some general UI changes (e.g. new buttons) coming to the admin, so I just posted as screenshot here after.jpg to have it available.

Version 0, edited 4 years ago by birgire (next)

@garrett-eclipse
4 years ago

Refresh to avoid the hunk on applying the latest patch

#26 @garrett-eclipse
4 years ago

Thanks @birgire I appreciate that. There was two minor issues with the admin css conflicting with privacy css but that is all resolved now. I caught that hunk on apply as well so just refreshed for a clean apply in 43974.4.diff

#27 in reply to: ↑ 23 ; follow-up: @SergeyBiryukov
4 years ago

Replying to garrett-eclipse:

Running the two workflows side by side they're now identical and provide the same experience which was the goal of this bug.

There seems to be a minor inconsistency where clicking the Erase Personal Data button shows "Erasure completed" in the Next Steps column, but clicking the Send Export Link button doesn't show anything in that column. Is the removal of the "Email sent" message intentional?

@garrett-eclipse
4 years ago

Restore Email Sent message

@garrett-eclipse
4 years ago

Email Sent restored on success action

#28 in reply to: ↑ 27 @garrett-eclipse
4 years ago

Replying to SergeyBiryukov:

There seems to be a minor inconsistency where clicking the Erase Personal Data button shows "Erasure completed" in the Next Steps column, but clicking the Send Export Link button doesn't show anything in that column. Is the removal of the "Email sent" message intentional?

Thanks @SergeyBiryukov good catch, I've restored the Email Sent message in the place of the action button after success. This is refreshed in 43974.5.diff

#29 @whyisjake
4 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 46412:

Privacy: Ensure that sending email, and remove data follow the same user experience.

Both personal data request processes should follow the same convention.

Fixes: #43974.
Props: garrett-eclipse, kjellr, Kerfred, birgire, marybaum, SergeyBiryukov.

Note: See TracTickets for help on using tickets.