WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 5 months ago

#44266 assigned enhancement

Add per-request-row delete row action for privacy actions

Reported by: allendav Owned by: xkon
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch needs-testing has-screenshots needs-refresh
Focuses: Cc:

Description

Right now the only way to delete a privacy request (e.g. ones created while doing periodic testing) is to use the bulk-delete flow. It could simplify things greatly to add a per-request-row Delete action below the email address.

Attachments (9)

44266.diff (5.8 KB) - added by xkon 6 months ago.
44266_preview.gif (845.0 KB) - added by xkon 6 months ago.
44266.2.diff (6.7 KB) - added by garrett-eclipse 5 months ago.
Refresh of initial patch
3057f33c547ab2e5ba8c03a035eef87d.gif (530.1 KB) - added by garrett-eclipse 5 months ago.
Export request screencast of the two new actions and their notices
e3242fe2f2569cd51b63a12c6950e248.gif (511.2 KB) - added by garrett-eclipse 5 months ago.
Screencast of the erasure row actions and notices
44266.3.diff (6.7 KB) - added by garrett-eclipse 5 months ago.
Added missed periods to make notices and aria labels sentences.
44266.4.diff (7.0 KB) - added by xkon 5 months ago.
Screen Shot 2020-02-07 at 10.52.53 AM.png (7.7 KB) - added by garrett-eclipse 5 months ago.
Concept: Export Row Actions shortened to match other screens
Screen Shot 2020-02-07 at 10.53.21 AM.png (9.6 KB) - added by garrett-eclipse 5 months ago.
Concept: Reduce erasure row actions for brevity

Download all attachments as: .zip

Change History (26)

#1 @allendav
2 years ago

  • Keywords ux-feedback added

#2 @allendav
2 years ago

Should also consider how this could work with an Archive flow #44222

#3 @desrosj
2 years ago

  • Milestone changed from Awaiting Review to Future Release

#4 @dejliglama
2 years ago

We should have in mind that the request for data export and erasure is a matter of documentation (logging), and should not be deleted lightly.

#5 @desrosj
2 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.


8 months ago

#7 @michaelarestad
8 months ago

  • Keywords changed from needs-patch, ux-feedback to needs-patch ux-feedback

Can you please add some testing instructions and a screenshot of existing UI?

#8 @xkon
6 months ago

  • Owner set to xkon
  • Status changed from new to assigned

@xkon
6 months ago

@xkon
6 months ago

#9 @xkon
6 months ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Milestone changed from Future Release to 5.4

44266.diff introduces 2 row actions for a Resend Confirmation Email as well as Delete Request on both Export/Erase personal data list tables.

I'll mark this for 5.4 as well in case we can have some extra eyes on testing :-).

#10 @xkon
6 months ago

  • Keywords 2nd-opinion added

#11 @xkon
6 months ago

  • Keywords has-screenshots added; ux-feedback removed

@garrett-eclipse
5 months ago

Refresh of initial patch

@garrett-eclipse
5 months ago

Export request screencast of the two new actions and their notices

@garrett-eclipse
5 months ago

Screencast of the erasure row actions and notices

#12 @garrett-eclipse
5 months ago

  • Keywords 2nd-opinion removed

Thanks @xkon I appreciate the initial patch here, it worked very nicely.

I've refreshed in 44266.2.diff to update the following;

  1. Updated the verbiage and classes to match #44674.
  2. Updated the translator comments to match the placeholder.
  3. Fixed some formatting for CS.
  4. Updated the delete action for the single request to match the resend by providing the email in the notice.
  5. Updated the resend multiple to keep it's plural form to support russian/arabic and other languages with multiple plural forms.

Uploaded screens to show the actions in action on both request tables.

If you can retest I think this can move forward. I was initially hesitant to use the bulk action here but it works nicely and when the list tables are updated in #47488 we can revisit how the row actions are handled.

@garrett-eclipse
5 months ago

Added missed periods to make notices and aria labels sentences.

@xkon
5 months ago

#13 @xkon
5 months ago

Thanks for the update @garrett-eclipse! Uploaded 44266.4.diff for some minor CS fixes (spaces instead tabs on some instances /shrug editors.. :D).

There is a small hiccup here, but not sure what would be the best approach to deal with it. When clicking the row actions you end up with a full URL on your browser depending on the action taken.

As an example, you click on "resend confirmation request" your URL should now have ?action=resend&request_id%5B0%5D=42&_wpnonce=8e47d5d5eb appended. This leads into a situation that if you hit a refresh page a confirmation is re-sent unintentionally.

We'll have to address this somehow before committing. I believe that we had the same issue for some other action in the past but couldn't find it at the moment.

#14 @birgire
5 months ago

Hi, it's very handy and makes it much easier to delete each request,

but I wonder if it would be benificial to look into the Trash feature, of the list table, for requests.

i.e. changing: "Delete Request" into "Trash Request"

and similar for bulk: "Delete Requests" into "Trash Requests"

to avoid accidental removal.

I think the older ticket had two forms on the same page interfering and it was solved by adding a action attribute to the other one, if I recall correctly. Not sure that would solve this one here.

I guess one could look into redirects like when posts are trashed from the list table, like when:

https://wp.test/wp-admin/post.php?post=157&action=trash&_wpnonce=c7dc10c655

is redirected to

https://wp.test/wp-admin/edit.php?post_type=post&ids=157

#15 @garrett-eclipse
5 months ago

  • Keywords needs-refresh added

Thanks for the refresh @xkon and flagging the nonce issue.

@birgire is correct the other issue #44047 was resolved by forcing an action onto the form to ensure its request didn't pull the nonce into its submission causing the invalid link. I don't believe this will apply here so will do some testing and see if I can come up with a solution before beta. *Marked for needs-refresh as we'll need to address this issue before finalizing.

@birgire as to your mention of the trash feature, that's accounted for in #44222 which I just milestoned to 5.5 as I'd like to work on that for next release.

@garrett-eclipse
5 months ago

Concept: Export Row Actions shortened to match other screens

@garrett-eclipse
5 months ago

Concept: Reduce erasure row actions for brevity

#16 @garrett-eclipse
5 months ago

@xkon / @birgire reviewing the other row-actions in core they in most cases use single words to make them more concise.
These new actions are quite verbose with 2-3 words per action forcing this to always be on two+ lines.

What are your thoughts on just going with Download | Resend | Delete or for the erasure Force Erase | Resend | Delete?

I uploaded some screens to illustrate.

#17 @xkon
5 months ago

  • Milestone changed from 5.4 to Future Release

We already changed the naming of the Bulk Actions on #44674 to Delete Requests and Resend Confirmation Requests to make it clear.

Unfortunately, since the tables show the e-mails it was misunderstood that the accounts might be getting deleted instead of just the requests. This is the reason why we also went in the way of including "big action titles" here following 44674 to always be clear about what the action is and avoid miscommunication.

I know that the general rule is the simplicity with "Download / Trash / Resend" etc but I believe that we can make an exception here as it is actually hard to distinguish the action, especially under the Erasures tables.

Also due to the 5.4 beta being close, I'm marking this for a future release so we can also take care of the URI issue here.

Note: See TracTickets for help on using tickets.