From 7b8d040a9c96e92717b9ee163ae2569f5a3a4a7f Mon Sep 17 00:00:00 2001 From: Robert Anderson Date: Wed, 23 Jan 2019 15:28:21 +1100 Subject: [PATCH] Have `canUser()` return `undefined` by default --- .../developers/data/data-core.md | 14 ++++++++------ packages/block-library/src/block/edit.js | 2 +- packages/core-data/src/selectors.js | 15 ++++++--------- packages/core-data/src/test/selectors.js | 11 ++--------- .../reusable-block-convert-button.js | 2 +- .../reusable-block-delete-button.js | 2 +- .../src/components/media-placeholder/index.js | 4 ++-- .../editor/src/components/media-upload/check.js | 7 ++++++- 8 files changed, 27 insertions(+), 30 deletions(-) diff --git a/docs/designers-developers/developers/data/data-core.md b/docs/designers-developers/developers/data/data-core.md index 1d96f05468230d..b556767e30c40d 100644 --- a/docs/designers-developers/developers/data/data-core.md +++ b/docs/designers-developers/developers/data/data-core.md @@ -136,7 +136,7 @@ get back from the oEmbed preview API. Is the preview for the URL an oEmbed link fallback. -### hasUploadPermissions +### hasUploadPermissions (deprecated) Returns whether the current user can upload media. @@ -145,6 +145,11 @@ Calling this may trigger an OPTIONS request to the REST API via the https://developer.wordpress.org/rest-api/reference/ +*Deprecated* + +Deprecated since 4.9. Callers should use the more generic `canUser()` selector instead of + `hasUploadPermissions()`, e.g. `canUser( 'create', 'media' )`. + *Parameters* * state: Data state. @@ -170,14 +175,11 @@ https://developer.wordpress.org/rest-api/reference/ * action: Action to check. One of: 'create', 'read', 'update', 'delete'. * resource: REST resource to check, e.g. 'media' or 'posts'. * id: Optional ID of the rest resource to check. - * defaultIsAllowed: What to return when we don't know if the current user can - perform the given action on the given resource. Defaults to - `false`. *Returns* -Whether or not the user can perform the action, or `defaultIsAllowed` if the - OPTIONS request is still being made. +Whether or not the user can perform the action, + or `undefined` if the OPTIONS request is still being made. ## Actions diff --git a/packages/block-library/src/block/edit.js b/packages/block-library/src/block/edit.js index 80f1c16be71699..62e4acf0640d15 100644 --- a/packages/block-library/src/block/edit.js +++ b/packages/block-library/src/block/edit.js @@ -162,7 +162,7 @@ export default compose( [ isFetching: isFetchingReusableBlock( ref ), isSaving: isSavingReusableBlock( ref ), block: reusableBlock ? getBlock( reusableBlock.clientId ) : null, - canUpdateBlock: !! reusableBlock && ! reusableBlock.isTemporary && canUser( 'update', 'blocks', ref ), + canUpdateBlock: !! reusableBlock && ! reusableBlock.isTemporary && !! canUser( 'update', 'blocks', ref ), }; } ), withDispatch( ( dispatch, ownProps ) => { diff --git a/packages/core-data/src/selectors.js b/packages/core-data/src/selectors.js index 4d614de3433454..6b84abe5dd216f 100644 --- a/packages/core-data/src/selectors.js +++ b/packages/core-data/src/selectors.js @@ -2,7 +2,7 @@ * External dependencies */ import createSelector from 'rememo'; -import { map, find, get, filter, compact } from 'lodash'; +import { map, find, get, filter, compact, defaultTo } from 'lodash'; /** * WordPress dependencies @@ -191,7 +191,7 @@ export function hasUploadPermissions( state ) { deprecated( "select( 'core' ).hasUploadPermissions()", { alternative: "select( 'core' ).canUser( 'create', 'media' )", } ); - return canUser( state, 'create', 'media', undefined, true ); + return defaultTo( canUser( state, 'create', 'media' ), true ); } /** @@ -207,14 +207,11 @@ export function hasUploadPermissions( state ) { * @param {string} action Action to check. One of: 'create', 'read', 'update', 'delete'. * @param {string} resource REST resource to check, e.g. 'media' or 'posts'. * @param {string=} id Optional ID of the rest resource to check. - * @param {boolean=} defaultIsAllowed What to return when we don't know if the current user can - * perform the given action on the given resource. Defaults to - * `false`. * - * @return {boolean} Whether or not the user can perform the action, or `defaultIsAllowed` if the - * OPTIONS request is still being made. + * @return {boolean|undefined} Whether or not the user can perform the action, + * or `undefined` if the OPTIONS request is still being made. */ -export function canUser( state, action, resource, id = undefined, defaultIsAllowed = false ) { +export function canUser( state, action, resource, id ) { const key = compact( [ action, resource, id ] ).join( '/' ); - return get( state, [ 'userPermissions', key ], defaultIsAllowed ); + return get( state, [ 'userPermissions', key ] ); } diff --git a/packages/core-data/src/test/selectors.js b/packages/core-data/src/test/selectors.js index 7e96dfcb3e7d6e..f2a2885e776628 100644 --- a/packages/core-data/src/test/selectors.js +++ b/packages/core-data/src/test/selectors.js @@ -120,18 +120,11 @@ describe( 'isPreviewEmbedFallback()', () => { } ); describe( 'canUser', () => { - it( 'returns false by default', () => { + it( 'returns undefined by default', () => { const state = deepFreeze( { userPermissions: {}, } ); - expect( canUser( state, 'create', 'media' ) ).toBe( false ); - } ); - - it( 'returns true by default if defaultIsAllowed is specified', () => { - const state = deepFreeze( { - userPermissions: {}, - } ); - expect( canUser( state, 'create', 'media', undefined, true ) ).toBe( true ); + expect( canUser( state, 'create', 'media' ) ).toBe( undefined ); } ); it( 'returns whether an action can be performed', () => { diff --git a/packages/editor/src/components/block-settings-menu/reusable-block-convert-button.js b/packages/editor/src/components/block-settings-menu/reusable-block-convert-button.js index 11bfdc752533e1..2e528a01e7d911 100644 --- a/packages/editor/src/components/block-settings-menu/reusable-block-convert-button.js +++ b/packages/editor/src/components/block-settings-menu/reusable-block-convert-button.js @@ -82,7 +82,7 @@ export default compose( [ ) ) && // Hide 'Add to Reusable Blocks' when current doesn't have permission to do that - canUser( 'create', 'blocks' ) + !! canUser( 'create', 'blocks' ) ); return { diff --git a/packages/editor/src/components/block-settings-menu/reusable-block-delete-button.js b/packages/editor/src/components/block-settings-menu/reusable-block-delete-button.js index 0bd65bbaa2b1db..5a73912b36ee7a 100644 --- a/packages/editor/src/components/block-settings-menu/reusable-block-delete-button.js +++ b/packages/editor/src/components/block-settings-menu/reusable-block-delete-button.js @@ -44,7 +44,7 @@ export default compose( [ null; return { - isVisible: !! reusableBlock && canUser( 'delete', 'blocks', reusableBlock.id ), + isVisible: !! reusableBlock && !! canUser( 'delete', 'blocks', reusableBlock.id ), isDisabled: reusableBlock && reusableBlock.isTemporary, }; } ), diff --git a/packages/editor/src/components/media-placeholder/index.js b/packages/editor/src/components/media-placeholder/index.js index e85d4e9d9d8223..11b80bcc58b0f6 100644 --- a/packages/editor/src/components/media-placeholder/index.js +++ b/packages/editor/src/components/media-placeholder/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { every, get, noop, startsWith } from 'lodash'; +import { every, get, noop, startsWith, defaultTo } from 'lodash'; import classnames from 'classnames'; /** @@ -261,7 +261,7 @@ const applyWithSelect = withSelect( ( select ) => { const { canUser } = select( 'core' ); return { - hasUploadPermissions: canUser( 'create', 'media', undefined, true ), + hasUploadPermissions: defaultTo( canUser( 'create', 'media' ), true ), }; } ); diff --git a/packages/editor/src/components/media-upload/check.js b/packages/editor/src/components/media-upload/check.js index a5832a6368f43b..5dde4c69fa827c 100644 --- a/packages/editor/src/components/media-upload/check.js +++ b/packages/editor/src/components/media-upload/check.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { defaultTo } from 'lodash'; + /** * WordPress dependencies */ @@ -11,6 +16,6 @@ export default withSelect( ( select ) => { const { canUser } = select( 'core' ); return { - hasUploadPermissions: canUser( 'create', 'media', undefined, true ), + hasUploadPermissions: defaultTo( canUser( 'create', 'media' ), true ), }; } )( MediaUploadCheck );