Opened 7 years ago
Last modified 7 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_xxxandmanage_xxx_extra_tablenavactions that accompany it - the ability to add custom columns (via a
xxx_columnsfilter) - the ability to add/remove row actions (via a
xxx_row_actionsfilter) - the ability to add "display states" (via a
display_xxx_statesfilter)
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
@
7 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
@
7 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
@
7 years ago
Thanks @pbiron good catch, I would say another ticket would be appropriate for that for sure
#6
in reply to:
↑ 5
@
7 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.
@
7 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
@
7 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_actionsfilter. 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 DataandForce Erase Personal Datarow 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_actionsfilter in addition to specific ones for each sub-class ofWP_Privacy_Request_Table(e.g.,export_personal_data_row_actionsandremove_personal_data_row_actions)?WP_Privacy_Data_Export_Requests_List_TableandWP_Privacy_Data_Removal_Requests_List_Tableare 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>?
@
7 years ago
example of the using a <span> tag in the builtin "Download Personal Data" and "Erase Personal Data" row actions
Related #44354