From 6a15fd5655458fdb196e7cbccde1a8979782472e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Wed, 18 May 2022 14:41:34 +0200 Subject: [PATCH 1/7] Squashed commit of the following: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit c86f5616a89cb1998ead4ea1f34694795e019267 Author: Adam Zieliński Date: Wed Feb 16 12:17:33 2022 +0100 Move the return type to TS commit eefec13ba88c7aa63e950b71dc242de4b9a46b16 Author: Adam Zieliński Date: Wed Feb 16 12:16:36 2022 +0100 Adjust TS types commit 12c74338a1b63d8b796b50e11160bf89fc2f5c49 Author: Adam Zieliński Date: Wed Feb 16 12:13:22 2022 +0100 Make useResourcePermissions return a tuple and make hasResolved the first item of the tuple forces the users to think about that variable and it's not easy to accidentally miss it. commit caa48a72800e6b47934a8cef5356b32bb8361788 Author: Adam Zieliński Date: Tue Feb 15 17:03:31 2022 +0100 Fix typo in the tests commit 33cdb4e59fd3a81c743430fa6016903373aff666 Author: Adam Zieliński Date: Mon Feb 14 15:05:09 2022 +0100 Expose __experimentalUseResourcePermissions as a public API commit cf30c5aa85dae7d7a488573b5e6459abc05086d5 Author: Adam Zieliński Date: Mon Feb 14 14:58:42 2022 +0100 Distinguish between global and local resoluion commit 0db7bd25a7106400188addc23892d77bd33b63ed Author: Adam Zieliński Date: Mon Feb 14 14:42:36 2022 +0100 Propose useResourcePermissions hook commit 2d5e270d67781783c02d1d8ff1fa0d3a671e3aff Author: Adam Zieliński Date: Mon Feb 14 14:48:09 2022 +0100 Move the status computation inside the enriched selectors commit e7ac34e5e9fdb3051d9d093fad5d4ab73833ba0a Author: Adam Zieliński Date: Mon Feb 14 14:01:11 2022 +0100 Propose useEntityRecords --- .../hooks/test/use-resource-permissions.js | 117 +++++++++++++++++ .../src/hooks/use-resource-permissions.ts | 120 ++++++++++++++++++ packages/core-data/src/index.js | 3 + 3 files changed, 240 insertions(+) create mode 100644 packages/core-data/src/hooks/test/use-resource-permissions.js create mode 100644 packages/core-data/src/hooks/use-resource-permissions.ts diff --git a/packages/core-data/src/hooks/test/use-resource-permissions.js b/packages/core-data/src/hooks/test/use-resource-permissions.js new file mode 100644 index 0000000000000..d1d41db3ddf36 --- /dev/null +++ b/packages/core-data/src/hooks/test/use-resource-permissions.js @@ -0,0 +1,117 @@ +/** + * WordPress dependencies + */ +import triggerFetch from '@wordpress/api-fetch'; +import { createRegistry, RegistryProvider } from '@wordpress/data'; + +jest.mock( '@wordpress/api-fetch' ); + +/** + * External dependencies + */ +import { act, render } from '@testing-library/react'; + +/** + * Internal dependencies + */ +import { store as coreDataStore } from '../../index'; +import useResourcePermissions from '../use-resource-permissions'; + +describe( 'useResourcePermissions', () => { + let registry; + beforeEach( () => { + jest.useFakeTimers(); + + registry = createRegistry(); + registry.register( coreDataStore ); + } ); + + afterEach( () => { + jest.runOnlyPendingTimers(); + jest.useRealTimers(); + } ); + + it( 'retrieves the relevant permissions for a key-less resource', async () => { + triggerFetch.mockImplementation( () => ( { + headers: { + Allow: 'POST', + }, + } ) ); + let data; + const TestComponent = () => { + data = useResourcePermissions( 'widgets' ); + return
; + }; + render( + + + + ); + expect( data ).toEqual( [ + false, + { + status: 'IDLE', + isResolving: false, + canCreate: false, + }, + ] ); + + // Required to make sure no updates happen outside of act() + await act( async () => { + jest.advanceTimersByTime( 1 ); + } ); + + expect( data ).toEqual( [ + true, + { + status: 'SUCCESS', + isResolving: false, + canCreate: true, + }, + ] ); + } ); + + it( 'retrieves the relevant permissions for a resource with a key', async () => { + triggerFetch.mockImplementation( () => ( { + headers: { + Allow: 'POST', + }, + } ) ); + let data; + const TestComponent = () => { + data = useResourcePermissions( 'widgets', 1 ); + return
; + }; + render( + + + + ); + expect( data ).toEqual( [ + false, + { + status: 'IDLE', + isResolving: false, + canCreate: false, + canUpdate: false, + canDelete: false, + }, + ] ); + + // Required to make sure no updates happen outside of act() + await act( async () => { + jest.advanceTimersByTime( 1 ); + } ); + + expect( data ).toEqual( [ + true, + { + status: 'SUCCESS', + isResolving: false, + canCreate: true, + 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 new file mode 100644 index 0000000000000..122eadc575980 --- /dev/null +++ b/packages/core-data/src/hooks/use-resource-permissions.ts @@ -0,0 +1,120 @@ +/** + * Internal dependencies + */ +import { store as coreStore } from '../'; +import { Status } from './constants'; +import useQuerySelect from './use-query-select'; + +interface GlobalResourcePermissionsResolution { + /** Can the current user create new resources of this type? */ + canCreate: boolean; +} +interface SpecificResourcePermissionsResolution { + /** Can the current user update resources of this type? */ + canUpdate: boolean; + /** Can the current user delete resources of this type? */ + canDelete: boolean; +} +interface ResolutionDetails { + /** Resolution status */ + status: Status; + /** + * Is the data still being resolved? + */ + isResolving: boolean; +} + +/** + * Is the data resolved by now? + */ +type HasResolved = boolean; + +type ResourcePermissionsResolution< IdType > = [ + HasResolved, + ResolutionDetails & + GlobalResourcePermissionsResolution & + ( IdType extends void ? SpecificResourcePermissionsResolution : {} ) +]; + +/** + * Resolves resource permissions. + * + * @param resource The resource in question, e.g. media. + * @param id ID of a specific resource entry, if needed, e.g. 10. + * + * @example + * ```js + * import { useResourcePermissions } from '@wordpress/core-data'; + * + * function PagesList() { + * const { canCreate, isResolving } = useResourcePermissions( 'pages' ); + * + * if ( isResolving ) { + * return 'Loading ...'; + * } + * + * return ( + *
+ * {canCreate ? () : false} + * // ... + *
+ * ); + * } + * + * // Rendered in the application: + * // + * ``` + * + * In the above example, when `PagesList` is rendered into an + * application, the appropriate permissions and the resolution details will be retrieved from + * the store state using `canUser()`, or resolved if missing. + * + * @return Entity records data. + * @template IdType + */ +export default function __experimentalUseResourcePermissions< IdType = void >( + resource: string, + id: IdType +): ResourcePermissionsResolution< IdType > { + return useQuerySelect( + ( resolve ) => { + const { canUser } = resolve( coreStore ); + const create = canUser( 'create', resource ); + if ( ! id ) { + return [ + create.hasResolved, + { + status: create.status, + isResolving: create.isResolving, + canCreate: create.hasResolved && create.data, + }, + ]; + } + + const update = canUser( 'update', resource, id ); + const _delete = canUser( 'delete', resource, id ); + const isResolving = + create.isResolving || update.isResolving || _delete.isResolving; + const hasResolved = + create.hasResolved && update.hasResolved && _delete.hasResolved; + + let status = Status.Idle; + if ( isResolving ) { + status = Status.Resolving; + } else if ( hasResolved ) { + status = Status.Success; + } + return [ + hasResolved, + { + status, + isResolving, + canCreate: hasResolved && create.data, + canUpdate: hasResolved && update.data, + canDelete: hasResolved && _delete.data, + }, + ]; + }, + [ resource, id ] + ); +} diff --git a/packages/core-data/src/index.js b/packages/core-data/src/index.js index 05cace07c992b..6914f376d72e4 100644 --- a/packages/core-data/src/index.js +++ b/packages/core-data/src/index.js @@ -68,6 +68,9 @@ export const store = createReduxStore( STORE_NAME, storeConfig() ); register( store ); export { default as EntityProvider } from './entity-provider'; +export { default as useEntityRecord } from './hooks/use-entity-record'; +export { default as useEntityRecords } from './hooks/use-entity-records'; +export { default as useResourcePermissions } from './hooks/use-resource-permissions'; export * from './entity-provider'; export * from './entity-types'; export * from './fetch'; From 9dcc6e11e1728af1d8d1e0785c5e8ec1d01be533 Mon Sep 17 00:00:00 2001 From: Adam Zielinski Date: Tue, 24 May 2022 14:30:15 +0200 Subject: [PATCH 2/7] Update packages/core-data/src/hooks/use-resource-permissions.ts Co-authored-by: Kai Hao --- packages/core-data/src/hooks/use-resource-permissions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-data/src/hooks/use-resource-permissions.ts b/packages/core-data/src/hooks/use-resource-permissions.ts index 122eadc575980..d9b1c7a35a500 100644 --- a/packages/core-data/src/hooks/use-resource-permissions.ts +++ b/packages/core-data/src/hooks/use-resource-permissions.ts @@ -74,7 +74,7 @@ type ResourcePermissionsResolution< IdType > = [ */ export default function __experimentalUseResourcePermissions< IdType = void >( resource: string, - id: IdType + id?: IdType ): ResourcePermissionsResolution< IdType > { return useQuerySelect( ( resolve ) => { From 6e07a1b37c74af2151c9c6a16002eefacaa3d8ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Wed, 15 Jun 2022 14:52:50 +0200 Subject: [PATCH 3/7] Migrate useNavigationMenu to useResourcePermissions --- .../src/navigation/use-navigation-menu.js | 105 ++++-------------- packages/core-data/src/index.js | 2 +- 2 files changed, 23 insertions(+), 84 deletions(-) diff --git a/packages/block-library/src/navigation/use-navigation-menu.js b/packages/block-library/src/navigation/use-navigation-menu.js index cf9498f6d200d..4dcc5c37ccb32 100644 --- a/packages/block-library/src/navigation/use-navigation-menu.js +++ b/packages/block-library/src/navigation/use-navigation-menu.js @@ -1,12 +1,22 @@ /** * WordPress dependencies */ -import { store as coreStore } from '@wordpress/core-data'; +import { + store as coreStore, + __experimentalUseResourcePermissions as useResourcePermissions, +} from '@wordpress/core-data'; import { useSelect } from '@wordpress/data'; export default function useNavigationMenu( ref ) { + const permissions = useResourcePermissions( 'navigation', ref ); + return useSelect( ( select ) => { + const [ + hasResolvedPermissions, + { canCreate, canUpdate, canDelete, isResolving }, + ] = permissions; + const { navigationMenus, isResolvingNavigationMenus, @@ -19,22 +29,6 @@ export default function useNavigationMenu( ref ) { isNavigationMenuMissing, } = selectExistingMenu( select, ref ); - const { - canUserCreateNavigationMenu, - isResolvingCanUserCreateNavigationMenu, - hasResolvedCanUserCreateNavigationMenu, - } = selectMenuCreatePermissions( select ); - - const { - canUserUpdateNavigationMenu, - hasResolvedCanUserUpdateNavigationMenu, - } = selectMenuUpdatePermissions( select, ref ); - - const { - canUserDeleteNavigationMenu, - hasResolvedCanUserDeleteNavigationMenu, - } = selectMenuDeletePermissions( select, ref ); - return { navigationMenus, isResolvingNavigationMenus, @@ -44,22 +38,22 @@ export default function useNavigationMenu( ref ) { isNavigationMenuResolved, isNavigationMenuMissing, - canUserCreateNavigationMenu, - isResolvingCanUserCreateNavigationMenu, - hasResolvedCanUserCreateNavigationMenu, - - canUserUpdateNavigationMenu, - hasResolvedCanUserUpdateNavigationMenu, - - canUserDeleteNavigationMenu, - hasResolvedCanUserDeleteNavigationMenu, - canSwitchNavigationMenu: ref ? navigationMenus?.length > 1 : navigationMenus?.length > 0, + + canUserCreateNavigationMenu: canCreate, + isResolvingCanUserCreateNavigationMenu: isResolving, + hasResolvedCanUserCreateNavigationMenu: hasResolvedPermissions, + + canUserUpdateNavigationMenu: canUpdate, + hasResolvedCanUserUpdateNavigationMenu: hasResolvedPermissions, + + canUserDeleteNavigationMenu: canDelete, + hasResolvedCanUserDeleteNavigationMenu: hasResolvedPermissions, }; }, - [ ref ] + [ ref, permissions ] ); } @@ -113,58 +107,3 @@ function selectExistingMenu( select, ref ) { : null, }; } - -function selectMenuCreatePermissions( select ) { - const { hasFinishedResolution, isResolving, canUser } = select( coreStore ); - - const args = [ 'create', 'navigation' ]; - return { - canUserCreateNavigationMenu: !! canUser( ...args ), - isResolvingCanUserCreateNavigationMenu: !! isResolving( - 'canUser', - args - ), - hasResolvedCanUserCreateNavigationMenu: !! hasFinishedResolution( - 'canUser', - args - ), - }; -} - -function selectMenuUpdatePermissions( select, ref ) { - if ( ! ref ) { - return { - canUserUpdateNavigationMenu: false, - hasResolvedCanUserUpdateNavigationMenu: false, - }; - } - - const { hasFinishedResolution, canUser } = select( coreStore ); - const args = [ 'update', 'navigation', ref ]; - return { - canUserUpdateNavigationMenu: !! canUser( ...args ), - hasResolvedCanUserUpdateNavigationMenu: !! hasFinishedResolution( - 'canUser', - args - ), - }; -} - -function selectMenuDeletePermissions( select, ref ) { - if ( ! ref ) { - return { - canUserDeleteNavigationMenu: false, - hasResolvedCanUserDeleteNavigationMenu: false, - }; - } - - const { hasFinishedResolution, canUser } = select( coreStore ); - const args = [ 'delete', 'navigation', ref ]; - return { - canUserDeleteNavigationMenu: !! canUser( ...args ), - hasResolvedCanUserDeleteNavigationMenu: !! hasFinishedResolution( - 'canUser', - args - ), - }; -} diff --git a/packages/core-data/src/index.js b/packages/core-data/src/index.js index 6914f376d72e4..c73deb6b4d532 100644 --- a/packages/core-data/src/index.js +++ b/packages/core-data/src/index.js @@ -70,7 +70,7 @@ register( store ); export { default as EntityProvider } from './entity-provider'; export { default as useEntityRecord } from './hooks/use-entity-record'; export { default as useEntityRecords } from './hooks/use-entity-records'; -export { default as useResourcePermissions } from './hooks/use-resource-permissions'; +export { default as __experimentalUseResourcePermissions } from './hooks/use-resource-permissions'; export * from './entity-provider'; export * from './entity-types'; export * from './fetch'; From 54afeee587d776be865afa3365a9dd7d8cc4f5e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Wed, 15 Jun 2022 15:05:07 +0200 Subject: [PATCH 4/7] Fix unit tests --- .../src/hooks/test/use-resource-permissions.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) 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 d1d41db3ddf36..f62ea0df738dd 100644 --- a/packages/core-data/src/hooks/test/use-resource-permissions.js +++ b/packages/core-data/src/hooks/test/use-resource-permissions.js @@ -24,6 +24,14 @@ describe( 'useResourcePermissions', () => { registry = createRegistry(); registry.register( coreDataStore ); + + triggerFetch.mockImplementation( () => ( { + headers: { + get: () => ( { + allow: 'POST', + } ), + }, + } ) ); } ); afterEach( () => { @@ -32,11 +40,6 @@ describe( 'useResourcePermissions', () => { } ); it( 'retrieves the relevant permissions for a key-less resource', async () => { - triggerFetch.mockImplementation( () => ( { - headers: { - Allow: 'POST', - }, - } ) ); let data; const TestComponent = () => { data = useResourcePermissions( 'widgets' ); @@ -72,11 +75,6 @@ describe( 'useResourcePermissions', () => { } ); it( 'retrieves the relevant permissions for a resource with a key', async () => { - triggerFetch.mockImplementation( () => ( { - headers: { - Allow: 'POST', - }, - } ) ); let data; const TestComponent = () => { data = useResourcePermissions( 'widgets', 1 ); From c1da2e31d8d59dabdd33180644acaf0e294b5571 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Thu, 16 Jun 2022 11:47:21 +0200 Subject: [PATCH 5/7] Fix unit tests --- .../navigation/test/use-navigation-menu.js | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 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 2c42bd2ad3ffc..05225fb562e93 100644 --- a/packages/block-library/src/navigation/test/use-navigation-menu.js +++ b/packages/block-library/src/navigation/test/use-navigation-menu.js @@ -89,10 +89,11 @@ describe( 'useNavigationMenus', () => { it( 'Should return no information when no data is resolved', () => { expect( useNavigationMenu() ).toEqual( { navigationMenus: null, + navigationMenu: undefined, canSwitchNavigationMenu: false, canUserCreateNavigationMenu: false, - canUserDeleteNavigationMenu: false, - canUserUpdateNavigationMenu: false, + canUserDeleteNavigationMenu: undefined, + canUserUpdateNavigationMenu: undefined, hasResolvedCanUserCreateNavigationMenu: false, hasResolvedCanUserDeleteNavigationMenu: false, hasResolvedCanUserUpdateNavigationMenu: false, @@ -109,13 +110,14 @@ describe( 'useNavigationMenus', () => { resolveCreatePermission( registry, true ); expect( useNavigationMenu() ).toEqual( { navigationMenus, + navigationMenu: undefined, canSwitchNavigationMenu: true, canUserCreateNavigationMenu: true, - canUserDeleteNavigationMenu: false, - canUserUpdateNavigationMenu: false, + canUserDeleteNavigationMenu: undefined, + canUserUpdateNavigationMenu: undefined, hasResolvedCanUserCreateNavigationMenu: true, - hasResolvedCanUserDeleteNavigationMenu: false, - hasResolvedCanUserUpdateNavigationMenu: false, + hasResolvedCanUserDeleteNavigationMenu: true, + hasResolvedCanUserUpdateNavigationMenu: true, hasResolvedNavigationMenus: true, isNavigationMenuMissing: true, isNavigationMenuResolved: false, @@ -170,6 +172,7 @@ describe( 'useNavigationMenus', () => { resolveRecords( registry, navigationMenus ); resolveCreatePermission( registry, true ); resolveUpdatePermission( registry, 1, true ); + resolveDeletePermission( registry, 1, false ); expect( useNavigationMenu( 1 ) ).toEqual( { navigationMenu: navigationMenu1, navigationMenus, @@ -178,7 +181,7 @@ describe( 'useNavigationMenus', () => { canUserDeleteNavigationMenu: false, canUserUpdateNavigationMenu: true, hasResolvedCanUserCreateNavigationMenu: true, - hasResolvedCanUserDeleteNavigationMenu: false, + hasResolvedCanUserDeleteNavigationMenu: true, hasResolvedCanUserUpdateNavigationMenu: true, hasResolvedNavigationMenus: true, isNavigationMenuMissing: false, @@ -190,6 +193,8 @@ describe( 'useNavigationMenus', () => { it( 'Should return correct permissions (delete only)', () => { resolveRecords( registry, navigationMenus ); + resolveCreatePermission( registry, false ); + resolveUpdatePermission( registry, 1, false ); resolveDeletePermission( registry, 1, true ); expect( useNavigationMenu( 1 ) ).toEqual( { navigationMenu: navigationMenu1, @@ -198,9 +203,9 @@ describe( 'useNavigationMenus', () => { canUserCreateNavigationMenu: false, canUserDeleteNavigationMenu: true, canUserUpdateNavigationMenu: false, - hasResolvedCanUserCreateNavigationMenu: false, + hasResolvedCanUserCreateNavigationMenu: true, hasResolvedCanUserDeleteNavigationMenu: true, - hasResolvedCanUserUpdateNavigationMenu: false, + hasResolvedCanUserUpdateNavigationMenu: true, hasResolvedNavigationMenus: true, isNavigationMenuMissing: false, isNavigationMenuResolved: false, From dd55fce31af3f81be33f5fea70a0b2e293794fff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Thu, 16 Jun 2022 16:43:51 +0200 Subject: [PATCH 6/7] Remove the http method check in navigation.test.js --- packages/e2e-tests/specs/editor/blocks/navigation.test.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js index 332bd1e380eb5..c081cf58ec444 100644 --- a/packages/e2e-tests/specs/editor/blocks/navigation.test.js +++ b/packages/e2e-tests/specs/editor/blocks/navigation.test.js @@ -400,11 +400,8 @@ describe( 'Navigation', () => { await setUpResponseMocking( [ { match: ( request ) => { - return ( - [ 'GET', 'OPTIONS' ].includes( request.method() ) && - decodeURIComponent( request.url() ).includes( - `navigation/${ testNavId }` - ) + return decodeURIComponent( request.url() ).includes( + `navigation/${ testNavId }` ); }, onRequestMatch: ( request ) => { From c9b3f586776f44f542443847b863798e18a6e091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Thu, 16 Jun 2022 17:33:48 +0200 Subject: [PATCH 7/7] Remove the testNavId from the mocked reponse endpoint in navigation.test.js --- packages/e2e-tests/specs/editor/blocks/navigation.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/e2e-tests/specs/editor/blocks/navigation.test.js b/packages/e2e-tests/specs/editor/blocks/navigation.test.js index c081cf58ec444..157c13de5f8ab 100644 --- a/packages/e2e-tests/specs/editor/blocks/navigation.test.js +++ b/packages/e2e-tests/specs/editor/blocks/navigation.test.js @@ -401,7 +401,7 @@ describe( 'Navigation', () => { { match: ( request ) => { return decodeURIComponent( request.url() ).includes( - `navigation/${ testNavId }` + `navigation/` ); }, onRequestMatch: ( request ) => {