Opened 87 minutes ago
Last modified 23 minutes ago
#49644 new enhancement
Move logic from register_post_type() into WP_Post_Type->register()
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Awaiting Review | Priority: | normal |
| Severity: | normal | Version: | 5.4 |
| Component: | Posts, Post Types | Keywords: | has-patch dev-feedback |
| Focuses: | Cc: |
Description
I have not looked at register_post_type() in several years, but was pleasantly surprised to find major improvements where most of the logic was refactored and moved to WP_Post_Type.
Still though, it appears there are a few missing capabilities that a bit more refactoring would resolve.
I propose we move most of the logic found in register_post_type() into WP_Post_Type->register() and a bit of the validation logic into WP_Post_Type->__construct() (see attached patch.)
Similarly, it would make sense to also move the logic in unregister_post_type() to a WP_Post_Type->unregister().
One of the benefits of this change is that currently register_post_type() has validation logic but WP_Post_Type->__construct() does not so someone can call new WP_Post_Type('this-is-a-far-too-long-post-type-name') and not get an error and allow a too-long post type name to be registered whereas register_post_type('this-is-a-far-too-long-post-type-name') currently throw an error and not let the name through. With this change, but approaches would validate.
The benefit of adding WP_Post_Type->register() and WP_Post_Type->unregister() is simply that those wanting to use the object to create their post types do not need to duplicate the logic in register_post_type(). The benefit of this is being able to write code like this and leverage PHP's validation of object property names:
<?php $post_type = new WP_Post_Type('my_widget'); $post_type->label = __( 'Widgets', 'my_app' ); $post_type->public = true; $post_type->menu_icon = 'dashicons-admin-generic'; $post_type->register();
Notice I added a backward-compatible additional parameter to the unregistered_post_type hook to indicate if the post type existed in $wp_post_types before the attempt to unregister it.
AFAICT there will be no backward compatibility or documentation issues with this change, so this should be a slam-dunk, right?
Attachments (2)
Change History (5)
@
85 minutes ago
#1
follow-up:
↓ 2
@
71 minutes ago
Just noting that class constructors cannot return a custom value thus the WP_Error object for a failed validation won't be returned.
#2
in reply to:
↑ 1
@
52 minutes ago
Replying to ocean90:
Just noting that class constructors cannot return a custom value thus the
WP_Errorobject for a failed validation won't be returned.
Good point. I shouldn't have rushed to get this pushed up before a conference call. :-)
Hopefully you agree the constructor should validate the post type length, right?
Do you think assigning to an last_error property be a workable solution?
@
25 minutes ago
Updates the patch to include a last_error. Also adds another $this as a 2nd parameter to unregistered_post_type hook
#3
@
23 minutes ago
So I proactively updated the patch to save the WP_Error to $post_type->last_error.
Also added a 2nd parameter to the 'unregistered_post_type' hook and moved the $removed parameter I added in the first patch to the 3rd parameter position.
Finally, I added PHPDoc headers that I missed in the first patch.
Moves code from register_post_type() to WP_Post_Type->register() and from unregister_post_type() to WP_Post_Type->unregister().