Skip to content

Commit

Permalink
Block Directory: Remove custom permission check in favor of canUser (
Browse files Browse the repository at this point in the history
…#23624)

* Block Directory: Switch to `canUser` helper

* Remove custom permissions checks

* Remove mocked OPTIONS response, only mock the GET requests

This lets the permissions request fall through to the site's API, which should give a correct reply (the previous issue was that the empty result was overriding all methods, making it seem like the permission check failed).

* Show the permission error only when hasPermission is known

When the request is pending, `hasPermission` is undefined.
  • Loading branch information
ryelle authored Jul 2, 2020
1 parent 480dc0d commit cbcc61e
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 135 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function DownloadableBlocksPanel( {
isWaiting,
debouncedSpeak,
} ) {
if ( ! hasPermission ) {
if ( false === hasPermission ) {
debouncedSpeak(
__(
'No blocks found in your library. Please contact your site administrator to install new blocks.'
Expand All @@ -38,7 +38,7 @@ function DownloadableBlocksPanel( {
);
}

if ( isLoading || isWaiting ) {
if ( typeof hasPermission === 'undefined' || isLoading || isWaiting ) {
return (
<p className="block-directory-downloadable-blocks-panel__description has-no-results">
<Spinner />
Expand Down Expand Up @@ -86,11 +86,13 @@ export default compose( [
withSelect( ( select, { filterValue } ) => {
const {
getDownloadableBlocks,
hasInstallBlocksPermission,
isRequestingDownloadableBlocks,
} = select( 'core/block-directory' );

const hasPermission = hasInstallBlocksPermission();
const hasPermission = select( 'core' ).canUser(
'read',
'block-directory/search'
);
const downloadableItems = hasPermission
? getDownloadableBlocks( filterValue )
: [];
Expand Down
12 changes: 0 additions & 12 deletions packages/block-directory/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,18 +38,6 @@ export function receiveDownloadableBlocks( downloadableBlocks, filterValue ) {
};
}

/**
* Returns an action object used in signalling that the user does not have
* permission to install blocks.
*
* @param {boolean} hasPermission User has permission to install blocks.
*
* @return {Object} Action object.
*/
export function setInstallBlocksPermission( hasPermission ) {
return { type: 'SET_INSTALL_BLOCKS_PERMISSION', hasPermission };
}

/**
* Action triggered to install a block plugin.
*
Expand Down
17 changes: 0 additions & 17 deletions packages/block-directory/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,22 +75,6 @@ export const blockManagement = (
return state;
};

/**
* Reducer returning an array of downloadable blocks.
*
* @param {Object} state Current state.
* @param {Object} action Dispatched action.
*
* @return {Object} Updated state.
*/
export function hasPermission( state = true, action ) {
if ( action.type === 'SET_INSTALL_BLOCKS_PERMISSION' ) {
return action.hasPermission;
}

return state;
}

/**
* Reducer returning an object of error notices.
*
Expand All @@ -113,6 +97,5 @@ export const errorNotices = ( state = {}, action ) => {
export default combineReducers( {
downloadableBlocks,
blockManagement,
hasPermission,
errorNotices,
} );
39 changes: 3 additions & 36 deletions packages/block-directory/src/store/resolvers.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { camelCase, get, hasIn, includes, mapKeys } from 'lodash';
import { camelCase, mapKeys } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -11,11 +11,7 @@ import { apiFetch } from '@wordpress/data-controls';
/**
* Internal dependencies
*/
import {
fetchDownloadableBlocks,
receiveDownloadableBlocks,
setInstallBlocksPermission,
} from './actions';
import { fetchDownloadableBlocks, receiveDownloadableBlocks } from './actions';

export default {
*getDownloadableBlocks( filterValue ) {
Expand All @@ -35,35 +31,6 @@ export default {
);

yield receiveDownloadableBlocks( blocks, filterValue );
} catch ( error ) {
if ( error.code === 'rest_block_directory_cannot_view' ) {
yield setInstallBlocksPermission( false );
}
}
},
*hasInstallBlocksPermission() {
try {
const response = yield apiFetch( {
method: 'OPTIONS',
path: `wp/v2/block-directory/search`,
parse: false,
} );

let allowHeader;
if ( hasIn( response, [ 'headers', 'get' ] ) ) {
// If the request is fetched using the fetch api, the header can be
// retrieved using the 'get' method.
allowHeader = response.headers.get( 'allow' );
} else {
// If the request was preloaded server-side and is returned by the
// preloading middleware, the header will be a simple property.
allowHeader = get( response, [ 'headers', 'Allow' ], '' );
}

const isAllowed = includes( allowHeader, 'GET' );
yield setInstallBlocksPermission( isAllowed );
} catch ( error ) {
yield setInstallBlocksPermission( false );
}
} catch ( error ) {}
},
};
11 changes: 0 additions & 11 deletions packages/block-directory/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,6 @@ export function getDownloadableBlocks( state, filterValue ) {
return state.downloadableBlocks[ filterValue ].results;
}

/**
* Returns true if user has permission to install blocks.
*
* @param {Object} state Global application state.
*
* @return {boolean} User has permission to install blocks.
*/
export function hasInstallBlocksPermission( state ) {
return state.hasPermission;
}

/**
* Returns the block types that have been installed on the server.
*
Expand Down
18 changes: 1 addition & 17 deletions packages/block-directory/src/store/test/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,7 @@ import deepFreeze from 'deep-freeze';
/**
* Internal dependencies
*/
import {
blockManagement,
downloadableBlocks,
errorNotices,
hasPermission,
} from '../reducer';
import { blockManagement, downloadableBlocks, errorNotices } from '../reducer';
import { blockTypeInstalled, downloadableBlock } from './fixtures';

describe( 'state', () => {
Expand Down Expand Up @@ -89,17 +84,6 @@ describe( 'state', () => {
} );
} );

describe( 'hasPermission()', () => {
it( 'should update permissions appropriately', () => {
const state = hasPermission( true, {
type: 'SET_INSTALL_BLOCKS_PERMISSION',
hasPermission: false,
} );

expect( state ).toEqual( false );
} );
} );

describe( 'errorNotices()', () => {
it( 'should set an error notice', () => {
const initialState = deepFreeze( {} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,53 +83,21 @@ const block = `( function() {
} );
} )();`;

const MOCK_OPTIONS = {
namespace: 'wp/v2',
methods: [ 'GET' ],
endpoints: [
{
methods: [ 'GET' ],
args: {},
},
],
schema: {
$schema: 'http://json-schema.org/draft-04/schema#',
title: 'block-directory-item',
type: 'object',
properties: {},
},
};

const MOCK_OPTIONS_RESPONSE = {
match: ( request ) =>
matchUrl( request.url(), SEARCH_URLS ) &&
request.method() === 'OPTIONS',
onRequestMatch: async ( request ) => {
const response = {
content: 'application/json',
body: JSON.stringify( MOCK_OPTIONS ),
headers: {
Allow: 'GET',
},
};

return request.respond( response );
},
};

const MOCK_EMPTY_RESPONSES = [
MOCK_OPTIONS_RESPONSE,
{
match: ( request ) => matchUrl( request.url(), SEARCH_URLS ),
match: ( request ) =>
matchUrl( request.url(), SEARCH_URLS ) &&
request.method() === 'GET',
onRequestMatch: createJSONResponse( [] ),
},
];

const MOCK_BLOCKS_RESPONSES = [
MOCK_OPTIONS_RESPONSE,
{
// Mock response for search with the block
match: ( request ) => matchUrl( request.url(), SEARCH_URLS ),
match: ( request ) =>
matchUrl( request.url(), SEARCH_URLS ) &&
request.method() === 'GET',
onRequestMatch: createJSONResponse( [ MOCK_BLOCK1, MOCK_BLOCK2 ] ),
},
{
Expand Down

0 comments on commit cbcc61e

Please sign in to comment.