From 57a22ca81e5b599c5445637fad9da5942ca28d0f Mon Sep 17 00:00:00 2001 From: ThierryA Date: Wed, 17 Jan 2018 15:18:34 +0100 Subject: [PATCH 01/11] Improved page def ault status logic and add overwrite ability --- assets/js/amp-post-meta-box.js | 6 +-- includes/admin/class-amp-post-meta-box.php | 62 ++++++++++------------ includes/amp-helper-functions.php | 42 ++++++++++++++- includes/class-amp-post-type-support.php | 14 ----- templates/admin/amp-status.php | 18 +++---- tests/test-amp-helper-functions.php | 24 +++++++++ tests/test-class-amp-meta-box.php | 14 ++--- tests/test-class-amp-post-type-support.php | 19 ------- 8 files changed, 108 insertions(+), 91 deletions(-) diff --git a/assets/js/amp-post-meta-box.js b/assets/js/amp-post-meta-box.js index 8f1816fbc59..4816fa8116d 100644 --- a/assets/js/amp-post-meta-box.js +++ b/assets/js/amp-post-meta-box.js @@ -19,7 +19,7 @@ var ampPostMetaBox = ( function( $ ) { */ data: { previewLink: '', - disabled: false, + enabled: '', statusInputName: '', l10n: { ampPreviewBtnLabel: '' @@ -58,7 +58,7 @@ var ampPostMetaBox = ( function( $ ) { component.boot = function boot( data ) { component.data = data; $( document ).ready( function() { - if ( ! component.data.disabled ) { + if ( component.data.enabled ) { component.addPreviewButton(); } component.listen(); @@ -161,7 +161,7 @@ var ampPostMetaBox = ( function( $ ) { $container.slideToggle( component.toggleSpeed ); // Update status. - if ( ! component.data.disabled ) { + if ( component.data.enabled ) { $container.data( 'amp-status', status ); $checked.prop( 'checked', true ); $( '.amp-status-text' ).text( $checked.next().text() ); diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index 34ac061e6b7..45c1415cd93 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -22,12 +22,28 @@ class AMP_Post_Meta_Box { const ASSETS_HANDLE = 'amp-post-meta-box'; /** - * The post meta key for whether the post is skipped. + * The enabled status post meta value. * * @since 0.6 * @var string */ - const DISABLED_POST_META_KEY = 'amp_disabled'; + const ENABLED_STATUS = 'enabled'; + + /** + * The disabled status post meta value. + * + * @since 0.6 + * @var string + */ + const DISABLED_STATUS = 'disabled'; + + /** + * The status post meta key. + * + * @since 0.6 + * @var string + */ + const STATUS_POST_META_KEY = 'amp_status'; /** * The field name for the enabled/disabled radio buttons. @@ -64,29 +80,6 @@ public function init() { add_action( 'save_post', array( $this, 'save_amp_status' ) ); add_filter( 'preview_post_link', array( $this, 'preview_post_link' ) ); } - - /** - * Get whether AMP is available for a given post. - * - * This is just calling `post_supports_amp()` but ignoring any user-supplied opt-out for AMP. - * - * @since 0.6 - * @see post_supports_amp() - * - * @param WP_Post $post Post. - * @return bool Whether or not AMP is available. - */ - protected function is_amp_available( $post ) { - $support_errors = AMP_Post_Type_Support::get_support_errors( $post ); - if ( empty( $support_errors ) ) { - return true; - } - if ( 1 === count( $support_errors ) && 'post-disabled' === $support_errors[0] ) { - return true; - } - return false; - } - /** * Enqueue admin assets. * @@ -122,7 +115,7 @@ public function enqueue_admin_assets() { wp_add_inline_script( self::ASSETS_HANDLE, sprintf( 'ampPostMetaBox.boot( %s );', wp_json_encode( array( 'previewLink' => esc_url_raw( add_query_arg( AMP_QUERY_VAR, '', get_preview_post_link( $post ) ) ), - 'disabled' => (bool) get_post_meta( $post->ID, self::DISABLED_POST_META_KEY, true ) || ! $this->is_amp_available( $post ), + 'enabled' => post_supports_amp( $post ), 'statusInputName' => self::STATUS_INPUT_NAME, 'l10n' => array( 'ampPreviewBtnLabel' => __( 'Preview changes in AMP (opens in new window)', 'amp' ), @@ -148,10 +141,9 @@ public function render_status( $post ) { return; } - $available = $this->is_amp_available( $post ); - $disabled = (bool) get_post_meta( $post->ID, self::DISABLED_POST_META_KEY, true ); - $status = $disabled || ! $available ? 'disabled' : 'enabled'; - $labels = array( + $errors = AMP_Post_Type_Support::get_support_errors( $post ); + $status = post_supports_amp( $post ) ? self::ENABLED_STATUS : self::DISABLED_STATUS; + $labels = array( 'enabled' => __( 'Enabled', 'amp' ), 'disabled' => __( 'Disabled', 'amp' ), ); @@ -182,11 +174,11 @@ public function save_amp_status( $post_id ) { ); if ( true === $verify ) { - if ( 'disabled' === $_POST[ self::STATUS_INPUT_NAME ] ) { - update_post_meta( $post_id, self::DISABLED_POST_META_KEY, true ); - } else { - delete_post_meta( $post_id, self::DISABLED_POST_META_KEY ); - } + update_post_meta( + $post_id, + self::STATUS_POST_META_KEY, + sanitize_key( wp_unslash( $_POST[ self::STATUS_INPUT_NAME ] ) ) + ); } } diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index a1f104a4d99..8f31a754fa6 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -55,7 +55,7 @@ function amp_get_permalink( $post_id ) { * Determine whether a given post supports AMP. * * @since 0.1 - * @since 0.6 Returns false when post has meta to disable AMP or when page is homepage or page for posts. + * @since 0.6 Returns false when post has meta to disable AMP. * @see AMP_Post_Type_Support::get_support_errors() * * @param WP_Post $post Post. @@ -63,7 +63,45 @@ function amp_get_permalink( $post_id ) { * @return bool Whether the post supports AMP. */ function post_supports_amp( $post ) { - return 0 === count( AMP_Post_Type_Support::get_support_errors( $post ) ); + // Return false if an error is found. + if ( ! empty( AMP_Post_Type_Support::get_support_errors( $post ) ) ) { + return false; + } + + switch ( get_post_meta( $post->ID, AMP_Post_Meta_Box::STATUS_POST_META_KEY, true ) ) { + case AMP_Post_Meta_Box::ENABLED_STATUS: + return true; + + case AMP_Post_Meta_Box::DISABLED_STATUS: + return false; + + // Disabled by default for custom page templates, page on front and page for posts. + default: + $enabled = ( + ! (bool) get_page_template_slug( $post ) + && + ! ( + 'page' === $post->post_type + && + 'page' === get_option( 'show_on_front' ) + && + in_array( (int) $post->ID, array( + (int) get_option( 'page_on_front' ), + (int) get_option( 'page_for_posts' ), + ), true ) + ) + ); + + /** + * Filters whether default AMP status should be enabled or not. + * + * @since 0.6 + * + * @param string $status Status. + * @param WP_Post $post Post. + */ + return apply_filters( 'amp_status_default_enabled', $enabled, $post ); + } } /** diff --git a/includes/class-amp-post-type-support.php b/includes/class-amp-post-type-support.php index 12b10c73f59..f0c32c78c42 100644 --- a/includes/class-amp-post-type-support.php +++ b/includes/class-amp-post-type-support.php @@ -76,20 +76,6 @@ public static function get_support_errors( $post ) { $errors[] = 'post-type-support'; } - // Skip based on postmeta. - if ( ! isset( $post->ID ) || (bool) get_post_meta( $post->ID, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ) { - $errors[] = 'post-disabled'; - } - - // Homepage and page for posts are not supported yet. - if ( 'page' === get_post_type( $post ) && 'page' === get_option( 'show_on_front' ) ) { - if ( (int) get_option( 'page_for_posts' ) === (int) $post->ID ) { - $errors[] = 'page-for-posts'; - } elseif ( (int) get_option( 'page_on_front' ) === (int) $post->ID ) { - $errors[] = 'page-on-front'; - } - } - if ( post_password_required( $post ) ) { $errors[] = 'password-protected'; } diff --git a/templates/admin/amp-status.php b/templates/admin/amp-status.php index 6575a3729a5..48f6ec3545c 100644 --- a/templates/admin/amp-status.php +++ b/templates/admin/amp-status.php @@ -13,11 +13,10 @@ /** * Inherited template vars. * - * @var array $labels Labels for enabled or disabled. - * @var string $status Enabled or disabled. - * @var bool $available Whether AMP is available. + * @var array $labels Labels for enabled or disabled. + * @var string $status Enabled or disabled. + * @var array $errors Support errors. */ - ?>
@@ -28,12 +27,12 @@
- +
- > + >
- > + >
@@ -44,9 +43,6 @@
- + diff --git a/tests/test-amp-helper-functions.php b/tests/test-amp-helper-functions.php index 2749c6340b8..9791dbd96b6 100644 --- a/tests/test-amp-helper-functions.php +++ b/tests/test-amp-helper-functions.php @@ -102,4 +102,28 @@ public function test_amp_get_permalink_with_pretty_permalinks() { $url = amp_get_permalink( $published_post ); $this->assertContains( 'current_filter=amp_get_permalink', $url ); } + + /** + * Test post_supports_amp(). + * + * @covers \post_supports_amp() + */ + public function test_post_supports_amp() { + // Test disabled by default for page for posts and show on front. + $post = $this->factory()->post->create_and_get( array( 'post_type' => 'page' ) ); + $this->assertTrue( post_supports_amp( $post ) ); + update_option( 'show_on_front', 'page' ); + $this->assertTrue( post_supports_amp( $post ) ); + update_option( 'page_for_posts', $post->ID ); + $this->assertFalse( post_supports_amp( $post ) ); + update_option( 'page_for_posts', '' ); + update_option( 'page_on_front', $post->ID ); + $this->assertFalse( post_supports_amp( $post ) ); + update_option( 'show_on_front', 'posts' ); + $this->assertTrue( post_supports_amp( $post ) ); + + // Test disabled by default for page templates. + update_post_meta( $post->ID, '_wp_page_template', 'foo.php' ); + $this->assertFalse( post_supports_amp( $post ) ); + } } diff --git a/tests/test-class-amp-meta-box.php b/tests/test-class-amp-meta-box.php index e9b7d7eaa71..e235fbef60e 100644 --- a/tests/test-class-amp-meta-box.php +++ b/tests/test-class-amp-meta-box.php @@ -108,7 +108,7 @@ public function test_render_status() { public function test_save_amp_status() { // Test failure. $post_id = $this->factory->post->create(); - $this->assertEmpty( get_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ); + $this->assertEmpty( get_post_meta( $post_id, AMP_Post_Meta_Box::STATUS_POST_META_KEY, true ) ); // Setup for success. wp_set_current_user( $this->factory->user->create( array( @@ -119,27 +119,27 @@ public function test_save_amp_status() { // Test revision bail. $post_id = $this->factory->post->create(); - delete_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY ); + delete_post_meta( $post_id, AMP_Post_Meta_Box::STATUS_POST_META_KEY ); wp_save_post_revision( $post_id ); - $this->assertEmpty( get_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ); + $this->assertEmpty( get_post_meta( $post_id, AMP_Post_Meta_Box::STATUS_POST_META_KEY, true ) ); // Test post update success to disable. $post_id = $this->factory->post->create(); - delete_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY ); + delete_post_meta( $post_id, AMP_Post_Meta_Box::STATUS_POST_META_KEY ); wp_update_post( array( 'ID' => $post_id, 'post_title' => 'updated', ) ); - $this->assertTrue( (bool) get_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ); + $this->assertTrue( (bool) get_post_meta( $post_id, AMP_Post_Meta_Box::STATUS_POST_META_KEY, true ) ); // Test post update success to enable. $_POST[ AMP_Post_Meta_Box::STATUS_INPUT_NAME ] = 'enabled'; - delete_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY ); + delete_post_meta( $post_id, AMP_Post_Meta_Box::STATUS_POST_META_KEY ); wp_update_post( array( 'ID' => $post_id, 'post_title' => 'updated', ) ); - $this->assertFalse( (bool) get_post_meta( $post_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ) ); + $this->assertEquals( AMP_Post_Meta_Box::ENABLED_STATUS, get_post_meta( $post_id, AMP_Post_Meta_Box::STATUS_POST_META_KEY, true ) ); } /** diff --git a/tests/test-class-amp-post-type-support.php b/tests/test-class-amp-post-type-support.php index 21a09279ae3..feb11c29e5d 100644 --- a/tests/test-class-amp-post-type-support.php +++ b/tests/test-class-amp-post-type-support.php @@ -95,12 +95,6 @@ public function test_get_support_error() { add_post_type_support( 'book', AMP_QUERY_VAR ); $this->assertEmpty( AMP_Post_Type_Support::get_support_errors( $book_id ) ); - // Disabled. - update_post_meta( $book_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY, true ); - $this->assertEquals( array( 'post-disabled' ), AMP_Post_Type_Support::get_support_errors( $book_id ) ); - delete_post_meta( $book_id, AMP_Post_Meta_Box::DISABLED_POST_META_KEY ); - $this->assertEmpty( AMP_Post_Type_Support::get_support_errors( $book_id ) ); - // Password-protected. add_filter( 'post_password_required', '__return_true' ); $this->assertEquals( array( 'password-protected' ), AMP_Post_Type_Support::get_support_errors( $book_id ) ); @@ -112,18 +106,5 @@ public function test_get_support_error() { $this->assertEquals( array( 'skip-post' ), AMP_Post_Type_Support::get_support_errors( $book_id ) ); remove_filter( 'amp_skip_post', '__return_true' ); $this->assertEmpty( AMP_Post_Type_Support::get_support_errors( $book_id ) ); - - // Page for posts and show on front. - $page_id = $this->factory()->post->create( array( 'post_type' => 'page' ) ); - $this->assertEmpty( AMP_Post_Type_Support::get_support_errors( $page_id ) ); - update_option( 'show_on_front', 'page' ); - $this->assertEmpty( AMP_Post_Type_Support::get_support_errors( $page_id ) ); - update_option( 'page_for_posts', $page_id ); - $this->assertEquals( array( 'page-for-posts' ), AMP_Post_Type_Support::get_support_errors( $page_id ) ); - update_option( 'page_for_posts', '' ); - update_option( 'page_on_front', $page_id ); - $this->assertEquals( array( 'page-on-front' ), AMP_Post_Type_Support::get_support_errors( $page_id ) ); - update_option( 'show_on_front', 'posts' ); - $this->assertEmpty( AMP_Post_Type_Support::get_support_errors( $page_id ) ); } } From 770407601e18fcc952956886c7b06a2ba909194b Mon Sep 17 00:00:00 2001 From: ThierryA Date: Wed, 17 Jan 2018 16:04:04 +0100 Subject: [PATCH 02/11] Fix PHP syntax compatibility --- includes/amp-helper-functions.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 8f31a754fa6..04857b4b4e7 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -63,8 +63,10 @@ function amp_get_permalink( $post_id ) { * @return bool Whether the post supports AMP. */ function post_supports_amp( $post ) { + $errors = AMP_Post_Type_Support::get_support_errors( $post ); + // Return false if an error is found. - if ( ! empty( AMP_Post_Type_Support::get_support_errors( $post ) ) ) { + if ( ! empty( $errors ) ) { return false; } From d8d2d89fda0b72cfe6859008a60e4a77969ce03b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 17 Jan 2018 11:05:40 -0800 Subject: [PATCH 03/11] Prevent mere post update from cementing enabled status postmeta --- assets/js/amp-post-meta-box.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/assets/js/amp-post-meta-box.js b/assets/js/amp-post-meta-box.js index 4816fa8116d..17b077c197d 100644 --- a/assets/js/amp-post-meta-box.js +++ b/assets/js/amp-post-meta-box.js @@ -58,6 +58,7 @@ var ampPostMetaBox = ( function( $ ) { component.boot = function boot( data ) { component.data = data; $( document ).ready( function() { + component.statusRadioInputs = $( '[name="' + component.data.statusInputName + '"]' ); if ( component.data.enabled ) { component.addPreviewButton(); } @@ -77,8 +78,10 @@ var ampPostMetaBox = ( function( $ ) { component.onAmpPreviewButtonClick(); } ); + component.statusRadioInputs.prop( 'disabled', true ); // Prevent cementing setting default status as overridden status. $( '.edit-amp-status, [href="#amp_status"]' ).click( function( e ) { e.preventDefault(); + component.statusRadioInputs.prop( 'disabled', false ); component.toggleAmpStatus( $( e.target ) ); } ); @@ -147,7 +150,7 @@ var ampPostMetaBox = ( function( $ ) { // Don't modify status on cancel button click. if ( ! $target.hasClass( 'button-cancel' ) ) { - status = $( '[name="' + component.data.statusInputName + '"]:checked' ).val(); + status = component.statusRadioInputs.filter( ':checked' ).val(); } $checked = $( '#amp-status-' + status ); From 8c51443059d227496554d075f8859526378cff4a Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 17 Jan 2018 11:21:37 -0800 Subject: [PATCH 04/11] Distinguish enabled-status from support-error --- assets/js/amp-post-meta-box.js | 5 +++-- includes/admin/class-amp-post-meta-box.php | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/assets/js/amp-post-meta-box.js b/assets/js/amp-post-meta-box.js index 17b077c197d..54485dfb36f 100644 --- a/assets/js/amp-post-meta-box.js +++ b/assets/js/amp-post-meta-box.js @@ -19,7 +19,8 @@ var ampPostMetaBox = ( function( $ ) { */ data: { previewLink: '', - enabled: '', + enabled: true, // Overridden by post_supports_amp( $post ). + canSupport: true, // Overridden by count( AMP_Post_Type_Support::get_support_errors( $post ) ) === 0. statusInputName: '', l10n: { ampPreviewBtnLabel: '' @@ -164,7 +165,7 @@ var ampPostMetaBox = ( function( $ ) { $container.slideToggle( component.toggleSpeed ); // Update status. - if ( component.data.enabled ) { + if ( component.data.canSupport ) { $container.data( 'amp-status', status ); $checked.prop( 'checked', true ); $( '.amp-status-text' ).text( $checked.next().text() ); diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index 45c1415cd93..4c0e1ab4ce8 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -116,6 +116,7 @@ public function enqueue_admin_assets() { wp_json_encode( array( 'previewLink' => esc_url_raw( add_query_arg( AMP_QUERY_VAR, '', get_preview_post_link( $post ) ) ), 'enabled' => post_supports_amp( $post ), + 'canSupport' => count( AMP_Post_Type_Support::get_support_errors( $post ) ) === 0, 'statusInputName' => self::STATUS_INPUT_NAME, 'l10n' => array( 'ampPreviewBtnLabel' => __( 'Preview changes in AMP (opens in new window)', 'amp' ), From c9acad7e7b376c2e615af996219429c8b7717b09 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 17 Jan 2018 20:46:51 -0800 Subject: [PATCH 05/11] Fix AMP queries for front page --- amp.php | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/amp.php b/amp.php index 74689924044..12b61fc224f 100644 --- a/amp.php +++ b/amp.php @@ -75,6 +75,7 @@ function amp_after_setup_theme() { add_filter( 'amp_post_template_analytics', 'amp_add_custom_analytics' ); add_action( 'wp_loaded', 'amp_post_meta_box' ); add_action( 'wp_loaded', 'amp_add_options_menu' ); + add_action( 'parse_query', 'amp_correct_query_when_is_front_page' ); AMP_Post_Type_Support::add_post_type_support(); } add_action( 'after_setup_theme', 'amp_after_setup_theme', 5 ); @@ -117,6 +118,46 @@ function amp_force_query_var_value( $query_vars ) { return $query_vars; } +/** + * Fix up WP_Query for front page when amp query var is present. + * + * Normally the front page would not get served if a query var is present other than preview, page, paged, and cpage. + * + * @since 0.6 + * @see WP_Query::parse_query() + * @link https://github.com/WordPress/wordpress-develop/blob/0baa8ae85c670d338e78e408f8d6e301c6410c86/src/wp-includes/class-wp-query.php#L951-L971 + * + * @param WP_Query $query Query. + */ +function amp_correct_query_when_is_front_page( WP_Query $query ) { + $is_front_page_query = ( + $query->is_main_query() + && + $query->is_home() + && + // Is AMP endpoint. + false !== $query->get( AMP_QUERY_VAR, false ) + && + // Is query not yet fixed uo up to be front page. + ! $query->is_front_page() + && + // Is showing pages on front. + 'page' === get_option( 'show_on_front' ) + && + // Has page on front set. + get_option( 'page_on_front' ) + && + // See line in WP_Query::parse_query() at . + 0 === count( array_diff( array_keys( wp_parse_args( $query->query ) ), array( AMP_QUERY_VAR, 'preview', 'page', 'paged', 'cpage' ) ) ) + ); + if ( $is_front_page_query ) { + $query->is_home = false; + $query->is_page = true; + $query->is_singular = true; + $query->set( 'page_id', get_option( 'page_on_front' ) ); + } +} + function amp_maybe_add_actions() { if ( ! is_singular() || is_feed() ) { return; From bb8743c671e6b085ddbbbf0415400f0fa4e90122 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 17 Jan 2018 20:57:32 -0800 Subject: [PATCH 06/11] Ensure page for posts can be served as AMP if enabled Remove workaround that was fixed in WordPress 4.4 --- amp.php | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/amp.php b/amp.php index 12b61fc224f..1167f348f0e 100644 --- a/amp.php +++ b/amp.php @@ -158,20 +158,27 @@ function amp_correct_query_when_is_front_page( WP_Query $query ) { } } +/** + * Add AMP actions when the request can be served as AMP. + * + * Actions will only be added if the request is for a singular post (including front page and page for posts), excluding feeds. + * + * @since 0.2 + */ function amp_maybe_add_actions() { - if ( ! is_singular() || is_feed() ) { + global $wp_query; + if ( ! ( is_singular() || $wp_query->is_posts_page ) || is_feed() ) { return; } - $is_amp_endpoint = is_amp_endpoint(); - // Cannot use `get_queried_object` before canonical redirect; see https://core.trac.wordpress.org/ticket/35344 - global $wp_query; - $post = $wp_query->post; - - $supports = post_supports_amp( $post ); - - if ( ! $supports ) { + /** + * Queried post object. + * + * @var WP_Post $post + */ + $post = get_queried_object(); + if ( ! post_supports_amp( $post ) ) { if ( $is_amp_endpoint ) { wp_safe_redirect( get_permalink( $post->ID ), 301 ); exit; From c224abf26165eb5af611ea1dd0bc6c8ae5dd043b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 17 Jan 2018 21:17:20 -0800 Subject: [PATCH 07/11] Ensure template_type for posts_page is page when loading parts --- includes/templates/class-amp-post-template.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/includes/templates/class-amp-post-template.php b/includes/templates/class-amp-post-template.php index 30624a746db..bd96bd53d87 100644 --- a/includes/templates/class-amp-post-template.php +++ b/includes/templates/class-amp-post-template.php @@ -199,7 +199,8 @@ public function get_customizer_setting( $name, $default = null ) { * Load and print the template parts for the given post. */ public function load() { - $template = is_page() ? 'page' : 'single'; + global $wp_query; + $template = is_page() || $wp_query->is_posts_page ? 'page' : 'single'; $this->load_parts( array( $template ) ); } From 5f6a0b74502344001d40d3141b10e827e91478ff Mon Sep 17 00:00:00 2001 From: ThierryA Date: Thu, 18 Jan 2018 13:15:53 +0100 Subject: [PATCH 08/11] Disable Page post type opt-in by default --- includes/class-amp-post-type-support.php | 3 ++- tests/test-amp-helper-functions.php | 6 ++++++ tests/test-class-amp-post-type-support.php | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/includes/class-amp-post-type-support.php b/includes/class-amp-post-type-support.php index f0c32c78c42..cbdd9e3415d 100644 --- a/includes/class-amp-post-type-support.php +++ b/includes/class-amp-post-type-support.php @@ -17,7 +17,7 @@ class AMP_Post_Type_Support { * @return string[] Post types. */ public static function get_builtin_supported_post_types() { - return array_filter( array( 'post', 'page' ), 'post_type_exists' ); + return array_filter( array( 'post' ), 'post_type_exists' ); } /** @@ -29,6 +29,7 @@ public static function get_builtin_supported_post_types() { public static function get_eligible_post_types() { return array_merge( self::get_builtin_supported_post_types(), + array( 'page' ), array_values( get_post_types( array( 'public' => true, diff --git a/tests/test-amp-helper-functions.php b/tests/test-amp-helper-functions.php index 9791dbd96b6..2d057e3cffa 100644 --- a/tests/test-amp-helper-functions.php +++ b/tests/test-amp-helper-functions.php @@ -109,7 +109,10 @@ public function test_amp_get_permalink_with_pretty_permalinks() { * @covers \post_supports_amp() */ public function test_post_supports_amp() { + add_post_type_support( 'page', AMP_QUERY_VAR ); + // Test disabled by default for page for posts and show on front. + update_option( 'show_on_front', 'page' ); $post = $this->factory()->post->create_and_get( array( 'post_type' => 'page' ) ); $this->assertTrue( post_supports_amp( $post ) ); update_option( 'show_on_front', 'page' ); @@ -125,5 +128,8 @@ public function test_post_supports_amp() { // Test disabled by default for page templates. update_post_meta( $post->ID, '_wp_page_template', 'foo.php' ); $this->assertFalse( post_supports_amp( $post ) ); + + // Reset. + remove_post_type_support( 'page', AMP_QUERY_VAR ); } } diff --git a/tests/test-class-amp-post-type-support.php b/tests/test-class-amp-post-type-support.php index feb11c29e5d..733d9ae46c2 100644 --- a/tests/test-class-amp-post-type-support.php +++ b/tests/test-class-amp-post-type-support.php @@ -28,7 +28,7 @@ public function tearDown() { * @covers AMP_Post_Type_Support::get_builtin_supported_post_types() */ public function test_get_builtin_supported_post_types() { - $this->assertEquals( array( 'post', 'page' ), AMP_Post_Type_Support::get_builtin_supported_post_types() ); + $this->assertEquals( array( 'post' ), AMP_Post_Type_Support::get_builtin_supported_post_types() ); } /** From 0a7f28c6ec106c3f634525353c0d7a091c99793f Mon Sep 17 00:00:00 2001 From: ThierryA Date: Thu, 18 Jan 2018 13:23:18 +0100 Subject: [PATCH 09/11] Updated newly introduced post status default filter name --- includes/amp-helper-functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 04857b4b4e7..b541c0a0488 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -102,7 +102,7 @@ function post_supports_amp( $post ) { * @param string $status Status. * @param WP_Post $post Post. */ - return apply_filters( 'amp_status_default_enabled', $enabled, $post ); + return apply_filters( 'amp_post_status_default_enabled', $enabled, $post ); } } From b0a32054d55ff35b4b22ad5d57160d4ee8f0d20a Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 18 Jan 2018 11:05:29 -0800 Subject: [PATCH 10/11] Use temporary redirect from non-supported AMP to canonical since support may be enabled later --- amp.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/amp.php b/amp.php index 1167f348f0e..68cbdb4b8db 100644 --- a/amp.php +++ b/amp.php @@ -180,7 +180,7 @@ function amp_maybe_add_actions() { $post = get_queried_object(); if ( ! post_supports_amp( $post ) ) { if ( $is_amp_endpoint ) { - wp_safe_redirect( get_permalink( $post->ID ), 301 ); + wp_safe_redirect( get_permalink( $post->ID ), 302 ); // Temporary redirect because AMP may be supported in future. exit; } return; From 313e7899f2180714819c010fc87440c5ecea5cf2 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 18 Jan 2018 12:05:02 -0800 Subject: [PATCH 11/11] Use register_meta() call for amp_status to expose in REST API and handle sanitization --- includes/admin/class-amp-post-meta-box.php | 31 +++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index 4c0e1ab4ce8..06427398033 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -75,11 +75,40 @@ class AMP_Post_Meta_Box { * @since 0.6 */ public function init() { + register_meta( 'post', self::STATUS_POST_META_KEY, array( + 'sanitize_callback' => array( $this, 'sanitize_status' ), + 'type' => 'string', + 'description' => __( 'AMP status.', 'amp' ), + 'show_in_rest' => true, + 'single' => true, + ) ); + add_action( 'admin_enqueue_scripts', array( $this, 'enqueue_admin_assets' ) ); add_action( 'post_submitbox_misc_actions', array( $this, 'render_status' ) ); add_action( 'save_post', array( $this, 'save_amp_status' ) ); add_filter( 'preview_post_link', array( $this, 'preview_post_link' ) ); } + + /** + * Sanitize status. + * + * @param string $status Status. + * @return string Sanitized status. Empty string when invalid. + */ + public function sanitize_status( $status ) { + $status = strtolower( trim( $status ) ); + if ( ! in_array( $status, array( 'enabled', 'disabled' ), true ) ) { + /* + * In lieu of actual validation being available, clear the status entirely + * so that the underlying default status will be used instead. + * In the future it would be ideal if register_meta() accepted a + * validate_callback as well which the REST API could leverage. + */ + $status = ''; + } + return $status; + } + /** * Enqueue admin assets. * @@ -178,7 +207,7 @@ public function save_amp_status( $post_id ) { update_post_meta( $post_id, self::STATUS_POST_META_KEY, - sanitize_key( wp_unslash( $_POST[ self::STATUS_INPUT_NAME ] ) ) + $_POST[ self::STATUS_INPUT_NAME ] // Note: The sanitize_callback has been supplied in the register_meta() call above. ); } }