From f77d3595d8aed6e8d0756b0f2e95d21e2d38f75f Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 17 Jul 2024 15:03:22 +0400 Subject: [PATCH 1/8] Code Data: Support entity queries in the 'useResourcePermissions' hook --- packages/core-data/README.md | 15 ++-- .../hooks/test/use-resource-permissions.js | 71 +++++++++++++++++++ .../src/hooks/use-resource-permissions.ts | 27 +++++-- 3 files changed, 102 insertions(+), 11 deletions(-) diff --git a/packages/core-data/README.md b/packages/core-data/README.md index 2f2094bed7f4d4..fd1074d757258b 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -1173,7 +1173,10 @@ _Usage_ import { useResourcePermissions } from '@wordpress/core-data'; function PagesList() { - const { canCreate, isResolving } = useResourcePermissions( 'pages' ); + const { canCreate, isResolving } = useResourcePermissions( { + kind: 'postType', + name: 'page', + } ); if ( isResolving ) { return 'Loading ...'; @@ -1196,7 +1199,11 @@ import { useResourcePermissions } from '@wordpress/core-data'; function Page( { pageId } ) { const { canCreate, canUpdate, canDelete, isResolving } = - useResourcePermissions( 'pages', pageId ); + useResourcePermissions( { + kind: 'postType', + name: 'page', + id: pageId, + } ); if ( isResolving ) { return 'Loading ...'; @@ -1222,8 +1229,8 @@ the store state using `canUser()`, or resolved if missing. _Parameters_ -- _resource_ `string`: The resource in question, e.g. media. -- _id_ `IdType`: ID of a specific resource entry, if needed, e.g. 10. +- _resource_ `string | EntityResource`: Entity resource to check. Accepts entity object `{ kind: 'root', name: 'media', id: 1 }` or REST base as a string - `media`. +- _id_ `IdType`: Optional ID of the rest resource to check. e.g. 10. _Returns_ diff --git a/packages/core-data/src/hooks/test/use-resource-permissions.js b/packages/core-data/src/hooks/test/use-resource-permissions.js index bb6c3a85c191f6..1ef35cab3353c9 100644 --- a/packages/core-data/src/hooks/test/use-resource-permissions.js +++ b/packages/core-data/src/hooks/test/use-resource-permissions.js @@ -93,4 +93,75 @@ describe( 'useResourcePermissions', () => { } ) ); } ); + + it( 'retrieves the relevant permissions for a id-less entity', async () => { + let data; + const TestComponent = () => { + data = useResourcePermissions( { + kind: 'root', + name: 'media', + } ); + return
; + }; + render( + + + + ); + expect( data ).toEqual( { + status: 'IDLE', + isResolving: false, + hasResolved: false, + canCreate: false, + canRead: false, + } ); + + await waitFor( () => + expect( data ).toEqual( { + status: 'SUCCESS', + isResolving: false, + hasResolved: true, + canCreate: true, + canRead: false, + } ) + ); + } ); + + it( 'retrieves the relevant permissions for an entity', async () => { + let data; + const TestComponent = () => { + data = useResourcePermissions( { + kind: 'root', + name: 'media', + id: 1, + } ); + return
; + }; + render( + + + + ); + expect( data ).toEqual( { + status: 'IDLE', + isResolving: false, + hasResolved: false, + canCreate: false, + canRead: false, + canUpdate: false, + canDelete: false, + } ); + + await waitFor( () => + expect( data ).toEqual( { + status: 'SUCCESS', + isResolving: false, + hasResolved: true, + canCreate: true, + canRead: false, + canUpdate: false, + canDelete: false, + } ) + ); + } ); } ); diff --git a/packages/core-data/src/hooks/use-resource-permissions.ts b/packages/core-data/src/hooks/use-resource-permissions.ts index 7da7189e2e5067..1f97ed5b45aa5e 100644 --- a/packages/core-data/src/hooks/use-resource-permissions.ts +++ b/packages/core-data/src/hooks/use-resource-permissions.ts @@ -2,6 +2,7 @@ * WordPress dependencies */ import deprecated from '@wordpress/deprecated'; +import { addQueryArgs } from '@wordpress/url'; /** * Internal dependencies @@ -41,20 +42,23 @@ type ResourcePermissionsResolution< IdType > = [ ( IdType extends void ? SpecificResourcePermissionsResolution : {} ), ]; +type EntityResource = { kind: string; name: string; id?: string | number }; + /** * Resolves resource permissions. * * @since 6.1.0 Introduced in WordPress core. * - * @param resource The resource in question, e.g. media. - * @param id ID of a specific resource entry, if needed, e.g. 10. + * @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. e.g. 10. * * @example * ```js * import { useResourcePermissions } from '@wordpress/core-data'; * * function PagesList() { - * const { canCreate, isResolving } = useResourcePermissions( 'pages' ); + * const { canCreate, isResolving } = useResourcePermissions( { kind: 'postType', name: 'page' } ); * * if ( isResolving ) { * return 'Loading ...'; @@ -82,7 +86,7 @@ type ResourcePermissionsResolution< IdType > = [ * canUpdate, * canDelete, * isResolving - * } = useResourcePermissions( 'pages', pageId ); + * } = useResourcePermissions( { kind: 'postType', name: 'page', id: pageId } ); * * if ( isResolving ) { * return 'Loading ...'; @@ -110,14 +114,23 @@ type ResourcePermissionsResolution< IdType > = [ * @template IdType */ export default function useResourcePermissions< IdType = void >( - resource: string, + resource: string | EntityResource, id?: IdType ): ResourcePermissionsResolution< IdType > { + // Serialize `resource` to a string that can be safely used as a React dep. + // We can't just pass `resource` as one of the deps, because if it is passed + // as an object literal, then it will be a different object on each call even + // if the values remain the same. + const resourceAsString = + typeof resource === 'object' ? addQueryArgs( '', resource ) : resource; + return useQuerySelect( ( resolve ) => { const { canUser } = resolve( coreStore ); const create = canUser( 'create', resource ); - if ( ! id ) { + + const hasId = typeof resource === 'object' ? !! resource.id : !! id; + if ( ! hasId ) { const read = canUser( 'read', resource ); const isResolving = create.isResolving || read.isResolving; @@ -168,7 +181,7 @@ export default function useResourcePermissions< IdType = void >( canDelete: hasResolved && _delete.data, }; }, - [ resource, id ] + [ resourceAsString, id ] ); } From cff0d1ab50fe0d8727664f43e6f1c09eb18b38f5 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 17 Jul 2024 15:59:07 +0400 Subject: [PATCH 2/8] Update hook usage --- .../src/navigation-link/link-ui.js | 22 +++++++------------ .../src/navigation/use-navigation-menu.js | 6 ++++- .../index.js | 17 +++++++------- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/packages/block-library/src/navigation-link/link-ui.js b/packages/block-library/src/navigation-link/link-ui.js index 6619c46253546e..deed35145d6dea 100644 --- a/packages/block-library/src/navigation-link/link-ui.js +++ b/packages/block-library/src/navigation-link/link-ui.js @@ -147,15 +147,18 @@ function LinkUIBlockInserter( { clientId, onBack, onSelectBlock } ) { } function UnforwardedLinkUI( props, ref ) { + const { label, url, opensInNewTab, type, kind } = props.link; + const postType = type || 'page'; + const [ addingBlock, setAddingBlock ] = useState( false ); const [ focusAddBlockButton, setFocusAddBlockButton ] = useState( false ); const { saveEntityRecord } = useDispatch( coreStore ); - const pagesPermissions = useResourcePermissions( 'pages' ); - const postsPermissions = useResourcePermissions( 'posts' ); + const permissions = useResourcePermissions( { + kind: 'postType', + name: postType, + } ); async function handleCreate( pageTitle ) { - const postType = props.link.type || 'page'; - const page = await saveEntityRecord( 'postType', postType, { title: pageTitle, status: 'draft', @@ -180,15 +183,6 @@ function UnforwardedLinkUI( props, ref ) { }; } - const { label, url, opensInNewTab, type, kind } = props.link; - - let userCanCreate = false; - if ( ! type || type === 'page' ) { - userCanCreate = pagesPermissions.canCreate; - } else if ( type === 'post' ) { - userCanCreate = postsPermissions.canCreate; - } - // Memoize link value to avoid overriding the LinkControl's internal state. // This is a temporary fix. See https://github.com/WordPress/gutenberg/issues/50976#issuecomment-1568226407. const link = useMemo( @@ -241,7 +235,7 @@ function UnforwardedLinkUI( props, ref ) { hasRichPreviews value={ link } showInitialSuggestions - withCreateSuggestion={ userCanCreate } + withCreateSuggestion={ permissions.canCreate } createSuggestion={ handleCreate } createSuggestionButtonText={ ( searchTerm ) => { let format; diff --git a/packages/block-library/src/navigation/use-navigation-menu.js b/packages/block-library/src/navigation/use-navigation-menu.js index 3834661ed6101b..5fd942c485bf84 100644 --- a/packages/block-library/src/navigation/use-navigation-menu.js +++ b/packages/block-library/src/navigation/use-navigation-menu.js @@ -14,7 +14,11 @@ import { useSelect } from '@wordpress/data'; import { PRELOADED_NAVIGATION_MENUS_QUERY } from './constants'; export default function useNavigationMenu( ref ) { - const permissions = useResourcePermissions( 'navigation', ref ); + const permissions = useResourcePermissions( { + kind: 'postType', + name: 'wp_navigation', + id: ref, + } ); const { navigationMenu, diff --git a/packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js b/packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js index 6d014a589252d9..af844fe9da4fe9 100644 --- a/packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js +++ b/packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js @@ -5,11 +5,7 @@ import { SlotFillProvider } from '@wordpress/components'; import { useViewportMatch } from '@wordpress/compose'; import { uploadMedia } from '@wordpress/media-utils'; import { useDispatch, useSelect } from '@wordpress/data'; -import { - useEntityBlockEditor, - store as coreStore, - useResourcePermissions, -} from '@wordpress/core-data'; +import { useEntityBlockEditor, store as coreStore } from '@wordpress/core-data'; import { useMemo } from '@wordpress/element'; import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; import { privateApis as editPatternsPrivateApis } from '@wordpress/patterns'; @@ -37,9 +33,9 @@ export default function WidgetAreasBlockEditorProvider( { children, ...props } ) { - const mediaPermissions = useResourcePermissions( 'media' ); const isLargeViewport = useViewportMatch( 'medium' ); const { + hasUploadPermissions, reusableBlocks, isFixedToolbarActive, keepCaretInsideBlock, @@ -55,6 +51,11 @@ export default function WidgetAreasBlockEditorProvider( { ? getEntityRecord( 'root', 'site' ) : undefined; return { + hasUploadPermissions: + canUser( 'create', { + kind: 'root', + name: 'media', + } ) ?? true, reusableBlocks: ALLOW_REUSABLE_BLOCKS ? getEntityRecords( 'postType', 'wp_block' ) : EMPTY_ARRAY, @@ -74,7 +75,7 @@ export default function WidgetAreasBlockEditorProvider( { const settings = useMemo( () => { let mediaUploadBlockEditor; - if ( mediaPermissions.canCreate ) { + if ( hasUploadPermissions ) { mediaUploadBlockEditor = ( { onError, ...argumentsObject } ) => { uploadMedia( { wpAllowedMimeTypes: blockEditorSettings.allowedMimeTypes, @@ -95,11 +96,11 @@ export default function WidgetAreasBlockEditorProvider( { pageForPosts, }; }, [ + hasUploadPermissions, blockEditorSettings, isFixedToolbarActive, isLargeViewport, keepCaretInsideBlock, - mediaPermissions.canCreate, reusableBlocks, setIsInserterOpened, pageOnFront, From 69ab28d4c009bf1a5feda9b2bafcac8fc95d8fdc Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 17 Jul 2024 16:44:20 +0400 Subject: [PATCH 3/8] Fix unit tests --- .../navigation/test/use-navigation-menu.js | 69 +++++++++++++++---- .../src/hooks/use-resource-permissions.ts | 10 ++- 2 files changed, 62 insertions(+), 17 deletions(-) diff --git a/packages/block-library/src/navigation/test/use-navigation-menu.js b/packages/block-library/src/navigation/test/use-navigation-menu.js index eb7e90aff22d1a..557fd30ac24a59 100644 --- a/packages/block-library/src/navigation/test/use-navigation-menu.js +++ b/packages/block-library/src/navigation/test/use-navigation-menu.js @@ -63,37 +63,76 @@ function resolveRecords( registry, menus ) { function resolveReadPermission( registry, allowed ) { const dispatch = registry.dispatch( coreStore ); - dispatch.receiveUserPermission( 'create/navigation', allowed ); - dispatch.startResolution( 'canUser', [ 'read', 'navigation' ] ); - dispatch.finishResolution( 'canUser', [ 'read', 'navigation' ] ); + dispatch.receiveUserPermission( 'read/postType/wp_navigation', allowed ); + dispatch.startResolution( 'canUser', [ + 'read', + { kind: 'postType', name: 'wp_navigation', id: undefined }, + ] ); + dispatch.finishResolution( 'canUser', [ + 'read', + { kind: 'postType', name: 'wp_navigation', id: undefined }, + ] ); } function resolveReadRecordPermission( registry, ref, allowed ) { const dispatch = registry.dispatch( coreStore ); - dispatch.receiveUserPermission( 'create/navigation', allowed ); - dispatch.startResolution( 'canUser', [ 'read', 'navigation', ref ] ); - dispatch.finishResolution( 'canUser', [ 'read', 'navigation', ref ] ); + dispatch.receiveUserPermission( + `read/postType/wp_navigation/${ ref }`, + allowed + ); + dispatch.startResolution( 'canUser', [ + 'read', + { kind: 'postType', name: 'wp_navigation', id: ref }, + ] ); + dispatch.finishResolution( 'canUser', [ + 'read', + { kind: 'postType', name: 'wp_navigation', id: ref }, + ] ); } function resolveCreatePermission( registry, allowed ) { const dispatch = registry.dispatch( coreStore ); - dispatch.receiveUserPermission( 'create/navigation', allowed ); - dispatch.startResolution( 'canUser', [ 'create', 'navigation' ] ); - dispatch.finishResolution( 'canUser', [ 'create', 'navigation' ] ); + dispatch.receiveUserPermission( 'create/postType/wp_navigation', allowed ); + dispatch.startResolution( 'canUser', [ + 'create', + { kind: 'postType', name: 'wp_navigation' }, + ] ); + dispatch.finishResolution( 'canUser', [ + 'create', + { kind: 'postType', name: 'wp_navigation' }, + ] ); } function resolveUpdatePermission( registry, ref, allowed ) { const dispatch = registry.dispatch( coreStore ); - dispatch.receiveUserPermission( `update/navigation/${ ref }`, allowed ); - dispatch.startResolution( 'canUser', [ 'update', 'navigation', ref ] ); - dispatch.finishResolution( 'canUser', [ 'update', 'navigation', ref ] ); + dispatch.receiveUserPermission( + `update/postType/wp_navigation/${ ref }`, + allowed + ); + dispatch.startResolution( 'canUser', [ + 'update', + { kind: 'postType', name: 'wp_navigation', id: ref }, + ] ); + dispatch.finishResolution( 'canUser', [ + 'update', + { kind: 'postType', name: 'wp_navigation', id: ref }, + ] ); } function resolveDeletePermission( registry, ref, allowed ) { const dispatch = registry.dispatch( coreStore ); - dispatch.receiveUserPermission( `delete/navigation/${ ref }`, allowed ); - dispatch.startResolution( 'canUser', [ 'delete', 'navigation', ref ] ); - dispatch.finishResolution( 'canUser', [ 'delete', 'navigation', ref ] ); + dispatch.receiveUserPermission( + `delete/postType/wp_navigation/${ ref }`, + allowed + ); + dispatch.startResolution( 'canUser', [ + 'delete', + { kind: 'postType', name: 'wp_navigation', id: ref }, + ] ); + dispatch.finishResolution( 'canUser', [ + 'delete', + { kind: 'postType', name: 'wp_navigation', id: ref }, + ] ); } describe( 'useNavigationMenus', () => { diff --git a/packages/core-data/src/hooks/use-resource-permissions.ts b/packages/core-data/src/hooks/use-resource-permissions.ts index 1f97ed5b45aa5e..2e93583eee8f85 100644 --- a/packages/core-data/src/hooks/use-resource-permissions.ts +++ b/packages/core-data/src/hooks/use-resource-permissions.ts @@ -126,10 +126,16 @@ export default function useResourcePermissions< IdType = void >( return useQuerySelect( ( resolve ) => { + const isEntity = typeof resource === 'object'; + const hasId = isEntity ? !! resource.id : !! id; const { canUser } = resolve( coreStore ); - const create = canUser( 'create', resource ); + const create = canUser( + 'create', + isEntity + ? { kind: resource.kind, name: resource.name } + : resource + ); - const hasId = typeof resource === 'object' ? !! resource.id : !! id; if ( ! hasId ) { const read = canUser( 'read', resource ); From d04d54f519b2fcae5687c25cbf75b32cf144429b Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Fri, 19 Jul 2024 11:04:11 +0400 Subject: [PATCH 4/8] Update cache key generation --- packages/core-data/src/hooks/use-resource-permissions.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core-data/src/hooks/use-resource-permissions.ts b/packages/core-data/src/hooks/use-resource-permissions.ts index 2e93583eee8f85..ce610397b5b44b 100644 --- a/packages/core-data/src/hooks/use-resource-permissions.ts +++ b/packages/core-data/src/hooks/use-resource-permissions.ts @@ -2,7 +2,6 @@ * WordPress dependencies */ import deprecated from '@wordpress/deprecated'; -import { addQueryArgs } from '@wordpress/url'; /** * Internal dependencies @@ -122,7 +121,7 @@ export default function useResourcePermissions< IdType = void >( // as an object literal, then it will be a different object on each call even // if the values remain the same. const resourceAsString = - typeof resource === 'object' ? addQueryArgs( '', resource ) : resource; + typeof resource === 'object' ? JSON.stringify( resource ) : resource; return useQuerySelect( ( resolve ) => { From 9594d4f63b61a53baffd3fdf030b90338b19e966 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Fri, 19 Jul 2024 11:08:56 +0400 Subject: [PATCH 5/8] Extract base entity in const --- .../navigation/test/use-navigation-menu.js | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/block-library/src/navigation/test/use-navigation-menu.js b/packages/block-library/src/navigation/test/use-navigation-menu.js index 557fd30ac24a59..7eaf648fab936e 100644 --- a/packages/block-library/src/navigation/test/use-navigation-menu.js +++ b/packages/block-library/src/navigation/test/use-navigation-menu.js @@ -9,6 +9,12 @@ import { store as coreStore } from '@wordpress/core-data'; */ import useNavigationMenu from '../use-navigation-menu'; +const BASE_ENTITY = { + kind: 'postType', + name: 'wp_navigation', + id: undefined, +}; + function createRegistryWithStores() { // Create a registry and register used stores. const registry = createRegistry(); @@ -64,14 +70,8 @@ function resolveRecords( registry, menus ) { function resolveReadPermission( registry, allowed ) { const dispatch = registry.dispatch( coreStore ); dispatch.receiveUserPermission( 'read/postType/wp_navigation', allowed ); - dispatch.startResolution( 'canUser', [ - 'read', - { kind: 'postType', name: 'wp_navigation', id: undefined }, - ] ); - dispatch.finishResolution( 'canUser', [ - 'read', - { kind: 'postType', name: 'wp_navigation', id: undefined }, - ] ); + dispatch.startResolution( 'canUser', [ 'read', BASE_ENTITY ] ); + dispatch.finishResolution( 'canUser', [ 'read', BASE_ENTITY ] ); } function resolveReadRecordPermission( registry, ref, allowed ) { @@ -82,11 +82,11 @@ function resolveReadRecordPermission( registry, ref, allowed ) { ); dispatch.startResolution( 'canUser', [ 'read', - { kind: 'postType', name: 'wp_navigation', id: ref }, + { ...BASE_ENTITY, id: ref }, ] ); dispatch.finishResolution( 'canUser', [ 'read', - { kind: 'postType', name: 'wp_navigation', id: ref }, + { ...BASE_ENTITY, id: ref }, ] ); } @@ -111,11 +111,11 @@ function resolveUpdatePermission( registry, ref, allowed ) { ); dispatch.startResolution( 'canUser', [ 'update', - { kind: 'postType', name: 'wp_navigation', id: ref }, + { ...BASE_ENTITY, id: ref }, ] ); dispatch.finishResolution( 'canUser', [ 'update', - { kind: 'postType', name: 'wp_navigation', id: ref }, + { ...BASE_ENTITY, id: ref }, ] ); } @@ -127,11 +127,11 @@ function resolveDeletePermission( registry, ref, allowed ) { ); dispatch.startResolution( 'canUser', [ 'delete', - { kind: 'postType', name: 'wp_navigation', id: ref }, + { ...BASE_ENTITY, id: ref }, ] ); dispatch.finishResolution( 'canUser', [ 'delete', - { kind: 'postType', name: 'wp_navigation', id: ref }, + { ...BASE_ENTITY, id: ref }, ] ); } From 92339b8a7081aee1ce9dd28e4965361c1f552835 Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Tue, 23 Jul 2024 07:32:45 +0400 Subject: [PATCH 6/8] Add function overloads --- packages/core-data/README.md | 2 +- .../src/hooks/use-resource-permissions.ts | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/core-data/README.md b/packages/core-data/README.md index fd1074d757258b..471d91f0125669 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -1230,7 +1230,7 @@ the store state using `canUser()`, or resolved if missing. _Parameters_ - _resource_ `string | EntityResource`: Entity resource to check. Accepts entity object `{ kind: 'root', name: 'media', id: 1 }` or REST base as a string - `media`. -- _id_ `IdType`: Optional ID of the rest resource to check. e.g. 10. +- _id_ `IdType`: Optional ID of the resource to check, e.g. 10. Note: The argument will be ignored when using an entity object as a resource to check permissions. _Returns_ diff --git a/packages/core-data/src/hooks/use-resource-permissions.ts b/packages/core-data/src/hooks/use-resource-permissions.ts index ce610397b5b44b..5495d7263c15bc 100644 --- a/packages/core-data/src/hooks/use-resource-permissions.ts +++ b/packages/core-data/src/hooks/use-resource-permissions.ts @@ -43,6 +43,16 @@ type ResourcePermissionsResolution< IdType > = [ type EntityResource = { kind: string; name: string; id?: string | number }; +function useResourcePermissions< IdType = void >( + resource: string, + id?: IdType +): ResourcePermissionsResolution< IdType >; + +function useResourcePermissions< IdType = void >( + resource: EntityResource, + id?: never +): ResourcePermissionsResolution< IdType >; + /** * Resolves resource permissions. * @@ -50,7 +60,8 @@ type EntityResource = { kind: string; name: string; id?: string | number }; * * @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. e.g. 10. + * @param id Optional ID of the resource to check, e.g. 10. Note: The argument will be ignored + * when using an entity object as a resource to check permissions. * * @example * ```js @@ -112,7 +123,7 @@ type EntityResource = { kind: string; name: string; id?: string | number }; * @return Entity records data. * @template IdType */ -export default function useResourcePermissions< IdType = void >( +function useResourcePermissions< IdType = void >( resource: string | EntityResource, id?: IdType ): ResourcePermissionsResolution< IdType > { @@ -190,6 +201,8 @@ export default function useResourcePermissions< IdType = void >( ); } +export default useResourcePermissions; + export function __experimentalUseResourcePermissions( resource: string, id?: unknown From bfdd334130fcae878754bfc34c1087dbe899803e Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Tue, 23 Jul 2024 13:17:51 +0400 Subject: [PATCH 7/8] Warn consumers about incorrect usage --- package-lock.json | 2 ++ packages/core-data/package.json | 1 + .../hooks/test/use-resource-permissions.js | 22 +++++++++++++++++++ .../src/hooks/use-resource-permissions.ts | 9 +++++++- packages/core-data/tsconfig.json | 3 ++- 5 files changed, 35 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0318a97778e11e..44d5e1a7210ef2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -52817,6 +52817,7 @@ "@wordpress/sync": "file:../sync", "@wordpress/undo-manager": "file:../undo-manager", "@wordpress/url": "file:../url", + "@wordpress/warning": "file:../warning", "change-case": "^4.1.2", "equivalent-key-map": "^0.2.2", "fast-deep-equal": "^3.1.3", @@ -67661,6 +67662,7 @@ "@wordpress/sync": "file:../sync", "@wordpress/undo-manager": "file:../undo-manager", "@wordpress/url": "file:../url", + "@wordpress/warning": "file:../warning", "change-case": "^4.1.2", "equivalent-key-map": "^0.2.2", "fast-deep-equal": "^3.1.3", diff --git a/packages/core-data/package.json b/packages/core-data/package.json index d94eb375287250..bed2ca97791bc5 100644 --- a/packages/core-data/package.json +++ b/packages/core-data/package.json @@ -47,6 +47,7 @@ "@wordpress/sync": "file:../sync", "@wordpress/undo-manager": "file:../undo-manager", "@wordpress/url": "file:../url", + "@wordpress/warning": "file:../warning", "change-case": "^4.1.2", "equivalent-key-map": "^0.2.2", "fast-deep-equal": "^3.1.3", diff --git a/packages/core-data/src/hooks/test/use-resource-permissions.js b/packages/core-data/src/hooks/test/use-resource-permissions.js index 1ef35cab3353c9..b1c43b7947874d 100644 --- a/packages/core-data/src/hooks/test/use-resource-permissions.js +++ b/packages/core-data/src/hooks/test/use-resource-permissions.js @@ -164,4 +164,26 @@ describe( 'useResourcePermissions', () => { } ) ); } ); + + it( 'should warn when called with incorrect arguments signature', () => { + const TestComponent = () => { + useResourcePermissions( + { + kind: 'root', + name: 'media', + }, + 1 + ); + return null; + }; + render( + + + + ); + + expect( console ).toHaveWarnedWith( + `When 'resource' is an entity object, passing 'id' as a separate argument isn't supported.` + ); + } ); } ); diff --git a/packages/core-data/src/hooks/use-resource-permissions.ts b/packages/core-data/src/hooks/use-resource-permissions.ts index 5495d7263c15bc..332e4da1fb615d 100644 --- a/packages/core-data/src/hooks/use-resource-permissions.ts +++ b/packages/core-data/src/hooks/use-resource-permissions.ts @@ -2,6 +2,7 @@ * WordPress dependencies */ import deprecated from '@wordpress/deprecated'; +import warning from '@wordpress/warning'; /** * Internal dependencies @@ -134,9 +135,15 @@ function useResourcePermissions< IdType = void >( const resourceAsString = typeof resource === 'object' ? JSON.stringify( resource ) : resource; + const isEntity = typeof resource === 'object'; + if ( isEntity && typeof id !== 'undefined' ) { + warning( + `When 'resource' is an entity object, passing 'id' as a separate argument isn't supported.` + ); + } + return useQuerySelect( ( resolve ) => { - const isEntity = typeof resource === 'object'; const hasId = isEntity ? !! resource.id : !! id; const { canUser } = resolve( coreStore ); const create = canUser( diff --git a/packages/core-data/tsconfig.json b/packages/core-data/tsconfig.json index 5db0de19f111e8..26602d82ab0c01 100644 --- a/packages/core-data/tsconfig.json +++ b/packages/core-data/tsconfig.json @@ -21,7 +21,8 @@ { "path": "../rich-text" }, { "path": "../sync" }, { "path": "../undo-manager" }, - { "path": "../url" } + { "path": "../url" }, + { "path": "../warning" } ], "include": [ "src/**/*" ] } From 56f1d2b6a72f9ac85b08300e99868b5a3fed741c Mon Sep 17 00:00:00 2001 From: George Mamadashvili Date: Wed, 24 Jul 2024 14:31:23 +0400 Subject: [PATCH 8/8] Latest feedback --- packages/core-data/README.md | 2 +- packages/core-data/src/hooks/use-resource-permissions.ts | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/packages/core-data/README.md b/packages/core-data/README.md index 471d91f0125669..de28e4b86be4bf 100644 --- a/packages/core-data/README.md +++ b/packages/core-data/README.md @@ -1230,7 +1230,7 @@ the store state using `canUser()`, or resolved if missing. _Parameters_ - _resource_ `string | EntityResource`: Entity resource to check. Accepts entity object `{ kind: 'root', name: 'media', id: 1 }` or REST base as a string - `media`. -- _id_ `IdType`: Optional ID of the resource to check, e.g. 10. Note: The argument will be ignored when using an entity object as a resource to check permissions. +- _id_ `IdType`: Optional ID of the resource to check, e.g. 10. Note: This argument is discouraged when using an entity object as a resource to check permissions and will be ignored. _Returns_ diff --git a/packages/core-data/src/hooks/use-resource-permissions.ts b/packages/core-data/src/hooks/use-resource-permissions.ts index 332e4da1fb615d..38dfc1ed77a3e6 100644 --- a/packages/core-data/src/hooks/use-resource-permissions.ts +++ b/packages/core-data/src/hooks/use-resource-permissions.ts @@ -61,8 +61,8 @@ function useResourcePermissions< IdType = void >( * * @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 resource to check, e.g. 10. Note: The argument will be ignored - * when using an entity object as a resource to check permissions. + * @param id Optional ID of the resource to check, e.g. 10. Note: This argument is discouraged + * when using an entity object as a resource to check permissions and will be ignored. * * @example * ```js @@ -132,10 +132,9 @@ function useResourcePermissions< IdType = void >( // We can't just pass `resource` as one of the deps, because if it is passed // as an object literal, then it will be a different object on each call even // if the values remain the same. - const resourceAsString = - typeof resource === 'object' ? JSON.stringify( resource ) : resource; - const isEntity = typeof resource === 'object'; + const resourceAsString = isEntity ? JSON.stringify( resource ) : resource; + if ( isEntity && typeof id !== 'undefined' ) { warning( `When 'resource' is an entity object, passing 'id' as a separate argument isn't supported.`