WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 9 months ago

Last modified 9 months ago

#14761 closed task (blessed) (fixed)

unregister_post_type()

Reported by: nacin Owned by:
Milestone: 4.5 Priority: normal
Severity: normal Version: 2.9
Component: Posts, Post Types Keywords: early has-patch has-unit-tests
Focuses: Cc:

Description

Two use cases:

  1. Remove a core post type. Means that the admin menus should respond in kind, though certain aspects of core like query/rewrite might not like this. Not the main use case regardless.
  1. Removing a post type of another plugin, or potentially more likely, a parent theme.

Example barebones function: http://wordpress.pastebin.com/VexHkgig

Related, unregister_taxonomy() #11058 and unregister_taxonomy_for_object_type(): #14482

Attachments (6)

14761.diff (4.3 KB) - added by swissspidy 12 months ago.
14761.2.diff (28.8 KB) - added by swissspidy 12 months ago.
14761.3.diff (11.3 KB) - added by swissspidy 12 months ago.
14761.4.diff (13.0 KB) - added by swissspidy 11 months ago.
14761.5.diff (13.3 KB) - added by swissspidy 11 months ago.
14761.6.diff (12.1 KB) - added by swissspidy 11 months ago.
Patch cleanup

Download all attachments as: .zip

Change History (63)

#1 @johnjamesjacoby
6 years ago

+1 to this. Would want to use something like this in the bbPress plugin to unload posts and pages for users who only want forums and nothing more.

#2 @scribu
6 years ago

  • Type changed from defect (bug) to enhancement

#3 @nacin
6 years ago

  • Keywords needs-patch 2nd-opinion added
  • Milestone changed from Awaiting Review to Future Release
  • Priority changed from normal to lowest
  • Version set to 2.9

#4 @nacin
6 years ago

The simple workaround here -- particularly for built-in post types -- would be to simply deny the capabilities to that post type, i.e. edit_post and friends or edit_page and friends.

#5 @johnkolbert
6 years ago

Ideally, though, you could de-register a post type and have it remove the menu item as well (specifically for core post types). There is a function to remove menu items here but it is a little 'hack-ish'

Last edited 6 years ago by johnkolbert (previous) (diff)

#6 @scribu
6 years ago

There is a native function as well: remove_menu_page().

#7 @nacin
6 years ago

Denying the caps will kill the menu. Killing the menu will not prevent the users from going to the page though.

#8 follow-up: @johnkolbert
6 years ago

What about doing something like this: https://gist.github.com/769160

This will be overly simplistic, but just as a starter example?

#9 @sorich87
6 years ago

  • Cc sorich87@… added

#10 @mikeschinkel
6 years ago

  • Cc mikeschinkel@… added

#11 @GaryJ
5 years ago

  • Cc gary@… added

#12 @andy
5 years ago

Deregistration would also be handy for plugins that are deactivating.

Typically you register post types on init. Some time after that, you get the deactivation action. A responsible plugin would remove its rewrite rules by flushing. However, it can't undo the registration, so the flush doesn't remove the plugin's rewrites.

The hacky workaround I've used is to just delete the rewrite_rules option on deactivation. They will be generated again when they are needed, so the only noticeable effect is that the next site view may take a bit longer.

#13 in reply to: ↑ 8 ; follow-ups: @johnbillion
5 years ago

I'm currently looking into whether it's workable to unregister the 'post' post type on a site. Two related issues I've found with unregistering post types:

  1. Any associated taxonomies should probably also be unregistered when unregistering a post type.
  2. The 'Right Now' dashboard widget has hardcoded output for posts, pages, categories and tags. These should only be shown if the corresponding post type / taxonomy is actually registered. I suspect there are other areas like this, this is the first one I've noticed.

Replying to johnkolbert:

What about doing something like this: https://gist.github.com/769160

You'd only need remove_menu_page() in there if you were unregistering a post type after the menu has been built. I'd say -1 on this, you should unregister post types on the 'init' hook (albeit probably with a later priority than normal).

#14 in reply to: ↑ 13 @SergeyBiryukov
5 years ago

Replying to johnbillion:

The 'Right Now' dashboard widget has hardcoded output for posts, pages, categories and tags. These should only be shown if the corresponding post type / taxonomy is actually registered.

See #19590.

#15 in reply to: ↑ 13 @GaryJ
5 years ago

Replying to johnbillion:

  1. Any associated taxonomies should probably also be unregistered when unregistering a post type.

