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

Eliminate obsolete re-validation when switching template modes #5100

Merged
merged 1 commit into from
Jul 27, 2020
Merged

Eliminate obsolete re-validation when switching template modes #5100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
125 changes: 0 additions & 125 deletions includes/options/class-amp-options-manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,6 @@ public static function validate_options( $new_options ) {
];
if ( isset( $new_options[ Option::THEME_SUPPORT ] ) && in_array( $new_options[ Option::THEME_SUPPORT ], $recognized_theme_supports, true ) ) {
$options[ Option::THEME_SUPPORT ] = $new_options[ Option::THEME_SUPPORT ];

// If this option was changed, display a notice with the new template mode.
if ( self::get_option( Option::THEME_SUPPORT ) !== $new_options[ Option::THEME_SUPPORT ] ) {
add_action( 'update_option_' . self::OPTION_NAME, [ __CLASS__, 'handle_updated_theme_support_option' ] );
}
}

// Validate post type support.
Expand Down Expand Up @@ -587,126 +582,6 @@ public static function insecure_connection_notice() {
}
}

/**
* Adds a message for an update of the theme support setting.
*/
public static function handle_updated_theme_support_option() {
$template_mode = self::get_option( Option::THEME_SUPPORT );

$url = amp_admin_get_preview_permalink();

$notice_type = 'updated';
$review_messages = [];
if ( $url ) {
$validation = AMP_Validation_Manager::validate_url_and_store( $url );

if ( is_wp_error( $validation ) ) {
$review_messages[] = esc_html__( 'However, there was an error when checking the AMP validity for your site.', 'amp' );
$review_messages[] = AMP_Validation_Manager::get_validate_url_error_message( $validation->get_error_code(), $validation->get_error_message() );
$notice_type = 'error';
} elseif ( is_array( $validation ) ) {
$new_errors = 0;
$rejected_errors = 0;

$errors = wp_list_pluck( $validation['results'], 'error' );
foreach ( $errors as $error ) {
$sanitization = AMP_Validation_Error_Taxonomy::get_validation_error_sanitization( $error );
$is_new_rejected = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS === $sanitization['status'];
if ( $is_new_rejected || AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS === $sanitization['status'] ) {
$new_errors++;
}
if ( $is_new_rejected || AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACK_REJECTED_STATUS === $sanitization['status'] ) {
$rejected_errors++;
}
}

$invalid_url_post_id = $validation['post_id'];
$invalid_url_screen_url = ! is_wp_error( $invalid_url_post_id ) ? get_edit_post_link( $invalid_url_post_id, 'raw' ) : null;

if ( $rejected_errors > 0 ) {
$notice_type = 'error';

$message = esc_html(
sprintf(
/* translators: %s is count of rejected errors */
_n(
'However, AMP is not yet available due to %s validation error (for one URL at least).',
'However, AMP is not yet available due to %s validation errors (for one URL at least).',
number_format_i18n( $rejected_errors ),
'amp'
),
$rejected_errors,
esc_url( $invalid_url_screen_url )
)
);

if ( $invalid_url_screen_url ) {
$message .= ' ' . sprintf(
/* translators: %s is URL to review issues */
_n(
'<a href="%s">Review Issue</a>.',
'<a href="%s">Review Issues</a>.',
$rejected_errors,
'amp'
),
esc_url( $invalid_url_screen_url )
);
}

$review_messages[] = $message;
} else {
$message = sprintf(
/* translators: %s is an AMP URL */
__( 'View an <a href="%s">AMP version of your site</a>.', 'amp' ),
esc_url( $url )
);

if ( $new_errors > 0 && $invalid_url_screen_url ) {
$message .= ' ' . sprintf(
/* translators: 1: URL to review issues. 2: count of new errors. */
_n(
'Please also <a href="%1$s">review %2$s issue</a> which may need to be fixed (for one URL at least).',
'Please also <a href="%1$s">review %2$s issues</a> which may need to be fixed (for one URL at least).',
$new_errors,
'amp'
),
esc_url( $invalid_url_screen_url ),
number_format_i18n( $new_errors )
);
}

$review_messages[] = $message;
}
}
}

switch ( $template_mode ) {
case AMP_Theme_Support::STANDARD_MODE_SLUG:
$message = esc_html__( 'Standard mode activated!', 'amp' );
if ( $review_messages ) {
$message .= ' ' . implode( ' ', $review_messages );
}
break;
case AMP_Theme_Support::TRANSITIONAL_MODE_SLUG:
$message = esc_html__( 'Transitional mode activated!', 'amp' );
if ( $review_messages ) {
$message .= ' ' . implode( ' ', $review_messages );
}
break;
case AMP_Theme_Support::READER_MODE_SLUG:
$message = sprintf(
/* translators: %s is an AMP URL */
__( 'Reader mode activated! View the <a href="%s">AMP version of a recent post</a>. It is recommended that you upgrade to Standard or Transitional mode.', 'amp' ),
esc_url( $url )
);
break;
}

if ( isset( $message ) ) {
self::add_settings_error( self::OPTION_NAME, 'template_mode_updated', wp_kses_post( $message ), $notice_type );
}
}

