From acf9849ef0b3094dfb18b4c15a0980f22d8b9b15 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Jul 2018 11:31:22 -0700 Subject: [PATCH] Restore available_callback but mark as deprecated Use _doing_it_wrong() instead of trigger_error() --- includes/admin/class-amp-post-meta-box.php | 5 + includes/class-amp-theme-support.php | 63 ++++++--- .../options/class-amp-options-manager.php | 26 ++-- includes/options/class-amp-options-menu.php | 132 ++++++++++-------- tests/test-class-amp-theme-support.php | 100 ++++++++++--- 5 files changed, 216 insertions(+), 110 deletions(-) diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index 58c14246363..325e81e7b9d 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -184,6 +184,11 @@ public function render_status( $post ) { return; } + /* + * When theme support is present then theme templates can be served in AMP and we check first if the template is available. + * Checking for template availability will include a check for get_support_errors. Otherwise, if theme support is not present + * then we just check get_support_errors. + */ if ( current_theme_supports( 'amp' ) ) { $availability = AMP_Theme_Support::get_template_availability( $post ); $status = $availability['supported'] ? self::ENABLED_STATUS : self::DISABLED_STATUS; diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index f373a004724..fbc25ba3050 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -157,10 +157,8 @@ public static function is_support_added_via_option() { * Read theme support and apply options from admin for whether theme support is enabled and via what template is enabled. * * @see AMP_Post_Type_Support::add_post_type_support() For where post type support is added, since it is irrespective of theme support. - * - * @param bool $check_args Whether the theme support args should be checked. */ - public static function read_theme_support( $check_args = WP_DEBUG ) { + public static function read_theme_support() { self::$support_added_via_option = false; self::$initial_theme_support_args = false; @@ -172,18 +170,20 @@ public static function read_theme_support( $check_args = WP_DEBUG ) { self::$initial_theme_support_args = array_shift( $support ); // Validate theme support usage. - if ( $check_args ) { - $keys = array( 'template_dir', 'comments_live_list', 'mode', 'optional', 'templates_supported' ); - if ( ! is_array( self::$initial_theme_support_args ) ) { - trigger_error( esc_html__( 'Expected AMP theme support arg to be array.', 'amp' ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error - } elseif ( count( array_diff( array_keys( self::$initial_theme_support_args ), $keys ) ) !== 0 ) { - trigger_error( esc_html( sprintf( // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error - /* translators: %1$s is expected keys and %2$s is actual keys */ - __( 'Expected AMP theme support to keys (%1$s) but saw (%2$s)', 'amp' ), - join( ', ', $keys ), - join( ', ', array_keys( self::$initial_theme_support_args ) ) - ) ) ); - } + $keys = array( 'template_dir', 'comments_live_list', 'mode', 'optional', 'templates_supported', 'available_callback' ); + if ( ! is_array( self::$initial_theme_support_args ) ) { + _doing_it_wrong( 'add_theme_support', esc_html__( 'Expected AMP theme support arg to be array.', 'amp' ), '1.0' ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error + } elseif ( count( array_diff( array_keys( self::$initial_theme_support_args ), $keys ) ) !== 0 ) { + _doing_it_wrong( 'add_theme_support', esc_html( sprintf( // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error + /* translators: %1$s is expected keys and %2$s is actual keys */ + __( 'Expected AMP theme support to keys (%1$s) but saw (%2$s)', 'amp' ), + join( ', ', $keys ), + join( ', ', array_keys( self::$initial_theme_support_args ) ) + ) ), '1.0' ); + } + + if ( isset( self::$initial_theme_support_args['available_callback'] ) ) { + _doing_it_wrong( 'add_theme_support', esc_html__( 'The available_callback is deprecated when adding amp theme support in favor of declaratively setting the supported_templates.', 'amp' ), '1.0' ); } } } @@ -359,7 +359,6 @@ public static function redirect_ampless_url( $exit = true ) { * When 'amp' theme support has not been added or canonical mode is enabled, then this returns false. * * @since 0.7 - * @since 1.0 This no longer looks at the available_callback bit instead calls get_template_availability. * * @see amp_is_canonical() * @return bool Whether available. @@ -457,6 +456,38 @@ public static function get_template_availability( $query = null ) { ); } + // Support available_callback from 0.7, though it is deprecated. + if ( isset( $theme_support_args['available_callback'] ) && is_callable( $theme_support_args['available_callback'] ) ) { + /** + * Queried object. + * + * @var WP_Post $queried_object + */ + $queried_object = $query->get_queried_object(); + if ( ( is_singular() || $query->is_posts_page ) && ! post_supports_amp( $queried_object ) ) { + return array_merge( + $default_response, + array( + 'errors' => array( 'no-post-support' ), + 'supported' => false, + 'immutable' => true, + ) + ); + } + + $response = array_merge( + $default_response, + array( + 'supported' => call_user_func( $theme_support_args['available_callback'] ), + 'immutable' => true, + ) + ); + if ( ! $response['supported'] ) { + $response['errors'][] = 'available_callback'; + } + return $response; + } + $all_templates_supported_by_theme_support = false; if ( isset( $theme_support_args['templates_supported'] ) ) { $all_templates_supported_by_theme_support = 'all' === $theme_support_args['templates_supported']; diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php index 40eb41f85ee..e18bdbc1bad 100644 --- a/includes/options/class-amp-options-manager.php +++ b/includes/options/class-amp-options-manager.php @@ -122,24 +122,24 @@ public static function validate_options( $new_options ) { $options['accept_tree_shaking'] = ! empty( $new_options['accept_tree_shaking'] ); $options['disable_admin_bar'] = ! empty( $new_options['disable_admin_bar'] ); + // Validate post type support. + $options['supported_post_types'] = array(); + if ( isset( $new_options['supported_post_types'] ) ) { + foreach ( $new_options['supported_post_types'] as $post_type ) { + if ( ! post_type_exists( $post_type ) ) { + add_settings_error( self::OPTION_NAME, 'unknown_post_type', __( 'Unrecognized post type.', 'amp' ) ); + } else { + $options['supported_post_types'][] = $post_type; + } + } + } + $theme_support_args = AMP_Theme_Support::get_theme_support_args( array( 'initial' => true ) ); $is_template_support_required = ( isset( $theme_support_args['templates_supported'] ) && 'all' === $theme_support_args['templates_supported'] ); - if ( ! $is_template_support_required ) { + if ( ! $is_template_support_required && ! isset( $theme_support_args['available_callback'] ) ) { $options['all_templates_supported'] = ! empty( $new_options['all_templates_supported'] ); - // Validate post type support. - $options['supported_post_types'] = array(); - if ( isset( $new_options['supported_post_types'] ) ) { - foreach ( $new_options['supported_post_types'] as $post_type ) { - if ( ! post_type_exists( $post_type ) ) { - add_settings_error( self::OPTION_NAME, 'unknown_post_type', __( 'Unrecognized post type.', 'amp' ) ); - } else { - $options['supported_post_types'][] = $post_type; - } - } - } - // Validate supported templates. $options['supported_templates'] = array(); if ( isset( $new_options['supported_templates'] ) ) { diff --git a/includes/options/class-amp-options-menu.php b/includes/options/class-amp-options-menu.php index 207cdfd2733..82672bc1df6 100644 --- a/includes/options/class-amp-options-menu.php +++ b/includes/options/class-amp-options-menu.php @@ -287,27 +287,35 @@ public function render_validation_handling() { */ public function render_supported_templates() { $theme_support_args = AMP_Theme_Support::get_theme_support_args( array( 'initial' => true ) ); // Initial so we can get before removed if optional. - ?> -
- -
+ + +
+ +
+

+ +

+
+

- +

-
- +

+ +

+ +
+ +

- -

-

- +

- - +
+
@@ -334,55 +342,57 @@ public function render_supported_templates() {
-
- -

- + +
+ +

+ + +
+ -
- - + assertInstanceOf( 'PHPUnit_Framework_Error_Notice', $e ); - $this->assertEquals( 'Expected AMP theme support arg to be array.', $e->getMessage() ); + AMP_Theme_Support::read_theme_support(); $this->assertTrue( current_theme_supports( 'amp' ) ); + } - // Test invalid args for theme support. + /** + * Test add_theme_support(amp) with invalid args. + * + * @expectedIncorrectUsage add_theme_support + * @covers \AMP_Theme_Support::read_theme_support() + * @covers \AMP_Theme_Support::get_theme_support_args() + */ + public function test_read_theme_support_bad_args_array() { $args = array( 'mode' => 'native', 'invalid_param_key' => array(), ); add_theme_support( 'amp', $args ); - try { - AMP_Theme_Support::read_theme_support( true ); - } catch ( Exception $exception ) { - $e = $exception; - } - $this->assertStringStartsWith( 'Expected AMP theme support to keys', $e->getMessage() ); + AMP_Theme_Support::read_theme_support(); + $this->assertTrue( current_theme_supports( 'amp' ) ); $this->assertEquals( $args, AMP_Theme_Support::get_theme_support_args() ); $this->assertEquals( $args, AMP_Theme_Support::get_theme_support_args( array( 'initial' => true ) ) ); $this->assertTrue( current_theme_supports( 'amp' ) ); + } + + /** + * Test add_theme_support(amp) with invalid args. + * + * @expectedIncorrectUsage add_theme_support + * @covers \AMP_Theme_Support::read_theme_support() + */ + public function test_read_theme_support_bad_available_callback() { + add_theme_support( 'amp', array( + 'available_callback' => function() { + return (bool) wp_rand( 0, 1 ); + }, + ) ); + AMP_Theme_Support::read_theme_support(); + $this->assertTrue( current_theme_supports( 'amp' ) ); + } + + /** + * Test read_theme_support, get_theme_support_args, and is_support_added_via_option. + * + * @covers \AMP_Theme_Support::read_theme_support() + * @covers \AMP_Theme_Support::is_support_added_via_option() + * @covers \AMP_Theme_Support::get_theme_support_args() + */ + public function test_read_theme_support_and_support_args() { // Test behavior of optional flag, that AMP is not enabled if the DB option is not set. $args = array( @@ -441,6 +462,45 @@ public function test_incorrect_usage_get_template_availability() { $this->assertNull( $availability['template'] ); } + /** + * Test get_template_availability with available_callback. + * + * @expectedIncorrectUsage add_theme_support + * @covers AMP_Theme_Support::get_template_availability() + */ + public function test_get_template_availability_with_available_callback() { + $this->go_to( get_permalink( $this->factory()->post->create() ) ); + add_theme_support( 'amp', array( + 'available_callback' => '__return_true', + ) ); + AMP_Theme_Support::init(); + $availability = AMP_Theme_Support::get_template_availability(); + $this->assertEquals( + $availability, + array( + 'supported' => true, + 'immutable' => true, + 'template' => null, + 'errors' => array(), + ) + ); + + add_theme_support( 'amp', array( + 'available_callback' => '__return_false', + ) ); + AMP_Theme_Support::init(); + $availability = AMP_Theme_Support::get_template_availability(); + $this->assertEquals( + $availability, + array( + 'supported' => false, + 'immutable' => true, + 'template' => null, + 'errors' => array( 'available_callback' ), + ) + ); + } + /** * Test get_template_availability. *