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

[allendav]

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.

[allendav]

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

[dejliglama]

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.

[desrosj]

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

[michaelarestad]

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

[xkon]

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 :-).

[garrett-eclipse]

Refresh of initial patch

[garrett-eclipse]

Export request screencast of the two new actions and their notices

[garrett-eclipse]

Screencast of the erasure row actions and notices

[garrett-eclipse]

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]

Added missed periods to make notices and aria labels sentences.

[xkon]

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.

[birgire]

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

[garrett-eclipse]

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]

Concept: Export Row Actions shortened to match other screens

[garrett-eclipse]

Concept: Reduce erasure row actions for brevity

[garrett-eclipse]

@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.

[xkon]

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.

[garrett-eclipse]

Refresh to improve verbiage and address the URI issue by using sendback passthrough.

[garrett-eclipse]

Erasure Row Actions

[garrett-eclipse]

Export Row Actions

[garrett-eclipse]

Resent request notice with email.

[garrett-eclipse]

Deleted notice

[garrett-eclipse]

Good point @xkon, I've refreshed in 44266.5.diff and feel it's pretty well there so am moving to 5.6 for extra eyes. Please take a minute to test if you can.

I've gone with the following verbiage which is clear enough but not overly verbose;
Download Data | Resend Confirmation | Delete Request
Force Erasure | Resent Confirmation | Delete Request

As to the URI and leaving param remnants during requests I adopted the posts/comments approach to using a sendback and redirect. To handle the redirect there's now a new handle_bulk_messages function that takes the info params from the redirect to display the message. This strips the wpnonce and other undesired items.

One drawback of the sendback method was the split mean passing the request id in order for us to lookup the email, but on delete actions the request is gone so we can't do the lookup. The only workaround is to pass the email via the param and I'd prefer not to have email in URL as that's saved to history etc. So instead for delete we only provide the count and not the email. The resent we do provide email as it's more important to know who was resent to than who you deleted.

Please test and review and let's move this forward.

P.S. One issue you may note, especially locally testing, is failed request emails aren't reported. I'm looking to address that via #44081.

[garrett-eclipse]

After testing found on removal requests we suppress actions. Updated so we only suppress confirm and erasure actions on confirmed requests. Delete and download will always be available.

[hellofromTonya]

📣 Call for testers 📣

This ticket is ready for commit once it gets some testing. Calling on our testers to help get this over the finish line to land in 5.6 Beta 1 (which is next week, ie 20th).

[hellofromTonya]

Screen capture: Tools > Erase Personal Data with patch 44266.6 applied. Browser: Firefox 81.0.2 on Mac

[hellofromTonya]

@garrett-eclipse Testing patch 42266.6 applied in both Firefox and Chrome:

• I did not get an admin notice after clicking on the Resend Confirmation link or via Bulk actions.
• I did get an admin notice for the delete action via the Delete Request link and bulk actions.

Is this the expected behavior? Should there be a resend notice?

[garrett-eclipse]

There should be a resend notice, but unfortunately I believe you're testing on local where emails won't successfully send so they aren't reported. And when there's a failure it's not reported currently so we have #44081 to fix that.

If you have a hosted server w/ mail you can test there, hopefully successfully.

[carike]

Tested on a single-site installation.
The remove link only appears at the end of the process, but it does remove the request from the list (checked both export and erasure).
Also, just a note, I needed to refresh the page before the link appeared. I'm assuming that this is intended behaviour / that you did not want to use AJAX.
The user did receive the confirmation e-mails.

Removing "has screenshots" tag as the screenshots are no longer representative of what the patch looks like.

[carike]

Resend confirmation request via the bulk action did send an e-mail with a new link on a live server.

[hellofromTonya]

Thanks @carike for testing in live site and confirming it works well. Marking this one for commit.

[garrett-eclipse]

Minor refresh to provide a message when all items failed either to delete or resend.

[garrett-eclipse]

Thanks @carike and @hellofromTonya after thinking on your issue with local mail failures I made a minor mod in 44266.7.diff to provide messaging for that failure. We'll greatly improve that in #44081 which is ready to land as a bug during beta.

[helen]

Late punt, not a fan of adding one-click delete without trash. See #44222.