Skip to content
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.

Sign up for GitHub

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

Post Actions: Use entity details for capability checks #63423

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Post Actions: Use entity details for capability checks #63423

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 28 additions & 27 deletions packages/editor/src/components/post-actions/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,28 +230,30 @@ const trashPostAction = {
},
};

function useCanUserEligibilityCheckPostType( capability, resource, action ) {
function useCanUserEligibilityCheckPostType( capability, postType, action ) {
const registry = useRegistry();
return useMemo(
() => ( {
...action,
isEligible( item ) {
return (
action.isEligible( item ) &&
registry
.select( coreStore )
.canUser( capability, resource, item.id )
registry.select( coreStore ).canUser( capability, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's something I don't understand here. how does this value change when it gets resolved. There's nothing triggering a re-render when the value of this selector changes and isEligible is not an event callback it's something that is called when rendering.

I'm sure we have a hack elsewhere that "hides" the bug, but this is not great.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original PR (#62589) might have more details, but I agree that this feels super "hacky."

kind: 'postType',
name: postType,
id: item.id,
} )
);
},
} ),
[ action, registry, capability, resource ]
[ action, registry, capability, postType ]
);
}

function useTrashPostAction( resource ) {
function useTrashPostAction( postType ) {
return useCanUserEligibilityCheckPostType(
'delete',
resource,
postType,
trashPostAction
);
}
Expand Down Expand Up @@ -347,10 +349,10 @@ const permanentlyDeletePostAction = {
},
};

function usePermanentlyDeletePostAction( resource ) {
function usePermanentlyDeletePostAction( postType ) {
return useCanUserEligibilityCheckPostType(
'delete',
resource,
postType,
permanentlyDeletePostAction
);
}
Expand Down Expand Up @@ -462,10 +464,10 @@ const restorePostAction = {
},
};

function useRestorePostAction( resource ) {
function useRestorePostAction( postType ) {
return useCanUserEligibilityCheckPostType(
'update',
resource,
postType,
restorePostAction
);
}
Expand Down Expand Up @@ -623,22 +625,21 @@ const renamePostAction = {
},
};

function useRenamePostAction( resource ) {
function useRenamePostAction( postType ) {
return useCanUserEligibilityCheckPostType(
'update',
resource,
postType,
renamePostAction
);
}

const useDuplicatePostAction = ( postType ) => {
const { userCanCreatePost } = useSelect(
const userCanCreatePost = useSelect(
( select ) => {
const { getPostType, canUser } = select( coreStore );
const resource = getPostType( postType )?.rest_base || '';
return {
userCanCreatePost: canUser( 'create', resource ),
};
Comment on lines -637 to -641
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prev code triggered a request to the wp/v2 namespace.

Screenshot

CleanShot 2024-07-11 at 15 00 21

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is a 6.6 bug?

Copy link
Member Author

@Mamaduka Mamaduka Jul 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it shouldn't affect users. It just makes an extra request.

First, it will call canUser( 'create', '' ), and then when the post type object is resolved, it will call it correctly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate action is also not included in the core, it is in the plugin only for now, so it should not have an impact on 6.6.

return select( coreStore ).canUser( 'create', {
kind: 'postType',
name: postType,
} );
},
[ postType ]
);
Expand Down Expand Up @@ -863,32 +864,32 @@ export function usePostActions( { postType, onActionPerformed, context } ) {
defaultActions,
postTypeObject,
userCanCreatePostType,
resource,
cachedCanUserResolvers,
} = useSelect(
( select ) => {
const { getPostType, canUser, getCachedResolvers } =
select( coreStore );
const { getEntityActions } = unlock( select( editorStore ) );
const _postTypeObject = getPostType( postType );
const _resource = _postTypeObject?.rest_base || '';
return {
postTypeObject: _postTypeObject,
defaultActions: getEntityActions( 'postType', postType ),
userCanCreatePostType: canUser( 'create', _resource ),
resource: _resource,
userCanCreatePostType: canUser( 'create', {
kind: 'postType',
name: postType,
} ),
cachedCanUserResolvers: getCachedResolvers()?.canUser,
};
},
[ postType ]
);

const duplicatePostAction = useDuplicatePostAction( postType );
const trashPostActionForPostType = useTrashPostAction( resource );
const trashPostActionForPostType = useTrashPostAction( postType );
const permanentlyDeletePostActionForPostType =
usePermanentlyDeletePostAction( resource );
const renamePostActionForPostType = useRenamePostAction( resource );
const restorePostActionForPostType = useRestorePostAction( resource );
usePermanentlyDeletePostAction( postType );
const renamePostActionForPostType = useRenamePostAction( postType );
const restorePostActionForPostType = useRestorePostAction( postType );
const isTemplateOrTemplatePart = [
TEMPLATE_POST_TYPE,
TEMPLATE_PART_POST_TYPE,
Expand Down
Loading