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.

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

Expose and store queried object for validated URL; show edit link #1426

Merged
merged 1 commit into from
Sep 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 8 additions & 3 deletions includes/class-amp-cli.php
Original file line number Diff line number Diff line change
Expand Up @@ -576,16 +576,21 @@ public static function validate_and_store_url( $url, $type ) {
self::$wp_cli_progress->tick();
}

AMP_Invalid_URL_Post_Type::store_validation_errors( $validity['validation_errors'], $validity['url'] );
$validation_errors = wp_list_pluck( $validity['results'], 'error' );
AMP_Invalid_URL_Post_Type::store_validation_errors(
$validation_errors,
$validity['url'],
wp_array_slice_assoc( $validity, array( 'queried_object' ) )
);
$unaccepted_error_count = count( array_filter(
$validity['validation_errors'],
$validation_errors,
function( $error ) {
$validation_status = AMP_Validation_Error_Taxonomy::get_validation_error_sanitization( $error );
return AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS !== $validation_status['term_status'];
}
) );

if ( count( $validity['validation_errors'] ) > 0 ) {
if ( count( $validation_errors ) > 0 ) {
self::$total_errors++;
}
if ( $unaccepted_error_count > 0 ) {
Expand Down
85 changes: 72 additions & 13 deletions includes/validation/class-amp-invalid-url-post-type.php
Original file line number Diff line number Diff line change
Expand Up @@ -346,17 +346,22 @@ public static function get_url_from_post( $post ) {
*
* If there are no validation errors provided, then any existing amp_invalid_url post is deleted.
*
* @param array $validation_errors Validation errors.
* @param string $url URL on which the validation errors occurred. Will be normalized to non-AMP version.
* @param int|WP_Post $post Post to update. Optional. If empty, then post is looked up by URL.
* @param array $validation_errors Validation errors.
* @param string $url URL on which the validation errors occurred. Will be normalized to non-AMP version.
* @param array $args {
* Args.
*
* @type int|WP_Post $invalid_url_post Post to update. Optional. If empty, then post is looked up by URL.
* @type array $queried_object Queried object, including keys for type and id. May be empty.
* }
* @return int|WP_Error $post_id The post ID of the custom post type used, or WP_Error on failure.
* @global WP $wp
*/
public static function store_validation_errors( $validation_errors, $url, $post = null ) {
public static function store_validation_errors( $validation_errors, $url, $args = array() ) {
$url = remove_query_arg( amp_get_slug(), $url ); // Only ever store the canonical version.
$slug = md5( $url );
if ( $post ) {
$post = get_post( $post );
if ( isset( $args['invalid_url_post'] ) ) {
$post = get_post( $args['invalid_url_post'] );
} else {
$post = get_page_by_path( $slug, OBJECT, self::POST_TYPE_SLUG );
if ( ! $post ) {
Expand Down Expand Up @@ -458,6 +463,9 @@ public static function store_validation_errors( $validation_errors, $url, $post
wp_set_object_terms( $post_id, wp_list_pluck( $terms, 'term_id' ), AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );

update_post_meta( $post_id, '_amp_validated_environment', self::get_validated_environment() );
if ( isset( $args['queried_object'] ) ) {
update_post_meta( $post_id, '_amp_queried_object', $args['queried_object'] );
}

return $post_id;
}
Expand Down Expand Up @@ -714,9 +722,14 @@ public static function handle_bulk_action( $redirect, $action, $items ) {
continue;
}

self::store_validation_errors( $validity['validation_errors'], $validity['url'], $post );
$validation_errors = wp_list_pluck( $validity['results'], 'error' );
self::store_validation_errors(
$validation_errors,
$validity['url'],
wp_array_slice_assoc( $validity, array( 'queried_object' ) )
);
$unaccepted_error_count = count( array_filter(
$validity['validation_errors'],
$validation_errors,
function( $error ) {
return ! AMP_Validation_Error_Taxonomy::is_validation_error_sanitized( $error );
}
Expand Down Expand Up @@ -890,14 +903,24 @@ public static function handle_validate_request() {
throw new Exception( esc_html( $validity->get_error_code() ) );
}

$stored = self::store_validation_errors( $validity['validation_errors'], $validity['url'], $post );
$errors = wp_list_pluck( $validity['results'], 'error' );
$stored = self::store_validation_errors(
$errors,
$validity['url'],
array_merge(
array(
'invalid_url_post' => $post,
),
wp_array_slice_assoc( $validity, array( 'queried_object' ) )
)
);
if ( is_wp_error( $stored ) ) {
throw new Exception( esc_html( $stored->get_error_code() ) );
}
$redirect = get_edit_post_link( $stored, 'raw' );

$error_count = count( array_filter(
$validity['validation_errors'],
$errors,
function ( $error ) {
return ! AMP_Validation_Error_Taxonomy::is_validation_error_sanitized( $error );
}
Expand Down Expand Up @@ -949,10 +972,20 @@ public static function recheck_post( $post ) {
return $validity;
}

$validation_errors = wp_list_pluck( $validity['results'], 'error' );
$validation_results = array();
self::store_validation_errors( $validity['validation_errors'], $validity['url'], $post->ID );
foreach ( $validity['validation_errors'] as $error ) {
$sanitized = AMP_Validation_Error_Taxonomy::is_validation_error_sanitized( $error );
self::store_validation_errors(
$validation_errors,
$validity['url'],
array_merge(
array(
'invalid_url_post' => $post,
),
wp_array_slice_assoc( $validity, array( 'queried_object' ) )
)
);
foreach ( $validation_errors as $error ) {
$sanitized = AMP_Validation_Error_Taxonomy::is_validation_error_sanitized( $error ); // @todo Consider re-using $validity['results'][x]['sanitized'], unless auto-sanitize is causing problem.

$validation_results[] = compact( 'error', 'sanitized' );
}
Expand Down Expand Up @@ -1155,6 +1188,32 @@ public static function print_status_meta_box( $post ) {
?>
<?php self::display_invalid_url_validation_error_counts_summary( $post ); ?>
</div>

<div class="misc-pub-section">
<?php
$view_label = __( 'View URL', 'amp' );
$queried_object = get_post_meta( $post->ID, '_amp_queried_object', true );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea to store the queried object. This will also help with displaying the title, like on this design:

will-help-with-title

if ( isset( $queried_object['id'] ) && isset( $queried_object['type'] ) ) {
$after = ' | ';
if ( 'post' === $queried_object['type'] && get_post( $queried_object['id'] ) && post_type_exists( get_post( $queried_object['id'] )->post_type ) ) {
$post_type_object = get_post_type_object( get_post( $queried_object['id'] )->post_type );
edit_post_link( $post_type_object->labels->edit_item, '', $after, $queried_object['id'] );
$view_label = $post_type_object->labels->view_item;
} elseif ( 'term' === $queried_object['type'] && get_term( $queried_object['id'] ) && taxonomy_exists( get_term( $queried_object['id'] )->taxonomy ) ) {
$taxonomy_object = get_taxonomy( get_term( $queried_object['id'] )->taxonomy );
edit_term_link( $taxonomy_object->labels->edit_item, '', $after, get_term( $queried_object['id'] ) );
Copy link
Contributor

@kienstra kienstra Sep 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit-term-link

$view_label = $taxonomy_object->labels->view_item;
} elseif ( 'user' === $queried_object['type'] ) {
$link = get_edit_user_link( $queried_object['id'] );
if ( $link ) {
printf( '<a href="%s">%s</a>%s', esc_url( $link ), esc_html__( 'Edit User', 'amp' ), esc_html( $after ) );
}
$view_label = __( 'View User', 'amp' );
}
}
printf( '<a href="%s">%s</a>', esc_url( self::get_url_from_post( $post ) ), esc_html( $view_label ) );
?>
</div>
</div>
</div>
<div id="major-publishing-actions">
Expand Down
66 changes: 52 additions & 14 deletions includes/validation/class-amp-validation-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,16 @@ function( $post ) {
} else {
$invalid_url_post_id = intval( get_post_meta( $post->ID, '_amp_invalid_url_post_id', true ) );

$validation_posts[ $post->ID ] = AMP_Invalid_URL_Post_Type::store_validation_errors( $validity['validation_errors'], $validity['url'], $invalid_url_post_id );
$validation_posts[ $post->ID ] = AMP_Invalid_URL_Post_Type::store_validation_errors(
wp_list_pluck( $validity['results'], 'error' ),
$validity['url'],
array_merge(
array(
'invalid_url_post' => $post,
Copy link
Member Author

@westonruter westonruter Sep 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was bad. Fixed in #1428

),
wp_array_slice_assoc( $validity, array( 'queried_object' ) )
)
);

// Remember the amp_invalid_url post so that when the slug changes the old amp_invalid_url post can be updated.
if ( ! is_wp_error( $validation_posts[ $post->ID ] ) && $invalid_url_post_id !== $validation_posts[ $post->ID ] ) {
Expand Down Expand Up @@ -1544,9 +1553,28 @@ public static function finalize_validation( DOMDocument $dom, $args = array() )
}

if ( $args['append_validation_status_comment'] ) {
$encoded = wp_json_encode( self::$validation_results, 128 /* JSON_PRETTY_PRINT */ );
$data = array(
'results' => self::$validation_results,
);
if ( get_queried_object() ) {
$data['queried_object'] = array();
if ( get_queried_object_id() ) {
$data['queried_object']['id'] = get_queried_object_id();
}
if ( get_queried_object() instanceof WP_Post ) {
$data['queried_object']['type'] = 'post';
} elseif ( get_queried_object() instanceof WP_Term ) {
$data['queried_object']['type'] = 'term';
} elseif ( get_queried_object() instanceof WP_User ) {
$data['queried_object']['type'] = 'user';
} elseif ( get_queried_object() instanceof WP_Post_Type ) {
$data['queried_object']['type'] = 'post_type';
}
}

$encoded = wp_json_encode( $data, 128 /* JSON_PRETTY_PRINT */ );
$encoded = str_replace( '--', '\u002d\u002d', $encoded ); // Prevent "--" in strings from breaking out of HTML comments.
$comment = $dom->createComment( 'AMP_VALIDATION_RESULTS:' . $encoded . "\n" );
$comment = $dom->createComment( 'AMP_VALIDATION:' . $encoded . "\n" );
$dom->documentElement->appendChild( $comment );
}
}
Expand Down Expand Up @@ -1608,13 +1636,18 @@ public static function validate_after_plugin_activation() {
if ( is_wp_error( $validity ) ) {
return $validity;
}
if ( is_array( $validity ) && count( $validity['validation_errors'] ) > 0 ) {
AMP_Invalid_URL_Post_Type::store_validation_errors( $validity['validation_errors'], $validity['url'] );
set_transient( self::PLUGIN_ACTIVATION_VALIDATION_ERRORS_TRANSIENT_KEY, $validity['validation_errors'], 60 );
$validation_errors = wp_list_pluck( $validity['results'], 'error' );
if ( is_array( $validity ) && count( $validation_errors ) > 0 ) { // @todo This should only warn when there are unaccepted validation errors.
AMP_Invalid_URL_Post_Type::store_validation_errors(
$validation_errors,
$validity['url'],
wp_array_slice_assoc( $validity, array( 'queried_object_id', 'queried_object_type' ) )
);
set_transient( self::PLUGIN_ACTIVATION_VALIDATION_ERRORS_TRANSIENT_KEY, $validation_errors, 60 );
} else {
delete_transient( self::PLUGIN_ACTIVATION_VALIDATION_ERRORS_TRANSIENT_KEY );
}
return $validity['validation_errors'];
return $validation_errors;
}

/**
Expand All @@ -1627,8 +1660,10 @@ public static function validate_after_plugin_activation() {
* @return WP_Error|array {
* Response.
*
* @type array $validation_errors Validation errors.
* @type string $url Final URL that was checked or redirected to.
* @type array $results Validation results, where each nested array contains an error key and sanitized key.
* @type string $url Final URL that was checked or redirected to.
* @type int $queried_object_id Queried object ID.
* @type string $queried_object_type Queried object type.
* }
*/
public static function validate_url( $url ) {
Expand Down Expand Up @@ -1709,15 +1744,18 @@ public static function validate_url( $url ) {
);

$response = wp_remote_retrieve_body( $r );
if ( ! preg_match( '#</body>.*?<!--\s*AMP_VALIDATION_RESULTS\s*:\s*(\[.*?\])\s*-->#s', $response, $matches ) ) {
if ( ! preg_match( '#</body>.*?<!--\s*AMP_VALIDATION\s*:\s*(\{.*?\})\s*-->#s', $response, $matches ) ) {
return new WP_Error( 'response_comment_absent' );
}
$validation_results = json_decode( $matches[1], true );
if ( ! is_array( $validation_results ) ) {
$validation = json_decode( $matches[1], true );
if ( json_last_error() || ! isset( $validation['results'] ) || ! is_array( $validation['results'] ) ) {
return new WP_Error( 'malformed_json_validation_errors' );
}
$validation_errors = wp_list_pluck( $validation_results, 'error' );
return compact( 'validation_errors', 'url' );

return array_merge(
$validation,
compact( 'url' )
);
}

/**
Expand Down
17 changes: 10 additions & 7 deletions tests/test-class-amp-cli.php
Original file line number Diff line number Diff line change
Expand Up @@ -467,24 +467,27 @@ function( $post ) {
}

/**
* Adds the AMP_VALIDATION_RESULTS: comment to the <html> body.
* Adds the AMP_VALIDATION: comment to the <html> body.
*
* @return array The response, with a comment in the body.
*/
public function add_comment() {
$mock_validation_results = array(
array(
'error' => array(
'code' => 'foo',
$mock_validation = array(
'results' => array(
array(
'error' => array(
'code' => 'foo',
),
'sanitized' => false,
),
'sanitized' => false,
),
'url' => home_url( '/' ),
);

return array(
'body' => sprintf(
'<html amp><head></head><body></body><!--%s--></html>',
'AMP_VALIDATION_RESULTS:' . wp_json_encode( $mock_validation_results )
'AMP_VALIDATION:' . wp_json_encode( $mock_validation )
),
'response' => array(
'code' => 200,
Expand Down
Loading