Skip to content

Commit

Permalink
Use template_dir consistently as signal for transitional mode
Browse files Browse the repository at this point in the history
  • Loading branch information
@westonruter
westonruter committed Sep 18, 2019
1 parent 93276f7 commit 94be3ad
Show file tree
Hide file tree
Showing 5 changed files with 206 additions and 56 deletions.
3 changes: 2 additions & 1 deletion includes/class-amp-theme-support.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ public static function read_theme_support() {
);
}

$is_paired = ! empty( $args[ self::PAIRED_FLAG ] );
// See amp_is_canonical().
$is_paired = isset( $args[ self::PAIRED_FLAG ] ) ? $args[ self::PAIRED_FLAG ] : ! empty( $args['template_dir'] );

self::$support_added_via_theme = $is_paired ? self::TRANSITIONAL_MODE_SLUG : self::STANDARD_MODE_SLUG;
self::$support_added_via_option = $theme_support_option;
Expand Down
5 changes: 2 additions & 3 deletions includes/options/class-amp-options-manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,8 @@ public static function get_options() {

$defaults['enable_response_caching'] = wp_using_ext_object_cache();

$args = AMP_Theme_Support::get_theme_support_args();
if ( false !== $args ) {
$defaults['theme_support'] = empty( $args[ AMP_Theme_Support::PAIRED_FLAG ] ) ? AMP_Theme_Support::STANDARD_MODE_SLUG : AMP_Theme_Support::TRANSITIONAL_MODE_SLUG;
if ( current_theme_supports( 'amp' ) ) {
$defaults['theme_support'] = amp_is_canonical() ? AMP_Theme_Support::STANDARD_MODE_SLUG : AMP_Theme_Support::TRANSITIONAL_MODE_SLUG;
}

$options = array_merge( $defaults, $options );
Expand Down
58 changes: 44 additions & 14 deletions tests/php/test-amp-helper-functions.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,15 @@ public function test_amp_add_frontend_actions() {
public function get_amphtml_urls() {
$post_id = self::factory()->post->create();
return [
'home' => [
'is_home' => [
home_url( '/' ),
add_query_arg( amp_get_slug(), '', home_url( '/' ) ),
],
'404' => [
'is_404' => [
home_url( '/no-existe/' ),
add_query_arg( amp_get_slug(), '', home_url( '/no-existe/' ) ),
],
'post' => [
'is_post' => [
get_permalink( $post_id ),
amp_get_permalink( $post_id ),
],
Expand All @@ -292,8 +292,45 @@ public function get_amphtml_urls() {
* @param string $canonical_url Canonical URL.
* @param string $amphtml_url The amphtml URL.
*/
public function test_amp_add_amphtml_link( $canonical_url, $amphtml_url ) {
public function test_amp_add_amphtml_link_reader_mode( $canonical_url, $amphtml_url ) {
$this->assertFalse( current_theme_supports( AMP_Theme_Support::SLUG ) );
$this->assertFalse( amp_is_canonical() );
$get_amp_html_link = static function() {
return get_echo( 'amp_add_amphtml_link' );
};

$assert_amphtml_link_present = function() use ( $amphtml_url, $get_amp_html_link ) {
$this->assertEquals(
sprintf( '<link rel="amphtml" href="%s">', esc_url( $amphtml_url ) ),
$get_amp_html_link()
);
};

$this->go_to( $canonical_url );
$assert_amphtml_link_present();

// Make sure adding the filter hides the amphtml link.
add_filter( 'amp_frontend_show_canonical', '__return_false' );
$this->assertEmpty( $get_amp_html_link() );
remove_filter( 'amp_frontend_show_canonical', '__return_false' );
$assert_amphtml_link_present();
}

/**
* Adding link when theme support in transitional mode.
*
* @dataProvider get_amphtml_urls
* @covers ::amp_add_amphtml_link()
* @param string $canonical_url Canonical URL.
* @param string $amphtml_url The amphtml URL.
*/
public function test_amp_add_amphtml_link_transitional_mode( $canonical_url, $amphtml_url ) {
AMP_Options_Manager::update_option( 'theme_support', AMP_Theme_Support::TRANSITIONAL_MODE_SLUG );
AMP_Options_Manager::update_option( 'auto_accept_sanitization', false );
AMP_Theme_Support::read_theme_support();
AMP_Theme_Support::init();
$this->assertTrue( current_theme_supports( AMP_Theme_Support::SLUG ) );
$this->assertFalse( amp_is_canonical() );

$get_amp_html_link = static function() {
return get_echo( 'amp_add_amphtml_link' );
Expand All @@ -316,26 +353,19 @@ public function test_amp_add_amphtml_link( $canonical_url, $amphtml_url ) {
$assert_amphtml_link_present();

// Make sure that the link is not provided when there are validation errors associated with the URL.
add_theme_support(
AMP_Theme_Support::SLUG,
[
'template_dir' => './',
]
);
AMP_Options_Manager::update_option( 'theme_support', AMP_Theme_Support::STANDARD_MODE_SLUG );
AMP_Theme_Support::read_theme_support();
AMP_Theme_Support::init();
$invalid_url_post_id = AMP_Validated_URL_Post_Type::store_validation_errors(
[
[ 'code' => 'foo' ],
],
$canonical_url
);
$this->assertNotInstanceOf( 'WP_Error', $invalid_url_post_id );
$this->assertContains( '<!--', $get_amp_html_link() );
$this->assertTrue( AMP_Theme_Support::is_paired_available() );

// Allow the URL when the errors are forcibly sanitized.
$this->assertContains( '<!--', $get_amp_html_link() );
add_filter( 'amp_validation_error_sanitized', '__return_true' );
$this->assertTrue( AMP_Theme_Support::is_paired_available() );
$assert_amphtml_link_present();
}

Expand Down
107 changes: 69 additions & 38 deletions tests/php/test-amp.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,49 +18,80 @@ public function tearDown() {
remove_theme_support( AMP_Theme_Support::SLUG );
}

/**
* Get data for test_amp_is_canonical.
*
* @return array
*/
public function get_amp_is_canonical_test_data() {
return [
'default' => [
null,
false,
null,
],
'no_args' => [
[],
true,
AMP_Theme_Support::STANDARD_MODE_SLUG,
],
'paired_implied' => [
[
'template_dir' => 'amp-templates',
],
false,
AMP_Theme_Support::TRANSITIONAL_MODE_SLUG,
],
'paired_with_template_dir' => [
[
AMP_Theme_Support::PAIRED_FLAG => true,
'template_dir' => 'amp-templates',
],
false,
AMP_Theme_Support::TRANSITIONAL_MODE_SLUG,
],
'canonical_with_template_dir' => [
[
// This should be a rare scenario, as standard mode should mean no separate templates.
AMP_Theme_Support::PAIRED_FLAG => false,
'template_dir' => 'amp-templates',
],
true,
AMP_Theme_Support::STANDARD_MODE_SLUG,
],
'paired_without_template_dir' => [
[
AMP_Theme_Support::PAIRED_FLAG => true,
],
false,
AMP_Theme_Support::TRANSITIONAL_MODE_SLUG,
],
];
}

/**
* Test amp_is_canonical().
*
* @dataProvider get_amp_is_canonical_test_data
* @covers ::amp_is_canonical()
* @covers AMP_Theme_Support::get_support_mode_added_via_theme()
* @covers AMP_Theme_Support::read_theme_support()
* @param mixed $theme_support_args Theme support args.
* @param bool $is_canonical Whether canonical.
* @param string $expected_mode Expected mode.
*/
public function test_amp_is_canonical() {
public function test_amp_is_canonical( $theme_support_args, $is_canonical, $expected_mode ) {
remove_theme_support( AMP_Theme_Support::SLUG );
$this->assertFalse( amp_is_canonical() );

add_theme_support( AMP_Theme_Support::SLUG );
$this->assertTrue( amp_is_canonical() );

add_theme_support(
AMP_Theme_Support::SLUG,
[
'template_dir' => 'amp-templates',
]
);
$this->assertFalse( amp_is_canonical() );

add_theme_support(
AMP_Theme_Support::SLUG,
[
AMP_Theme_Support::PAIRED_FLAG => false,
'template_dir' => 'amp-templates',
]
);
$this->assertTrue( amp_is_canonical() );

add_theme_support(
AMP_Theme_Support::SLUG,
[
AMP_Theme_Support::PAIRED_FLAG => true,
]
);
$this->assertFalse( amp_is_canonical() );

add_theme_support(
AMP_Theme_Support::SLUG,
[
'custom_prop' => 'something',
]
);
$this->assertTrue( amp_is_canonical() );
delete_option( AMP_Options_Manager::OPTION_NAME );
if ( isset( $theme_support_args ) ) {
if ( is_array( $theme_support_args ) ) {
add_theme_support( AMP_Theme_Support::SLUG, $theme_support_args );
} else {
add_theme_support( AMP_Theme_Support::SLUG );
}
}
AMP_Theme_Support::read_theme_support();
$this->assertSame( $expected_mode, AMP_Theme_Support::get_support_mode_added_via_theme() );
$this->assertSame( $is_canonical, amp_is_canonical() );
}
}
89 changes: 89 additions & 0 deletions tests/php/test-class-amp-options-manager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,95 @@ public function test_get_and_set_options() {
}
}


public function get_test_get_options_defaults_data() {
return [
'reader' => [
null,
AMP_Theme_Support::READER_MODE_SLUG,
],
'transitional_without_template_dir' => [
[
'paired' => true,
],
AMP_Theme_Support::TRANSITIONAL_MODE_SLUG,
],
'transitional_implied_by_template_dir' => [
[
'template_dir' => 'amp',
],
AMP_Theme_Support::TRANSITIONAL_MODE_SLUG,
],
'standard_paired_false' => [
[
'paired' => false,
],
AMP_Theme_Support::STANDARD_MODE_SLUG,
],
'standard_paired_false' => [
[
'paired' => false,
],
AMP_Theme_Support::STANDARD_MODE_SLUG,
],
'standard_no_args' => [
[],
AMP_Theme_Support::STANDARD_MODE_SLUG,
],
'standard_via_native' => [
null,
AMP_Theme_Support::STANDARD_MODE_SLUG,
[
'theme_support' => 'native',
],
],
'standard_via_native' => [
null,
AMP_Theme_Support::TRANSITIONAL_MODE_SLUG,
[
'theme_support' => 'paired',
],
],
'standard_upon_upgrade' => [
[
'paired' => false,
],
AMP_Theme_Support::STANDARD_MODE_SLUG,
[
'theme_support' => 'disabled',
],
],
'transitional_upon_upgrade' => [
[
'paired' => true,
],
AMP_Theme_Support::TRANSITIONAL_MODE_SLUG,
[
'theme_support' => 'disabled',
],
],
];
}

/**
* Test get_options defaults.
*
* @dataProvider get_test_get_options_defaults_data
* @covers AMP_Options_Manager::get_options()
* @covers AMP_Options_Manager::get_option()
*
* @param array|null $args Theme support args.
* @param string $expected_mode Expected mode.
* @param array $initial_option Initial option in DB.
*/
public function test_get_options_theme_support_defaults( $args, $expected_mode, $initial_option = [] ) {
update_option( AMP_Options_Manager::OPTION_NAME, $initial_option );
if ( isset( $args ) ) {
add_theme_support( 'amp', $args );
}
$this->assertEquals( $expected_mode, AMP_Options_Manager::get_option( 'theme_support' ) );
}

/**
* Test check_supported_post_type_update_errors.
*
Expand Down

0 comments on commit 94be3ad

Please sign in to comment.