WordPress.org

Make WordPress Core

Opened 16 months ago

Last modified 4 months ago

#44354 assigned enhancement

Improve `WP_Privacy_Requests_Table` to manage columns

Reported by: 7studio Owned by: garrett-eclipse
Milestone: Future Release Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch needs-testing
Focuses: administration Cc:
PR Number:

Description

It could be interesting to improve WP_Privacy_Requests_Table with the help of actions and filters, for consistency with other post list tables.

The best place to start is to add a filter like "manage_{$post_type}_posts_columns" and an action like "manage_{$post->post_type}_posts_custom_column" to enhance columns contents
or add new columns with content recovered by submission form or other.

This issue goes in the same way as #43960, #44015, #44025 or #44267

Hope you will consider this ticket and it contributes to WP.

Attachments (3)

44354.diff (2.0 KB) - added by birgire 16 months ago.
custom-column.jpg (32.5 KB) - added by birgire 16 months ago.
44354.2.diff (3.8 KB) - added by pbiron 4 months ago.
patch refresh and addition of 2 more hooks.

Download all attachments as: .zip

Change History (12)

#1 @desrosj
16 months ago

  • Focuses administration added
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

@birgire
16 months ago

#2 follow-up: @birgire
16 months ago

Thanks for the ticket @7studio

44354.diff is a first pass to improve the column management for the Requests list table.

We add the dynamic action in the default column handler:

do_action( "manage_{$this->request_type}_custom_column", $column_name, $item );

that generates the following actions:

  • manage_export_personal_data_custom_column.
  • manage_remove_personal_data_custom_column

We also add the following columns filter:

return apply_filters( "manage_{$this->request_type}_columns", $columns );

that generates the following filters:

  • manage_export_personal_data_columns.
  • manage_remove_personal_data_columns

Another naming suggestion could be:

  • "manage_{$this->request_type}_requests_custom_column"
  • "manage_{$this->request_type}_requests_columns"

but that generates longer hook names.

It's interesting to see the mismatch regarding the output of column methods in various list tables.

It's both echo and return, but from the WP_List_Table::single_row_columns() it seems to expect return:

} elseif ( method_exists( $this, 'column_' . $column_name ) ) {
	echo "<td $attributes>";
	echo call_user_func( array( $this, 'column_' . $column_name ), $item );
	echo $this->handle_row_actions( $item, $column_name, $primary );
	echo '</td>';
} else {
	echo "<td $attributes>";
	echo $this->column_default( $item, $column_name );
	echo $this->handle_row_actions( $item, $column_name, $primary );
	echo '</td>';
}

Test Example

Here's a test example where we add a custom column called "Resend Email":

add_action( 'manage_export_personal_data_custom_column', function( $column_name, $item ) {
    if ( 'resend_email' === $column_name ) {
        printf( '<input type="button" class="button" value="%s" />', esc_attr( __( 'Resend Email' ) ) );
	}
}, 10, 2 );

add_filter('manage_export_personal_data_columns', function( $columns ) {
    if( is_array( $columns ) && ! isset( $columns['resend_email'] ) ) {
        $columns['resend_email'] = __( 'Resend Email' );
	}
    return $columns;
} );


See the screenshot in custom-column.jpg.

#3 @birgire
16 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

#4 follow-up: @7studio
15 months ago

Sorry for this late answer @birgire

Thank you very much for this patch :D IMHO, long hook names are not a problem. Most of the time, it's more understandable ;)

It's interesting to see the mismatch regarding the output of column methods in various list tables.

It's the same thing in examples on the Web but I've always used echo ;)

If I must do something let me know.

#5 in reply to: ↑ 4 @birgire
14 months ago

Replying to 7studio:

Sorry for this late answer @birgire

No problem :-)

Thank you very much for this patch :D IMHO, long hook names are not a problem. Most of the time, it's more understandable ;)

So you would prefer, the longer hook name version here?

It's interesting to see the mismatch regarding the output of column methods in various list tables.

It's the same thing in examples on the Web but I've always used echo ;)

Yes, since both approaches work, it's understandable that both versions are being used out there.

If I must do something let me know.

It would be great if you could test the patch and the example above, when you've time. Thanks.

#6 @garrett-eclipse
8 months ago

  • Keywords needs-refresh added
  • Owner set to garrett-eclipse
  • Status changed from new to assigned

Hi @birgire thank you for the patch.

Applying it I found the table vanished and gone due to PHP errors exhausting memory;
PHP Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 20480 bytes) in .../wp-includes/class-wp-hook.php on line 271
Same error affected a few files. Looking deeper the cause is the update to column_default which introduced the "manage_{$this->request_type}_custom_column" action. The column_default function expects a return so returning with apply_filter should work.

Reviewing the original code I believe we want to preserve the @return, as well as the $cell_value = $item->$column_name; and then apply a filter to that with a return call.
*Although maybe just use $column_name not sure why the original code needed to access it from $item.

An existing example in core;
https://github.com/WordPress/WordPress/blob/56c162fbc9867f923862f64f1b4570d885f1ff03/wp-admin/includes/class-wp-terms-list-table.php#L579-L598

All the best

#8 in reply to: ↑ 2 @pbiron
4 months ago

Replying to birgire:

It's interesting to see the mismatch regarding the output of column methods in various list tables.

It's both echo and return, but from the WP_List_Table::single_row_columns() it seems to expect return:

Yeah, single_row_columns() appears to expect a value, which implies that we should use a filter instead of an action (as in the example from WP_Terms_List_Table mentioned by garrett-eclipse).

However, of the 11 core list tables that currently support custom columns, only 2 of them use a filter and return a value (WP_Users_List_Table is the other one). You can see that by searching the Code Reference for hooks that use "custom_column".

So, my vote is to go with action, as it is more prevalent in core already.

@pbiron
4 months ago

patch refresh and addition of 2 more hooks.

#9 @pbiron
4 months ago

  • Keywords needs-refresh removed

44354.2.diff refreshes the patch after the file reorganization in #43895. It also updates the @since tags to `5.3.0'.

Additionally, it adds 2 new hooks:

  1. manage_privacy_requests_columns (in get_columns(), this is a filter). This allows for adding columns that are "agnostic" as to the request type (similar to wp_insert_post()'s use of both save_post and save_post_{$post_type}). The request type is passed as the 2nd param, so it can also be used to output columns that are specific to the request type.
  2. manage_privacy_request_custom_column (in column_default(), this is an action, see comment 8). Again, this allows for outputting values for custom columns that are both "agnostic" as to request-type and request-type-specific columns.

The hooks in 44354.diff are still there. These new hooks just give plugin authors different ways to add/output custom columns.

p.s. I have not experienced the memory problems mentioned by garrett-eclipse. I didn't try the initial patch until after #43895...maybe that had something to do with it?

Note: See TracTickets for help on using tickets.