Skip to content

Commit

Permalink
Add link to validation error details in Gutenberg notice
Browse files Browse the repository at this point in the history
  • Loading branch information
@westonruter
westonruter committed Apr 12, 2018
1 parent 21e60f0 commit 8372dbd
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 53 deletions.
21 changes: 15 additions & 6 deletions assets/js/amp-block-validation.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var ampBlockValidation = ( function() {
*/
data: {
i18n: {},
restValidationErrorsField: ''
ampValidityRestField: ''
},

/**
Expand Down Expand Up @@ -94,7 +94,7 @@ var ampBlockValidation = ( function() {
* @return {void}
*/
handleValidationErrorsStateChange: function handleValidationErrorsStateChange() {
var currentPost, validationErrors, blockValidationErrors, noticeMessage, blockErrorCount;
var currentPost, validationErrors, blockValidationErrors, noticeElement, noticeMessage, blockErrorCount, ampValidity;

// @todo Gutenberg currently is not persisting isDirty state if changes are made during save request. Block order mismatch.
// We can only align block validation errors with blocks in editor when in saved state, since only here will the blocks be aligned with the validation errors.
Expand All @@ -103,7 +103,8 @@ var ampBlockValidation = ( function() {
}

currentPost = wp.data.select( 'core/editor' ).getCurrentPost();
validationErrors = currentPost[ module.data.restValidationErrorsField ];
ampValidity = currentPost[ module.data.ampValidityRestField ] || {};
validationErrors = ampValidity.errors;

// Short-circuit if there was no change to the validation errors.
if ( ! validationErrors || _.isEqual( module.lastValidationErrors, validationErrors ) ) {
Expand Down Expand Up @@ -172,8 +173,16 @@ var ampBlockValidation = ( function() {
}

noticeMessage += ' ' + wp.i18n.__( 'Invalid code is stripped when displaying AMP.', 'amp' );

module.validationWarningNoticeId = wp.data.dispatch( 'core/editor' ).createWarningNotice( noticeMessage ).notice.id;
noticeElement = wp.element.createElement( 'p', {}, [
noticeMessage + ' ',
ampValidity.link && wp.element.createElement(
'a',
{ key: 'details', href: ampValidity.link, target: '_blank' },
wp.i18n.__( 'Details', 'amp' )
)
] );

module.validationWarningNoticeId = wp.data.dispatch( 'core/editor' ).createWarningNotice( noticeElement, { spokenMessage: noticeMessage } ).notice.id;
},

/**
Expand Down Expand Up @@ -202,7 +211,7 @@ var ampBlockValidation = ( function() {
var blockValidationErrorsByUid, editorSelect, currentPost, blockOrder, validationErrors, otherValidationErrors;
editorSelect = wp.data.select( 'core/editor' );
currentPost = editorSelect.getCurrentPost();
validationErrors = currentPost[ module.data.restValidationErrorsField ];
validationErrors = currentPost[ module.data.ampValidityRestField ].errors;
blockOrder = module.getFlattenedBlockOrder( editorSelect.getBlocks() );

otherValidationErrors = [];
Expand Down
73 changes: 53 additions & 20 deletions includes/utils/class-amp-validation-utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class AMP_Validation_Utils {
*
* @var string
*/
const REST_FIELD_NAME = 'amp_validation_errors';
const VALIDITY_REST_FIELD_NAME = 'amp_validity';

/**
* The errors encountered when validating.
Expand Down Expand Up @@ -365,6 +365,8 @@ public static function handle_save_post_prompting_validation( $post_id, $post )
* Validate the posts pending frontend validation.
*
* @see AMP_Validation_Utils::handle_save_post_prompting_validation()
*
* @return array Mapping of post ID to the result of validating or storing the validation result.
*/
public static function validate_queued_posts_on_frontend() {
$posts = array_filter(
Expand All @@ -374,10 +376,13 @@ function( $post ) {
}
);

$validation_posts = array();

// @todo Only validate the first and then queue the rest in WP Cron?
foreach ( $posts as $post ) {
$url = amp_get_permalink( $post->ID );
if ( ! $url ) {
$validation_posts[ $post->ID ] = new WP_Error( 'no_amp_permalink' );
continue;
}

Expand All @@ -386,11 +391,13 @@ function( $post ) {

$validation_errors = self::validate_url( $url );
if ( is_wp_error( $validation_errors ) ) {
continue;
$validation_posts[ $post->ID ] = $validation_errors;
} else {
$validation_posts[ $post->ID ] = self::store_validation_errors( $validation_errors, $url );
}

self::store_validation_errors( $validation_errors, $url );
}

return $validation_posts;
}

/**
Expand Down Expand Up @@ -1264,7 +1271,7 @@ public static function send_validation_errors_header() {
*
* @param array $validation_errors Validation errors.
* @param string $url URL on which the validation errors occurred.
* @return int|null $post_id The post ID of the custom post type used, or null.
* @return int|WP_Error $post_id The post ID of the custom post type used, null if post was deleted due to no validation errors, or WP_Error on failure.
* @global WP $wp
*/
public static function store_validation_errors( $validation_errors, $url ) {
Expand Down Expand Up @@ -1309,9 +1316,9 @@ public static function store_validation_errors( $validation_errors, $url ) {
'post_name' => $post_name,
'post_content' => $encoded_errors,
'post_status' => 'publish',
) ) );
if ( ! $post_id ) {
return null;
) ), true );
if ( is_wp_error( $post_id ) ) {
return $post_id;
}
if ( ! in_array( $url, get_post_meta( $post_id, self::AMP_URL_META, false ), true ) ) {
add_post_meta( $post_id, self::AMP_URL_META, wp_slash( $url ), false );
Expand Down Expand Up @@ -1934,8 +1941,8 @@ public static function enqueue_block_validation() {
);

$data = wp_json_encode( array(
'i18n' => gutenberg_get_jed_locale_data( 'amp' ), // @todo POT file.
'restValidationErrorsField' => self::REST_FIELD_NAME,
'i18n' => gutenberg_get_jed_locale_data( 'amp' ), // @todo POT file.
'ampValidityRestField' => self::VALIDITY_REST_FIELD_NAME,
) );
wp_add_inline_script( $slug, sprintf( 'ampBlockValidation.boot( %s );', $data ) );
}
Expand All @@ -1959,11 +1966,11 @@ public static function add_rest_api_fields() {

register_rest_field(
$object_types,
self::REST_FIELD_NAME,
self::VALIDITY_REST_FIELD_NAME,
array(
'get_callback' => array( __CLASS__, 'rest_field_amp_validation' ),
'get_callback' => array( __CLASS__, 'get_amp_validity_rest_field' ),
'schema' => array(
'description' => __( 'AMP validation results', 'amp' ),
'description' => __( 'AMP validity status', 'amp' ),
'type' => 'object',
),
)
Expand All @@ -1981,16 +1988,42 @@ public static function add_rest_api_fields() {
* @param WP_REST_Request $request The name of the field to add.
* @return array|null $validation_data Validation data if it's available, or null.
*/
public static function rest_field_amp_validation( $post_data, $field_name, $request ) {
$post_id = $post_data['id'];
$post = get_post( $post_id );
public static function get_amp_validity_rest_field( $post_data, $field_name, $request ) {
unset( $field_name );
if ( ! current_user_can( 'edit_post', $post_data['id'] ) ) {
return null;
}
$post = get_post( $post_data['id'] );

$validation_status_post = null;
if ( in_array( $request->get_method(), array( 'PUT', 'POST' ), true ) ) {
if ( ! isset( self::$posts_pending_frontend_validation[ $post_id ] ) ) {
self::$posts_pending_frontend_validation[ $post_id ] = true;
if ( ! isset( self::$posts_pending_frontend_validation[ $post->ID ] ) ) {
self::$posts_pending_frontend_validation[ $post->ID ] = true;
}
$results = self::validate_queued_posts_on_frontend();
if ( isset( $results[ $post->ID ] ) && is_int( $results[ $post->ID ] ) ) {
$validation_status_post = get_post( $results[ $post->ID ] );
}
self::validate_queued_posts_on_frontend();
}
return self::get_existing_validation_errors( $post );

if ( empty( $validation_status_post ) ) {
// @todo Consider process_markup() if not post type is not viewable and if post type supports editor.
$validation_status_post = self::get_validation_status_post( amp_get_permalink( $post->ID ) );
}

if ( ! $validation_status_post ) {
$field = array(
'errors' => array(),
'link' => null,
);
} else {
$field = array(
'errors' => json_decode( $validation_status_post->post_content, true ),
'link' => get_edit_post_link( $validation_status_post->ID, 'raw' ),
);
}

return $field;
}

/**
Expand Down
70 changes: 43 additions & 27 deletions tests/test-class-amp-validation-utils.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1449,7 +1449,7 @@ public function test_enqueue_block_validation() {
$this->assertEquals( AMP__VERSION, $script->ver );
$this->assertTrue( in_array( $slug, wp_scripts()->queue, true ) );
$this->assertContains( 'ampBlockValidation.boot', $inline_script );
$this->assertContains( AMP_Validation_Utils::REST_FIELD_NAME, $inline_script );
$this->assertContains( AMP_Validation_Utils::VALIDITY_REST_FIELD_NAME, $inline_script );
$this->assertContains( '"domain":"amp"', $inline_script );
}

Expand Down Expand Up @@ -1485,48 +1485,64 @@ public function test_add_rest_api_fields() {
*/
public function assert_rest_api_field_present( $post_types ) {
foreach ( $post_types as $post_type ) {
$field = $GLOBALS['wp_rest_additional_fields'][ $post_type ][ AMP_Validation_Utils::REST_FIELD_NAME ];
$field = $GLOBALS['wp_rest_additional_fields'][ $post_type ][ AMP_Validation_Utils::VALIDITY_REST_FIELD_NAME ];
$this->assertEquals(
$field['schema'],
array(
'description' => 'AMP validation results',
'description' => 'AMP validity status',
'type' => 'object',
)
);
$this->assertEquals( $field['get_callback'], array( self::TESTED_CLASS, 'rest_field_amp_validation' ) );
$this->assertEquals( $field['get_callback'], array( self::TESTED_CLASS, 'get_amp_validity_rest_field' ) );
}
}

/**
* Test rest_field_amp_validation.
* Test get_amp_validity_rest_field.
*
* @covers AMP_Validation_Utils::rest_field_amp_validation()
* @covers AMP_Validation_Utils::get_amp_validity_rest_field()
*/
public function test_rest_field_amp_validation() {
$post_id = $this->factory()->post->create();
$this->assertEquals(
null,
AMP_Validation_Utils::rest_field_amp_validation(
array(
'id' => $post_id,
),
'',
new WP_REST_Request( 'PUT' )
)
);
AMP_Validation_Utils::register_post_type();
$id = $this->factory()->post->create();
$this->assertNull( AMP_Validation_Utils::get_amp_validity_rest_field(
compact( 'id' ),
'',
new WP_REST_Request( 'GET' )
) );

// Create an error custom post for the ID, so this will return the errors in the field.
$this->create_custom_post( amp_get_permalink( $post_id ) );
$this->assertEquals(
$this->get_mock_errors(),
AMP_Validation_Utils::rest_field_amp_validation(
array(
'id' => $post_id,
),
'',
new WP_REST_Request( 'PUT' )
)
$this->create_custom_post( amp_get_permalink( $id ) );

// Make sure capability check is honored.
$this->assertNull( AMP_Validation_Utils::get_amp_validity_rest_field(
compact( 'id' ),
'',
new WP_REST_Request( 'GET' )
) );

wp_set_current_user( $this->factory()->user->create( array( 'role' => 'administrator' ) ) );

// GET request.
$field = AMP_Validation_Utils::get_amp_validity_rest_field(
compact( 'id' ),
'',
new WP_REST_Request( 'GET' )
);
$this->assertArrayHasKey( 'errors', $field );
$this->assertArrayHasKey( 'link', $field );
$this->assertEquals( $field['errors'], $this->get_mock_errors() );

// @todo Test successful loopback request to test.
// PUT request.
$field = AMP_Validation_Utils::get_amp_validity_rest_field(
compact( 'id' ),
'',
new WP_REST_Request( 'PUT' )
);
$this->assertArrayHasKey( 'errors', $field );
$this->assertArrayHasKey( 'link', $field );
$this->assertEquals( $field['errors'], $this->get_mock_errors() );
}

/**
Expand Down

0 comments on commit 8372dbd

Please sign in to comment.