Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block Directory: Fix installing blocks #23096

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions packages/block-directory/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,13 @@ export function* installBlockType( block ) {
throw new Error( __( 'Block has no assets.' ) );
}
yield setIsInstalling( block.id, true );
const response = yield apiFetch( {
yield apiFetch( {
path: '__experimental/block-directory/install',
data: {
slug: block.id,
},
method: 'POST',
} );
if ( response.success !== true ) {
throw new Error( __( 'Unable to install this block.' ) );
}
yield addInstalledBlockType( block );

yield loadAssets( assets );
Expand Down
29 changes: 21 additions & 8 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, mapKeys } from 'lodash';
import { camelCase, get, hasIn, includes, mapKeys } from 'lodash';

/**
* WordPress dependencies
Expand Down Expand Up @@ -36,21 +36,34 @@ export default {

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

let allowHeader;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to have this here until we are out of experimental status and move to:

export function* canUser( action, resource, id ) {

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 );
}
},
};
2 changes: 1 addition & 1 deletion packages/block-directory/src/store/test/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe( 'actions', () => {
message: 'Plugin not found.',
data: null,
};
expect( generator.next( apiError ).value ).toMatchObject( {
expect( generator.throw( apiError ).value ).toMatchObject( {
type: 'SET_ERROR_NOTICE',
blockId: item.id,
} );
Expand Down
58 changes: 57 additions & 1 deletion packages/e2e-tests/specs/experiments/block-directory-add.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ const MOCK_BLOCK1 = {
humanized_updated: '5 months ago',
};

const MOCK_INSTALLED_BLOCK_PLUGIN_DETAILS = {
plugin: 'block-directory-test-block',
status: 'active',
name: 'Block Directory',
plugin_uri: '',
author: 'No Author',
author_uri: '',
description: {
raw: 'This plugin is useful for the block.',
rendered: 'This plugin is useful for the block.',
},
version: '1.0',
network_only: false,
requires_wp: '',
requires_php: '',
text_domain: 'block-directory-test-block',
};

const MOCK_BLOCK2 = {
...MOCK_BLOCK1,
name: 'block-directory-test-block/secondary-block',
Expand All @@ -73,14 +91,50 @@ const block = `( function() {
} );
} )();`;

const MOCK_OPTIONS = {
namespace: '__experimental',
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 ),
onRequestMatch: createJSONResponse( [] ),
},
];

const MOCK_BLOCKS_RESPONSES = [
MOCK_OPTIONS_RESPONSE,
{
// Mock response for search with the block
match: ( request ) => matchUrl( request.url(), SEARCH_URLS ),
Expand All @@ -89,7 +143,9 @@ const MOCK_BLOCKS_RESPONSES = [
{
// Mock response for install
match: ( request ) => matchUrl( request.url(), INSTALL_URLS ),
onRequestMatch: createJSONResponse( { success: true } ),
onRequestMatch: createJSONResponse(
MOCK_INSTALLED_BLOCK_PLUGIN_DETAILS
),
},
{
// Mock the response for the js asset once it gets injected
Expand Down