/**
* Register a settings error to be displayed to the user.
*
Expand Down
150 changes: 0 additions & 150 deletions tests/php/test-class-amp-options-manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,154 +671,4 @@ public function test_get_options_theme_support_defaults( $args, $expected_mode,
}
$this->assertEquals( $expected_mode, AMP_Options_Manager::get_option( Option::THEME_SUPPORT ) );
}

/**
* Test handle_updated_theme_support_option for reader mode.
*
* @covers AMP_Options_Manager::handle_updated_theme_support_option()
* @covers ::amp_admin_get_preview_permalink()
*/
public function test_handle_updated_theme_support_option_disabled() {
wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) );
AMP_Validation_Manager::init();

$page_id = self::factory()->post->create( [ 'post_type' => 'page' ] );
AMP_Options_Manager::update_option( Option::SUPPORTED_POST_TYPES, [ 'page' ] );
AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG );
AMP_Options_Manager::handle_updated_theme_support_option();
$amp_settings_errors = get_settings_errors( AMP_Options_Manager::OPTION_NAME );
$new_error = end( $amp_settings_errors );
$this->assertStringStartsWith( 'Reader mode activated!', $new_error['message'] );
$this->assertStringContains( esc_url( amp_get_permalink( $page_id ) ), $new_error['message'], 'Expect amp_admin_get_preview_permalink() to return a page since it is the only post type supported.' );
$this->assertCount( 0, get_posts( [ 'post_type' => AMP_Validated_URL_Post_Type::POST_TYPE_SLUG ] ) );
}

/**
* Test handle_updated_theme_support_option for standard when there is one auto-accepted issue.
*
* @covers AMP_Options_Manager::handle_updated_theme_support_option()
* @covers ::amp_admin_get_preview_permalink()
*/
public function test_handle_updated_theme_support_option_standard_success_but_error() {
wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) );

$post_id = self::factory()->post->create( [ 'post_type' => 'post' ] );
AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG );
AMP_Options_Manager::update_option( Option::SUPPORTED_POST_TYPES, [ 'post' ] );

