Issue 1364: Improved "Error isolation" view

assets/css/amp-validation-error-taxonomy.css

@@ -37,11 +37,10 @@ details[open] .details-attributes__summary {
 	margin-bottom: 15px;
 }
 
-.details-attributes__summary::-webkit-details-marker {
+.details-attributes__summary::-webkit-details-marker, .notice details > summary::-webkit-details-marker {
 	display: none;
 }
-
-.details-attributes__summary::after {
+.details-attributes__summary::after, .single-error-detail-summary::after {
 	order: 99;
 	width: 12px;
 	height: 12px;
@@ -51,7 +50,7 @@ details[open] .details-attributes__summary {
 	content: "";
 }
 
-details[open] .details-attributes__summary::after {
+details[open] .details-attributes__summary::after, details.single-error-detail[open] .single-error-detail-summary::after {
 	transform: rotate(180deg);
 }
 
@@ -145,4 +144,52 @@ details[open] .details-attributes__summary::after {
 .status-text.rejected::before {
 	background-image: url( '../images/baseline-error-blue.svg' );
 }
+.single-error-detail {
+	margin: 5px 0 5px 0;
+}
+.single-error-detail-summary:after {
+	display: inline-block;
+}
+.single-error-detail-summary strong {
+	margin-right: 10px;
+	font-size: 15px;
+}
+.single-error-detail ul.secondary-details-array .details-attributes__attr {
+	margin-left: 20px;
+}
+.single-error-detail ul.secondary-details-array .details-attributes__value {
+	margin-left: 30px;
+}
+.single-error-detail .details-attributes__value {
+	margin-left: 10px;
+}
 
+.notice.accept-reject-error {
+	display: flex;
+}
+
+.notice.accept-reject-error > p {
+	display: inline-block;
+	font-size: 14px;
+	flex-grow: 10;
+	margin-right: 20px;
+}
+.notice.accept-reject-error > .button {
+	display: inline-block;
+	margin: 5px 5px 0 5px;
+	padding: 0 26px 2px;
+	flex-grow: 1;
+	text-align: center;
+}
+.notice.accept-reject-error > .button.accept {
+	/* @todo Add green colors */
+}
+.notice.accept-reject-error > .button.reject {
+	/* @todo Add red colors */
+}
+
+.wp-heading-inline .status-text {
+	display: inline-flex;
+	margin-left: 10px;
+	vertical-align: middle;
+}

includes/validation/class-amp-invalid-url-post-type.php

@@ -153,24 +153,25 @@ public static function add_admin_hooks() {
 		add_filter( 'bulk_post_updated_messages', array( __CLASS__, 'filter_bulk_post_updated_messages' ), 10, 2 );
 
 		// Hide irrelevant "published" label in the invalid URL post list.
-		add_filter( 'post_date_column_status', function( $status, $post ) {
+		add_filter( 'post_date_column_status', function ( $status, $post ) {
 			if ( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG === get_post_type( $post ) ) {
 				$status = '';
 			}
+
 			return $status;
 		}, 10, 2 );
 
 		// Prevent query vars from persisting after redirect.
-		add_filter( 'removable_query_args', function( $query_vars ) {
+		add_filter( 'removable_query_args', function ( $query_vars ) {
 			$query_vars[] = 'amp_actioned';
 			$query_vars[] = 'amp_taxonomy_terms_updated';
 			$query_vars[] = AMP_Invalid_URL_Post_Type::REMAINING_ERRORS;
 			$query_vars[] = 'amp_urls_tested';
 			$query_vars[] = 'amp_validate_error';
+
 			return $query_vars;
 		} );
 	}
-
 	/**
 	 * Enqueue style.
 	 */
@@ -717,6 +718,13 @@ public static function add_post_columns( $columns ) {
 			$columns['date'] = esc_html__( 'Last Checked', 'amp' );
 		}
 
+		if ( ! empty( $_GET[ \AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ] ) ) { // WPCS: CSRF OK.
+			unset( $columns['error_status'], $columns[ AMP_Validation_Error_Taxonomy::REMOVED_ELEMENTS ], $columns[ AMP_Validation_Error_Taxonomy::REMOVED_ATTRIBUTES ] );
+			$columns[ AMP_Validation_Error_Taxonomy::SOURCES_INVALID_OUTPUT ] = esc_html__( 'Sources', 'amp' );
+			$columns['date']  = esc_html__( 'Last Checked', 'amp' );
+			$columns['title'] = esc_html__( 'URL', 'amp' );
+		}
+
 		return $columns;
 	}
 
@@ -1026,6 +1034,126 @@ public static function print_admin_notice() {
 				wp_kses_post( $message )
 			);
 		}
+
+		/**
+		 * Adds notices to the single error page.
+		 * 1. Notice with detailed error information in an expanding box.
+		 * 2. Notice with accept and reject buttons.
+		 */
+		if ( ! empty( $_GET[ \AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ] ) && isset( $_GET['post_type'] ) && self::POST_TYPE_SLUG === $_GET['post_type'] ) { // WPCS: CSRF OK.
+			$error_id = sanitize_key( wp_unslash( $_GET[ \AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ] ) ); // WPCS: CSRF OK.
+
+			// @todo When PR #1429 is merged switch this to use AMP_Validation_Error_Taxonomy::get_term()
+			$error = get_term_by( 'slug', $error_id, \AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
+			if ( ! $error ) {
+				return;
+			}
+
+			// @todo Update this to use the method which will be developed in PR #1429 AMP_Validation_Error_Taxonomy::get_term_error() .
+			$description  = json_decode( $error->description, true );
+			$sanitization = \AMP_Validation_Error_Taxonomy::get_validation_error_sanitization( $description );
+			$status_text  = \AMP_Validation_Error_Taxonomy::get_status_text_with_icon( $sanitization['term_status'], $sanitization['forced'] );
+			$error_code   = isset( $description['code'] ) ? $description['code'] : 'error';
+			$error_title  = \AMP_Validation_Error_Taxonomy::get_error_title_from_code( $error_code );
+
+			$output = '';
+			foreach ( $description as $desc_name => $desc_info ) {
+				if ( ! is_array( $desc_info ) ) {
+					$output .= sprintf( '<li><span class="details-attributes__title">%s:</span><br/><span class="details-attributes__value">%s</span></li>', $desc_name, $desc_info );
+				} else {
+					$output .= sprintf( '<ul class="secondary-details-array"><span class="details-attributes__title">%s:</span><br/>', $desc_name );
+					foreach ( $desc_info as $sec_desc_info_name => $sec_desc_info ) {
+						$output .= sprintf( '<li><span class="details-attributes__attr">%s:</span><br/><span class="details-attributes__value">%s</span></li>', $sec_desc_info_name, $sec_desc_info );
+					}
+					$output .= '</ul>';
+				}
+			}
+
+			printf(
+				'<div class="notice"><details class="single-error-detail"><summary class="single-error-detail-summary"><strong>%s</strong></summary><ul>%s</ul></details></div>',
+				esc_html( $error_title ),
+				wp_kses_post( $output )
+			);
+
+			$accept_all_url = wp_nonce_url(
+				add_query_arg(
+					array(
+						'action'  => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPT_ACTION,
+						'term_id' => $error->term_id,
+					)
+				),
+				AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPT_ACTION
+			);
+			$reject_all_url = wp_nonce_url(
+				add_query_arg(
+					array(
+						'action'  => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECT_ACTION,
+						'term_id' => $error->term_id,
+					)
+				),
+				AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECT_ACTION
+			);
+
+			if ( ! $sanitization['forced'] ) {
+				echo '<div class="notice accept-reject-error">';
+
+				// @todo Update once https://github.com/Automattic/amp-wp/pull/1429 is merged.
+				if ( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS === $sanitization['term_status'] ) {
+					if ( amp_is_canonical() ) {
+						$info = __( 'Rejecting an error means that any URL on which it occurs will not be served as AMP.', 'amp' );
+					} else {
+						$info = __( 'Rejecting an error means that any URL on which it occurs will redirect to the non-AMP version.', 'amp' );
+					}
+					printf(
+						'<p>%s</p><a class="button button-primary reject" href="%s">%s</a>',
+						esc_html__( 'Reject this validation error for all instances.', 'amp' ) . ' ' . esc_html( $info ),
+						esc_url( $reject_all_url ),
+						esc_html__( 'Reject', 'amp' )
+					);
+				} elseif ( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECTED_STATUS === $sanitization['term_status'] ) {
+					if ( amp_is_canonical() ) {
+						$info = __( 'Accepting all validation errors which occur on a URL will allow it to be served as AMP.', 'amp' );
+					} else {
+						$info = __( 'Accepting all validation errors which occur on a URL will allow it to be served as AMP.', 'amp' );
+					}
+					printf(
+						'<p>%s</p><a class="button button-primary accept" href="%s">%s</a>',
+						esc_html__( 'Accept this error for all instances.', 'amp' ) . ' ' . esc_html( $info ),
+						esc_url( $accept_all_url ),
+						esc_html__( 'Accept', 'amp' )
+					);
+				} else {
+					if ( amp_is_canonical() ) {
+						$info = __( 'Rejecting an error means that any URL on which it occurs will not be served as AMP. If all errors occurring on a URL are accepted, then it will be served as AMP.', 'amp' );
+					} else {
+						$info = __( 'Rejecting an error means that any URL on which it occurs will redirect to the non-AMP version. If all errors occurring on a URL are accepted, then it will not redirect.', 'amp' );
+					}
+					printf(
+						'<p>%s</p><a class="button reject" href="%s">%s</a><a class="button button-primary accept" href="%s">%s</a>',
+						esc_html__( 'Accept or Reject this error for all instances.', 'amp' ) . ' ' . esc_html( $info ),
+						esc_url( $reject_all_url ),
+						esc_html__( 'Reject', 'amp' ),
+						esc_url( $accept_all_url ),
+						esc_html__( 'Accept', 'amp' )
+					);
+				}
+				echo '</div>';
+			}
+
+			$heading = sprintf(
+				'%s: <code>%s</code>%s',
+				esc_html( $error_title ),
+				esc_html( $description['node_name'] ),
+				wp_kses_post( $status_text )
+			);
+			?>
+			<script type="text/javascript">
+				jQuery( function( $ ) {
+					$( 'h1.wp-heading-inline' ).html( <?php echo wp_json_encode( $heading ); ?> );
+				});
+			</script>
+			<?php
+		}
 	}
 
 	/**

includes/validation/class-amp-validation-error-taxonomy.php

@@ -588,6 +588,7 @@ public static function add_admin_hooks() {
 		add_filter( 'posts_where', array( __CLASS__, 'filter_posts_where_for_validation_error_status' ), 10, 2 );
 		add_filter( 'handle_bulk_actions-edit-' . self::TAXONOMY_SLUG, array( __CLASS__, 'handle_validation_error_update' ), 10, 3 );
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'handle_inline_edit_request' ) );
+		add_action( 'load-edit.php', array( __CLASS__, 'handle_inline_edit_request' ) );
 
 		// Prevent query vars from persisting after redirect.
 		add_filter( 'removable_query_args', function( $query_vars ) {
@@ -1361,25 +1362,7 @@ public static function filter_manage_custom_columns( $content, $column_name, $te
 				break;
 			case 'status':
 				$sanitization = self::get_validation_error_sanitization( $validation_error );
-				if ( self::VALIDATION_ERROR_ACCEPTED_STATUS === $sanitization['term_status'] ) {
-					if ( $sanitization['forced'] && $sanitization['term_status'] !== $sanitization['status'] ) {
-						$class = 'sanitized';
-					} else {
-						$class = 'accepted';
-					}
-					$text = __( 'Accepted', 'amp' );
-				} elseif ( self::VALIDATION_ERROR_REJECTED_STATUS === $sanitization['term_status'] ) {
-					if ( $sanitization['forced'] && $sanitization['term_status'] !== $sanitization['status'] ) {
-						$class = 'sanitized';
-					} else {
-						$class = 'rejected';
-					}
-					$text = __( 'Rejected', 'amp' );
-				} else {
-					$class = 'new';
-					$text  = __( 'New', 'amp' );
-				}
-				$content .= sprintf( '<span class="status-text %s">%s</span>', esc_attr( $class ), esc_html( $text ) );
+				$content     .= self::get_status_text_with_icon( $sanitization['term_status'], $sanitization['forced'] );
 				break;
 			case 'created_date_gmt':
 				$created_datetime = null;
@@ -1485,7 +1468,18 @@ public static function filter_manage_custom_columns( $content, $column_name, $te
 	 * Handle inline edit links.
 	 */
 	public static function handle_inline_edit_request() {
-		if ( self::TAXONOMY_SLUG !== get_current_screen()->taxonomy || ! isset( $_GET['action'] ) || ! isset( $_GET['_wpnonce'] ) || ! isset( $_GET['term_id'] ) ) { // WPCS: CSRF ok.
+		// Check for necessary arguments.
+		if ( ! isset( $_GET['action'] ) || ! isset( $_GET['_wpnonce'] ) || ! isset( $_GET['term_id'] ) ) {  // WPCS: CSRF ok.
+			return;
+		}
+
+		// Check if we are on either the taxonomy page or a single error page (which has the post_type argument).
+		if ( self::TAXONOMY_SLUG !== get_current_screen()->taxonomy && ! isset( $_GET['post_type'] ) ) { // WPCS: CSRF ok.
+			return;
+		}
+
+		// If we have a post_type check that it is the correct one.
+		if ( isset( $_GET['post_type'] ) && \AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG !== $_GET['post_type'] ) { // WPCS: CSRF ok.
 			return;
 		}
 		$action = sanitize_key( $_GET['action'] ); // WPCS: CSRF ok.
@@ -1544,4 +1538,63 @@ public static function handle_validation_error_update( $redirect_to, $action, $t
 
 		return $redirect_to;
 	}
+
+	/**
+	 * Get Error Title from Code
+	 *
+	 * @param string $error_code Error code.
+	 *
+	 * @return string
+	 */
+	public static function get_error_title_from_code( $error_code ) {
+		$error_title = 'Error';
+		if ( self::INVALID_ELEMENT_CODE === $error_code ) {
+			$error_title = __( 'Invalid element', 'amp' );
+		} elseif ( self::INVALID_ATTRIBUTE_CODE === $error_code ) {
+			$error_title = __( 'Invalid attribute', 'amp' );
+		} elseif ( 'file_path_not_allowed' === $error_code ) {
+			$error_title = __( 'File path not allowed', 'amp' );
+		} elseif ( 'excessive_css' === $error_code ) {
+			$error_title = __( 'Excessive CSS', 'amp' );
+		} elseif ( 'illegal_css_at_rule' === $error_code ) {
+			$error_title = __( 'Illegal CSS @ rule', 'amp' );
+		} elseif ( 'disallowed_file_extension' === $error_code ) {
+			$error_title = __( 'Disallowed file extension', 'amp' );
+		} elseif ( 'file_path_not_allowed' === $error_code ) {
+			$error_title = __( 'File path not allowed', 'amp' );
+		} elseif ( 'removed_unused_css_rules' === $error_code ) {
+			$error_title = __( 'Remove unused CSS rules', 'amp' );
+		}
+		return $error_title;
+	}
+
+	/**
+	 * Get Status Text with Icon
+	 *
+	 * @param string $status              Status.
+	 * @param string $sanitization_forced Sanitization forced.
+	 *
+	 * @return string
+	 */
+	public static function get_status_text_with_icon( $status, $sanitization_forced ) {
+		if ( self::VALIDATION_ERROR_ACCEPTED_STATUS === $status ) {
+			if ( $sanitization_forced && $sanitization_forced !== $status ) {
+				$class = 'sanitized';
+			} else {
+				$class = 'accepted';
+			}
+			$text = __( 'Accepted', 'amp' );
+		} elseif ( self::VALIDATION_ERROR_REJECTED_STATUS === $status ) {
+			if ( $sanitization_forced && $sanitization_forced !== $status ) {
+				$class = 'sanitized';
+			} else {
+				$class = 'rejected';
+			}
+			$text = __( 'Rejected', 'amp' );
+		} else {
+			$class = 'new';
+			$text  = __( 'New', 'amp' );
+		}
+		return sprintf( '<span class="status-text %s">%s</span>', esc_attr( $class ), esc_html( $text ) );
+	}
 }