...unless they are also being used by another post type.

#16 @Ipstenu
5 years ago

  • Cc ipstenu@… added

#17 @scottbasgaard
4 years ago

  • Cc mail@… added

#18 @lkraav
4 years ago

  • Cc lkraav added

#19 @eddiemoya
4 years ago

  • Cc eddie.moya+wptrac@… added

#20 @lightningspirit
4 years ago

  • Cc lightningspirit@… added

#21 @sc0ttkclark
4 years ago

  • Cc lol@… added

+100 on this

#22 @ryanduff
4 years ago

  • Cc ryan@… added

#23 @DrewAPicture
4 years ago

  • Cc xoodrew@… added

#24 follow-ups: @ericlewis
4 years ago

Introducing the function into the API is the first step. Something to think about in tandem is the ability to enable/disable specific core post types via a Settings page.

#25 in reply to: ↑ 24 ; follow-up: @ryanduff
4 years ago

Replying to ericlewis:

Introducing the function into the API is the first step. Something to think about in tandem is the ability to enable/disable specific core post types via a Settings page.

Not really sure I agree with the idea of more settings in the admin panel. Plugin material?

Should be easy enough to hook into the settings API to show checkboxes that would then filter on/off registered post types

#26 in reply to: ↑ 25 @ericlewis
4 years ago

Replying to ryanduff:

Replying to ericlewis:

Introducing the function into the API is the first step. Something to think about in tandem is the ability to enable/disable specific core post types via a Settings page.

Not really sure I agree with the idea of more settings in the admin panel. Plugin material?

Should be easy enough to hook into the settings API to show checkboxes that would then filter on/off registered post types

I'm reminded of BuddyPress' component administration here, in which end-users can easily enable/disable BP components.

End-users who don't want a blog on their site and just want to use WP as a CMS seems like a large demographic in my mind (no data to reference though), and giving the ability for an admin to enable / disable seems like a reasonable idea, while also supporting the "WordPress is more than just a blog" movement.

#27 in reply to: ↑ 24 @DrewAPicture
4 years ago

Replying to ericlewis:

Introducing the function into the API is the first step. Something to think about in tandem is the ability to enable/disable specific core post types via a Settings page.

+1 for introducing the function as a good first step.

#28 follow-up: @nacin
4 years ago

A unregister_post_type() function needs to un-roll quite a bit:

  • Remove the query var from the WP class, if one was registered.
  • Remove registered custom meta capabilities from _post_type_meta_capabilities().
  • Remove all post type support.
  • Remove any rewrite tags, permastructs, and rules.
  • Remove the callback for handling any meta boxes.
  • Remove the post type from any taxonomies.
  • Remove the future post hook callback.

An unregister_post_type() function should disallow the unregistration of core post types until core does not make existing assumptions all over the place. I'm not sure that will happen any time soon for 'post' and 'page'. The other types, revisions, attachments, and nav menu items, should probably never be allowed to be unregistered.

#29 in reply to: ↑ 28 ; follow-ups: @MikeSchinkel
4 years ago

  • Cc MikeSchinkel added

Replying to nacin:

The other types, revisions, attachments, and nav menu items, should probably never be allowed to be unregistered.

Agreed on 'revision' and 'attachment' but you might consider 'nav_menu_item' as something we can unregister. The current Nav Menu system is pretty heavy and for fully custom commissioned themes some professional site builders are just building menus directly into the themes since their designs don't anticipate new menu items without developer involvement. For those situations it would be nice to be able to fully disable the Nav Menu systems.

#30 @sbruner
4 years ago

  • Cc sbruner added

#31 @ejdanderson
4 years ago

  • Cc ejdanderson@… added

#32 @jtsternberg
3 years ago

  • Cc justin@… added

#33 @chriscct7
19 months ago

  • Keywords early 4.4-early added; 2nd-opinion removed
  • Owner set to chriscct7
  • Priority changed from lowest to normal
  • Status changed from new to assigned

I'm not sure why, but this ticket doesn't show up on the {43} report even though it qualifies under the params.

That being said this is a great idea and something that should proceed.

This is going to require alot of time to test it thoroughly, but I wouldn't mind tackling this for 4.4, so let's call it 4.4-early, unless someone wants to give this a go for 4.3 (which would need to happen *very* soon given the major feature freeze is in 2 weeks)

#34 in reply to: ↑ 29 @chriscct7
19 months ago

