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.

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

Improve determination of eligible post types for AMP #6883

Closed
westonruter opened this issue Feb 3, 2022 · 2 comments · Fixed by #6897
Closed

Improve determination of eligible post types for AMP #6883

westonruter opened this issue Feb 3, 2022 · 2 comments · Fixed by #6897
Labels
Changelogged Whether the issue/PR has been added to release notes. Enhancement New feature or improvement of an existing one
Milestone

Comments

@westonruter
Copy link
Member

Feature Description

As noted in #6881 (comment):

the only post types that are eligible for being AMP-enabled are those which have 'public' => true:

public static function get_eligible_post_types() {
$post_types = array_values(
get_post_types(
[
'public' => true,
],
'names'
)
);
/**
* Filters the list of post types which may be supported for AMP.
*
* By default the list includes those which are public.
*
* @since 2.0
*
* @param string[] $post_types Post types.
*/
return array_values( (array) apply_filters( 'amp_supportable_post_types', $post_types ) );
}

It's true that a post type could have public set to true but have publicly_queryable set to false, I think, and in that case it may be possible that we could accidentally show or hide the preview button. It would be marginally better to here instead do the following since there is now a is_post_type_viewable filter in WP 5.9:

diff --git a/includes/class-amp-post-type-support.php b/includes/class-amp-post-type-support.php
index 3d4560641..17eb6519b 100644
--- a/includes/class-amp-post-type-support.php
+++ b/includes/class-amp-post-type-support.php
@@ -42,14 +42,9 @@ public static function get_builtin_supported_post_types() {
 	 * @return string[] Post types eligible for AMP.
 	 */
 	public static function get_eligible_post_types() {
-		$post_types = array_values(
-			get_post_types(
-				[
-					'public' => true,
-				],
-				'names'
-			)
-		);
+		$post_types = array_values( get_post_types( [], 'names' ) );
+
+		$post_types = array_filter( $post_types, 'is_post_type_viewable' );
 
 		/**
 		 * Filters the list of post types which may be supported for AMP.

Acceptance Criteria

Only post types for which is_post_type_viewable() returns true should be listed among the post types for which AMP can be enabled. Only these post types should have an AMP preview button shown in a paired AMP mode (transitional or reader).

Implementation Brief

No response

QA Testing Instructions

No response

Demo

No response

Changelog Entry

No response

@westonruter westonruter added the Enhancement New feature or improvement of an existing one label Feb 3, 2022
@westonruter westonruter added this to the v2.2.2 milestone Feb 3, 2022
@dhaval-parekh
Copy link
Collaborator

Step for QA is provided here #6897 (comment)

@pooja-muchandikar
Copy link

QA Passed ✅

Eligible post types for AMP are visible in Supported Templates section

Screenshot 2022-03-31 at 2 21 20 PM

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. Enhancement New feature or improvement of an existing one
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants