Opened 8 years ago
Last modified 7 years ago
#40340 reopened defect (bug)
"Attach to existing content" modal shows posts and pages of other users
Reported by: | menakas | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Role/Capability | Keywords: | has-screenshots needs-patch needs-unit-tests |
Focuses: | Cc: |
Description
From the Media library, a user can attach media (through the Attach link) that were uploaded by himself only. The first image shows the Attach links appearing for certain media only.
In the same way, the "Attach to existing content" should also list only those posts/pages for which the user has the permissions to attach.
Instead it lists all posts (including private) and pages of all users, and then, upon selection, gives the message "Sorry, you are not allowed to edit this post."
Attachments (2)
Change History (8)
@
8 years ago
All posts (including private) and pages for which user does not have edit permissions listed - Expected different behaviour
#2
@
8 years ago
- Keywords close added; needs-patch removed
- Resolution set to maybelater
- Status changed from new to closed
To my understanding the current setting is doing good.
If posts/user to be filtered out in the popup then there would be another check to be added - like Admin has the power to do everything, a Moderator have different privilege etc. meaning which posts to be listed based on current user's role would be an additional burden and to achieve this a series of if...else statements will have to be incorporated which I believe is not very necessary here.
#3
@
8 years ago
- Keywords close removed
- Resolution maybelater deleted
- Status changed from closed to reopened
meaning which posts to be listed based on current user's role would be an additional burden and to achieve this a series of if...else statements will have to be incorporated which I believe is not very necessary here.
Not really. WordPress has a robust capabilities API. This would be as simple as current_user_can( 'edit_post', $post_id )
for example.
#4
@
8 years ago
Yes, that's true. But when listing posts in popup why should I loop through all posts to find out what posts the 'current_user_can' edit? Don't you think it creates overhead if there are say 5000 posts and WP has to go though all of them in order to just decide whether or not to place it in the popup?
This should be checked when the action is being performed, i.e. attaching a media to a post which the current_user_can NOT edit (as per the issue reported here), and I believe this is how it is working now.
The issue as reported here is to filter out only "user's own posts" in the popup when somebody clicks on Attach link of a specific media. So if that has to be achieved current_user_can
check needs to check every single post available in the database. I believe this is an extra burden. I may be wrong!
#5
@
8 years ago
Firstly, at the very least, I would expect the private posts of other users to not be displayed. As of now, other users' private posts are also displayed.
Secondly, should we let implementation concerns affect the interface design/behavior?
Thirdly, whether user has permissions to attach an image/media or not has been checked for each image; so why not check for posts too?
Fourth,
a) an editor can attach an image to any public post, so pick all posts and pages that are not private.
b) an author can attach an image to his posts only, so we can use the query arg 'author'
to be current user's id. Perhaps, using current_user_can
for each post is not necessary?
#6
@
7 years ago
- Focuses ui removed
- Keywords needs-patch needs-unit-tests added
- Version 4.7.3 deleted
WP_Query
has the perm => editable
parameter just for this (which can be added to the query args in wp_ajax_find_posts()
), but it doesn't appear to work in conjunction with the status
parameter. Needs some more investigation.
Attach link available for media for which user has permissions - Correct behaviour