Replying to MikeSchinkel:

Replying to nacin:

The other types, revisions, attachments, and nav menu items, should probably never be allowed to be unregistered.

Agreed on 'revision' and 'attachment' but you might consider 'nav_menu_item' as something we can unregister. The current Nav Menu system is pretty heavy and for fully custom commissioned themes some professional site builders are just building menus directly into the themes since their designs don't anticipate new menu items without developer involvement. For those situations it would be nice to be able to fully disable the Nav Menu systems.

That's an interesting point of view. The problem with that is it could have unintended circumstances (plugins doing weird things or having a dependency somehow on that post type), but I'll take a look at it after 4.3 is out.

Last edited 19 months ago by chriscct7 (previous) (diff)

#35 in reply to: ↑ 29 @kraftbj
18 months ago

Replying to MikeSchinkel:

Replying to nacin:

The other types, revisions, attachments, and nav menu items, should probably never be allowed to be unregistered.

Agreed on 'revision' and 'attachment' but you might consider 'nav_menu_item' as something we can unregister. The current Nav Menu system is pretty heavy and for fully custom commissioned themes some professional site builders are just building menus directly into the themes since their designs don't anticipate new menu items without developer involvement. For those situations it would be nice to be able to fully disable the Nav Menu systems.

There's probably a cleaner way to deactivate the nav menu system (even if, initially, it simply remove all admin UI items) instead of deactivating the dependent post_type. That post type exists for the nav menu system, the nav menu system doesn't exist to support the post type.

#36 @chriscct7
16 months ago

  • Keywords 4.4-early removed
  • Owner chriscct7 deleted

@swissspidy
12 months ago

#37 @swissspidy
12 months ago

14761.diff is the beginning of a patch for this, based on #28.

  • Remove the query var from the WP class, if one was registered.
  • Remove registered custom meta capabilities from _post_type_meta_capabilities().
  • Remove all post type support.
  • Remove any rewrite tags, permastructs, and rules.
  • Remove the callback for handling any meta boxes.
  • Remove the post type from any taxonomies.
  • Remove the future post hook callback.

Need to implement remove_rewrite_tag and figure out how to properly remove meta caps. Any help appreciated.

@swissspidy
12 months ago

#38 @swissspidy
12 months ago

  • Keywords has-patch has-unit-tests dev-feedback added; needs-patch removed
  • Milestone changed from Future Release to 4.5

14761.2.diff fully implements unregister_post_type(). The following changed compared to the previous patch:

  • Properly removes the post type from the $wp_post_types global
  • Implemented remove_rewrite_tag()
  • Replaces _unregister_post_type() with the new function everywhere
  • Properly handles post type meta capabilities by storing them in a global instead of a static variable

All unit tests pass, of course.

@swissspidy
12 months ago

#39 @swissspidy
12 months ago

14761.3.diff is a new patch that includes the suggested improvements from #35227.

Relies on the patches of the following tickets, which makes this one much more readable:

  • #35234 for WP::remove_query_var()
  • #35235 for remove_permastruct()
  • #35236 for remove_rewrite_tag()

@swissspidy
11 months ago

#40 @swissspidy
11 months ago

14761.4.diff is a refreshed patch with some more tests after feedback on #35227 and the other tickets related to this. Every bit of the function is now covered, so I'm certain that it works.

The main thing that I'd love to hear some feedback on is making $post_type_meta_caps a global variable. Currently it's a static variable inside _post_type_meta_capabilities(), which means deleting meta caps is not possible.

However, it's a fairly small change, so I'm not worried about it.

@swissspidy
11 months ago

#41 @swissspidy
11 months ago

14761.5.diff adds an addition test like the one added in [36247] for taxonomies.

#42 @swissspidy
11 months ago

  • Keywords commit added; dev-feedback removed

@swissspidy
11 months ago

Patch cleanup

#43 @swissspidy
11 months ago

  • Owner set to swissspidy
  • Resolution set to fixed
  • Status changed from assigned to closed

In 36316:

Post Types: Introduce unregister_post_type().

This new function can be used to completely unregister non built-in post types.

Fixes #14761.

#44 follow-ups: @DrewAPicture
10 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I think we should rename unregister_post_type() to deregister_post_type(). Semantics? Maybe. But I think it's worth discussing.

The basic issue is that "unregister" isn't really a word. In fact, the only context in which "unregister" is really valid is when describing the state of a post type registration. The action is registration vs deregistration.

