Skip to content

Commit

Permalink
Merge pull request #4534 from ampproject/fix/4448-filter-list-table-r…
Browse files Browse the repository at this point in the history
…ow-actions

Ensure other plugins won't interfere with validated URLs' post_row_actions or meta boxes
  • Loading branch information
westonruter authored Apr 21, 2020
2 parents 07b638e + 2a2c0d7 commit 28a33fb
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 13 deletions.
28 changes: 25 additions & 3 deletions includes/validation/class-amp-validated-url-post-type.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public static function add_admin_hooks() {

// Edit post screen hooks.
add_action( 'admin_enqueue_scripts', [ __CLASS__, 'enqueue_edit_post_screen_scripts' ] );
add_action( 'add_meta_boxes', [ __CLASS__, 'add_meta_boxes' ] );
add_action( 'add_meta_boxes', [ __CLASS__, 'add_meta_boxes' ], PHP_INT_MAX );
add_action( 'edit_form_after_title', [ __CLASS__, 'render_single_url_list_table' ] );
add_filter( 'edit_' . AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_per_page', [ __CLASS__, 'get_terms_per_page' ] );
add_action( 'admin_init', [ __CLASS__, 'add_taxonomy' ] );
Expand Down Expand Up @@ -226,7 +226,7 @@ static function() {
add_action( 'admin_action_' . self::VALIDATE_ACTION, [ __CLASS__, 'handle_validate_request' ] );
add_action( 'post_action_' . self::UPDATE_POST_TERM_STATUS_ACTION, [ __CLASS__, 'handle_validation_error_status_update' ] );
add_action( 'admin_menu', [ __CLASS__, 'add_admin_menu_new_invalid_url_count' ] );
add_filter( 'post_row_actions', [ __CLASS__, 'filter_post_row_actions' ], 10, 2 );
add_filter( 'post_row_actions', [ __CLASS__, 'filter_post_row_actions' ], PHP_INT_MAX, 2 );
add_filter( sprintf( 'views_edit-%s', self::POST_TYPE_SLUG ), [ __CLASS__, 'filter_table_views' ] );
add_filter( 'bulk_post_updated_messages', [ __CLASS__, 'filter_bulk_post_updated_messages' ], 10, 2 );
add_filter( 'admin_title', [ __CLASS__, 'filter_admin_title' ] );
Expand Down Expand Up @@ -1823,9 +1823,13 @@ public static function enqueue_edit_post_screen_scripts() {
/**
* Adds the meta boxes to the CPT post.php page.
*
* @global array $wp_meta_boxes
* @return void
*/
public static function add_meta_boxes() {
global $wp_meta_boxes;
$stylesheets_metabox_id = 'amp_stylesheets';

remove_meta_box( 'submitdiv', self::POST_TYPE_SLUG, 'side' );
remove_meta_box( 'slugdiv', self::POST_TYPE_SLUG, 'normal' );
add_meta_box(
Expand All @@ -1838,14 +1842,27 @@ public static function add_meta_boxes() {
[ '__back_compat_meta_box' => true ]
);
add_meta_box(
'amp_stylesheets',
$stylesheets_metabox_id,
__( 'Stylesheets', 'amp' ),
[ __CLASS__, 'print_stylesheets_meta_box' ],
self::POST_TYPE_SLUG,
'normal',
'default',
[ '__back_compat_meta_box' => true ]
);

// Ensure only the expected metaboxes are shown on this screen.
// Note the O(n^3) complexity here is not a concern because each nested array has only a few items.
$allowed_metaboxes = [ 'slugdiv', 'submitdiv', self::STATUS_META_BOX, $stylesheets_metabox_id ];
foreach ( $wp_meta_boxes[ self::POST_TYPE_SLUG ] as $context => $metabox_contexts ) {
foreach ( $metabox_contexts as $metabox_priorities ) {
foreach ( array_keys( $metabox_priorities ) as $id ) {
if ( ! in_array( $id, $allowed_metaboxes, true ) ) {
remove_meta_box( $id, self::POST_TYPE_SLUG, $context );
}
}
}
}
}

/**
Expand Down Expand Up @@ -2856,6 +2873,11 @@ public static function filter_post_row_actions( $actions, $post ) {
);
}

$actions = wp_array_slice_assoc(
$actions,
[ 'edit', self::VALIDATE_ACTION, 'view', 'delete' ]
);

return $actions;
}

Expand Down
8 changes: 7 additions & 1 deletion includes/validation/class-amp-validation-error-taxonomy.php
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ static function() {
);
add_filter( 'terms_clauses', [ __CLASS__, 'filter_terms_clauses_for_description_search' ], 10, 3 );
add_action( 'admin_notices', [ __CLASS__, 'add_admin_notices' ] );
add_filter( self::TAXONOMY_SLUG . '_row_actions', [ __CLASS__, 'filter_tag_row_actions' ], 10, 2 );
add_filter( self::TAXONOMY_SLUG . '_row_actions', [ __CLASS__, 'filter_tag_row_actions' ], PHP_INT_MAX, 2 );
if ( get_taxonomy( self::TAXONOMY_SLUG )->show_in_menu ) {
add_action( 'admin_menu', [ __CLASS__, 'add_admin_menu_validation_error_item' ] );
}
Expand Down Expand Up @@ -1679,6 +1679,12 @@ public static function filter_tag_row_actions( $actions, WP_Term $tag ) {
);
}
}

$actions = wp_array_slice_assoc(
$actions,
[ 'details', self::VALIDATION_ERROR_ACCEPT_ACTION, self::VALIDATION_ERROR_REJECT_ACTION ]
);

return $actions;
}

Expand Down
72 changes: 67 additions & 5 deletions tests/php/validation/test-class-amp-validated-url-post-type.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ public function test_add_admin_hooks() {
$this->assertEquals( 10, has_action( 'rightnow_end', [ self::TESTED_CLASS, 'print_dashboard_glance_styles' ] ) );

$this->assertEquals( 10, has_action( 'admin_enqueue_scripts', [ self::TESTED_CLASS, 'enqueue_edit_post_screen_scripts' ] ) );
$this->assertEquals( 10, has_action( 'add_meta_boxes', [ self::TESTED_CLASS, 'add_meta_boxes' ] ) );
$this->assertEquals( PHP_INT_MAX, has_action( 'add_meta_boxes', [ self::TESTED_CLASS, 'add_meta_boxes' ] ) );
$this->assertEquals( 10, has_action( 'edit_form_top', [ self::TESTED_CLASS, 'print_url_as_title' ] ) );

$this->assertEquals( 10, has_filter( 'the_title', [ self::TESTED_CLASS, 'filter_the_title_in_post_list_table' ] ) );
$this->assertEquals( 10, has_filter( 'restrict_manage_posts', [ self::TESTED_CLASS, 'render_post_filters' ] ) );
$this->assertEquals( 10, has_filter( 'manage_' . AMP_Validated_URL_Post_Type::POST_TYPE_SLUG . '_posts_columns', [ self::TESTED_CLASS, 'add_post_columns' ] ) );
$this->assertEquals( 10, has_filter( 'manage_' . AMP_Validated_URL_Post_Type::POST_TYPE_SLUG . '_columns', [ self::TESTED_CLASS, 'add_single_post_columns' ] ) );
$this->assertEquals( 10, has_action( 'manage_posts_custom_column', [ self::TESTED_CLASS, 'output_custom_column' ] ) );
$this->assertEquals( 10, has_filter( 'post_row_actions', [ self::TESTED_CLASS, 'filter_post_row_actions' ] ) );
$this->assertEquals( PHP_INT_MAX, has_filter( 'post_row_actions', [ self::TESTED_CLASS, 'filter_post_row_actions' ] ) );
$this->assertEquals( 10, has_filter( 'bulk_actions-edit-' . AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, [ self::TESTED_CLASS, 'filter_bulk_actions' ] ) );
$this->assertEquals( 10, has_filter( 'bulk_actions-' . AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, '__return_false' ) );
$this->assertEquals( 10, has_filter( 'handle_bulk_actions-edit-' . AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, [ self::TESTED_CLASS, 'handle_bulk_action' ] ) );
Expand Down Expand Up @@ -1167,7 +1167,15 @@ public function test_render_link_to_error_index_screen() {
*/
public function test_add_meta_boxes() {
global $wp_meta_boxes;
AMP_Validated_URL_Post_Type::add_meta_boxes();
AMP_Validated_URL_Post_Type::add_admin_hooks();
add_action(
'add_meta_boxes',
function () {
add_meta_box( 'bogus', 'Bogus', '__return_empty_string', AMP_Validated_URL_Post_Type::POST_TYPE_SLUG );
},
100
);
do_action( 'add_meta_boxes' );
$side_meta_box = $wp_meta_boxes[ AMP_Validated_URL_Post_Type::POST_TYPE_SLUG ]['side']['default'][ AMP_Validated_URL_Post_Type::STATUS_META_BOX ];
$this->assertEquals( AMP_Validated_URL_Post_Type::STATUS_META_BOX, $side_meta_box['id'] );
$this->assertEquals( 'Status', $side_meta_box['title'] );
Expand All @@ -1183,10 +1191,12 @@ public function test_add_meta_boxes() {
$side_meta_box['args']
);

$contexts = $wp_meta_boxes[ AMP_Validated_URL_Post_Type::POST_TYPE_SLUG ]['side'];
foreach ( $contexts as $context ) {
foreach ( $wp_meta_boxes[ AMP_Validated_URL_Post_Type::POST_TYPE_SLUG ]['side'] as $context ) {
$this->assertFalse( $context['submitdiv'] );
}
foreach ( $wp_meta_boxes[ AMP_Validated_URL_Post_Type::POST_TYPE_SLUG ]['advanced'] as $context ) {
$this->assertFalse( $context['bogus'] );
}
}

/**
Expand Down Expand Up @@ -1678,4 +1688,56 @@ public function get_mock_errors() {
],
];
}

/**
* Test that the code ensures other plugins won't mess up the validation URL action links in the post list table.
*/
public function test_post_row_actions_filter() {
wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) );
AMP_Validated_URL_Post_Type::add_admin_hooks();

$post = self::factory()->post->create_and_get(
[
'post_type' => AMP_Validated_URL_Post_Type::POST_TYPE_SLUG,
'status' => 'publish',
]
);

add_filter(
'post_row_actions',
static function ( $actions, $post ) {
$actions['edit'] = sprintf(
'<a href="%s">%s</a>',
esc_url( get_edit_post_link( $post ) ),
'Unwanted Edit Action'
);

$actions['other_action'] = sprintf(
'<a href="%s">%s</a>',
'https://example.com',
'Unwanted Other Action'
);

return $actions;
},
10,
2
);

$initial_actions = [
'trash' => sprintf( '<a href="%s">Trash</a>', get_delete_post_link( $post->ID ) ),
];

$actions = apply_filters( 'post_row_actions', $initial_actions, $post );

$this->assertInternalType( 'array', $actions );
$this->assertArrayHasKey( 'edit', $actions );
$this->assertArrayHasKey( 'view', $actions );
$this->assertArrayHasKey( 'delete', $actions );
$this->assertArrayHasKey( 'amp_validate', $actions );
$this->assertArrayNotHasKey( 'other_action', $actions );

$this->assertStringContains( __( 'Details', 'amp' ), $actions['edit'] );
$this->assertStringNotContains( 'Unwanted Edit Action', $actions['edit'] );
}
}
25 changes: 21 additions & 4 deletions tests/php/validation/test-class-amp-validation-error-taxonomy.php
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ public function test_add_admin_hooks() {
$this->assertEquals( 10, has_filter( 'user_has_cap', [ self::TESTED_CLASS, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ] ) );
$this->assertEquals( 10, has_filter( 'terms_clauses', [ self::TESTED_CLASS, 'filter_terms_clauses_for_description_search' ] ) );
$this->assertEquals( 10, has_action( 'admin_notices', [ self::TESTED_CLASS, 'add_admin_notices' ] ) );
$this->assertEquals( 10, has_filter( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_row_actions', [ self::TESTED_CLASS, 'filter_tag_row_actions' ] ) );
$this->assertEquals( PHP_INT_MAX, has_filter( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_row_actions', [ self::TESTED_CLASS, 'filter_tag_row_actions' ] ) );
$this->assertEquals( 10, has_action( 'admin_menu', [ self::TESTED_CLASS, 'add_admin_menu_validation_error_item' ] ) );
$this->assertEquals( 10, has_filter( 'parse_term_query', [ self::TESTED_CLASS, 'parse_post_php_term_query' ] ) );
$this->assertEquals( 10, has_filter( 'manage_' . AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_custom_column', [ self::TESTED_CLASS, 'filter_manage_custom_columns' ] ) );
Expand Down Expand Up @@ -985,6 +985,7 @@ public function test_filter_tag_row_actions() {
AMP_Validation_Error_Taxonomy::register();
$initial_actions = [
'delete' => '<a href="#">Delete</a>',
'bad' => 'So bad!',
];

// The term is for this taxonomy, so this should filter the actions.
Expand All @@ -994,9 +995,25 @@ public function test_filter_tag_row_actions() {
'description' => wp_json_encode( $this->get_mock_error() ),
]
);
$filtered_actions = AMP_Validation_Error_Taxonomy::filter_tag_row_actions( $initial_actions, $term_this_taxonomy );
$reject_action = $filtered_actions[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECT_ACTION ];
$accept_action = $filtered_actions[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPT_ACTION ];

AMP_Validation_Error_Taxonomy::add_admin_hooks();

add_filter(
AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_row_actions',
function ( $actions ) {
$actions['also_bad'] = 'Also bad!';
return $actions;
},
1000
);

$filtered_actions = apply_filters( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_row_actions', $initial_actions, get_term( $term_this_taxonomy ) );
$this->assertEqualSets(
[ 'details', AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPT_ACTION, AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECT_ACTION ],
array_keys( $filtered_actions )
);
$reject_action = $filtered_actions[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECT_ACTION ];
$accept_action = $filtered_actions[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPT_ACTION ];
$this->assertStringContains( strval( $term_this_taxonomy->term_id ), $reject_action );
$this->assertStringContains( strval( $term_this_taxonomy->term_id ), $accept_action );
$this->assertStringContains( 'Keep', $reject_action );
Expand Down

0 comments on commit 28a33fb

Please sign in to comment.