diff --git a/amp.php b/amp.php index 56756588371..0e08b153977 100644 --- a/amp.php +++ b/amp.php @@ -17,6 +17,7 @@ require_once AMP__DIR__ . '/back-compat/back-compat.php'; require_once AMP__DIR__ . '/includes/amp-helper-functions.php'; +require_once AMP__DIR__ . '/includes/amp-post-types-support.php'; require_once AMP__DIR__ . '/includes/admin/functions.php'; require_once AMP__DIR__ . '/includes/admin/class-amp-customizer.php'; require_once AMP__DIR__ . '/includes/settings/class-amp-customizer-settings.php'; @@ -54,14 +55,11 @@ function amp_init() { return; } - define( 'AMP_QUERY_VAR', apply_filters( 'amp_query_var', 'amp' ) ); - do_action( 'amp_init' ); load_plugin_textdomain( 'amp', false, plugin_basename( AMP__DIR__ ) . '/languages' ); add_rewrite_endpoint( AMP_QUERY_VAR, EP_PERMALINK ); - add_post_type_support( 'post', AMP_QUERY_VAR ); add_filter( 'request', 'amp_force_query_var_value' ); add_action( 'wp', 'amp_maybe_add_actions' ); @@ -74,6 +72,19 @@ function amp_init() { } } +/** + * Define AMP query var. + * + * This function must be invoked through the 'after_setup_theme' action to allow + * the AMP setting to declare the post types support earlier than plugins/theme. + * + * @since 0.6 + */ +function define_query_var() { + define( 'AMP_QUERY_VAR', apply_filters( 'amp_query_var', 'amp' ) ); +} +add_action( 'after_setup_theme', 'define_query_var', 7 ); + // Make sure the `amp` query var has an explicit value. // Avoids issues when filtering the deprecated `query_string` hook. function amp_force_query_var_value( $query_vars ) { diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 62568471a31..8ba5cdcac00 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -18,11 +18,8 @@ function amp_get_permalink( $post_id ) { } function post_supports_amp( $post ) { - // Listen to post types settings. - $is_enabled_via_setting = (bool) AMP_Settings_Post_Types::get_instance()->get_settings_value( $post->post_type ); - - // Because `add_rewrite_endpoint` doesn't let us target specific post_types. - if ( ! post_type_supports( $post->post_type, AMP_QUERY_VAR ) && true !== $is_enabled_via_setting ) { + // Because `add_rewrite_endpoint` doesn't let us target specific post_types :(. + if ( ! post_type_supports( $post->post_type, AMP_QUERY_VAR ) ) { return false; } diff --git a/includes/amp-post-types-support.php b/includes/amp-post-types-support.php new file mode 100644 index 00000000000..3a5a8683d64 --- /dev/null +++ b/includes/amp-post-types-support.php @@ -0,0 +1,35 @@ +get_settings() as $post_type => $enabled ) { + if ( true === $enabled ) { + add_post_type_support( $post_type, AMP_QUERY_VAR ); + } + } +} +add_action( 'after_setup_theme', 'amp_custom_post_types_support' ); diff --git a/includes/settings/class-amp-settings-post-types.php b/includes/settings/class-amp-settings-post-types.php index b53b6c0727b..cf70b2c9d54 100644 --- a/includes/settings/class-amp-settings-post-types.php +++ b/includes/settings/class-amp-settings-post-types.php @@ -41,13 +41,18 @@ public function __construct() { public function init() { add_action( 'admin_init', array( $this, 'register_settings' ) ); add_action( 'update_option_' . AMP_Settings::SETTINGS_KEY, 'flush_rewrite_rules' ); + add_action( 'amp_settings_screen', array( $this, 'errors' ) ); } /** * Register the current page settings. */ public function register_settings() { - register_setting( AMP_Settings::SETTINGS_KEY, AMP_Settings::SETTINGS_KEY ); + register_setting( + AMP_Settings::SETTINGS_KEY, + AMP_Settings::SETTINGS_KEY, + array( $this, 'validate' ) + ); add_settings_section( $this->section_id, false, @@ -57,7 +62,7 @@ public function register_settings() { add_settings_field( $this->setting['id'], $this->setting['label'], - array( $this, 'render_setting' ), + array( $this, 'render' ), AMP_Settings::MENU_SLUG, $this->section_id ); @@ -67,16 +72,24 @@ public function register_settings() { * Getter for settings value. * * @param string $post_type The post type name. - * @return bool Return the setting value. + * @return bool|array Return true if the post type is always on; the setting value otherwise. */ - public function get_settings_value( $post_type ) { - $settings = get_option( AMP_Settings::SETTINGS_KEY, array() ); + public function get_settings( $post_type = false ) { + $settings = $this->validate( get_option( AMP_Settings::SETTINGS_KEY, array() ) ); + + if ( false !== $post_type ) { + if ( isset( $settings[ $this->setting['id'] ][ $post_type ] ) ) { + return $settings[ $this->setting['id'] ][ $post_type ]; + } - if ( isset( $settings[ $this->setting['id'] ][ $post_type ] ) ) { - return (bool) $settings[ $this->setting['id'] ][ $post_type ]; + return false; } - return false; + if ( empty( $settings[ $this->setting['id'] ] ) || ! is_array( $settings[ $this->setting['id'] ] ) ) { + return array(); + } + + return $settings[ $this->setting['id'] ]; } /** @@ -102,16 +115,77 @@ public function get_supported_post_types() { * @param string $post_type The post type name. * @return object The setting HTML input name attribute. */ - public function get_setting_name( $post_type ) { + public function get_name_attribute( $post_type ) { $id = $this->setting['id']; - return AMP_Settings::SETTINGS_KEY . "[{$id}][{$post_type}]"; } + /** + * Check whether the post type should be disabled or not. + * + * Since we can't flag a post type which is not enabled by setting and removed by plugin/theme, + * we can't disable the checkbox but the errors() takes care of this scenario. + * + * @param string $post_type The post type name. + * @return bool True if disabled; false otherwise. + */ + public function disabled( $post_type ) { + $settings = $this->get_settings(); + + // Disable if post type support was not added by setting and added by plugin/theme. + if ( post_type_supports( $post_type, AMP_QUERY_VAR ) && ! isset( $settings[ $post_type ] ) ) { + return true; + } + + return false; + } + + /** + * Handle errors. + */ + public function errors() { + $settings = $this->get_settings(); + + foreach ( $settings as $post_type => $value ) { + // Throw error if post type support was added by setting and removed by plugin/theme. + if ( true === $value && ! post_type_supports( $post_type, AMP_QUERY_VAR ) ) { + $post_type_object = get_post_type_object( $post_type ); + + add_settings_error( + $post_type, + $post_type, + sprintf( + /* Translators: %s: Post type name. */ + __( '"%s" could not be activated because support was removed by a plugin or theme', 'amp' ), + isset( $post_type_object->label ) ? $post_type_object->label : $post_type + ) + ); + } + } + } + + /** + * Validate and sanitize the settings. + * + * @param array $settings The post types settings. + * @return array The post types settings. + */ + public function validate( $settings ) { + if ( ! isset( $settings[ $this->setting['id'] ] ) || ! is_array( $settings[ $this->setting['id'] ] ) ) { + return array(); + } + + foreach ( $settings[ $this->setting['id'] ] as $key => $value ) { + $settings[ $this->setting['id'] ][ $key ] = (bool) $value; + } + + return $settings; + } + /** * Setting renderer. */ - public function render_setting() { + public function render() { require_once AMP__DIR__ . '/templates/admin/settings/fields/checkbox-post-types.php'; } diff --git a/includes/settings/class-amp-settings.php b/includes/settings/class-amp-settings.php index 88c7db3eacd..2ee29c78444 100644 --- a/includes/settings/class-amp-settings.php +++ b/includes/settings/class-amp-settings.php @@ -63,6 +63,12 @@ public function render_screen() { return; } + /** + * Fires before the AMP settings screen is rendered. + * + * @since 0.6 + */ + do_action( 'amp_settings_screen' ); include_once AMP__DIR__ . '/templates/admin/settings/screen.php'; } diff --git a/templates/admin/settings/fields/checkbox-post-types.php b/templates/admin/settings/fields/checkbox-post-types.php index e34166eb0ff..4321dc80515 100644 --- a/templates/admin/settings/fields/checkbox-post-types.php +++ b/templates/admin/settings/fields/checkbox-post-types.php @@ -13,7 +13,7 @@
get_supported_post_types() as $post_type ) : ?>
diff --git a/tests/test-amp-post-types-support.php b/tests/test-amp-post-types-support.php new file mode 100644 index 00000000000..e23c59be530 --- /dev/null +++ b/tests/test-amp-post-types-support.php @@ -0,0 +1,45 @@ +assertTrue( post_type_supports( 'post', AMP_QUERY_VAR ) ); + remove_post_type_support( 'post', AMP_QUERY_VAR ); + } + + /** + * Test amp_custom_post_types_support. + * + * @see amp_custom_post_types_support() + */ + public function test_amp_custom_post_types_support() { + update_option( AMP_Settings::SETTINGS_KEY, array( + 'post_types_support' => array( + 'foo' => true, + 'bar' => true, + ), + ) ); + amp_custom_post_types_support(); + $this->assertTrue( post_type_supports( 'foo', AMP_QUERY_VAR ) ); + $this->assertTrue( post_type_supports( 'bar', AMP_QUERY_VAR ) ); + delete_option( AMP_Settings::SETTINGS_KEY ); + remove_post_type_support( 'foo', AMP_QUERY_VAR ); + remove_post_type_support( 'bar', AMP_QUERY_VAR ); + } + +} diff --git a/tests/test-class-amp-settings-post-types.php b/tests/test-class-amp-settings-post-types.php index 725c334fdaa..f87890cbb6c 100644 --- a/tests/test-class-amp-settings-post-types.php +++ b/tests/test-class-amp-settings-post-types.php @@ -75,12 +75,14 @@ public function test_register_settings() { } /** - * Test get_settings_value. + * Test get_value. * - * @see AMP_Settings_Post_Types::get_settings_value() + * @see AMP_Settings_Post_Types::get_settings() */ - public function test_get_settings_value() { - $this->assertFalse( $this->instance->get_settings_value( 'foo' ) ); + public function test_get_settings() { + $this->assertEmpty( $this->instance->get_settings() ); + $this->assertInternalType( 'array', $this->instance->get_settings() ); + $this->assertFalse( $this->instance->get_settings( 'foo' ) ); update_option( AMP_Settings::SETTINGS_KEY, array( 'post_types_support' => array( @@ -88,7 +90,9 @@ public function test_get_settings_value() { ), ) ); - $this->assertTrue( $this->instance->get_settings_value( 'post' ) ); + $this->assertContains( 'post', $this->instance->get_settings() ); + $this->assertInternalType( 'array', $this->instance->get_settings() ); + $this->assertTrue( $this->instance->get_settings( 'post' ) ); // Cleanup. delete_option( AMP_Settings::SETTINGS_KEY ); @@ -105,22 +109,67 @@ public function test_get_supported_post_types() { } /** - * Test get_setting_name. + * Test get_name_attribute. * - * @see AMP_Settings_Post_Types::get_setting_name() + * @see AMP_Settings_Post_Types::get_name_attribute() */ - public function test_get_setting_name() { - $this->assertEquals( AMP_Settings::SETTINGS_KEY . '[post_types_support][post]', $this->instance->get_setting_name( 'post' ) ); + public function test_get_name_attribute() { + $this->assertEquals( AMP_Settings::SETTINGS_KEY . '[post_types_support][post]', $this->instance->get_name_attribute( 'post' ) ); } /** - * Test render_setting. + * Test disabled. * - * @see AMP_Settings_Post_Types::render_setting() + * @see AMP_Settings_Post_Types::disabled() */ - public function test_render_setting() { + public function test_disabled() { + $this->assertFalse( $this->instance->disabled( 'foo' ) ); + add_post_type_support( 'foo', AMP_QUERY_VAR ); + $this->assertTrue( $this->instance->disabled( 'foo' ) ); + } + + /** + * Test errors. + * + * @see AMP_Settings_Post_Types::errors() + */ + public function test_errors() { + update_option( AMP_Settings::SETTINGS_KEY, array( + 'post_types_support' => array( + 'foo' => true, + ), + ) ); + remove_post_type_support( 'foo', AMP_QUERY_VAR ); + $this->instance->errors(); + $this->assertNotEmpty( get_settings_errors() ); + delete_option( AMP_Settings::SETTINGS_KEY ); + } + + /** + * Test validate. + * + * @see AMP_Settings_Post_Types::validate() + */ + public function test_validate() { + $this->assertInternalType( 'array', $this->instance->validate( array() ) ); + update_option( AMP_Settings::SETTINGS_KEY, array( + 'post_types_support' => array( + 'foo' => true, + ), + ) ); + $settings = $this->instance->validate( get_option( AMP_Settings::SETTINGS_KEY ) ); + $this->assertInternalType( 'bool', $settings['post_types_support']['foo'] ); + delete_option( AMP_Settings::SETTINGS_KEY ); + } + + /** + * Test render. + * + * @see AMP_Settings_Post_Types::render() + */ + public function test_render() { ob_start(); - $this->instance->render_setting(); + $this->instance->render(); $this->assertContains( '
', ob_get_clean() ); }