$filter = static function() {
$validation = [
'results' => [
[
'error' => [ 'code' => 'example' ],
'sanitized' => false,
],
],
];
return [
'body' => wp_json_encode( $validation ),
];
};
add_filter( 'pre_http_request', $filter, 10, 3 );
AMP_Options_Manager::handle_updated_theme_support_option();
remove_filter( 'pre_http_request', $filter );
$amp_settings_errors = get_settings_errors( AMP_Options_Manager::OPTION_NAME );
$new_error = end( $amp_settings_errors );
$this->assertStringStartsWith( 'Standard mode activated!', $new_error['message'] );
$this->assertStringContains( esc_url( amp_get_permalink( $post_id ) ), $new_error['message'], 'Expect amp_admin_get_preview_permalink() to return a post since it is the only post type supported.' );
$invalid_url_posts = get_posts(
[
'post_type' => AMP_Validated_URL_Post_Type::POST_TYPE_SLUG,
'fields' => 'ids',
]
);
$this->assertEquals( 'updated', $new_error['type'] );
$this->assertCount( 1, $invalid_url_posts );
$this->assertStringContains( 'review 1 issue', $new_error['message'] );
$this->assertStringContains( esc_url( get_edit_post_link( $invalid_url_posts[0], 'raw' ) ), $new_error['message'], 'Expect edit post link for the invalid URL post to be present.' );
}

/**
* Test handle_updated_theme_support_option for standard when there is one auto-accepted issue.
*
* @covers AMP_Options_Manager::handle_updated_theme_support_option()
* @covers ::amp_admin_get_preview_permalink()
*/
public function test_handle_updated_theme_support_option_standard_validate_error() {
wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) );
self::factory()->post->create( [ 'post_type' => 'post' ] );

AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG );
AMP_Options_Manager::update_option( Option::SUPPORTED_POST_TYPES, [ 'post' ] );

$filter = static function() {
return [
'body' => '<html amp><head></head><body></body>',
];
};
add_filter( 'pre_http_request', $filter, 10, 3 );
AMP_Options_Manager::handle_updated_theme_support_option();
remove_filter( 'pre_http_request', $filter );

$amp_settings_errors = get_settings_errors( AMP_Options_Manager::OPTION_NAME );
$new_error = end( $amp_settings_errors );
$this->assertStringStartsWith( 'Standard mode activated!', $new_error['message'] );
$invalid_url_posts = get_posts(
[
'post_type' => AMP_Validated_URL_Post_Type::POST_TYPE_SLUG,
'fields' => 'ids',
]
);
$this->assertCount( 0, $invalid_url_posts );
$this->assertEquals( 'error', $new_error['type'] );
}

/**
* Test handle_updated_theme_support_option for transitional mode.
*
* @covers AMP_Options_Manager::handle_updated_theme_support_option()
* @covers ::amp_admin_get_preview_permalink()
*/
public function test_handle_updated_theme_support_option_paired() {
wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) );

$post_id = self::factory()->post->create( [ 'post_type' => 'post' ] );
AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG );
AMP_Options_Manager::update_option( Option::SUPPORTED_POST_TYPES, [ 'post' ] );

$filter = static function() {
$validation = [
'results' => [
[
'error' => [ 'code' => 'foo' ],
'sanitized' => false,
],
[
'error' => [ 'code' => 'bar' ],
'sanitized' => false,
],
],
];
return [
'body' => wp_json_encode( $validation ),
];
};
add_filter( 'pre_http_request', $filter, 10, 3 );
AMP_Options_Manager::handle_updated_theme_support_option();
remove_filter( 'pre_http_request', $filter );
$amp_settings_errors = get_settings_errors( AMP_Options_Manager::OPTION_NAME );
$new_error = end( $amp_settings_errors );
$this->assertStringStartsWith( 'Transitional mode activated!', $new_error['message'] );
$this->assertStringContains( esc_url( amp_get_permalink( $post_id ) ), $new_error['message'], 'Expect amp_admin_get_preview_permalink() to return a post since it is the only post type supported.' );
$invalid_url_posts = get_posts(
[
'post_type' => AMP_Validated_URL_Post_Type::POST_TYPE_SLUG,
'fields' => 'ids',
]
);
$this->assertEquals( 'updated', $new_error['type'] );
$this->assertCount( 1, $invalid_url_posts );
$this->assertStringContains( 'review 2 issues', $new_error['message'] );
$this->assertStringContains( esc_url( get_edit_post_link( $invalid_url_posts[0], 'raw' ) ), $new_error['message'], 'Expect edit post link for the invalid URL post to be present.' );
}
}