Skip to content

Commit

Permalink
Introduce amp_get_slug() to replace AMP_QUERY_VAR
Browse files Browse the repository at this point in the history
  • Loading branch information
westonruter committed Mar 10, 2018
1 parent c42dd3c commit d024c4c
Show file tree
Hide file tree
Showing 13 changed files with 76 additions and 49 deletions.
36 changes: 12 additions & 24 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ function amp_deactivate() {
// We need to manually remove the amp endpoint
global $wp_rewrite;
foreach ( $wp_rewrite->endpoints as $index => $endpoint ) {
if ( AMP_QUERY_VAR === $endpoint[1] ) {
if ( amp_get_slug() === $endpoint[1] ) {
unset( $wp_rewrite->endpoints[ $index ] );
break;
}
Expand All @@ -73,24 +73,12 @@ function amp_deactivate() {
* @since 0.6
*/
function amp_after_setup_theme() {
amp_get_slug(); // Ensure AMP_QUERY_VAR is set.

if ( false === apply_filters( 'amp_is_enabled', true ) ) {
// Set a fallback value to avoid constant not defined warnings
if ( ! defined( 'AMP_QUERY_VAR' ) ) {
define( 'AMP_QUERY_VAR', false );
}
return;
}

if ( ! defined( 'AMP_QUERY_VAR' ) ) {
/**
* Filter the AMP query variable.
*
* @since 0.3.2
* @param string $query_var The AMP query variable.
*/
define( 'AMP_QUERY_VAR', apply_filters( 'amp_query_var', 'amp' ) );
}

add_action( 'init', 'amp_init', 0 ); // Must be 0 because widgets_init happens at init priority 1.
}
add_action( 'after_setup_theme', 'amp_after_setup_theme', 5 );
Expand All @@ -111,7 +99,7 @@ function amp_init() {

load_plugin_textdomain( 'amp', false, plugin_basename( AMP__DIR__ ) . '/languages' );

add_rewrite_endpoint( AMP_QUERY_VAR, EP_PERMALINK );
add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );

AMP_Validation_Utils::init();
AMP_Theme_Support::init();
Expand All @@ -136,8 +124,8 @@ function amp_init() {
// 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 ) {
if ( isset( $query_vars[ AMP_QUERY_VAR ] ) && '' === $query_vars[ AMP_QUERY_VAR ] ) {
$query_vars[ AMP_QUERY_VAR ] = 1;
if ( isset( $query_vars[ amp_get_slug() ] ) && '' === $query_vars[ amp_get_slug() ] ) {
$query_vars[ amp_get_slug() ] = 1;
}
return $query_vars;
}
Expand Down Expand Up @@ -206,7 +194,7 @@ function amp_correct_query_when_is_front_page( WP_Query $query ) {
$query->is_home()
&&
// Is AMP endpoint.
false !== $query->get( AMP_QUERY_VAR, false )
false !== $query->get( amp_get_slug(), false )
&&
// Is query not yet fixed uo up to be front page.
! $query->is_front_page()
Expand All @@ -218,7 +206,7 @@ function amp_correct_query_when_is_front_page( WP_Query $query ) {
get_option( 'page_on_front' )
&&
// See line in WP_Query::parse_query() at <https://github.com/WordPress/wordpress-develop/blob/0baa8ae/src/wp-includes/class-wp-query.php#L961>.
0 === count( array_diff( array_keys( wp_parse_args( $query->query ) ), array( AMP_QUERY_VAR, 'preview', 'page', 'paged', 'cpage' ) ) )
0 === count( array_diff( array_keys( wp_parse_args( $query->query ) ), array( amp_get_slug(), 'preview', 'page', 'paged', 'cpage' ) ) )
);
if ( $is_front_page_query ) {
$query->is_home = false;
Expand Down Expand Up @@ -308,9 +296,9 @@ function amp_render_post( $post ) {
* which is not ideal for any code that expects to run in an AMP context.
* Let's force the value to be true while we render AMP.
*/
$was_set = isset( $wp_query->query_vars[ AMP_QUERY_VAR ] );
$was_set = isset( $wp_query->query_vars[ amp_get_slug() ] );
if ( ! $was_set ) {
$wp_query->query_vars[ AMP_QUERY_VAR ] = true;
$wp_query->query_vars[ amp_get_slug() ] = true;
}

// Prevent New Relic from causing invalid AMP responses due the NREUM script it injects after the meta charset.
Expand All @@ -332,7 +320,7 @@ function amp_render_post( $post ) {
$template->load();

if ( ! $was_set ) {
unset( $wp_query->query_vars[ AMP_QUERY_VAR ] );
unset( $wp_query->query_vars[ amp_get_slug() ] );
}
}

Expand Down Expand Up @@ -363,7 +351,7 @@ function _amp_bootstrap_customizer() {
function amp_redirect_old_slug_to_new_url( $link ) {

if ( is_amp_endpoint() ) {
$link = trailingslashit( trailingslashit( $link ) . AMP_QUERY_VAR );
$link = trailingslashit( trailingslashit( $link ) . amp_get_slug() );
}

return $link;
Expand Down
2 changes: 1 addition & 1 deletion includes/admin/class-amp-customizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public function add_customizer_scripts() {

wp_add_inline_script( 'amp-customize-controls', sprintf( 'ampCustomizeControls.boot( %s );',
wp_json_encode( array(
'queryVar' => AMP_QUERY_VAR,
'queryVar' => amp_get_slug(),
'panelId' => self::PANEL_ID,
'ampUrl' => amp_admin_get_preview_permalink(),
'l10n' => array(
Expand Down
4 changes: 2 additions & 2 deletions includes/admin/class-amp-post-meta-box.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,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 ) ) ),
'previewLink' => esc_url_raw( add_query_arg( amp_get_slug(), '', get_preview_post_link( $post ) ) ),
'canonical' => amp_is_canonical(),
'enabled' => post_supports_amp( $post ),
'canSupport' => count( AMP_Post_Type_Support::get_support_errors( $post ) ) === 0,
Expand Down Expand Up @@ -237,7 +237,7 @@ public function preview_post_link( $link ) {
);

if ( $is_amp ) {
$link = add_query_arg( AMP_QUERY_VAR, true, $link );
$link = add_query_arg( amp_get_slug(), true, $link );
}

return $link;
Expand Down
2 changes: 1 addition & 1 deletion includes/admin/functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ function amp_admin_get_preview_permalink() {
*/
$post_type = (string) apply_filters( 'amp_customizer_post_type', 'post' );

if ( ! post_type_supports( $post_type, AMP_QUERY_VAR ) ) {
if ( ! post_type_supports( $post_type, amp_get_slug() ) ) {
return null;
}

Expand Down
40 changes: 35 additions & 5 deletions includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,36 @@
* @package AMP
*/

/**
* Get the slug used in AMP for the query var, endpoint, and post type support.
*
* The return value can be overridden by previously defining a AMP_QUERY_VAR
* constant or by adding a 'amp_query_var' filter, but *warning* this ability
* may be deprecated in the future. Normally the slug should be just 'amp'.
*
* @since 0.7
* @return string Slug used for query var, endpoint, and post type support.
*/
function amp_get_slug() {
if ( defined( 'AMP_QUERY_VAR' ) ) {
return AMP_QUERY_VAR;
}

/**
* Filter the AMP query variable.
*
* Warning: This filter may become deprecated.
*
* @since 0.3.2
* @param string $query_var The AMP query variable.
*/
$query_var = apply_filters( 'amp_query_var', 'amp' );

define( 'AMP_QUERY_VAR', $query_var );

return $query_var;
}

/**
* Retrieves the full AMP-specific permalink for the given post ID.
*
Expand Down Expand Up @@ -38,9 +68,9 @@ function amp_get_permalink( $post_id ) {
$parsed_url = wp_parse_url( get_permalink( $post_id ) );
$structure = get_option( 'permalink_structure' );
if ( empty( $structure ) || ! empty( $parsed_url['query'] ) || is_post_type_hierarchical( get_post_type( $post_id ) ) ) {
$amp_url = add_query_arg( AMP_QUERY_VAR, '', get_permalink( $post_id ) );
$amp_url = add_query_arg( amp_get_slug(), '', get_permalink( $post_id ) );
} else {
$amp_url = trailingslashit( get_permalink( $post_id ) ) . user_trailingslashit( AMP_QUERY_VAR, 'single_amp' );
$amp_url = trailingslashit( get_permalink( $post_id ) ) . user_trailingslashit( amp_get_slug(), 'single_amp' );
}
}

Expand All @@ -66,10 +96,10 @@ function amp_get_permalink( $post_id ) {
function amp_remove_endpoint( $url ) {

// Strip endpoint.
$url = preg_replace( ':/' . preg_quote( AMP_QUERY_VAR, ':' ) . '(?=/?(\?|#|$)):', '', $url );
$url = preg_replace( ':/' . preg_quote( amp_get_slug(), ':' ) . '(?=/?(\?|#|$)):', '', $url );

// Strip query var.
$url = remove_query_arg( AMP_QUERY_VAR, $url );
$url = remove_query_arg( amp_get_slug(), $url );

return $url;
}
Expand Down Expand Up @@ -149,7 +179,7 @@ function is_amp_endpoint() {
_doing_it_wrong( __FUNCTION__, sprintf( esc_html__( "is_amp_endpoint() was called before the 'parse_query' hook was called. This function will always return 'false' before the 'parse_query' hook is called.", 'amp' ) ), '0.4.2' );
}

return false !== get_query_var( AMP_QUERY_VAR, false );
return false !== get_query_var( amp_get_slug(), false );
}

/**
Expand Down
4 changes: 2 additions & 2 deletions includes/class-amp-post-type-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public static function add_post_type_support() {
AMP_Options_Manager::get_option( 'supported_post_types', array() )
);
foreach ( $post_types as $post_type ) {
add_post_type_support( $post_type, AMP_QUERY_VAR );
add_post_type_support( $post_type, amp_get_slug() );
}
}

Expand All @@ -73,7 +73,7 @@ public static function get_support_errors( $post ) {
$errors = array();

// Because `add_rewrite_endpoint` doesn't let us target specific post_types.
if ( isset( $post->post_type ) && ! post_type_supports( $post->post_type, AMP_QUERY_VAR ) ) {
if ( isset( $post->post_type ) && ! post_type_supports( $post->post_type, amp_get_slug() ) ) {
$errors[] = 'post-type-support';
}

Expand Down
2 changes: 1 addition & 1 deletion includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public static function finish_init() {
* @since 0.7
*/
public static function redirect_canonical_amp() {
if ( false !== get_query_var( AMP_QUERY_VAR, false ) ) { // Because is_amp_endpoint() now returns true if amp_is_canonical().
if ( false !== get_query_var( amp_get_slug(), false ) ) { // Because is_amp_endpoint() now returns true if amp_is_canonical().
$url = preg_replace( '#^(https?://.+?)(/.*)$#', '$1', home_url( '/' ) );
if ( isset( $_SERVER['REQUEST_URI'] ) ) {
$url .= wp_unslash( $_SERVER['REQUEST_URI'] );
Expand Down
2 changes: 1 addition & 1 deletion includes/options/class-amp-options-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public static function check_supported_post_type_update_errors() {
continue;
}

$post_type_supported = post_type_supports( $post_type->name, AMP_QUERY_VAR );
$post_type_supported = post_type_supports( $post_type->name, amp_get_slug() );
$is_support_elected = in_array( $post_type->name, $supported_types, true );

$error = null;
Expand Down
2 changes: 1 addition & 1 deletion includes/options/class-amp-options-menu.php
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public function render_post_types_support() {
id="<?php echo esc_attr( $element_id ); ?>"
name="<?php echo esc_attr( $element_name ); ?>"
value="<?php echo esc_attr( $post_type->name ); ?>"
<?php checked( true, amp_is_canonical() || post_type_supports( $post_type->name, AMP_QUERY_VAR ) ); ?>
<?php checked( true, amp_is_canonical() || post_type_supports( $post_type->name, amp_get_slug() ) ); ?>
<?php disabled( $is_builtin ); ?>
>
<label for="<?php echo esc_attr( $element_id ); ?>">
Expand Down
13 changes: 11 additions & 2 deletions tests/test-amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ public function return_example_url( $url, $post_id ) {
return 'http://overridden.example.com/?' . build_query( compact( 'url', 'post_id', 'current_filter' ) );
}

/**
* Test amp_get_slug().
*
* @covers amp_get_slug()
*/
public function test_amp_get_slug() {
$this->assertSame( 'amp', amp_get_slug() );
}

/**
* Test amp_get_permalink() without pretty permalinks.
*
Expand Down Expand Up @@ -261,7 +270,7 @@ public function test_amp_get_content_sanitizers_deprecated_param() {
* @covers \post_supports_amp()
*/
public function test_post_supports_amp() {
add_post_type_support( 'page', AMP_QUERY_VAR );
add_post_type_support( 'page', amp_get_slug() );

// Test disabled by default for page for posts and show on front.
update_option( 'show_on_front', 'page' );
Expand All @@ -282,7 +291,7 @@ public function test_post_supports_amp() {
$this->assertFalse( post_supports_amp( $post ) );

// Reset.
remove_post_type_support( 'page', AMP_QUERY_VAR );
remove_post_type_support( 'page', amp_get_slug() );
}

/**
Expand Down
6 changes: 3 additions & 3 deletions tests/test-class-amp-meta-box.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@ public function test_render_status() {
$this->instance->render_status( $post );
$this->assertContains( $amp_status_markup, ob_get_clean() );

remove_post_type_support( 'post', AMP_QUERY_VAR );
remove_post_type_support( 'post', amp_get_slug() );

ob_start();
$this->instance->render_status( $post );
$this->assertEmpty( ob_get_clean() );

add_post_type_support( 'post', AMP_QUERY_VAR );
add_post_type_support( 'post', amp_get_slug() );
wp_set_current_user( $this->factory->user->create( array(
'role' => 'subscriber',
) ) );
Expand Down Expand Up @@ -160,7 +160,7 @@ public function test_preview_post_link() {
$link = 'https://foo.bar';
$this->assertEquals( 'https://foo.bar', $this->instance->preview_post_link( $link ) );
$_POST['amp-preview'] = 'do-preview';
$this->assertEquals( 'https://foo.bar?' . AMP_QUERY_VAR . '=1', $this->instance->preview_post_link( $link ) );
$this->assertEquals( 'https://foo.bar?' . amp_get_slug() . '=1', $this->instance->preview_post_link( $link ) );
}

}
4 changes: 2 additions & 2 deletions tests/test-class-amp-options-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public function test_check_supported_post_type_update_errors() {
$this->assertEmpty( get_settings_errors() );

// Activation error.
remove_post_type_support( 'foo', AMP_QUERY_VAR );
remove_post_type_support( 'foo', amp_get_slug() );
AMP_Options_Manager::check_supported_post_type_update_errors();
$errors = get_settings_errors();
$this->assertCount( 1, $errors );
Expand All @@ -181,7 +181,7 @@ public function test_check_supported_post_type_update_errors() {

// Deactivation error.
AMP_Options_Manager::update_option( 'supported_post_types', array() );
add_post_type_support( 'foo', AMP_QUERY_VAR );
add_post_type_support( 'foo', amp_get_slug() );
AMP_Options_Manager::check_supported_post_type_update_errors();
$errors = get_settings_errors();
$this->assertCount( 1, $errors );
Expand Down
8 changes: 4 additions & 4 deletions tests/test-class-amp-post-type-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ public function test_add_post_type_support() {
AMP_Options_Manager::update_option( 'supported_post_types', array( 'poem' ) );

AMP_Post_Type_Support::add_post_type_support();
$this->assertTrue( post_type_supports( 'post', AMP_QUERY_VAR ) );
$this->assertTrue( post_type_supports( 'poem', AMP_QUERY_VAR ) );
$this->assertFalse( post_type_supports( 'book', AMP_QUERY_VAR ) );
$this->assertTrue( post_type_supports( 'post', amp_get_slug() ) );
$this->assertTrue( post_type_supports( 'poem', amp_get_slug() ) );
$this->assertFalse( post_type_supports( 'book', amp_get_slug() ) );
}

/**
Expand All @@ -92,7 +92,7 @@ public function test_get_support_error() {
// Post type support.
$book_id = $this->factory()->post->create( array( 'post_type' => 'book' ) );
$this->assertEquals( array( 'post-type-support' ), AMP_Post_Type_Support::get_support_errors( $book_id ) );
add_post_type_support( 'book', AMP_QUERY_VAR );
add_post_type_support( 'book', amp_get_slug() );
$this->assertEmpty( AMP_Post_Type_Support::get_support_errors( $book_id ) );

// Password-protected.
Expand Down

0 comments on commit d024c4c

Please sign in to comment.