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

Core Data: Resolve user capabilities when fetching an entity #63430

Merged
merged 6 commits into from
Jul 17, 2024
Merged

Core Data: Resolve user capabilities when fetching an entity #63430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a726f88
Core Data: Resolve user capabilities when fetching an entity
Mamaduka Jul 11, 2024
b2a0f40
Mark canUser selector as resolved
Mamaduka Jul 14, 2024
3ca0afa
Fix unit tests
Mamaduka Jul 14, 2024
a9b931e
Dedupe logic
Mamaduka Jul 15, 2024
23ea0c1
Showcase: Update Pattern block to benefit from new user permission re…
Mamaduka Jul 15, 2024
c8ae4b6
Clarify comment
Mamaduka Jul 16, 2024
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
89 changes: 58 additions & 31 deletions packages/block-library/src/block/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,51 @@ export default function ReusableBlockEditRecursionWrapper( props ) {
);
}

function ReusableBlockControl( {
recordId,
canOverrideBlocks,
hasContent,
handleEditOriginal,
resetContent,
} ) {
const canUserEdit = useSelect(
( select ) =>
!! select( coreStore ).canUser( 'update', {
kind: 'postType',
name: 'wp_block',
id: recordId,
} ),
[ recordId ]
);

return (
<>
{ canUserEdit && !! handleEditOriginal && (
<BlockControls>
<ToolbarGroup>
<ToolbarButton onClick={ handleEditOriginal }>
{ __( 'Edit original' ) }
</ToolbarButton>
</ToolbarGroup>
</BlockControls>
) }

{ canOverrideBlocks && (
<BlockControls>
<ToolbarGroup>
<ToolbarButton
onClick={ resetContent }
disabled={ ! hasContent }
>
{ __( 'Reset' ) }
</ToolbarButton>
</ToolbarGroup>
</BlockControls>
) }
</>
);
}

