From 1eff14a6d9d04a3a85c61889cb6422f005853adb Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 17 Jul 2024 15:12:06 +0400 Subject: [PATCH] Core Data: Resolve user capabilities when fetching an entity (#63430) * Core Data: Resolve user capabilities when fetching an entity * Mark canUser selector as resolved * Fix unit tests * Dedupe logic * Showcase: Update Pattern block to benefit from new user permission resolutions * Clarify comment Co-authored-by: Mamaduka Co-authored-by: youknowriad Co-authored-by: jsnajdr Co-authored-by: tyxla --- packages/block-library/src/block/edit.js | 89 ++++++++++++------- .../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/resolvers.js | 68 +++++++------- packages/core-data/src/selectors.ts | 9 +- packages/core-data/src/test/resolvers.js | 35 ++++---- packages/core-data/src/utils/index.js | 5 ++ .../core-data/src/utils/user-permissions.js | 39 ++++++++ packages/editor/src/store/test/actions.js | 12 ++- 10 files changed, 181 insertions(+), 93 deletions(-) create mode 100644 packages/core-data/src/utils/user-permissions.js diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 86c9483c98b34..53422e1c4cb8c 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 ? ( diff --git a/packages/block-library/src/block/test/edit.native.js b/packages/block-library/src/block/test/edit.native.js index f184fa164ce71..46c6d39034f8b 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 7a0fd15c85447..5cf653321b2be 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 1bfb13f38ac15..d83957abe06a1 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/resolvers.js b/packages/core-data/src/resolvers.js index 73528a365c4c5..0b0cc017086cc 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'; @@ -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 @@ -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 } ); + 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( + permissionKey, + permissions[ action ] + ); + dispatch.finishResolution( 'canUser', [ + action, + { kind, name, id: key }, + ] ); + } + } ); } } finally { dispatch.__unstableReleaseStoreLock( lock ); @@ -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.` ); } @@ -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; } @@ -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 ] ); diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index 6ff8e26d3684e..aeec14782ce4f 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/test/resolvers.js b/packages/core-data/src/test/resolvers.js index 35c3f10b89e07..fa0d53d79aa7d 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/core-data/src/utils/index.js b/packages/core-data/src/utils/index.js index d4d491fab9827..bb4b554443502 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 0000000000000..a81c83f9e5af5 --- /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 React 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; +} diff --git a/packages/editor/src/store/test/actions.js b/packages/editor/src/store/test/actions.js index a2e683cc23cc9..fae30c6fc271e 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 {