Skip to content

Commit

Permalink
Fix: Restrict block insert, move, and replace attending to allowedBlo…
Browse files Browse the repository at this point in the history
…cks, locking, and child blocks (#14003)

Supersedes: https://github.com/WordPress/gutenberg/pull/7301/files
Uses the select control implemented in #13924.
This PR changes the action creators to be a generator and have all the required validations before yielding the action.

Fixes: #14568;
Fixes: #10186;
Fixes: #13099;

Missing the update to the unit tests.

## How has this been tested?
Test block: [gist.github.com/jorgefilipecosta/b7194daac3ce26827522694fb4252c1c#file-testallowedblocksmiddleware-js](https://gist.github.com/jorgefilipecosta/b7194daac3ce26827522694fb4252c1c#file-testallowedblocksmiddleware-js)

I pasted the test block referred in the browser console.
I added the Product block.
I verified I cannot move using drag & drop the Buy button out of the Product block. (child block restriction.
I verified I cannot move using drag & drop a paragraph created outside of the Product block inside the product block. (allowed blocks restriction).
I verified that if I press enter in the buy button or image inside the product block, a paragraph is not created (allowed blocks restriction).
I added two galleries outside the product block, multi-selected them, and copied them ( ctrl + c ), I pasted them inside the list block inside the Product block and I verified they were not added (allowed blocks restriction).

I verified that when we have a template lock and we press enter on the title we now do not insert unallowed blocks.
Template lock code:

```
add_filter( 'allowed_block_types', function( $allowed_block_types, $post ) {
	if ( $post->post_type === 'post' ) {
	    return $allowed_block_types;
	}
	return [ 'core/image', 'core/button' ];
}, 10, 2 );
```
I verified that if we disable the paragraph, e.g., with a filter or inside the test block, pressing enter on the title or in placeholders does not add paragraph blocks. Pressing the sibling inserter also does not insert the paragraph (PR #7226).
Code to disable paragraph:

```
function myplugin_register_book_lock_post_type() {
    $args = array(
        'public' => true,
	  	'template_lock' => 'all',
        'label'  => 'Books Lock',
        'show_in_rest' => true,
        'template' => array(
            array( 'core/image', array(
            ) ),
            array( 'core/heading', array(
                'placeholder' => 'Add Author...',
            ) ),
            array( 'core/paragraph', array(
                'placeholder' => 'Add Description...',
            ) ),
        ),
    );
    register_post_type( 'book_lock', $args );
}
add_action( 'init', 'myplugin_register_book_lock_post_type' );
```
I added two image blocks, multi-selected them, copied them and tried to paste them on the list inside the test block. I checked the image blocks were not added inside the test block.

All these actions are possible in master.

Todo: If we accept this approach, end 2 end tests will be created before the merge that replicate this manual tests.
  • Loading branch information
jorgefilipecosta authored and aduth committed Apr 16, 2019
1 parent 8913322 commit e234570
Show file tree
Hide file tree
Showing 4 changed files with 417 additions and 91 deletions.
158 changes: 128 additions & 30 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* External dependencies
*/
import { castArray } from 'lodash';
import { castArray, first } from 'lodash';

/**
* WordPress dependencies
Expand All @@ -13,6 +13,25 @@ import { getDefaultBlockName, createBlock } from '@wordpress/blocks';
*/
import { select } from './controls';

/**
* Generator which will yield a default block insert action if there
* are no other blocks at the root of the editor. This generator should be used
* in actions which may result in no blocks remaining in the editor (removal,
* replacement, etc).
*/
function* ensureDefaultBlock() {
const count = yield select(
'core/block-editor',
'getBlockCount',
);

// To avoid a focus loss when removing the last block, assure there is
// always a default block if the last of the blocks have been removed.
if ( count === 0 ) {
yield insertDefaultBlock();
}
}

/**
* Returns an action object used in signalling that blocks state should be
* reset to the specified array of blocks, taking precedence over any other
Expand Down Expand Up @@ -202,15 +221,36 @@ export function toggleSelection( isSelectionEnabled = true ) {
* @param {(string|string[])} clientIds Block client ID(s) to replace.
* @param {(Object|Object[])} blocks Replacement block(s).
*
* @return {Object} Action object.
* @yields {Object} Action object.
*/
export function replaceBlocks( clientIds, blocks ) {
return {
export function* replaceBlocks( clientIds, blocks ) {
clientIds = castArray( clientIds );
blocks = castArray( blocks );
const rootClientId = yield select(
'core/block-editor',
'getBlockRootClientId',
first( clientIds )
);
// Replace is valid if the new blocks can be inserted in the root block.
for ( let index = 0; index < blocks.length; index++ ) {
const block = blocks[ index ];
const canInsertBlock = yield select(
'core/block-editor',
'canInsertBlockType',
block.name,
rootClientId
);
if ( ! canInsertBlock ) {
return;
}
}
yield {
type: 'REPLACE_BLOCKS',
clientIds: castArray( clientIds ),
blocks: castArray( blocks ),
clientIds,
blocks,
time: Date.now(),
};
yield* ensureDefaultBlock();
}

/**
Expand Down Expand Up @@ -256,16 +296,51 @@ export const moveBlocksUp = createOnMove( 'MOVE_BLOCKS_UP' );
* @param {?string} toRootClientId Root client ID destination.
* @param {number} index The index to move the block into.
*
* @return {Object} Action object.
* @yields {Object} Action object.
*/
export function moveBlockToPosition( clientId, fromRootClientId, toRootClientId, index ) {
return {
export function* moveBlockToPosition( clientId, fromRootClientId, toRootClientId, index ) {
const templateLock = yield select(
'core/block-editor',
'getTemplateLock',
fromRootClientId
);

// If locking is equal to all on the original clientId (fromRootClientId),
// it is not possible to move the block to any other position.
if ( templateLock === 'all' ) {
return;
}

const action = {
type: 'MOVE_BLOCK_TO_POSITION',
fromRootClientId,
toRootClientId,
clientId,
index,
};
// If moving inside the same root block the move is always possible.
if ( fromRootClientId === toRootClientId ) {
yield action;
return;
}

const blockName = yield select(
'core/block-editor',
'getBlockName',
clientId
);

const canInsertBlock = yield select(
'core/block-editor',
'canInsertBlockType',
blockName,
toRootClientId
);

// If moving to other parent block, the move is possible if we can insert a block of the same type inside the new parent block.
if ( canInsertBlock ) {
yield action;
}
}

/**
Expand All @@ -279,8 +354,18 @@ export function moveBlockToPosition( clientId, fromRootClientId, toRootClientId,
*
* @return {Object} Action object.
*/
export function insertBlock( block, index, rootClientId, updateSelection = true ) {
return insertBlocks( [ block ], index, rootClientId, updateSelection );
export function insertBlock(
block,
index,
rootClientId,
updateSelection = true,
) {
return insertBlocks(
[ block ],
index,
rootClientId,
updateSelection
);
}

/**
Expand All @@ -292,17 +377,37 @@ export function insertBlock( block, index, rootClientId, updateSelection = true
* @param {?string} rootClientId Optional root client ID of block list on which to insert.
* @param {?boolean} updateSelection If true block selection will be updated. If false, block selection will not change. Defaults to true.
*
* @return {Object} Action object.
*/
export function insertBlocks( blocks, index, rootClientId, updateSelection = true ) {
return {
type: 'INSERT_BLOCKS',
blocks: castArray( blocks ),
index,
rootClientId,
time: Date.now(),
updateSelection,
};
* @return {Object} Action object.
*/
export function* insertBlocks(
blocks,
index,
rootClientId,
updateSelection = true
) {
blocks = castArray( blocks );
const allowedBlocks = [];
for ( const block of blocks ) {
const isValid = yield select(
'core/block-editor',
'canInsertBlockType',
block.name,
rootClientId
);
if ( isValid ) {
allowedBlocks.push( block );
}
}
if ( allowedBlocks.length ) {
return {
type: 'INSERT_BLOCKS',
blocks: allowedBlocks,
index,
rootClientId,
time: Date.now(),
updateSelection,
};
}
}

/**
Expand Down Expand Up @@ -394,16 +499,9 @@ export function* removeBlocks( clientIds, selectPrevious = true ) {
clientIds,
};

const count = yield select(
'core/block-editor',
'getBlockCount',
);

// To avoid a focus loss when removing the last block, assure there is
// always a default block if the last of the blocks have been removed.
if ( count === 0 ) {
yield insertDefaultBlock();
}
yield* ensureDefaultBlock();
}

/**
Expand Down
22 changes: 0 additions & 22 deletions packages/block-editor/src/store/effects.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@ import {
replaceBlocks,
selectBlock,
setTemplateValidity,
insertDefaultBlock,
resetBlocks,
} from './actions';
import {
getBlock,
getBlocks,
getSelectedBlockCount,
getBlockCount,
getTemplateLock,
getTemplate,
isValidTemplate,
Expand Down Expand Up @@ -60,23 +58,6 @@ export function validateBlocksToTemplate( action, store ) {
}
}

/**
* Effect handler which will return a default block insertion action if there
* are no other blocks at the root of the editor. This is expected to be used
* in actions which may result in no blocks remaining in the editor (removal,
* replacement, etc).
*
* @param {Object} action Action which had initiated the effect handler.
* @param {Object} store Store instance.
*
* @return {?Object} Default block insert action, if no other blocks exist.
*/
export function ensureDefaultBlock( action, store ) {
if ( ! getBlockCount( store.getState() ) ) {
return insertDefaultBlock();
}
}

export default {
MERGE_BLOCKS( action, store ) {
const { dispatch } = store;
Expand Down Expand Up @@ -127,9 +108,6 @@ export default {
RESET_BLOCKS: [
validateBlocksToTemplate,
],
REPLACE_BLOCKS: [
ensureDefaultBlock,
],
MULTI_SELECT: ( action, { getState } ) => {
const blockCount = getSelectedBlockCount( getState() );

Expand Down
Loading

0 comments on commit e234570

Please sign in to comment.