function ReusableBlockEdit( {
name,
attributes: { ref, content },
Expand All @@ -143,29 +188,20 @@ function ReusableBlockEdit( {

const {
innerBlocks,
userCanEdit,
onNavigateToEntityRecord,
editingMode,
hasPatternOverridesSource,
} = useSelect(
( select ) => {
const { canUser } = select( coreStore );
const {
getBlocks,
getSettings,
getBlockEditingMode: _getBlockEditingMode,
} = select( blockEditorStore );
const { getBlockBindingsSource } = unlock( select( blocksStore ) );
const canEdit = canUser( 'update', {
kind: 'postType',
name: 'wp_block',
id: ref,
} );

// For editing link to the site editor if the theme and user permissions support it.
return {
innerBlocks: getBlocks( patternClientId ),
userCanEdit: canEdit,
getBlockEditingMode: _getBlockEditingMode,
onNavigateToEntityRecord:
getSettings().onNavigateToEntityRecord,
Expand All @@ -175,7 +211,7 @@ function ReusableBlockEdit( {
),
};
},
[ patternClientId, ref ]
[ patternClientId ]
);

// Sync the editing mode of the pattern block with the inner blocks.
Expand Down Expand Up @@ -256,27 +292,18 @@ function ReusableBlockEdit( {

return (
<>
{ userCanEdit && onNavigateToEntityRecord && (
<BlockControls>
<ToolbarGroup>
<ToolbarButton onClick={ handleEditOriginal }>
{ __( 'Edit original' ) }
</ToolbarButton>
</ToolbarGroup>
</BlockControls>
) }

{ canOverrideBlocks && (
<BlockControls>
<ToolbarGroup>
<ToolbarButton
onClick={ resetContent }
disabled={ ! content }
>
{ __( 'Reset' ) }
</ToolbarButton>
</ToolbarGroup>
</BlockControls>
{ hasResolved && (
<ReusableBlockControl
recordId={ ref }
canOverrideBlocks={ canOverrideBlocks }
hasContent={ !! content }
handleEditOriginal={
onNavigateToEntityRecord
? handleEditOriginal
: undefined
}
resetContent={ resetContent }
/>
Comment on lines +295 to +306
Copy link
Member Author

Choose a reason for hiding this comment

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

Unrelated change showcases the benefits of this proposal. I am happy to extract it into a separate PR.

Requests before with four pattern blocks on canvas:
CleanShot 2024-07-15 at 16 52 13

Requests after:
CleanShot 2024-07-15 at 16 52 36

) }

{ children === null ? (
Expand Down
8 changes: 6 additions & 2 deletions packages/block-library/src/block/test/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,9 @@ describe( 'Synced patterns', () => {
if ( path.startsWith( endpoint ) ) {
response = getMockedReusableBlock( id );
}
return Promise.resolve( response );
return Promise.resolve( {
json: () => Promise.resolve( response ),
} );
} );

const screen = await initializeEditor( {
Expand Down Expand Up @@ -229,7 +231,9 @@ describe( 'Synced patterns', () => {
response.content.raw = `<!-- wp:image {"id":1,"sizeSlug":"large","linkDestination":"none"} -->
<figure class="wp-block-image size-large"><img src="https://cldup.com/cXyG__fTLN.jpg" alt="" class="wp-image-1"/></figure>
<!-- /wp:image -->`;
return Promise.resolve( response );
return Promise.resolve( {
json: () => Promise.resolve( response ),
} );
} );

const screen = await initializeEditor( {
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/image/test/edit.native.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ function mockGetMedia( media ) {
const FETCH_MEDIA = {
request: {
path: `/wp/v2/media/1?context=edit`,
parse: false,
},
response: {
source_url: 'https://cldup.com/cXyG__fTLN.jpg',
Expand Down
8 changes: 5 additions & 3 deletions packages/core-data/src/hooks/test/use-entity-record.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,11 @@ describe( 'useEntityRecord', () => {
} );

const TEST_RECORD = { id: 1, hello: 'world' };
const TEST_RECORD_RESPONSE = { json: () => Promise.resolve( TEST_RECORD ) };

it( 'resolves the entity record when missing from the state', async () => {
// Provide response
triggerFetch.mockImplementation( () => TEST_RECORD );
triggerFetch.mockImplementation( () => TEST_RECORD_RESPONSE );

let data;
const TestComponent = () => {
Expand Down Expand Up @@ -60,6 +61,7 @@ describe( 'useEntityRecord', () => {
await waitFor( () =>
expect( triggerFetch ).toHaveBeenCalledWith( {
path: '/wp/v2/widgets/1?context=edit',
parse: false,
} )
);

Expand All @@ -79,7 +81,7 @@ describe( 'useEntityRecord', () => {

it( 'applies edits to the entity record', async () => {
// Provide response
triggerFetch.mockImplementation( () => TEST_RECORD );
triggerFetch.mockImplementation( () => TEST_RECORD_RESPONSE );

let widget;
const TestComponent = () => {
Expand Down Expand Up @@ -119,7 +121,7 @@ describe( 'useEntityRecord', () => {
} );

it( 'does not resolve entity record when disabled via options', async () => {
triggerFetch.mockImplementation( () => TEST_RECORD );
triggerFetch.mockImplementation( () => TEST_RECORD_RESPONSE );

let data;
const TestComponent = ( { enabled } ) => {
Expand Down
68 changes: 36 additions & 32 deletions packages/core-data/src/resolvers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ import apiFetch from '@wordpress/api-fetch';
*/
import { STORE_NAME } from './name';
import { getOrLoadEntitiesConfig, DEFAULT_ENTITY_KEY } from './entities';
import { forwardResolver, getNormalizedCommaSeparable } from './utils';
import {
forwardResolver,
getNormalizedCommaSeparable,
getUserPermissionCacheKey,
getUserPermissionsFromResponse,
ALLOWED_RESOURCE_ACTIONS,
} from './utils';
import { getSyncProvider } from './sync';
import { fetchBlockPatterns } from './fetch';

Expand Down Expand Up @@ -58,7 +64,7 @@ export const getCurrentUser =
*/
export const getEntityRecord =
( kind, name, key = '', query ) =>
async ( { select, dispatch } ) => {
async ( { select, dispatch, registry } ) => {
const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) );
const entityConfig = configs.find(
( config ) => config.name === name && config.kind === kind
Expand Down Expand Up @@ -165,8 +171,29 @@ export const getEntityRecord =
}
}

const record = await apiFetch( { path } );
dispatch.receiveEntityRecords( kind, name, record, query );
const response = await apiFetch( { path, parse: false } );
Copy link
Member

Choose a reason for hiding this comment

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

Setting parse: false will disable the fetchAllMiddleware. See how it returns early when options.parse is false. Is this a concern here?

fetchAllMiddleware is relevant for fetching collections, and getEntityRecord fetches only one item, so it shouldn't be relevant. But what if I do:

getEntityRecord( 'postType', 'post', '', { per_page: -1 } );

This will fetch the entire collection anyway, won't it? And should trigger fetchAllMiddleware.

Copy link
Member Author

Choose a reason for hiding this comment

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

I also had similar concerns during the original discussion: #43480 (comment). But it shouldn't affect the getEntityRecord resolver.

The getEntityRecord supports only fetching a single entity. Any other usage is incorrect, and it's up to consumers to fix it.

Calling wp.data.select( 'core' ).getEntityRecord( 'postType', 'post', '', { per_page: -1 } ); will make a request, but then will return undefined.

const record = await response.json();
const permissions = getUserPermissionsFromResponse( response );

registry.batch( () => {
dispatch.receiveEntityRecords( kind, name, record, query );

for ( const action of ALLOWED_RESOURCE_ACTIONS ) {
const permissionKey = getUserPermissionCacheKey(
action,
{ kind, name, id: key }
);

dispatch.receiveUserPermission(
Mamaduka marked this conversation as resolved.
Show resolved Hide resolved
permissionKey,
permissions[ action ]
);
dispatch.finishResolution( 'canUser', [
action,
{ kind, name, id: key },
] );
}
} );
}
} finally {
dispatch.__unstableReleaseStoreLock( lock );
Expand Down Expand Up @@ -355,9 +382,7 @@ export const getEmbedPreview =
export const canUser =
( requestedAction, resource, id ) =>
async ( { dispatch, registry } ) => {
const retrievedActions = [ 'create', 'read', 'update', 'delete' ];

if ( ! retrievedActions.includes( requestedAction ) ) {
if ( ! ALLOWED_RESOURCE_ACTIONS.includes( requestedAction ) ) {
throw new Error( `'${ requestedAction }' is not a valid action.` );
}

Expand Down Expand Up @@ -389,7 +414,7 @@ export const canUser =
const { hasStartedResolution } = registry.select( STORE_NAME );

// Prevent resolving the same resource twice.
for ( const relatedAction of retrievedActions ) {
for ( const relatedAction of ALLOWED_RESOURCE_ACTIONS ) {
if ( relatedAction === requestedAction ) {
continue;
}
Expand All @@ -416,31 +441,10 @@ export const canUser =
return;
}

// Optional chaining operator is used here because the API requests don't
// return the expected result in the native version. Instead, API requests
// only return the result, without including response properties like the headers.
const allowedMethods = response.headers?.get( 'allow' ) || '';

const permissions = {};
const methods = {
create: 'POST',
read: 'GET',
update: 'PUT',
delete: 'DELETE',
};
for ( const [ actionName, methodName ] of Object.entries( methods ) ) {
permissions[ actionName ] = allowedMethods.includes( methodName );
}

const permissions = getUserPermissionsFromResponse( response );
registry.batch( () => {
for ( const action of retrievedActions ) {
const key = (
typeof resource === 'object'
? [ action, resource.kind, resource.name, resource.id ]
: [ action, resource, id ]
)
.filter( Boolean )
.join( '/' );
for ( const action of ALLOWED_RESOURCE_ACTIONS ) {
const key = getUserPermissionCacheKey( action, resource, id );

dispatch.receiveUserPermission( key, permissions[ action ] );

Expand Down
9 changes: 2 additions & 7 deletions packages/core-data/src/selectors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
isRawAttribute,
setNestedValue,
isNumericID,
getUserPermissionCacheKey,
} from './utils';
import type * as ET from './entity-types';
import type { UndoManager } from '@wordpress/undo-manager';
Expand Down Expand Up @@ -1156,13 +1157,7 @@ export function canUser(
return false;
}

const key = (
isEntity
? [ action, resource.kind, resource.name, resource.id ]
: [ action, resource, id ]
)
.filter( Boolean )
.join( '/' );
const key = getUserPermissionCacheKey( action, resource, id );

return state.userPermissions[ key ];
}
Expand Down
Loading
Loading