For instance: An unregistered (state) post type is one that has been deregistered (verb).

#45 in reply to: ↑ 44 ; follow-ups: @johnjamesjacoby
10 months ago

Replying to DrewAPicture:

I don't disagree with the semantics, but WordPress already uses unregister_ in a few other places, and does not use deregister_ anywhere.

If semantics now matters more than consistency, we should open a new ticket to deprecate semantically incorrect function names and use them as wrappers for newly named ones.

#46 in reply to: ↑ 45 @DrewAPicture
10 months ago

Replying to johnjamesjacoby:

If semantics now matters more than consistency, we should open a new ticket to deprecate semantically incorrect function names and use them as wrappers for newly named ones.

So the argument is essentially: we've always done it wrong so let's do it wrong in perpetuity.

It's too late for the others, but I think there's value in trying not to perpetuate consistent semantic incorrectness :-)

#47 in reply to: ↑ 45 ; follow-up: @GaryJ
10 months ago

Replying to johnjamesjacoby:

Replying to DrewAPicture:

I don't disagree with the semantics, but WordPress already uses unregister_ in a few other places, and does not use deregister_ anywhere.

There is wp_deregister_style() and wp_deregister_script(), and their related dequeue functions.

If semantics now matters more than consistency, we should open a new ticket to deprecate semantically incorrect function names and use them as wrappers for newly named ones.

deregister vs unregister, prefixed vs not prefixed.

#48 in reply to: ↑ 47 @johnjamesjacoby
10 months ago

Replying to GaryJ:

In that case, wp_deregister_ best fits all of our current philosophies, if we can consistently rename & deprecate across the board.

I enjoy the Orwellian sound of unregister but think consistent environments are double-plus good.

#49 in reply to: ↑ 44 @swissspidy
10 months ago

I think we should rename unregister_post_type() to deregister_post_type(). Semantics? Maybe. But I think it's worth discussing.

The basic issue is that "unregister" isn't really a word. In fact, the only context in which "unregister" is really valid is when describing the state of a post type registration. The action is registration vs deregistration.

There's also unregister_taxonomy() which would need to be renamed, see #35227.

I'm not opposed to renaming these two functions, but I'm also not a native speaker.

#50 @boonebgorges
10 months ago

The basic issue is that "unregister" isn't really a word. In fact, the only context in which "unregister" is really valid is when describing the state of a post type registration. The action is registration vs deregistration.

I am unconvinced by the "isn't really a word" argument. If it's widely used, then it's a word. And it's used in many places throughout WP: wp_embed_unregister_handler(), wp_unregister_sidebar_widget(), unregister_nav_menu(), unregister_setting(), as well in other software projects (eg TinyMCE).

I like deregister a bit more, but it's really just an aesthetic thing; to my ears, deregister more clearly says that it was once registered and now is not. TBH, both of them sound very awkward, and I don't think there's a "right" one. We should just make an arbitrary decision and standardize for new functions like these. There is no reason to go back and change existing function names.

Given that it's register_post_type() and register_taxonomy(), we should not use the wp_ prefix for the new functions.

#51 @GaryJ
10 months ago

All of the entries, and comments (like the one about undo) on http://english.stackexchange.com/questions/25931/unregister-vs-deregister may be influential to any naming decision.

#52 @swissspidy
10 months ago

  • Keywords commit removed
  • Owner swissspidy deleted
  • Status changed from reopened to assigned

This ticket was mentioned in Slack in #core by jorbin. View the logs.


10 months ago

#54 @jorbin
10 months ago

  • Type changed from enhancement to task (blessed)

As it's just some finalization of naming at this point, I'm blessing this as a task.

#55 follow-up: @swissspidy
9 months ago

Just stumbled upon wp_embed_unregister_handler() today. In the test suite, _unregister_post_type() has been around for quite some time. I feel like we should just keep this one as-is.

#56 in reply to: ↑ 55 @DrewAPicture
9 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

Replying to swissspidy:

Just stumbled upon wp_embed_unregister_handler() today. In the test suite, _unregister_post_type() has been around for quite some time. I feel like we should just keep this one as-is.

Yeah, I think it's fine. Let's call this fixed.

#57 @DrewAPicture
9 months ago

In 36944:

Docs: Improve the accuracy of the return description for unregister_post_type(), introduced in [36316].

See #14761. See #35986.

Note: See TracTickets for help on using tickets.