Opened 4 years ago
Last modified 3 years ago
#52094 new defect (bug)
Queries with perm readable/editable will not work for multiple post types and status queries
Reported by: | leogermani | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Query | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
This ticket is a continuation of #48556, which fixes permission checks when querying for multiple post types.
If you make a query for multiple post types, and also specifying one or more post statuses, there will be a couple of problems in the results if your query also includes perm=readable
or perm=editable
.
perm=readable
is supposed to do a check to see if the current user has permission to read the queried post statuses before adding it to the SQL statement. It works fine if you're querying for only one post type, but will fail for multiple post types.
The problem is the same we found in #48556. When multiple post types are queried, the permission checked will be read_private_multiple_post_types
, instead of individually check for each post type's capabilities.
(The same happens for perm=editable
, which will check for edit_others_multiple_post_types
)
Another problem: Custom post statuses
When querying for multiple post types and perm=readable
, WP_Query also will not handle custom post statuses properly.
Any custom post status will be treated as a public
post status, ignoring how it was registered. You can see it happening here: https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/class-wp-query.php#L2478
Fix
The fix should be similar to what we are doing in #48556, and I think we should work on the patch after we finish #48556.
Reproducing
I'm going to attach a test file that reproduces all the bugs described here.
Attachments (1)
Change History (9)
#1
@
4 years ago
Here's a first proposed fix -> https://github.com/leogermani/wordpress-develop/pull/2
This is a PR that depends on the PR that fixes #48556 and a patch should be properly created once #48556 is merged. (or we should turn both in one single patch)
This patch also addresses the bug described in #51094 and adds a specific test for that.
What this patch does:
- It fixed #51094 by checking if at least one valid post status was found in the query
- It fixes support for custom post statuses in the block of code that goes:
<?php foreach ( get_post_stati( array(), 'objects' ) as $status => $status_object ) { if ( in_array( $status, $q_status, true ) ) { if ( $status_object->private ) {
- It adds the current post Status logic inside a Loop and treat each post type differently, depending on the specific capabilities for each of them
Note
This patch introduces the same logic introduced by #48556 to the status
query, so there's a bit of duplicate logic.
I considered doing everything in the same block of code and simplifying the whole thing, but this would "fix" some current inconsistencies and, therefore, change some behavior I'm not sure we want to change. It could also end up being a bit more complicated to read. But I can give it a go if anyone thinks it's worth it. It's not very difficult
The main "inconsistency" is that when we query for multiple post types, we do not consider the perm
parameter. We basically assume perm=readable
but there's no support for perm=editable
for example. So we would start supporting it.
Also there is how we handle "protected states that should show in the admin all list", that are not considered when querying for post statuses.
This ticket was mentioned in PR #1183 on WordPress/wordpress-develop by leogermani.
4 years ago
#2
- Keywords has-patch has-unit-tests added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/52094
#3
@
4 years ago
- Keywords needs-patch added; has-patch has-unit-tests removed
Edit: Here's a PR that joins everything in a single block, fixing both this and #48556
This ticket was mentioned in PR #2810 on WordPress/wordpress-develop by leogermani.
3 years ago
#5
Add tests and refactor the creation of status and post types query
Trac ticket: https://core.trac.wordpress.org/ticket/52094
leogermani commented on PR #1183:
3 years ago
#6
closing in favor of https://github.com/WordPress/wordpress-develop/pull/2810
#7
@
3 years ago
I've created a brand new patch for this ticket: https://github.com/WordPress/wordpress-develop/pull/2810
cc @peterwilsoncc
What this patch does is it joins the situations where post_status
query is used with the multiple post type logic fixed in #48556.
So now it's possible to query for multiple post types and multiple post_status at the same time and it will work as expected.
# Bug or feature
There's one thing that I'm not fixing here because I'm not sure it's a bug or feature.
If you query for post_status=any
, the perm=readable
param is ignored. Meaning that if you query for any post status, all post statuses will be returned regardless of the permission the current user has. Protected statuses will be removed, but private statuses will be included. This is how it behaves now and I didn't change it.
Should we change it? Or should we just update the docs to make it more explicit?
Tests that reproduces the bugs described in this ticket