Opened 6 years ago
Last modified 6 years ago
#47488 new enhancement
add "extensibility" to WP_Privacy_Requests_Table
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Privacy | Keywords: | needs-patch has-screenshots |
Focuses: | Cc: |
Description
WP_Privacy_Requests_Table
(and its 2 sub-classes WP_Privacy_Data_Removal_Requests_List_Table
and WP_Privacy_Data_Export_Requests_List_Table
) lack some of the "extension" points that other list tables (e.g., WP_Posts_List_Table
) in core have.
A short list of what is missing includes:
- an
extra_tablenav()
method and therestrict_manage_xxx
andmanage_xxx_extra_tablenav
actions that accompany it - the ability to add custom columns (via a
xxx_columns
filter) - the ability to add/remove row actions (via a
xxx_row_actions
filter) - the ability to add "display states" (via a
display_xxx_states
filter)
I do not have a concrete use-case for needing any of the above at this time, but feel that all core list tables should support them unless there is a concrete reason not to.
I'll add a patch shortly.
Attachments (4)
Change History (12)
#3
@
6 years ago
Thanks @pbiron as there's already work and a patch for the custom columns (even though needs a refresh) probably best to keep the work separate, I've added a cross-relation to the other ticket so they're taken into account together.
#4
@
6 years ago
While working on the patch for this, I noticed the following.
Other core list tables that have a get_views()
method do not output a view if the count for that view is 0, e.g., WP_Posts_List_Table
doesn't output "Pending (0)" if there are no posts with `$post_status === 'pending').
However, WP_Privacy_Requests_Table
does output "Pending (0)" if there are no pending requests.
For consistency's sake, I think WP_Privacy_Requests_Table
should skip views with count of 0.
Should I open another ticket for that?
#5
follow-up:
↓ 6
@
6 years ago
Thanks @pbiron good catch, I would say another ticket would be appropriate for that for sure
#6
in reply to:
↑ 5
@
6 years ago
Replying to garrett-eclipse:
Thanks @pbiron good catch, I would say another ticket would be appropriate for that for sure
See #47495, with patch added.
@
6 years ago
example of the problem caused by using a <div> tag in the builtin "Download Personal Data" and "Erase Personal Data" row actions
#7
@
6 years ago
Looking for some advice on a couple of matters related to row actions for the privacy list tables.
- all other core list tables that have "builtin" row actions allow some/all of those builtin actions to be filtered out with their respective
xxx_row_actions
filter. That is, they build up their builtin row actions array and then pass it toapply_filters( 'xxx_row_actions', $actions, $item )
and then pass the result to$this->row_actions( $actions )
. It seems to be, however, that that might not be appropriate for the privacy list tables...since theDownload Personal Data
andForce Erase Personal Data
row actions are really how a user "operates" on the row (i.e., there is no equivalent "edit" action as in all other list tables). So, my question is: should a plugin be allowed to remove those row actions? I ask because I don't have any real-world experience processing privacy requests and don't know what the implications would be if a plugin were to filter them out.
- would it be useful to have a "generic"
privacy_request_row_actions
filter in addition to specific ones for each sub-class ofWP_Privacy_Request_Table
(e.g.,export_personal_data_row_actions
andremove_personal_data_row_actions
)?WP_Privacy_Data_Export_Requests_List_Table
andWP_Privacy_Data_Removal_Requests_List_Table
are unique in core in that they extend an abstract class that itself extendsWP_List_Table
. There is precedent in core for both a "specific" and a "generic" hook, e.g.,save_post_{$post->post_type}
andsave_post
.
- the markup for the 2 "builtin" row actions uses a
<div>
tag to wrap a couple of<span>
tags. That<div>
causes whenWP_List_Table::row_action()
builds up a string with each action separated by|
(see screenshot). As far as I can tell, it does not need to be a<div>
, and everything works fine if it's changed to a<span>
. Does anyone know of a reason it needs to stay a<div>
?
@
6 years ago
example of the using a <span> tag in the builtin "Download Personal Data" and "Erase Personal Data" row actions
Related #44354