search

boonebgorges / buddypress-docs

GNU General Public License v3.0
106 stars 44 forks source link

'bp_docs_pending' post status is added to every 'post_status' WP_Query for users with the 'bp_moderate' capability #707

Open r-a-y opened 2 years ago

r-a-y commented 2 years ago

In WP_Query, WordPress calls on every public post status here: https://github.com/WordPress/WordPress/blob/2ea5eb36fe22ebeffff1193c4d9edde59675e16d/wp-includes/class-wp-query.php#L2567-L2568

And appends any public post status to the query.

Because BP Docs uses register_post_status() with the 'public' argument set to current_user_can( 'bp_moderate' ): https://github.com/boonebgorges/buddypress-docs/blob/d69ab86a2fe6c6f3a61dca6425bc60a9208c3949/includes/addon-moderation.php#L41

All post queries targeting 'post_status' will include the bp_docs_pending post status for users with the 'bp_moderate' capability.

It might be better to restrict when the 'public' argument should be set to true, but I'm not sure how the pending status works in the context of BP Docs to make any recommendations, so I thought I'd post an issue.

boonebgorges commented 2 years ago

Hi @r-a-y - In the abstract, it makes sense that, since the status is specific to the bp_doc post type, then it should be referenced by WP_Query when bp_doc is one of the queried post types. But as far as I can see, WP doesn't offer a way to register a post status so that it's only applied to a given post type.

For context, why is it a problem if the 'bp_docs_pending' status is part of the WP_Query? Is it for aesthetic reasons - we just don't like the fact that we unnecessarily reference a given status? Or are there more specific problems - with performance, with extensibility, etc? It feels like this is a limitation of WP's API (see https://core.trac.wordpress.org/ticket/12706) and I don't want to jump through hoops to work around it unless necessary.

r-a-y commented 2 years ago

Is it for aesthetic reasons - we just don't like the fact that we unnecessarily reference a given status?

Mostly aesthetic. Was debugging some other queries and was wondering why the bp_doc_pending post status was in the query.

If the post status is only meant to be queried when the bp_doc post type is added, it's possible to register the post status at a later hook on pre_get_posts and in the admin area, can register the post status on the current_screen hook.

It was bugging me enough to add a PR :) See #708. Let me know what you think.

boonebgorges commented 2 years ago

Mostly aesthetic.

Fair enough, I just wanted to be sure :)

The logic in your patch seems fine so far as it goes. My one concern is that you might have a WP_Query with 'any' post type - that is, it could match bp_doc without specifying it in 'post_type'. This could result in non-moderated posts being exposed. Probably not a security issue, especially as it would happen only during custom queries (ie not in buddypress-docs's native UI) but it might be worth accounting for?