Add `WP_Post_Status` class

[flixos90]

Similar to #36217 and #36224, a dedicated class for post statuses would make it easier to work with those as they are currently represented through raw objects.

The global variable $wp_post_statuses would then hold objects of WP_Post_Status which would provide the benefits of autocompletion in IDEs, and we could possibly add methods to the class in the future to make post status management easier.

[flixos90]

initial patch implementing WP_Post_Status

[flixos90]

36492.diff​ is an initial patch implementing the class and using it in register_post_status() and get_post_status_object().

Update: This patch is broken, please use 36492.2.diff​.

[flixos90]

fix for last patch

[flixos90]

I forgot to do svn add in the first patch, so please ignore it :)

36492.2.diff​ is an actually valid patch.

[sebastian.pisula]

I like this !

[ocean90]

A WP_Post_Status class was already proposed 3 years ago in #12706, see ticket:12706:135.

@DrewAPicture Any chance you could review this patch?

[DrewAPicture]

Docs fixes + coding standards

[DrewAPicture]

@flixos90 I've iterated on 36492.2.diff with 36492.3.diff​. Changes mostly comprise coding standards and docs fixes. Some notes not implemented in the new patch:

My primary concern with the current approach is that we're doing argument handling in the constructor.

Outside of the Customizer APIs and list table classes, we don't usually do much argument handling in constructors, as they're typically relegated to populating properties in core classes.

With that in mind, I think we'd do well to implement a base/sub-class pattern with something like WP_Status – as suggested by @johnjamesjacoby in #12706 – that could be then extended for the posts, or users, or comments, or * use cases.

Implementing argument handling in the constructor could throw up some roadblocks when it comes to implementing a base class.

All that said, I wouldn't really have a problem with committing a WP_Post_Status first-run, though I would package it with a WP_Status base class in the same release. There's enough cross-over and possibility for standardized methods of handling underlying status architecture elsewhere in core to give the base/sub-class approach due consideration. Post statuses is an excellent place to start.

[flixos90]

Replying to DrewAPicture:

My primary concern with the current approach is that we're doing argument handling in the constructor.

Outside of the Customizer APIs and list table classes, we don't usually do much argument handling in constructors, as they're typically relegated to populating properties in core classes.

The thing is the arguments are only validated and filled with defaults to populate the properties afterwards. I agree in so far that the constructor should not do anything external with the arguments. But we can think about alternatives, for example we could put all the arguments into an $args property in the class and then move the logic from the constructor into a validate() method. We could then use magic getters to access these values as properties. I definitely think the validating logic should stay in the class and not in the register_post_status() function. Anyway, we should probably discuss this in a broader scope since there are also a WP_Post_Type and WP_Taxonomy class in the making which use a similar approach (see #36217 and #36224).

With that in mind, I think we'd do well to implement a base/sub-class pattern with something like WP_Status – as suggested by @johnjamesjacoby in #12706 – that could be then extended for the posts, or users, or comments, or * use cases.

That definitely makes sense. Just for an overview, can you outline to me which parts of WordPress have statuses? Not sure atm if there are others beside posts and comments. I would also like to broaden this discussion for the above two tickets - a post type and a taxonomy (and a comment type) are basically "content types", similar to how a post status and a comment status are both statuses.

[swissspidy]

Replying to flixos90:

Anyway, we should probably discuss this in a broader scope since there are also a WP_Post_Type and WP_Taxonomy class in the making which use a similar approach (see #36217 and #36224).

Sure! Drew mentioned pointed them out to me as well. I have yet to review the most recent patches on those tickets though.

I like the idea of handling arguments in a get_instance() method or similar. I'm not 100% fond of the current approaches anyway.

This shouldn't stop us from introducing WP_Status in a revised patch. Such base class will surely help with handling the arguments in a better way, because it makes us think more about it :-)

That definitely makes sense. Just for an overview, can you outline to me which parts of WordPress have statuses? Not sure atm if there are others beside posts and comments. I would also like to broaden this discussion for the above two tickets - a post type and a taxonomy (and a comment type) are basically "content types", similar to how a post status and a comment status are both statuses.

Posts, comments and users all currently have some kind of statuses. See also #36594 and #34316.

[flixos90]

I will work on a new patch in the next days. Should I attach that patch to this ticket? Or should it be in a new ticket for a WP_Status base class? I'm not sure how we should handle something like this on Trac since an abstract WP_Status really doesn't belong to one component.

[swissspidy]

@flixos90 It's fine to do this in this ticket.

[flixos90]

WP_Status as base class for WP_Post_Status

[flixos90]

36492.4.diff​ introduces an abstract WP_Status class, and the WP_Post_Status class extends it. I put all properties which should also apply to a possible WP_Comment_Status and WP_User_Status into the base class. So for now, all that the WP_Post_Status class still takes care of are the exclude_from_search and publicly_queryable properties since I feel they are post-specific properties (correct me if I'm wrong here).

For the overall approach, I modified the class to follow the structure proposed by @boonebgorges in the WP_Post_Type discussion.

@DrewAPicture: I wasn't completely sure what would be the best way to document the two classes, especially in terms of duplicate docs or unclear docs because of the abstraction. These definitely need some review.

[flixos90]

update from 36492.3.diff (without abstract class)

[flixos90]

After discussion in weekly Core meeting, it was decided not to use abstract classes at this point.

Therefore 36492.5.diff​ is an iteration built upon 36492.3.diff​ which however includes the internal structural improvements from 36492.4.diff​.

[johnjamesjacoby]

Replying to flixos90:

After discussion in weekly Core meeting, it was decided not to use abstract classes at this point.

That's too bad. It's not unlike the base list-table class, where we have a specific problem that requires solving, and foreseeable problems that we can also architect for and begin solving at the same time.

In WordPress core, users, comments, and taxonomy terms will (all, eventually) derive a great deal of value from a structure around what a status includes and details. Without a base class, so far, each of these objects has redefined what a status is in a unique (and mostly terrible) way. BuddyPress and bbPress have followed suit, unfortunately.

In plugins, any other imaginable object will likely need a status. Events, Downloads, Shopping cart items, Activity streams, Friendship requests, Group memberships, etc...

The current WP_Post_Status class is a great start, and hopefully we'll be able to use it as a jumping off point for refactoring statuses for the other WordPress objects at a later date.

[boonebgorges]

I think we should hold off on discussion of a base class (or, better, some technique for extensibility that doesn't rely on inheritance) until we have clear concept of what a "status" is, in general: what are the properties that make something a status (rather than a post, or a term, etc), and what abilities a status should have. As things stand, *post* statuses are not even very well conceptualized, much less a general "status" type that could inform other implementations.

36492.5.diff​ seems OK to me. A couple thoughts and questions:

• I don't think we need to describe default values for object properties in the documentation for those properties. We already do that in set_props().
• What is the benefit of the get_instance() method? Methods like this are generally used when the constructor is private - ie for a singleton. It seems like a mixing of concerns to have the object make reference to the global $wp_post_statuses. I'd remove the method and move that logic back to get_post_status_object().
• Code formatting: ! $args['internal'], not $args['internal'], etc

[schlessera]

Was there any consideration for implementing post statuses as an actual "enumerated" (or "enum") data type (or at least a PHP simulation thereof)?

Benefits would be:

• Correct data type, as it is the correct representation of a finite set of values.
• Proper, semantic equality checking.
• Proper, semantic way of getting all accepted values.
• Type-hinting on the actual status value if wanted.

A useful implementation example would be ​https://github.com/marc-mabe/php-enum, which could probably be rewritten to work without namespaces.

[GaryJ]

Was there any consideration for implementing post statuses as an actual "enumerated" (or "enum") data type (or at least a PHP simulation thereof)?

An enumerated list wouldn't allow much in the way of custom statuses.

and what abilities a status should have

This is crying out for an interface to be created first, so that each WP_*_Status, including WP_Post_Status can implement it.