-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix: Include permission management on permanently delete, rename, and restore. #62754
Fix: Include permission management on permanently delete, rename, and restore. #62754
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: +56 B (0%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
@@ -307,7 +307,7 @@ const trashPostAction = { | |||
}, | |||
}; | |||
|
|||
function useTrashPostAction( postType ) { | |||
function useCanUserEligibilityCheckPostType( postType, capability, action ) { | |||
const registry = useRegistry(); | |||
const { resource, cachedCanUserResolvers } = useSelect( | |||
( select ) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we avoid creating this subscription many times, and just do it once?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ellatrix, I applied a change and now this PR does not add any store subscription.
Hi @ellatrix, I think that's ok, the edit means open in the site editor, but in my tests the site editor does permission checks and for example, if you open a page there but don't have permission to edit you will not be able to change the page content. I guess if you have permission to change the template you may change the template but not the content. |
… restore post actions.
72b7f6e
to
471e144
Compare
registry | ||
.select( coreStore ) | ||
.canUser( 'delete', resource, item.id ) | ||
.canUser( capability, resource, item.id ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it doesn't really happen in the case, but the problem here is that isEligible
will not be called if the user capability changes, right? So maybe all of these need to be precalculated/subscribed to at some point so the buttons re-render.
Handled in #62823. |
Expands what was done in #62589 to other three actions where it makes sense: Permanently delete, rename, and restore post action.
Testing Instructions
I moved a page to the trash.
I added the following code to a PHP file:
I went to the trash page views and verified the permanently delete page action is unavailable.
I went to drafts view and verified the trash post action is still not available.
I added the following code to a PHP file:
I went to the drafts pages list and verified the ability to rename a page was not shown.
I went to the trash pages list and verified the ability to restore pages was not present.