[wp-trac] [WordPress Trac] #35567: New argument `is_embeddable` for `register_post_type()`

[WordPress Trac]

#35567: New argument `is_embeddable` for `register_post_type()`
-------------------------------------------------+-------------------------
 Reporter:  pampfelimetten                       |       Owner:  bradleyt
     Type:  enhancement                          |      Status:  assigned
 Priority:  normal                               |   Milestone:  Awaiting
Component:  Embeds                               |  Review
 Severity:  normal                               |     Version:  4.4
 Keywords:  good-first-bug needs-unit-tests      |  Resolution:
  has-patch                                      |     Focuses:
-------------------------------------------------+-------------------------
Changes (by DrewAPicture):

 * keywords:  needs-patch good-first-bug needs-unit-tests => good-first-bug
     needs-unit-tests has-patch
 * owner:   => bradleyt
 * status:  reopened => assigned


Comment:

 Replying to [comment:4 bradleyt]:
 > I think the attahced diff does what is required.
 >
 > One thing I was unsure of was if the {{{register_post_type}}} arguments
 are in any particular order?
 >
 > I am also uncomfortable adding tests, so maybe someone else could do
 this.

 @bradleyt thanks for the patch! Some notes:

 '''Overall:'''
 * Needs unit tests (`/tests/post/types.php` is a good place to start)
 * Any new or changed code should follow
 [https://make.wordpress.org/core/handbook/ core coding standards],
 specifically in regards to spacing.
 * I'm not completely sold on 'is_embeddable' for an argument name.
 ''Maybe'' 'embeddable' or 'can_embed' for the argument would be better.

 '''`wp_oembed_add_discovery_links()`:'''

 * `$post` is not passed to this function as it assumes it's in the loop.
 In this case we can simply use `$post_type = get_post_type()`. Either way,
 you'd want to then check whether a post type was returned before trying to
 use it, perhaps by combining it with the `is_singular()` check, e.g. `if (
 is_singular() && $post_type ) {`
 * The DocBlock is missing an `@since` tag. Use "x.x.x" for now.

 '''`get_oembed_response_data()`:'''
 * `$post` is required here, so the `get_post()` call is moot. Instead, you
 could do `$post_type = get_post_type( $post );`, or even just pass
 `get_post_type()` directly to `is_post_type_embeddable()` since it's only
 used once.
 * The `@return` tag description should be updated to reflect the new
 condition for the possibility of returning false.

 '''`register_post_type()`:'''

 * DocBlock is missing a changelog entry for "x.x.x" version for the new
 argument
 * See '''Overall''' above about argument naming

 '''`is_post_type_embeddable()`:'''

 * The `is_scalar()` check is unnecessary since it's already done in
 `get_post_type_object()`. Should be a `post_type_exists()` check instead.
 * The DocBlock is missing an `@since` tag. Use "x.x.x" for now.

 ----

 Assigning the ticket to mark the `good-first-bug` as "claimed".

--
Ticket URL: <https://core.trac.wordpress.org/ticket/35567#comment:11>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform