Make WordPress Core

Opened 5 years ago

Last modified 5 years ago

#47488 new enhancement

add "extensibility" to WP_Privacy_Requests_Table

Reported by: pbiron's profile pbiron 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 the restrict_manage_xxx and manage_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)

privacy-row-actions-div-problem.png (7.4 KB) - added by pbiron 5 years ago.
example of the problem caused by using a <div> tag in the builtin "Download Personal Data" and "Erase Personal Data" row actions
privacy-row-actions-with-span.png (6.7 KB) - added by pbiron 5 years ago.
example of the using a <span> tag in the builtin "Download Personal Data" and "Erase Personal Data" row actions
privacy-display-states.png (12.5 KB) - added by pbiron 5 years ago.
example showing 1 possible use of display_states...to indicate a request from a "VIP"
privacy-extra-tablenav.png (11.9 KB) - added by pbiron 5 years ago.
example of use of extra_tablenav() to filter requests by date of request and a custom taxonomy

Download all attachments as: .zip

Change History (12)

#1 follow-up: @birgire
5 years ago

Related #44354

#2 in reply to: ↑ 1 @pbiron
5 years ago

Replying to birgire:

Related #44354

Thanx for pointing out that there was already a ticket for part of what I'm proposing, wasn't aware of that ticket.

Should I not worry about custom columns in my patch for this ticket (i.e., leave that part to #44354)?

#3 @garrett-eclipse
5 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 @pbiron
5 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: @garrett-eclipse
5 years ago

Thanks @pbiron good catch, I would say another ticket would be appropriate for that for sure

#6 in reply to: ↑ 5 @pbiron
5 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.

@pbiron
5 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 @pbiron
5 years ago

Looking for some advice on a couple of matters related to row actions for the privacy list tables.

  1. 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 to apply_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 the Download Personal Data and Force 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.
  1. would it be useful to have a "generic" privacy_request_row_actions filter in addition to specific ones for each sub-class of WP_Privacy_Request_Table (e.g., export_personal_data_row_actions and remove_personal_data_row_actions)? WP_Privacy_Data_Export_Requests_List_Table and WP_Privacy_Data_Removal_Requests_List_Table are unique in core in that they extend an abstract class that itself extends WP_List_Table. There is precedent in core for both a "specific" and a "generic" hook, e.g., save_post_{$post->post_type} and save_post.
  1. the markup for the 2 "builtin" row actions uses a <div> tag to wrap a couple of <span> tags. That <div> causes when WP_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>?

@pbiron
5 years ago

example of the using a <span> tag in the builtin "Download Personal Data" and "Erase Personal Data" row actions

@pbiron
5 years ago

example showing 1 possible use of display_states...to indicate a request from a "VIP"

@pbiron
5 years ago

example of use of extra_tablenav() to filter requests by date of request and a custom taxonomy

#8 @pbiron
5 years ago

  • Keywords has-screenshots added

All the screenshots I've been adding are from a small plugin I'm writing to test the in-progress patch for this ticket. When I'm ready to post the patch I'll make that plugin available on GitHub.

Note: See TracTickets for help on using tickets.