From a726f886a35fb214d41a29d8cc1b240f784505c1 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Thu, 11 Jul 2024 15:47:53 +0400 Subject: [PATCH 1/6] Core Data: Resolve user capabilities when fetching an entity --- packages/core-data/src/resolvers.js | 42 ++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index 73528a365c4c5f..af81c5515064c9 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -58,7 +58,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 @@ -165,8 +165,44 @@ export const getEntityRecord = } } - const record = await apiFetch( { path } ); - dispatch.receiveEntityRecords( kind, name, record, query ); + const response = await apiFetch( { path, parse: false } ); + const record = await response.json(); + + const allowHeader = response.headers?.get( 'allow' ); + const allowedMethods = allowHeader?.allow || allowHeader || ''; + 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 ); + } + + registry.batch( () => { + dispatch.receiveEntityRecords( kind, name, record, query ); + + for ( const action of [ + 'create', + 'read', + 'update', + 'delete', + ] ) { + const permissionKey = [ action, kind, name, key ] + .filter( Boolean ) + .join( '/' ); + + dispatch.receiveUserPermission( + permissionKey, + permissions[ action ] + ); + } + } ); } } finally { dispatch.__unstableReleaseStoreLock( lock ); From b2a0f4049cf3e6ab64b1fdac9adc6ecfe41049aa Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Sun, 14 Jul 2024 11:09:57 +0400 Subject: [PATCH 2/6] Mark canUser selector as resolved --- packages/core-data/src/resolvers.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index af81c5515064c9..59a2e4a2d2b286 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -201,6 +201,10 @@ export const getEntityRecord = permissionKey, permissions[ action ] ); + dispatch.finishResolution( 'canUser', [ + action, + { kind, name, id: key }, + ] ); } } ); } From 3ca0afadf41055c7ea423eb5c58742ab454c4ffb Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Sun, 14 Jul 2024 11:53:10 +0400 Subject: [PATCH 3/6] Fix unit tests --- .../src/block/test/edit.native.js | 8 +++-- .../src/image/test/edit.native.js | 1 + .../src/hooks/test/use-entity-record.js | 8 +++-- packages/core-data/src/test/resolvers.js | 35 +++++++++++-------- packages/editor/src/store/test/actions.js | 12 +++++-- 5 files changed, 41 insertions(+), 23 deletions(-) diff --git a/packages/block-library/src/block/test/edit.native.js b/packages/block-library/src/block/test/edit.native.js index f184fa164ce71b..46c6d39034f8b0 100644 --- a/packages/block-library/src/block/test/edit.native.js +++ b/packages/block-library/src/block/test/edit.native.js @@ -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( { @@ -229,7 +231,9 @@ describe( 'Synced patterns', () => { response.content.raw = `
`; - return Promise.resolve( response ); + return Promise.resolve( { + json: () => Promise.resolve( response ), + } ); } ); const screen = await initializeEditor( { diff --git a/packages/block-library/src/image/test/edit.native.js b/packages/block-library/src/image/test/edit.native.js index 7a0fd15c854470..5cf653321b2be0 100644 --- a/packages/block-library/src/image/test/edit.native.js +++ b/packages/block-library/src/image/test/edit.native.js @@ -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', diff --git a/packages/core-data/src/hooks/test/use-entity-record.js b/packages/core-data/src/hooks/test/use-entity-record.js index 1bfb13f38ac156..d83957abe06a1b 100644 --- a/packages/core-data/src/hooks/test/use-entity-record.js +++ b/packages/core-data/src/hooks/test/use-entity-record.js @@ -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 = () => { @@ -60,6 +61,7 @@ describe( 'useEntityRecord', () => { await waitFor( () => expect( triggerFetch ).toHaveBeenCalledWith( { path: '/wp/v2/widgets/1?context=edit', + parse: false, } ) ); @@ -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 = () => { @@ -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 } ) => { diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js index 35c3f10b89e076..fa0d53d79aa7d8 100644 --- a/packages/core-data/src/test/resolvers.js +++ b/packages/core-data/src/test/resolvers.js @@ -19,6 +19,7 @@ import { describe( 'getEntityRecord', () => { const POST_TYPE = { slug: 'post' }; + const POST_TYPE_RESPONSE = { json: () => Promise.resolve( POST_TYPE ) }; const ENTITIES = [ { name: 'postType', @@ -27,28 +28,37 @@ describe( 'getEntityRecord', () => { baseURLParams: { context: 'edit' }, }, ]; + const registry = { batch: ( callback ) => callback() }; + let dispatch; beforeEach( async () => { - triggerFetch.mockReset(); - } ); - - it( 'yields with requested post type', async () => { - const dispatch = Object.assign( jest.fn(), { + dispatch = Object.assign( jest.fn(), { receiveEntityRecords: jest.fn(), __unstableAcquireStoreLock: jest.fn(), __unstableReleaseStoreLock: jest.fn(), + receiveUserPermission: jest.fn(), + finishResolution: jest.fn(), } ); + triggerFetch.mockReset(); + } ); + + it( 'yields with requested post type', async () => { // Provide entities dispatch.mockReturnValueOnce( ENTITIES ); // Provide response - triggerFetch.mockImplementation( () => POST_TYPE ); + triggerFetch.mockImplementation( () => POST_TYPE_RESPONSE ); - await getEntityRecord( 'root', 'postType', 'post' )( { dispatch } ); + await getEntityRecord( + 'root', + 'postType', + 'post' + )( { dispatch, registry } ); // Fetch request should have been issued. expect( triggerFetch ).toHaveBeenCalledWith( { path: '/wp/v2/types/post?context=edit', + parse: false, } ); // The record should have been received. @@ -75,24 +85,18 @@ describe( 'getEntityRecord', () => { const select = { hasEntityRecords: jest.fn( () => {} ), }; - - const dispatch = Object.assign( jest.fn(), { - receiveEntityRecords: jest.fn(), - __unstableAcquireStoreLock: jest.fn(), - __unstableReleaseStoreLock: jest.fn(), - } ); // Provide entities dispatch.mockReturnValueOnce( ENTITIES ); // Provide response - triggerFetch.mockImplementation( () => POST_TYPE ); + triggerFetch.mockImplementation( () => POST_TYPE_RESPONSE ); await getEntityRecord( 'root', 'postType', 'post', query - )( { dispatch, select } ); + )( { dispatch, select, registry } ); // Check resolution cache for an existing entity that fulfills the request with query. expect( select.hasEntityRecords ).toHaveBeenCalledWith( @@ -104,6 +108,7 @@ describe( 'getEntityRecord', () => { // Trigger apiFetch, test that the query is present in the url. expect( triggerFetch ).toHaveBeenCalledWith( { path: '/wp/v2/types/post?context=view&_envelope=1', + parse: false, } ); // The record should have been received. diff --git a/packages/editor/src/store/test/actions.js b/packages/editor/src/store/test/actions.js index a2e683cc23cc99..fae30c6fc271ec 100644 --- a/packages/editor/src/store/test/actions.js +++ b/packages/editor/src/store/test/actions.js @@ -90,7 +90,9 @@ describe( 'Post actions', () => { method === 'GET' && path.startsWith( '/wp/v2/types/post' ) ) { - return {}; + return { + json: () => Promise.resolve( {} ), + }; } throw { @@ -178,7 +180,9 @@ describe( 'Post actions', () => { path.startsWith( '/wp/v2/types/post' ) || path.startsWith( `/wp/v2/posts/${ postId }` ) ) { - return {}; + return { + json: () => Promise.resolve( {} ), + }; } } @@ -262,7 +266,9 @@ describe( 'Post actions', () => { method === 'GET' && path.startsWith( '/wp/v2/types/post' ) ) { - return {}; + return { + json: () => Promise.resolve( {} ), + }; } throw { From a9b931e500ab1d18cce1c994760a707336317666 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Mon, 15 Jul 2024 16:30:11 +0400 Subject: [PATCH 4/6] Dedupe logic --- packages/core-data/src/resolvers.js | 72 +++++-------------- packages/core-data/src/selectors.ts | 9 +-- packages/core-data/src/utils/index.js | 5 ++ .../core-data/src/utils/user-permissions.js | 39 ++++++++++ 4 files changed, 64 insertions(+), 61 deletions(-) create mode 100644 packages/core-data/src/utils/user-permissions.js diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index 59a2e4a2d2b286..0b0cc017086cc7 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -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'; @@ -167,35 +173,16 @@ export const getEntityRecord = const response = await apiFetch( { path, parse: false } ); const record = await response.json(); - - const allowHeader = response.headers?.get( 'allow' ); - const allowedMethods = allowHeader?.allow || allowHeader || ''; - 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( () => { dispatch.receiveEntityRecords( kind, name, record, query ); - for ( const action of [ - 'create', - 'read', - 'update', - 'delete', - ] ) { - const permissionKey = [ action, kind, name, key ] - .filter( Boolean ) - .join( '/' ); + for ( const action of ALLOWED_RESOURCE_ACTIONS ) { + const permissionKey = getUserPermissionCacheKey( + action, + { kind, name, id: key } + ); dispatch.receiveUserPermission( permissionKey, @@ -395,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.` ); } @@ -429,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; } @@ -456,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 ] ); diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index 6ff8e26d3684e7..aeec14782ce4fb 100644 --- a/packages/core-data/src/selectors.ts +++ b/packages/core-data/src/selectors.ts @@ -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'; @@ -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 ]; } diff --git a/packages/core-data/src/utils/index.js b/packages/core-data/src/utils/index.js index d4d491fab9827d..bb4b5544435021 100644 --- a/packages/core-data/src/utils/index.js +++ b/packages/core-data/src/utils/index.js @@ -9,3 +9,8 @@ export { default as isRawAttribute } from './is-raw-attribute'; export { default as setNestedValue } from './set-nested-value'; export { default as getNestedValue } from './get-nested-value'; export { default as isNumericID } from './is-numeric-id'; +export { + getUserPermissionCacheKey, + getUserPermissionsFromResponse, + ALLOWED_RESOURCE_ACTIONS, +} from './user-permissions'; diff --git a/packages/core-data/src/utils/user-permissions.js b/packages/core-data/src/utils/user-permissions.js new file mode 100644 index 00000000000000..48c9c8bd2652fd --- /dev/null +++ b/packages/core-data/src/utils/user-permissions.js @@ -0,0 +1,39 @@ +export const ALLOWED_RESOURCE_ACTIONS = [ + 'create', + 'read', + 'update', + 'delete', +]; + +export function getUserPermissionsFromResponse( response ) { + const permissions = {}; + + // 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 methods = { + create: 'POST', + read: 'GET', + update: 'PUT', + delete: 'DELETE', + }; + for ( const [ actionName, methodName ] of Object.entries( methods ) ) { + permissions[ actionName ] = allowedMethods.includes( methodName ); + } + + return permissions; +} + +export function getUserPermissionCacheKey( action, resource, id ) { + const key = ( + typeof resource === 'object' + ? [ action, resource.kind, resource.name, resource.id ] + : [ action, resource, id ] + ) + .filter( Boolean ) + .join( '/' ); + + return key; +} From 23ea0c16c572a7120133ca8b9017845fd52c8945 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Mon, 15 Jul 2024 16:48:08 +0400 Subject: [PATCH 5/6] Showcase: Update Pattern block to benefit from new user permission resolutions --- packages/block-library/src/block/edit.js | 89 +++++++++++++++--------- 1 file changed, 58 insertions(+), 31 deletions(-) diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 86c9483c98b343..53422e1c4cb8c0 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -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 && ( + + + + { __( 'Edit original' ) } + + + + ) } + + { canOverrideBlocks && ( + + + + { __( 'Reset' ) } + + + + ) } + + ); +} + function ReusableBlockEdit( { name, attributes: { ref, content }, @@ -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, @@ -175,7 +211,7 @@ function ReusableBlockEdit( { ), }; }, - [ patternClientId, ref ] + [ patternClientId ] ); // Sync the editing mode of the pattern block with the inner blocks. @@ -256,27 +292,18 @@ function ReusableBlockEdit( { return ( <> - { userCanEdit && onNavigateToEntityRecord && ( - - - - { __( 'Edit original' ) } - - - - ) } - - { canOverrideBlocks && ( - - - - { __( 'Reset' ) } - - - + { hasResolved && ( + ) } { children === null ? ( From c8ae4b6abc7b90f1d1758907dcc753fc53d2e9f5 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Tue, 16 Jul 2024 17:10:33 +0400 Subject: [PATCH 6/6] Clarify comment Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> --- packages/core-data/src/utils/user-permissions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-data/src/utils/user-permissions.js b/packages/core-data/src/utils/user-permissions.js index 48c9c8bd2652fd..a81c83f9e5af50 100644 --- a/packages/core-data/src/utils/user-permissions.js +++ b/packages/core-data/src/utils/user-permissions.js @@ -9,7 +9,7 @@ export function getUserPermissionsFromResponse( response ) { const permissions = {}; // Optional chaining operator is used here because the API requests don't - // return the expected result in the native version. Instead, API requests + // return the expected result in the React native version. Instead, API requests // only return the result, without including response properties like the headers. const allowedMethods = response.headers?.get( 'allow' ) || '';