Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filters on taxonomy and post page #1373

Merged
merged 30 commits into from
Sep 5, 2018
Merged

Add filters on taxonomy and post page #1373

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0017edf
Add a 'type' to all validation errors
kienstra Aug 28, 2018
3e9a81e
If an on* attribute is removed, make type js_error
kienstra Aug 28, 2018
0ae789d
In unit test assertions, change to js_error
kienstra Aug 28, 2018
3433791
Add a query var to filter by type
kienstra Aug 29, 2018
8dd3f01
Filter the term redirect location, to possibly add a query var
kienstra Aug 29, 2018
5f92cc2
Render the taxonomy filter for this taxonomy type
kienstra Aug 29, 2018
3d17734
Add a filter <select> element for error type, like 'Accepted'
kienstra Aug 29, 2018
c2656a3
Remove views for filtering taxonomy, but reuse their counts
kienstra Aug 29, 2018
fe43d31
Add the option value for 'All Statuses' to the whitelist
kienstra Aug 29, 2018
1e135d2
Use the new terminology for the error taxonomy page name
kienstra Aug 29, 2018
f1d4bea
Address failed unit tests in Travis build
kienstra Aug 29, 2018
caa2a26
Restore error counts by status in <option>
kienstra Aug 30, 2018
7b937b1
Merge branch 'develop' into add/1360-taxonomy-error-type
kienstra Aug 30, 2018
13d5caf
Add error types for HTML (Element) and HTML (Attribute)
kienstra Aug 30, 2018
b74854c
Refactor logic in render_taxonomy_filters() into 2 methods
kienstra Aug 31, 2018
0397a52
Add filters on the 'Errors by URL' page
kienstra Aug 31, 2018
228eb81
Filter for error type on Errors by URL page
kienstra Sep 3, 2018
a567047
Output the 'View errors by URL' link from the design
kienstra Sep 3, 2018
3bcfc03
Simplify add_term_filter_query_var()
kienstra Sep 3, 2018
13a8648
Make the error status counts accurate on the Errors by URL page
kienstra Sep 3, 2018
6c5c06d
On the 'Errors by URL' page, use 'With New Errors' instead of 'New Er…
kienstra Sep 3, 2018
6c262f0
Remove extra empty line to address Travis failure
kienstra Sep 3, 2018
3694bb9
Fix an issue where filtering by 'All Error Types' wasn't reflected in…
kienstra Sep 3, 2018
8443fd8
Improve inline documentation, address Travis error.
kienstra Sep 3, 2018
a13c91e
Skip showing status dropdown if there will be no options
westonruter Sep 5, 2018
61c6095
Simplify logic conditional in render_post_filters
westonruter Sep 5, 2018
364944e
Only add WHERE conditions when required in filter_posts_where_for_val…
westonruter Sep 5, 2018
ea15a2e
Fix translatability of strings which vary 'with'
westonruter Sep 5, 2018
2882262
Harden reading of input vars
westonruter Sep 5, 2018
3b87595
Opt to output all status options even when empty to prevent de-select…
westonruter Sep 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions includes/sanitizers/class-amp-base-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,11 @@ public function prepare_validation_error( array $error = array(), array $data =
if ( ! isset( $error['code'] ) ) {
$error['code'] = AMP_Validation_Error_Taxonomy::INVALID_ELEMENT_CODE;
}

if ( ! isset( $error['type'] ) ) {
$error['type'] = 'script' === $node->nodeName ? AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE : AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE;
}

$error['node_attributes'] = array();
foreach ( $node->attributes as $attribute ) {
$error['node_attributes'][ $attribute->nodeName ] = $attribute->nodeValue;
Expand All @@ -456,6 +461,10 @@ public function prepare_validation_error( array $error = array(), array $data =
if ( ! isset( $error['code'] ) ) {
$error['code'] = AMP_Validation_Error_Taxonomy::INVALID_ATTRIBUTE_CODE;
}
if ( ! isset( $error['type'] ) ) {
// If this is an attribute that begins with on, like onclick, it should be a js_error.
$error['type'] = preg_match( '/^on[a-zA-Z]/', $node->nodeName ) ? AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE : AMP_Validation_Error_Taxonomy::HTML_ATTRIBUTE_ERROR_TYPE;
}
$error['element_attributes'] = array();
if ( $node->parentNode && $node->parentNode->hasAttributes() ) {
foreach ( $node->parentNode->attributes as $attribute ) {
Expand Down
19 changes: 19 additions & 0 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,7 @@ private function process_link_element( DOMElement $element ) {
$this->remove_invalid_child( $element, array(
'code' => $css_file_path->get_error_code(),
'message' => $css_file_path->get_error_message(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
) );
return;
} else {
Expand All @@ -682,6 +683,7 @@ private function process_link_element( DOMElement $element ) {
$this->remove_invalid_child( $element, array(
'code' => $css_file_path->get_error_code(),
'message' => $css_file_path->get_error_message(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
) );
return;
} else {
Expand All @@ -691,6 +693,7 @@ private function process_link_element( DOMElement $element ) {
if ( false === $stylesheet ) {
$this->remove_invalid_child( $element, array(
'code' => 'stylesheet_file_missing',
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
) );
return;
}
Expand Down Expand Up @@ -887,6 +890,7 @@ private function parse_import_stylesheet( Import $item, CSSList $css_list, $opti
$error = array(
'code' => $contents->get_error_code(),
'message' => $contents->get_error_message(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
if ( $sanitized ) {
Expand All @@ -901,6 +905,7 @@ private function parse_import_stylesheet( Import $item, CSSList $css_list, $opti
$error = array(
'code' => $css_file_path->get_error_code(),
'message' => $css_file_path->get_error_message(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
if ( $sanitized ) {
Expand Down Expand Up @@ -988,6 +993,7 @@ function ( $value ) {
$error = array(
'code' => 'css_parse_error',
'message' => $exception->getMessage(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);

/*
Expand Down Expand Up @@ -1239,6 +1245,7 @@ private function process_css_list( CSSList $css_list, $options ) {
$error = array(
'code' => 'illegal_css_at_rule',
'at_rule' => $css_item->atRuleName(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
$results[] = compact( 'error', 'sanitized' );
Expand All @@ -1259,6 +1266,7 @@ private function process_css_list( CSSList $css_list, $options ) {
$error = array(
'code' => 'illegal_css_at_rule',
'at_rule' => $css_item->atRuleName(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
$results[] = compact( 'error', 'sanitized' );
Expand All @@ -1275,6 +1283,7 @@ private function process_css_list( CSSList $css_list, $options ) {
$error = array(
'code' => 'illegal_css_at_rule',
'at_rule' => $css_item->atRuleName(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
$results[] = compact( 'error', 'sanitized' );
Expand All @@ -1290,13 +1299,15 @@ private function process_css_list( CSSList $css_list, $options ) {
$error = array(
'code' => 'illegal_css_at_rule',
'at_rule' => $css_item->atRuleName(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
$results[] = compact( 'error', 'sanitized' );
} else {
$error = array(
'code' => 'unrecognized_css',
'item' => get_class( $css_item ),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
$results[] = compact( 'error', 'sanitized' );
Expand Down Expand Up @@ -1382,6 +1393,7 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l
'code' => 'illegal_css_property',
'property_name' => $property->getRule(),
'property_value' => $property->getValue(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
if ( $sanitized ) {
Expand All @@ -1398,6 +1410,7 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l
'code' => 'illegal_css_property',
'property_name' => $property->getRule(),
'property_value' => (string) $property->getValue(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
if ( $sanitized ) {
Expand Down Expand Up @@ -1556,6 +1569,7 @@ private function process_css_keyframes( KeyFrame $css_list, $options ) {
$error = array(
'code' => 'unrecognized_css',
'item' => get_class( $rules ),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
if ( $sanitized ) {
Expand All @@ -1578,6 +1592,7 @@ private function process_css_keyframes( KeyFrame $css_list, $options ) {
'code' => 'illegal_css_property',
'property_name' => $property->getRule(),
'property_value' => (string) $property->getValue(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
if ( $sanitized ) {
Expand Down Expand Up @@ -1623,6 +1638,7 @@ private function transform_important_qualifiers( RuleSet $ruleset, CSSList $css_
} else {
$error = array(
'code' => 'illegal_css_important',
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
$sanitized = $this->should_sanitize_validation_error( $error );
if ( $sanitized ) {
Expand Down Expand Up @@ -1901,6 +1917,7 @@ private function finalize_styles() {
if ( ! $body ) {
$this->should_sanitize_validation_error( array(
'code' => 'missing_body_element',
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
) );
} else {
$style_element = $this->dom->createElement( 'style' );
Expand Down Expand Up @@ -1997,6 +2014,7 @@ private function finalize_stylesheet_set( $stylesheet_set ) {
if ( $is_too_much_css && $should_tree_shake && empty( $this->args['accept_tree_shaking'] ) ) {
$should_tree_shake = $this->should_sanitize_validation_error( array(
'code' => self::TREE_SHAKING_ERROR_CODE,
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
) );
}

Expand Down Expand Up @@ -2066,6 +2084,7 @@ private function finalize_stylesheet_set( $stylesheet_set ) {
if ( $final_size + $sheet_size > $stylesheet_set['cdata_spec']['max_bytes'] ) {
$validation_error = array(
'code' => 'excessive_css',
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);
if ( isset( $pending_stylesheet['sources'] ) ) {
$validation_error['sources'] = $pending_stylesheet['sources'];
Expand Down
137 changes: 20 additions & 117 deletions includes/validation/class-amp-invalid-url-post-type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,8 @@ public static function add_admin_hooks() {
add_action( 'add_meta_boxes', array( __CLASS__, 'add_meta_boxes' ) );
add_action( 'edit_form_top', array( __CLASS__, 'print_url_as_title' ) );
add_filter( 'the_title', array( __CLASS__, 'filter_the_title_in_post_list_table' ), 10, 2 );
add_action( 'restrict_manage_posts', array( __CLASS__, 'render_post_filters' ), 10, 2 );

add_filter( 'views_edit-' . self::POST_TYPE_SLUG, array( __CLASS__, 'filter_views_edit' ) );
add_filter( 'manage_' . self::POST_TYPE_SLUG . '_posts_columns', array( __CLASS__, 'add_post_columns' ) );
add_action( 'manage_posts_custom_column', array( __CLASS__, 'output_custom_column' ), 10, 2 );
add_filter( 'post_row_actions', array( __CLASS__, 'filter_row_actions' ), 10, 2 );
Expand Down Expand Up @@ -497,122 +497,6 @@ public static function get_post_staleness( $post ) {
return $staleness;
}

/**
* Add views for filtering validation errors by status.
*
* @param array $views Views.
* @return array Views
*/
public static function filter_views_edit( $views ) {
unset( $views['publish'] );

$args = array(
'post_type' => self::POST_TYPE_SLUG,
'update_post_meta_cache' => false,
'update_post_term_cache' => false,
);

$with_new_query = new WP_Query( array_merge(
$args,
array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS )
) );
$with_rejected_query = new WP_Query( array_merge(
$args,
array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECTED_STATUS )
) );
$with_accepted_query = new WP_Query( array_merge(
$args,
array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS )
) );

$current_url = remove_query_arg(
array_merge(
wp_removable_query_args(),
array( 's' ) // For some reason behavior of posts list table is to not persist the search query.
),
wp_unslash( $_SERVER['REQUEST_URI'] )
);

$current_status = null;
if ( isset( $_GET[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ) { // WPCS: CSRF ok.
$value = intval( $_GET[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR ] ); // WPCS: CSRF ok.
if ( in_array( $value, array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS, AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS, AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECTED_STATUS ), true ) ) {
$current_status = $value;
}
}

$views['new'] = sprintf(
'<a href="%s" class="%s">%s</a>',
esc_url(
add_query_arg(
AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR,
AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS,
$current_url
)
),
AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS === $current_status ? 'current' : '',
sprintf(
/* translators: %s: the post count. */
_nx(
'With New Errors <span class="count">(%s)</span>',
'With New Errors <span class="count">(%s)</span>',
$with_new_query->found_posts,
'posts',
'amp'
),
number_format_i18n( $with_new_query->found_posts )
)
);

$views['rejected'] = sprintf(
'<a href="%s" class="%s">%s</a>',
esc_url(
add_query_arg(
AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR,
AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECTED_STATUS,
$current_url
)
),
AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECTED_STATUS === $current_status ? 'current' : '',
sprintf(
/* translators: %s: the post count. */
_nx(
'With Rejected Errors <span class="count">(%s)</span>',
'With Rejected Errors <span class="count">(%s)</span>',
$with_rejected_query->found_posts,
'posts',
'amp'
),
number_format_i18n( $with_rejected_query->found_posts )
)
);

$views['accepted'] = sprintf(
'<a href="%s" class="%s">%s</a>',
esc_url(
add_query_arg(
AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR,
AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS,
$current_url
)
),
AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS === $current_status ? 'current' : '',
sprintf(
/* translators: %s: the post count. */
_nx(
'With Accepted Errors <span class="count">(%s)</span>',
'With Accepted Errors <span class="count">(%s)</span>',
$with_accepted_query->found_posts,
'posts',
'amp'
),
number_format_i18n( $with_accepted_query->found_posts )
)
);

return $views;
}

/**
* Adds post columns to the UI for the validation errors.
*
Expand Down Expand Up @@ -1475,6 +1359,25 @@ public static function filter_the_title_in_post_list_table( $title, $post ) {
return $title;
}

/**
* Renders the filters on the invalid URL post type edit.php page.
*
* @param string $post_type The slug of the post type.
* @param string $which The location for the markup, either 'top' or 'bottom'.
*/
public static function render_post_filters( $post_type, $which ) {
if (
self::POST_TYPE_SLUG !== $post_type
||
'top' !== $which
) {
return;
}

AMP_Validation_Error_Taxonomy::render_error_status_filter();
AMP_Validation_Error_Taxonomy::render_error_type_filter();
}

/**
* Gets the URL to recheck the post for AMP validity.
*
Expand Down
Loading