From 4ea1c4f1be9fd6344fa53b663883e0a36cb27192 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Tue, 9 Jul 2024 21:52:56 +0400 Subject: [PATCH 01/10] Core Data: Support entities in the 'canUser' selector --- docs/reference-guides/data/data-core.md | 2 +- packages/core-data/README.md | 2 +- packages/core-data/src/resolvers.js | 41 ++++++++++++++++++++----- packages/core-data/src/selectors.ts | 11 +++++-- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/docs/reference-guides/data/data-core.md b/docs/reference-guides/data/data-core.md index ba77f065584cf..23503ce411eda 100644 --- a/docs/reference-guides/data/data-core.md +++ b/docs/reference-guides/data/data-core.md @@ -18,7 +18,7 @@ _Parameters_ - _state_ `State`: Data state. - _action_ `string`: Action to check. One of: 'create', 'read', 'update', 'delete'. -- _resource_ `string`: REST resource to check, e.g. 'media' or 'posts'. +- _resource_ `string | Record< string, any >`: REST resource to check, e.g. 'media' or 'posts'. - _id_ `EntityRecordKey`: Optional ID of the rest resource to check. _Returns_ diff --git a/packages/core-data/README.md b/packages/core-data/README.md index 694f780dafb99..90f53a61a2833 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -339,7 +339,7 @@ _Parameters_ - _state_ `State`: Data state. - _action_ `string`: Action to check. One of: 'create', 'read', 'update', 'delete'. -- _resource_ `string`: REST resource to check, e.g. 'media' or 'posts'. +- _resource_ `string | Record< string, any >`: REST resource to check, e.g. 'media' or 'posts'. - _id_ `EntityRecordKey`: Optional ID of the rest resource to check. _Returns_ diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index bff8c8cb0f678..a6a6a86a732af 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -360,15 +360,35 @@ export const getEmbedPreview = export const canUser = ( requestedAction, resource, id ) => async ( { dispatch, registry } ) => { - const { hasStartedResolution } = registry.select( STORE_NAME ); - - const resourcePath = id ? `${ resource }/${ id }` : resource; const retrievedActions = [ 'create', 'read', 'update', 'delete' ]; if ( ! retrievedActions.includes( requestedAction ) ) { throw new Error( `'${ requestedAction }' is not a valid action.` ); } + let resourcePath = null; + if ( typeof resource === 'object' ) { + const configs = await dispatch( + getOrLoadEntitiesConfig( resource.kind, resource.name ) + ); + const entityConfig = configs.find( + ( config ) => + config.name === resource.name && + config.kind === resource.name + ); + if ( ! entityConfig ) { + return; + } + + resourcePath = + entityConfig.baseURL + ( resource.id ? '/' + resource.id : '' ); + } else { + // @todo: Maybe warn when detecting a legacy usage. + resourcePath = `/wp/v2/${ resource }` + ( id ? '/' + id : '' ); + } + + const { hasStartedResolution } = registry.select( STORE_NAME ); + // Prevent resolving the same resource twice. for ( const relatedAction of retrievedActions ) { if ( relatedAction === requestedAction ) { @@ -387,7 +407,7 @@ export const canUser = let response; try { response = await apiFetch( { - path: `/wp/v2/${ resourcePath }`, + path: resourcePath, method: 'OPTIONS', parse: false, } ); @@ -416,10 +436,15 @@ export const canUser = registry.batch( () => { for ( const action of retrievedActions ) { - dispatch.receiveUserPermission( - `${ action }/${ resourcePath }`, - permissions[ action ] - ); + const key = ( + typeof resource === 'object' + ? [ action, resource.kind, resource.name, resource.id ] + : [ action, resource, id ] + ) + .filter( Boolean ) + .join( '/' ); + + dispatch.receiveUserPermission( key, permissions[ action ] ); } } ); }; diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index 425a537cad736..7d3e76ca988a1 100644 --- a/packages/core-data/src/selectors.ts +++ b/packages/core-data/src/selectors.ts @@ -1145,10 +1145,17 @@ export function isPreviewEmbedFallback( state: State, url: string ): boolean { export function canUser( state: State, action: string, - resource: string, + resource: string | Record< string, any >, id?: EntityRecordKey ): boolean | undefined { - const key = [ action, resource, id ].filter( Boolean ).join( '/' ); + const key = ( + typeof resource === 'object' + ? [ action, resource.kind, resource.name, resource.id ] + : [ action, resource, id ] + ) + .filter( Boolean ) + .join( '/' ); + return state.userPermissions[ key ]; } From 2cf879fb7fcab1fc6d7e9397f9adb165cdc4891f Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 10 Jul 2024 09:13:18 +0400 Subject: [PATCH 02/10] Cleanup 'canUser' unit tests --- packages/core-data/src/test/resolvers.js | 33 +++--------------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js index 4e900615df386..2187f0275593b 100644 --- a/packages/core-data/src/test/resolvers.js +++ b/packages/core-data/src/test/resolvers.js @@ -283,7 +283,7 @@ describe( 'getEmbedPreview', () => { } ); describe( 'canUser', () => { - let registry; + let dispatch, registry; beforeEach( async () => { registry = { select: jest.fn( () => ( { @@ -291,14 +291,13 @@ describe( 'canUser', () => { } ) ), batch: ( callback ) => callback(), }; + dispatch = Object.assign( jest.fn(), { + receiveUserPermission: jest.fn(), + } ); triggerFetch.mockReset(); } ); it( 'does nothing when there is an API error', async () => { - const dispatch = Object.assign( jest.fn(), { - receiveUserPermission: jest.fn(), - } ); - triggerFetch.mockImplementation( () => Promise.reject( { status: 404 } ) ); @@ -315,10 +314,6 @@ describe( 'canUser', () => { } ); it( 'receives false when the user is not allowed to perform an action', async () => { - const dispatch = Object.assign( jest.fn(), { - receiveUserPermission: jest.fn(), - } ); - triggerFetch.mockImplementation( () => ( { headers: new Map( [ [ 'allow', 'GET' ] ] ), } ) ); @@ -338,10 +333,6 @@ describe( 'canUser', () => { } ); it( 'receives true when the user is allowed to perform an action', async () => { - const dispatch = Object.assign( jest.fn(), { - receiveUserPermission: jest.fn(), - } ); - triggerFetch.mockImplementation( () => ( { headers: new Map( [ [ 'allow', 'POST, GET, PUT, DELETE' ] ] ), } ) ); @@ -361,10 +352,6 @@ describe( 'canUser', () => { } ); it( 'receives true when the user is allowed to perform an action on a specific resource', async () => { - const dispatch = Object.assign( jest.fn(), { - receiveUserPermission: jest.fn(), - } ); - triggerFetch.mockImplementation( () => ( { headers: new Map( [ [ 'allow', 'POST, GET, PUT, DELETE' ] ] ), } ) ); @@ -384,10 +371,6 @@ describe( 'canUser', () => { } ); it( 'runs apiFetch only once per resource', async () => { - const dispatch = Object.assign( jest.fn(), { - receiveUserPermission: jest.fn(), - } ); - registry = { ...registry, select: () => ( { @@ -415,10 +398,6 @@ describe( 'canUser', () => { } ); it( 'retrieves all permissions even when ID is not given', async () => { - const dispatch = Object.assign( jest.fn(), { - receiveUserPermission: jest.fn(), - } ); - registry = { ...registry, select: () => ( { @@ -454,10 +433,6 @@ describe( 'canUser', () => { } ); it( 'runs apiFetch only once per resource ID', async () => { - const dispatch = Object.assign( jest.fn(), { - receiveUserPermission: jest.fn(), - } ); - registry = { ...registry, select: () => ( { From 2230256e9c0e16bd371ebdad889f8a97135bb3e8 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 10 Jul 2024 09:33:23 +0400 Subject: [PATCH 03/10] Add unit tests --- packages/core-data/src/resolvers.js | 2 +- packages/core-data/src/test/resolvers.js | 182 +++++++++++++++++++++++ packages/core-data/src/test/selectors.js | 11 ++ 3 files changed, 194 insertions(+), 1 deletion(-) diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index a6a6a86a732af..855439f6589c9 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -374,7 +374,7 @@ export const canUser = const entityConfig = configs.find( ( config ) => config.name === resource.name && - config.kind === resource.name + config.kind === resource.kind ); if ( ! entityConfig ) { return; diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js index 2187f0275593b..e99f9f2b4e098 100644 --- a/packages/core-data/src/test/resolvers.js +++ b/packages/core-data/src/test/resolvers.js @@ -283,6 +283,21 @@ describe( 'getEmbedPreview', () => { } ); describe( 'canUser', () => { + const ENTITIES = [ + { + name: 'media', + kind: 'root', + baseURL: '/wp/v2/media', + baseURLParams: { context: 'edit' }, + }, + { + name: 'wp_block', + kind: 'postType', + baseURL: '/wp/v2/blocks', + baseURLParams: { context: 'edit' }, + }, + ]; + let dispatch, registry; beforeEach( async () => { registry = { @@ -294,6 +309,7 @@ describe( 'canUser', () => { dispatch = Object.assign( jest.fn(), { receiveUserPermission: jest.fn(), } ); + dispatch.mockReturnValue( ENTITIES ); triggerFetch.mockReset(); } ); @@ -303,6 +319,10 @@ describe( 'canUser', () => { ); await canUser( 'create', 'media' )( { dispatch, registry } ); + await canUser( 'create', { kind: 'root', name: 'media' } )( { + dispatch, + registry, + } ); expect( triggerFetch ).toHaveBeenCalledWith( { path: '/wp/v2/media', @@ -332,6 +352,28 @@ describe( 'canUser', () => { ); } ); + it( 'receives false when the user is not allowed to perform an action on entities', async () => { + triggerFetch.mockImplementation( () => ( { + headers: new Map( [ [ 'allow', 'GET' ] ] ), + } ) ); + + await canUser( 'create', { kind: 'root', name: 'media' } )( { + dispatch, + registry, + } ); + + expect( triggerFetch ).toHaveBeenCalledWith( { + path: '/wp/v2/media', + method: 'OPTIONS', + parse: false, + } ); + + expect( dispatch.receiveUserPermission ).toHaveBeenCalledWith( + 'create/root/media', + false + ); + } ); + it( 'receives true when the user is allowed to perform an action', async () => { triggerFetch.mockImplementation( () => ( { headers: new Map( [ [ 'allow', 'POST, GET, PUT, DELETE' ] ] ), @@ -351,6 +393,28 @@ describe( 'canUser', () => { ); } ); + it( 'receives true when the user is allowed to perform an action on entities', async () => { + triggerFetch.mockImplementation( () => ( { + headers: new Map( [ [ 'allow', 'POST, GET, PUT, DELETE' ] ] ), + } ) ); + + await canUser( 'create', { kind: 'root', name: 'media' } )( { + dispatch, + registry, + } ); + + expect( triggerFetch ).toHaveBeenCalledWith( { + path: '/wp/v2/media', + method: 'OPTIONS', + parse: false, + } ); + + expect( dispatch.receiveUserPermission ).toHaveBeenCalledWith( + 'create/root/media', + true + ); + } ); + it( 'receives true when the user is allowed to perform an action on a specific resource', async () => { triggerFetch.mockImplementation( () => ( { headers: new Map( [ [ 'allow', 'POST, GET, PUT, DELETE' ] ] ), @@ -370,6 +434,32 @@ describe( 'canUser', () => { ); } ); + it( 'receives true when the user is allowed to perform an action on a specific entity', async () => { + triggerFetch.mockImplementation( () => ( { + headers: new Map( [ [ 'allow', 'POST, GET, PUT, DELETE' ] ] ), + } ) ); + + await canUser( 'create', { + kind: 'postType', + name: 'wp_block', + id: 123, + } )( { + dispatch, + registry, + } ); + + expect( triggerFetch ).toHaveBeenCalledWith( { + path: '/wp/v2/blocks/123', + method: 'OPTIONS', + parse: false, + } ); + + expect( dispatch.receiveUserPermission ).toHaveBeenCalledWith( + 'create/postType/wp_block/123', + true + ); + } ); + it( 'runs apiFetch only once per resource', async () => { registry = { ...registry, @@ -397,6 +487,45 @@ describe( 'canUser', () => { ); } ); + it( 'runs apiFetch only once per entity', async () => { + registry = { + ...registry, + select: () => ( { + hasStartedResolution: ( _, [ action ] ) => action === 'read', + } ), + }; + + triggerFetch.mockImplementation( () => ( { + headers: new Map( [ [ 'allow', 'POST, GET' ] ] ), + } ) ); + + await canUser( 'create', { + kind: 'postType', + name: 'wp_block', + } )( { + dispatch, + registry, + } ); + await canUser( 'read', { + kind: 'postType', + name: 'wp_block', + } )( { + dispatch, + registry, + } ); + + expect( triggerFetch ).toHaveBeenCalledTimes( 1 ); + + expect( dispatch.receiveUserPermission ).toHaveBeenCalledWith( + 'create/postType/wp_block', + true + ); + expect( dispatch.receiveUserPermission ).toHaveBeenCalledWith( + 'read/postType/wp_block', + true + ); + } ); + it( 'retrieves all permissions even when ID is not given', async () => { registry = { ...registry, @@ -468,6 +597,59 @@ describe( 'canUser', () => { true ); } ); + + it( 'runs apiFetch only once per entity ID', async () => { + registry = { + ...registry, + select: () => ( { + hasStartedResolution: ( _, [ action ] ) => action === 'create', + } ), + }; + + triggerFetch.mockImplementation( () => ( { + headers: new Map( [ [ 'allow', 'POST, GET, PUT, DELETE' ] ] ), + } ) ); + + await canUser( 'create', { + kind: 'postType', + name: 'wp_block', + id: 123, + } )( { dispatch, registry } ); + await canUser( 'read', { + kind: 'postType', + name: 'wp_block', + id: 123, + } )( { dispatch, registry } ); + await canUser( 'update', { + kind: 'postType', + name: 'wp_block', + id: 123, + } )( { dispatch, registry } ); + await canUser( 'delete', { + kind: 'postType', + name: 'wp_block', + id: 123, + } )( { dispatch, registry } ); + + expect( triggerFetch ).toHaveBeenCalledTimes( 1 ); + + expect( dispatch.receiveUserPermission ).toHaveBeenCalledWith( + 'create/postType/wp_block/123', + true + ); + expect( dispatch.receiveUserPermission ).toHaveBeenCalledWith( + 'read/postType/wp_block/123', + true + ); + expect( dispatch.receiveUserPermission ).toHaveBeenCalledWith( + 'update/postType/wp_block/123', + true + ); + expect( dispatch.receiveUserPermission ).toHaveBeenCalledWith( + 'delete/postType/wp_block/123', + true + ); + } ); } ); describe( 'getAutosaves', () => { diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js index 43c84a3e97891..841bccd2ce413 100644 --- a/packages/core-data/src/test/selectors.js +++ b/packages/core-data/src/test/selectors.js @@ -690,24 +690,35 @@ describe( 'canUser', () => { userPermissions: {}, } ); expect( canUser( state, 'create', 'media' ) ).toBe( undefined ); + expect( + canUser( state, 'create', { kind: 'root', name: 'media' } ) + ).toBe( undefined ); } ); it( 'returns whether an action can be performed', () => { const state = deepFreeze( { userPermissions: { 'create/media': false, + 'create/root/media': false, }, } ); expect( canUser( state, 'create', 'media' ) ).toBe( false ); + expect( + canUser( state, 'create', { kind: 'root', name: 'media' } ) + ).toBe( false ); } ); it( 'returns whether an action can be performed for a given resource', () => { const state = deepFreeze( { userPermissions: { 'create/media/123': false, + 'create/root/media/123': false, }, } ); expect( canUser( state, 'create', 'media', 123 ) ).toBe( false ); + expect( + canUser( state, 'create', { kind: 'root', name: 'media', id: 123 } ) + ).toBe( false ); } ); } ); From b99da12981b5db037fd17049a04664ce03b3559e Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 10 Jul 2024 09:44:01 +0400 Subject: [PATCH 04/10] Replace 'canUserEditEntityRecord' selector usages --- packages/block-library/src/post-title/edit.js | 10 +++++----- packages/block-library/src/utils/hooks.js | 6 +++++- packages/editor/src/bindings/post-meta.js | 10 +++++----- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/block-library/src/post-title/edit.js b/packages/block-library/src/post-title/edit.js index 5b11ec3a9452a..3963016e34282 100644 --- a/packages/block-library/src/post-title/edit.js +++ b/packages/block-library/src/post-title/edit.js @@ -42,11 +42,11 @@ export default function PostTitleEdit( { if ( isDescendentOfQueryLoop ) { return false; } - return select( coreStore ).canUserEditEntityRecord( - 'postType', - postType, - postId - ); + return select( coreStore ).canUser( 'update', { + kind: 'postType', + name: postType, + id: postId, + } ); }, [ isDescendentOfQueryLoop, postType, postId ] ); diff --git a/packages/block-library/src/utils/hooks.js b/packages/block-library/src/utils/hooks.js index 43733d7f49a04..f9ad52297c53e 100644 --- a/packages/block-library/src/utils/hooks.js +++ b/packages/block-library/src/utils/hooks.js @@ -18,7 +18,11 @@ import { useViewportMatch } from '@wordpress/compose'; export function useCanEditEntity( kind, name, recordId ) { return useSelect( ( select ) => - select( coreStore ).canUserEditEntityRecord( kind, name, recordId ), + select( coreStore ).canUser( 'update', { + kind, + name, + id: recordId, + } ), [ kind, name, recordId ] ); } diff --git a/packages/editor/src/bindings/post-meta.js b/packages/editor/src/bindings/post-meta.js index 0b46e58542a35..a2fb596466397 100644 --- a/packages/editor/src/bindings/post-meta.js +++ b/packages/editor/src/bindings/post-meta.js @@ -59,11 +59,11 @@ export default { } // Check that the user has the capability to edit post meta. - const canUserEdit = select( coreDataStore ).canUserEditEntityRecord( - 'postType', - context?.postType, - context?.postId - ); + const canUserEdit = select( coreDataStore ).canUser( 'update', { + kind: 'postType', + name: context?.postType, + id: context?.postId, + } ); if ( ! canUserEdit ) { return false; } From b6aeb12bb82839a9f342dad86aec1583f46e4bf9 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 10 Jul 2024 09:48:22 +0400 Subject: [PATCH 05/10] Make 'canUserEditEntityRecord' forwarded selector/resolver --- packages/core-data/src/resolvers.js | 11 +---- packages/core-data/src/selectors.ts | 8 +--- packages/core-data/src/test/selectors.js | 56 ------------------------ 3 files changed, 2 insertions(+), 73 deletions(-) diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index 855439f6589c9..4882a79df2c82 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -460,16 +460,7 @@ export const canUser = export const canUserEditEntityRecord = ( kind, name, recordId ) => async ( { dispatch } ) => { - const configs = await dispatch( getOrLoadEntitiesConfig( kind, name ) ); - const entityConfig = configs.find( - ( config ) => config.name === name && config.kind === kind - ); - if ( ! entityConfig ) { - return; - } - - const resource = entityConfig.__unstable_rest_base; - await dispatch( canUser( 'update', resource, recordId ) ); + await dispatch( canUser( 'update', { kind, name, id: recordId } ) ); }; /** diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index 7d3e76ca988a1..60aff2dab78bc 100644 --- a/packages/core-data/src/selectors.ts +++ b/packages/core-data/src/selectors.ts @@ -1180,13 +1180,7 @@ export function canUserEditEntityRecord( name: string, recordId: EntityRecordKey ): boolean | undefined { - const entityConfig = getEntityConfig( state, kind, name ); - if ( ! entityConfig ) { - return false; - } - const resource = entityConfig.__unstable_rest_base; - - return canUser( state, 'update', resource, recordId ); + return canUser( state, 'update', { kind, name, id: recordId } ); } /** diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js index 841bccd2ce413..7c917f703eeb6 100644 --- a/packages/core-data/src/test/selectors.js +++ b/packages/core-data/src/test/selectors.js @@ -18,7 +18,6 @@ import { getEmbedPreview, isPreviewEmbedFallback, canUser, - canUserEditEntityRecord, getAutosave, getAutosaves, getCurrentUser, @@ -722,61 +721,6 @@ describe( 'canUser', () => { } ); } ); -describe( 'canUserEditEntityRecord', () => { - it( 'returns false by default', () => { - const state = deepFreeze( { - userPermissions: {}, - entities: { records: {} }, - } ); - expect( canUserEditEntityRecord( state, 'postType', 'post' ) ).toBe( - false - ); - } ); - - it( 'returns whether the user can edit', () => { - const state = deepFreeze( { - userPermissions: { - 'create/posts': false, - 'update/posts/1': true, - }, - entities: { - config: [ - { - kind: 'postType', - name: 'post', - __unstable_rest_base: 'posts', - }, - ], - records: { - root: { - postType: { - queriedData: { - items: { - default: { - post: { - slug: 'post', - __unstable: 'posts', - }, - }, - }, - itemIsComplete: { - default: { - post: true, - }, - }, - queries: {}, - }, - }, - }, - }, - }, - } ); - expect( - canUserEditEntityRecord( state, 'postType', 'post', '1' ) - ).toBe( true ); - } ); -} ); - describe( 'getAutosave', () => { const testAutosave = { author: 1, From 4e185e1a6edf738767020ac3d1ed4943ff1cd783 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 10 Jul 2024 15:47:09 +0400 Subject: [PATCH 06/10] Add checks when resource is an entity --- docs/reference-guides/data/data-core.md | 4 ++-- packages/core-data/README.md | 4 ++-- packages/core-data/src/resolvers.js | 4 ++++ packages/core-data/src/selectors.ts | 11 ++++++++--- packages/core-data/src/test/resolvers.js | 19 +++++++++++++++++++ packages/core-data/src/test/selectors.js | 8 ++++++++ 6 files changed, 43 insertions(+), 7 deletions(-) diff --git a/docs/reference-guides/data/data-core.md b/docs/reference-guides/data/data-core.md index 23503ce411eda..063052d1a4388 100644 --- a/docs/reference-guides/data/data-core.md +++ b/docs/reference-guides/data/data-core.md @@ -23,7 +23,7 @@ _Parameters_ _Returns_ -- `boolean | undefined`: Whether or not the user can perform the action, or `undefined` if the OPTIONS request is still being made. +- `boolean | undefined | null`: Whether or not the user can perform the action, or `undefined` if the OPTIONS request is still being made. ### canUserEditEntityRecord @@ -42,7 +42,7 @@ _Parameters_ _Returns_ -- `boolean | undefined`: Whether or not the user can edit, or `undefined` if the OPTIONS request is still being made. +- `boolean | undefined | null`: Whether or not the user can edit, or `undefined` if the OPTIONS request is still being made. ### getAuthors diff --git a/packages/core-data/README.md b/packages/core-data/README.md index 90f53a61a2833..d6e6d97e9a1d7 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -344,7 +344,7 @@ _Parameters_ _Returns_ -- `boolean | undefined`: Whether or not the user can perform the action, or `undefined` if the OPTIONS request is still being made. +- `boolean | undefined | null`: Whether or not the user can perform the action, or `undefined` if the OPTIONS request is still being made. ### canUserEditEntityRecord @@ -363,7 +363,7 @@ _Parameters_ _Returns_ -- `boolean | undefined`: Whether or not the user can edit, or `undefined` if the OPTIONS request is still being made. +- `boolean | undefined | null`: Whether or not the user can edit, or `undefined` if the OPTIONS request is still being made. ### getAuthors diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index 4882a79df2c82..446ba8466161d 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -368,6 +368,10 @@ export const canUser = let resourcePath = null; if ( typeof resource === 'object' ) { + if ( ! resource.kind || ! resource.name ) { + return; + } + const configs = await dispatch( getOrLoadEntitiesConfig( resource.kind, resource.name ) ); diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index 60aff2dab78bc..7c82982553727 100644 --- a/packages/core-data/src/selectors.ts +++ b/packages/core-data/src/selectors.ts @@ -1147,9 +1147,14 @@ export function canUser( action: string, resource: string | Record< string, any >, id?: EntityRecordKey -): boolean | undefined { +): boolean | undefined | null { + const isEntity = typeof resource === 'object'; + if ( isEntity && ( ! resource.kind || ! resource.name ) ) { + return null; + } + const key = ( - typeof resource === 'object' + isEntity ? [ action, resource.kind, resource.name, resource.id ] : [ action, resource, id ] ) @@ -1179,7 +1184,7 @@ export function canUserEditEntityRecord( kind: string, name: string, recordId: EntityRecordKey -): boolean | undefined { +): boolean | undefined | null { return canUser( state, 'update', { kind, name, id: recordId } ); } diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js index e99f9f2b4e098..c751a67f73d46 100644 --- a/packages/core-data/src/test/resolvers.js +++ b/packages/core-data/src/test/resolvers.js @@ -333,6 +333,25 @@ describe( 'canUser', () => { expect( dispatch.receiveUserPermission ).not.toHaveBeenCalled(); } ); + it( 'does nothing when entity kind or name is missing', async () => { + triggerFetch.mockImplementation( () => + Promise.reject( { status: 404 } ) + ); + + await canUser( 'create', { kind: 'root', name: 'media' } )( { + dispatch, + registry, + } ); + await canUser( 'create', { name: 'wp_block' } )( { + dispatch, + registry, + } ); + + expect( triggerFetch ).not.toHaveBeenCalledWith(); + + expect( dispatch.receiveUserPermission ).not.toHaveBeenCalled(); + } ); + it( 'receives false when the user is not allowed to perform an action', async () => { triggerFetch.mockImplementation( () => ( { headers: new Map( [ [ 'allow', 'GET' ] ] ), diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js index 7c917f703eeb6..b3edbab9b6926 100644 --- a/packages/core-data/src/test/selectors.js +++ b/packages/core-data/src/test/selectors.js @@ -694,6 +694,14 @@ describe( 'canUser', () => { ).toBe( undefined ); } ); + it( 'returns null when entity kind or name is missing', () => { + const state = deepFreeze( { + userPermissions: {}, + } ); + expect( canUser( state, 'create', { name: 'media' } ) ).toBe( null ); + expect( canUser( state, 'create', { kind: 'root' } ) ).toBe( null ); + } ); + it( 'returns whether an action can be performed', () => { const state = deepFreeze( { userPermissions: { From 92b2859435efa75cb715a7fc270105e7b64f1a71 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 10 Jul 2024 17:30:59 +0400 Subject: [PATCH 07/10] Return false when entity arg is malformed --- docs/reference-guides/data/data-core.md | 4 ++-- packages/core-data/README.md | 4 ++-- packages/core-data/src/selectors.ts | 6 +++--- packages/core-data/src/test/selectors.js | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/reference-guides/data/data-core.md b/docs/reference-guides/data/data-core.md index 063052d1a4388..23503ce411eda 100644 --- a/docs/reference-guides/data/data-core.md +++ b/docs/reference-guides/data/data-core.md @@ -23,7 +23,7 @@ _Parameters_ _Returns_ -- `boolean | undefined | null`: Whether or not the user can perform the action, or `undefined` if the OPTIONS request is still being made. +- `boolean | undefined`: Whether or not the user can perform the action, or `undefined` if the OPTIONS request is still being made. ### canUserEditEntityRecord @@ -42,7 +42,7 @@ _Parameters_ _Returns_ -- `boolean | undefined | null`: Whether or not the user can edit, or `undefined` if the OPTIONS request is still being made. +- `boolean | undefined`: Whether or not the user can edit, or `undefined` if the OPTIONS request is still being made. ### getAuthors diff --git a/packages/core-data/README.md b/packages/core-data/README.md index d6e6d97e9a1d7..90f53a61a2833 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -344,7 +344,7 @@ _Parameters_ _Returns_ -- `boolean | undefined | null`: Whether or not the user can perform the action, or `undefined` if the OPTIONS request is still being made. +- `boolean | undefined`: Whether or not the user can perform the action, or `undefined` if the OPTIONS request is still being made. ### canUserEditEntityRecord @@ -363,7 +363,7 @@ _Parameters_ _Returns_ -- `boolean | undefined | null`: Whether or not the user can edit, or `undefined` if the OPTIONS request is still being made. +- `boolean | undefined`: Whether or not the user can edit, or `undefined` if the OPTIONS request is still being made. ### getAuthors diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index 7c82982553727..010cf347eee53 100644 --- a/packages/core-data/src/selectors.ts +++ b/packages/core-data/src/selectors.ts @@ -1147,10 +1147,10 @@ export function canUser( action: string, resource: string | Record< string, any >, id?: EntityRecordKey -): boolean | undefined | null { +): boolean | undefined { const isEntity = typeof resource === 'object'; if ( isEntity && ( ! resource.kind || ! resource.name ) ) { - return null; + return false; } const key = ( @@ -1184,7 +1184,7 @@ export function canUserEditEntityRecord( kind: string, name: string, recordId: EntityRecordKey -): boolean | undefined | null { +): boolean | undefined { return canUser( state, 'update', { kind, name, id: recordId } ); } diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js index b3edbab9b6926..4b5e8417ad202 100644 --- a/packages/core-data/src/test/selectors.js +++ b/packages/core-data/src/test/selectors.js @@ -698,8 +698,8 @@ describe( 'canUser', () => { const state = deepFreeze( { userPermissions: {}, } ); - expect( canUser( state, 'create', { name: 'media' } ) ).toBe( null ); - expect( canUser( state, 'create', { kind: 'root' } ) ).toBe( null ); + expect( canUser( state, 'create', { name: 'media' } ) ).toBe( false ); + expect( canUser( state, 'create', { kind: 'root' } ) ).toBe( false ); } ); it( 'returns whether an action can be performed', () => { From c6375827a0bda4fa4067775f7b169af70d9ed90e Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 10 Jul 2024 18:33:22 +0400 Subject: [PATCH 08/10] Update types and JSDoc --- docs/reference-guides/data/data-core.md | 2 +- packages/core-data/README.md | 2 +- packages/core-data/src/resolvers.js | 9 +++++---- packages/core-data/src/selectors.ts | 7 +++++-- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/reference-guides/data/data-core.md b/docs/reference-guides/data/data-core.md index 23503ce411eda..0ddd3858c9760 100644 --- a/docs/reference-guides/data/data-core.md +++ b/docs/reference-guides/data/data-core.md @@ -18,7 +18,7 @@ _Parameters_ - _state_ `State`: Data state. - _action_ `string`: Action to check. One of: 'create', 'read', 'update', 'delete'. -- _resource_ `string | Record< string, any >`: REST resource to check, e.g. 'media' or 'posts'. +- _resource_ `string | EntityResource`: Entity resource to check. Accepts entity object `{ kind: 'root', name: 'media', id: 1 }` or REST base as a string - `media`. - _id_ `EntityRecordKey`: Optional ID of the rest resource to check. _Returns_ diff --git a/packages/core-data/README.md b/packages/core-data/README.md index 90f53a61a2833..8c262ebeee8d1 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -339,7 +339,7 @@ _Parameters_ - _state_ `State`: Data state. - _action_ `string`: Action to check. One of: 'create', 'read', 'update', 'delete'. -- _resource_ `string | Record< string, any >`: REST resource to check, e.g. 'media' or 'posts'. +- _resource_ `string | EntityResource`: Entity resource to check. Accepts entity object `{ kind: 'root', name: 'media', id: 1 }` or REST base as a string - `media`. - _id_ `EntityRecordKey`: Optional ID of the rest resource to check. _Returns_ diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index 446ba8466161d..d9b90e593620b 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -352,10 +352,11 @@ export const getEmbedPreview = * Checks whether the current user can perform the given action on the given * REST resource. * - * @param {string} requestedAction Action to check. One of: 'create', 'read', 'update', - * 'delete'. - * @param {string} resource REST resource to check, e.g. 'media' or 'posts'. - * @param {?string} id ID of the rest resource to check. + * @param {string} requestedAction Action to check. One of: 'create', 'read', 'update', + * 'delete'. + * @param {string|Object} resource Entity resource to check. Accepts entity object `{ kind: 'root', name: 'media', id: 1 }` + * or REST base as a string - `media`. + * @param {?string} id ID of the rest resource to check. */ export const canUser = ( requestedAction, resource, id ) => diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index 010cf347eee53..67c70d0310bcd 100644 --- a/packages/core-data/src/selectors.ts +++ b/packages/core-data/src/selectors.ts @@ -120,6 +120,8 @@ type EntityRecordArgs = | [ string, string, EntityRecordKey ] | [ string, string, EntityRecordKey, GetRecordsHttpQuery ]; +type EntityResource = { kind: string; name: string; id?: EntityRecordKey }; + /** * Shared reference to an empty object for cases where it is important to avoid * returning a new object reference on every invocation, as in a connected or @@ -1136,7 +1138,8 @@ export function isPreviewEmbedFallback( state: State, url: string ): boolean { * * @param state Data state. * @param action Action to check. One of: 'create', 'read', 'update', 'delete'. - * @param resource REST resource to check, e.g. 'media' or 'posts'. + * @param resource Entity resource to check. Accepts entity object `{ kind: 'root', name: 'media', id: 1 }` + * or REST base as a string - `media`. * @param id Optional ID of the rest resource to check. * * @return Whether or not the user can perform the action, @@ -1145,7 +1148,7 @@ export function isPreviewEmbedFallback( state: State, url: string ): boolean { export function canUser( state: State, action: string, - resource: string | Record< string, any >, + resource: string | EntityResource, id?: EntityRecordKey ): boolean | undefined { const isEntity = typeof resource === 'object'; From 5ff940732e97b7ebc4feeadb3e0c85ad43c3ec1d Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 10 Jul 2024 18:52:30 +0400 Subject: [PATCH 09/10] Throw an error when an entity resource object is malformed --- packages/core-data/src/resolvers.js | 2 +- packages/core-data/src/test/resolvers.js | 24 +++++++----------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/packages/core-data/src/resolvers.js b/packages/core-data/src/resolvers.js index d9b90e593620b..d3ca23221eadd 100644 --- a/packages/core-data/src/resolvers.js +++ b/packages/core-data/src/resolvers.js @@ -370,7 +370,7 @@ export const canUser = let resourcePath = null; if ( typeof resource === 'object' ) { if ( ! resource.kind || ! resource.name ) { - return; + throw new Error( 'The entity resource object is not valid.' ); } const configs = await dispatch( diff --git a/packages/core-data/src/test/resolvers.js b/packages/core-data/src/test/resolvers.js index c751a67f73d46..95a70b5e5c45f 100644 --- a/packages/core-data/src/test/resolvers.js +++ b/packages/core-data/src/test/resolvers.js @@ -333,23 +333,13 @@ describe( 'canUser', () => { expect( dispatch.receiveUserPermission ).not.toHaveBeenCalled(); } ); - it( 'does nothing when entity kind or name is missing', async () => { - triggerFetch.mockImplementation( () => - Promise.reject( { status: 404 } ) - ); - - await canUser( 'create', { kind: 'root', name: 'media' } )( { - dispatch, - registry, - } ); - await canUser( 'create', { name: 'wp_block' } )( { - dispatch, - registry, - } ); - - expect( triggerFetch ).not.toHaveBeenCalledWith(); - - expect( dispatch.receiveUserPermission ).not.toHaveBeenCalled(); + it( 'throws an error when an entity resource object is malformed', async () => { + await expect( + canUser( 'create', { name: 'wp_block' } )( { + dispatch, + registry, + } ) + ).rejects.toThrow( 'The entity resource object is not valid.' ); } ); it( 'receives false when the user is not allowed to perform an action', async () => { From df1dbd24ca04c01b2cd9610574967039ca595559 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 10 Jul 2024 21:39:20 +0400 Subject: [PATCH 10/10] Deprecate canUserEditEntityRecord --- packages/core-data/src/selectors.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/core-data/src/selectors.ts b/packages/core-data/src/selectors.ts index 67c70d0310bcd..6ff8e26d3684e 100644 --- a/packages/core-data/src/selectors.ts +++ b/packages/core-data/src/selectors.ts @@ -1188,6 +1188,11 @@ export function canUserEditEntityRecord( name: string, recordId: EntityRecordKey ): boolean | undefined { + deprecated( `wp.data.select( 'core' ).canUserEditEntityRecord()`, { + since: '6.7', + alternative: `wp.data.select( 'core' ).canUser( 'update', { kind, name, id } )`, + } ); + return canUser( state, 'update', { kind, name, id: recordId } ); }