Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Account for ambiguity in matching templates by incorporating template priority order from core #1938

Merged
merged 3 commits into from
Mar 8, 2019
Merged

Account for ambiguity in matching templates by incorporating template priority order from core #1938

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e631c8b
Account for ambiguity in matching template by incorporating template …
westonruter Mar 7, 2019
bef18cf
Restore patches_applied to composer.lock
westonruter Mar 7, 2019
7017d58
Clean compiled files and do so before building
westonruter Mar 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
node_modules
wiki
amp.zip
**/*-compiled.js
assets/js/*-compiled.js
built
/amphtml
7 changes: 6 additions & 1 deletion Gruntfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ module.exports = function( grunt ) {

// Clean up the build.
clean: {
compiled: {
src: [ 'assets/js/*-compiled.js' ]
},
build: {
src: [ 'build' ]
}
Expand Down Expand Up @@ -72,6 +75,9 @@ module.exports = function( grunt ) {
spawnQueue = [];
stdout = [];

// Clear out all existing compiled files first.
grunt.task.run( 'clean' );

grunt.task.run( 'shell:webpack_production' );

spawnQueue.push(
Expand Down Expand Up @@ -101,7 +107,6 @@ module.exports = function( grunt ) {
paths.push( 'vendor/fasterimage/fasterimage/src/**' );
paths.push( 'vendor/willwashburn/stream/src/**' );

grunt.task.run( 'clean' );
grunt.config.set( 'copy', {
build: {
src: paths,
Expand Down
7 changes: 6 additions & 1 deletion composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

59 changes: 58 additions & 1 deletion includes/class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,11 +569,68 @@ public static function get_template_availability( $query = null ) {
}
}

// The is_home() condition is the default so discard it if there are other matching templates.
// If there is more than 1 matching template, the is_home() condition is the default so discard it if there are other matching templates.
if ( count( $matching_templates ) > 1 && isset( $matching_templates['is_home'] ) ) {
unset( $matching_templates['is_home'] );
}

/*
* When there is still more than one matching template, account for ambiguous cases, informed by the order in template-loader.php.
* See <https://github.com/WordPress/wordpress-develop/blob/5.1.0/src/wp-includes/template-loader.php#L49-L68>.
*/
if ( count( $matching_templates ) > 1 ) {
$template_conditional_priority_order = array(
'is_embed',
'is_404',
'is_search',
'is_front_page',
'is_home',
'is_post_type_archive',
'is_tax',
'is_attachment',
'is_single',
'is_page',
'is_singular',
'is_category',
'is_tag',
'is_author',
'is_date',
'is_archive',
);

// Obtain the template conditionals for each matching template ID (e.g. 'is_post_type_archive[product]' => 'is_post_type_archive').
$template_conditional_id_mapping = array();
foreach ( array_keys( $matching_templates ) as $template_id ) {
$template_conditional_id_mapping[ strtok( $template_id, '[' ) ] = $template_id;
}

// If there are any custom supportable templates, only consider them since they would override the conditional logic in core.
$custom_template_conditions = array_diff(
array_keys( $template_conditional_id_mapping ),
$template_conditional_priority_order
);
if ( ! empty( $custom_template_conditions ) ) {
$matching_templates = wp_array_slice_assoc(
$matching_templates,
array_values( wp_array_slice_assoc( $template_conditional_id_mapping, $custom_template_conditions ) )
);
} else {
/*
* Otherwise, iterate over the template conditionals in the order they occur in the if/elseif/else conditional chain.
* to then populate $matching_templates with just this one entry.
*/
foreach ( $template_conditional_priority_order as $template_conditional ) {
if ( isset( $template_conditional_id_mapping[ $template_conditional ] ) ) {
$template_id = $template_conditional_id_mapping[ $template_conditional ];
$matching_templates = array(
$template_id => $matching_templates[ $template_id ],
);
break;
}
}
}
}

/*
* If there are more than one matching templates, then something is probably not right.
* Template conditions need to be set up properly to prevent this from happening.
Expand Down
65 changes: 60 additions & 5 deletions tests/test-class-amp-theme-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,29 +562,36 @@ public function test_get_template_availability() {
add_filter(
'amp_supportable_templates',
function( $templates ) {
$templates['is_single'] = array(
$templates['is_single'] = array(
'label' => 'Single post',
'supported' => false,
'parent' => 'is_singular',
);
$templates['is_special'] = array(
$templates['is_special'] = array(
'label' => 'Special post',
'parent' => 'is_single',
'callback' => function( WP_Query $query ) {
return $query->is_singular() && 'special' === get_post( $query->get_queried_object_id() )->post_name;
},
);
$templates['is_page'] = array(
$templates['is_page'] = array(
'label' => 'Page',
'supported' => true,
'parent' => 'is_singular',
);
$templates['is_custom'] = array(
$templates['is_custom'] = array(
'label' => 'Custom',
'callback' => function( WP_Query $query ) {
return false !== $query->get( 'custom', false );
},
);
$templates['is_custom[thing]'] = array(
'label' => 'Custom Thing',
'parent' => 'is_custom',
'callback' => function( WP_Query $query ) {
return 'thing' === $query->get( 'custom', false );
},
);
return $templates;
}
);
Expand Down Expand Up @@ -621,11 +628,59 @@ function( $vars ) {
$availability = AMP_Theme_Support::get_template_availability( $this->factory()->post->create_and_get( array( 'post_type' => 'page' ) ) );
$this->assertTrue( $availability['supported'] );

// Test custom.
// Test is_custom.
$this->go_to( '/?custom=1' );
$availability = AMP_Theme_Support::get_template_availability();
$this->assertTrue( $availability['supported'] );
$this->assertEquals( 'is_custom', $availability['template'] );

// Test is_custom[thing].
$this->go_to( '/?custom=thing' );
$availability = AMP_Theme_Support::get_template_availability();
$this->assertFalse( $availability['supported'] );
$this->assertEquals( 'is_custom[thing]', $availability['template'] );
}

/**
* Test get_template_availability with ambiguous matching templates.
*
* @covers AMP_Theme_Support::get_template_availability()
*/
public function test_get_template_availability_with_ambiguity() {
AMP_Options_Manager::update_option( 'all_templates_supported', true );
add_theme_support( AMP_Theme_Support::SLUG );
$custom_post_type = 'book';
register_post_type(
$custom_post_type,
array(
'has_archive' => true,
'publicly_queryable' => true,
)
);
$this->factory()->post->create(
array(
'post_type' => $custom_post_type,
'post_title' => 'test',
)
);

// Test that when doing a post_type archive, we get the post type archive as expected.
$this->go_to( "/?post_type=$custom_post_type" );
$this->assertTrue( is_post_type_archive( $custom_post_type ) );
$this->assertFalse( is_search() );
$availability = AMP_Theme_Support::get_template_availability();
$this->assertTrue( $availability['supported'] );
$this->assertEmpty( $availability['errors'] );
$this->assertEquals( "is_post_type_archive[$custom_post_type]", $availability['template'] );

// Test that when doing a search and a post_type archive, the search wins.
$this->go_to( "/?s=test&post_type=$custom_post_type" );
$this->assertTrue( is_post_type_archive( $custom_post_type ) );
$this->assertTrue( is_search() );
$availability = AMP_Theme_Support::get_template_availability();
$this->assertTrue( $availability['supported'] );
$this->assertEmpty( $availability['errors'] );
$this->assertEquals( 'is_search', $availability['template'] );
}

/**
Expand Down