Add widget id to blocks in the widgets screen (#28379)

WordPress · Jan 27, 2021 · 68debe9 · 68debe9
1 parent d5f58c6
commit 68debe9
Show file tree

Hide file tree

Showing 15 changed files with 377 additions and 141 deletions.
diff --git a/packages/block-editor/src/components/provider/test/use-block-sync.js b/packages/block-editor/src/components/provider/test/use-block-sync.js
@@ -1,3 +1,8 @@
+/**
+ * WordPress dependencies
+ */
+import { registerBlockType } from '@wordpress/blocks';
+
 /**
  * External dependencies
  */
@@ -19,6 +24,15 @@ const TestWrapper = withRegistryProvider( ( props ) => {
 } );
 
 describe( 'useBlockSync hook', () => {
+	beforeAll( () => {
+		registerBlockType( 'test/test-block', {
+			title: 'Test block',
+			attributes: {
+				foo: { type: 'number' },
+			},
+		} );
+	} );
+
 	afterEach( () => {
 		jest.clearAllMocks();
 	} );
@@ -101,7 +115,12 @@ describe( 'useBlockSync hook', () => {
 		);
 
 		const testBlocks = [
-			{ clientId: 'a', innerBlocks: [], attributes: { foo: 1 } },
+			{
+				name: 'test/test-block',
+				clientId: 'a',
+				innerBlocks: [],
+				attributes: { foo: 1 },
+			},
 		];
 		await act( async () => {
 			root.update(
@@ -131,7 +150,12 @@ describe( 'useBlockSync hook', () => {
 		const onInput = jest.fn();
 
 		const value1 = [
-			{ clientId: 'a', innerBlocks: [], attributes: { foo: 1 } },
+			{
+				name: 'test/test-block',
+				clientId: 'a',
+				innerBlocks: [],
+				attributes: { foo: 1 },
+			},
 		];
 		let root;
 		let registry;
@@ -282,7 +306,12 @@ describe( 'useBlockSync hook', () => {
 		const onInput = jest.fn();
 
 		const value1 = [
-			{ clientId: 'a', innerBlocks: [], attributes: { foo: 1 } },
+			{
+				name: 'test/test-block',
+				clientId: 'a',
+				innerBlocks: [],
+				attributes: { foo: 1 },
+			},
 		];
 
 		await act( async () => {
@@ -325,7 +354,12 @@ describe( 'useBlockSync hook', () => {
 		const onInput = jest.fn();
 
 		const value1 = [
-			{ clientId: 'a', innerBlocks: [], attributes: { foo: 1 } },
+			{
+				name: 'test/test-block',
+				clientId: 'a',
+				innerBlocks: [],
+				attributes: { foo: 1 },
+			},
 		];
 
 		let registry;
@@ -352,7 +386,14 @@ describe( 'useBlockSync hook', () => {
 
 		expect( replaceInnerBlocks ).not.toHaveBeenCalled();
 		expect( onChange ).toHaveBeenCalledWith(
-			[ { clientId: 'a', innerBlocks: [], attributes: { foo: 2 } } ],
+			[
+				{
+					name: 'test/test-block',
+					clientId: 'a',
+					innerBlocks: [],
+					attributes: { foo: 2 },
+				},
+			],
 			{ selectionEnd: {}, selectionStart: {} }
 		);
 		expect( onInput ).not.toHaveBeenCalled();

diff --git a/packages/block-editor/src/components/provider/use-block-sync.js b/packages/block-editor/src/components/provider/use-block-sync.js
@@ -63,8 +63,6 @@ import { cloneBlock } from '@wordpress/blocks';
  *                                change has been made in the block-editor blocks
  *                                for the given clientId. When this is called,
  *                                controlling sources do not become dirty.
- * @param {boolean} props.__unstableCloneValue Whether or not to clone each of
- *                                the blocks provided in the value prop.
  */
 export default function useBlockSync( {
 	clientId = null,
@@ -73,7 +71,6 @@ export default function useBlockSync( {
 	selectionEnd: controlledSelectionEnd,
 	onChange = noop,
 	onInput = noop,
-	__unstableCloneValue = true,
 } ) {
 	const registry = useRegistry();
 
@@ -101,12 +98,9 @@ export default function useBlockSync( {
 		if ( clientId ) {
 			setHasControlledInnerBlocks( clientId, true );
 			__unstableMarkNextChangeAsNotPersistent();
-			let storeBlocks = controlledBlocks;
-			if ( __unstableCloneValue ) {
-				storeBlocks = controlledBlocks.map( ( block ) =>
-					cloneBlock( block )
-				);
-			}
+			const storeBlocks = controlledBlocks.map( ( block ) =>
+				cloneBlock( block )
+			);
 			if ( subscribed.current ) {
 				pendingChanges.current.incoming = storeBlocks;
 			}

diff --git a/packages/blocks/CHANGELOG.md b/packages/blocks/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Breaking Change
+
+-   `cloneBlock` now sanitizes the attributes to match the same logic `createBlock` has. [#28379](https://github.com/WordPress/gutenberg/pull/28379)
+
 ## 6.25.0 (2020-12-17)
 
 ### New Feature

diff --git a/packages/blocks/src/api/factory.js b/packages/blocks/src/api/factory.js
@@ -4,7 +4,6 @@
 import { v4 as uuid } from 'uuid';
 import {
 	every,
-	reduce,
 	castArray,
 	findIndex,
 	isObjectLike,
@@ -31,7 +30,7 @@ import {
 	getBlockTypes,
 	getGroupingBlockName,
 } from './registration';
-import { normalizeBlockType } from './utils';
+import { normalizeBlockType, sanitizeBlockAttributes } from './utils';
 
 /**
  * Returns a block object given its type and attributes.
@@ -43,40 +42,7 @@ import { normalizeBlockType } from './utils';
  * @return {Object} Block object.
  */
 export function createBlock( name, attributes = {}, innerBlocks = [] ) {
-	// Get the type definition associated with a registered block.
-	const blockType = getBlockType( name );
-
-	if ( undefined === blockType ) {
-		throw new Error( `Block type '${ name }' is not registered.` );
-	}
-
-	// Ensure attributes contains only values defined by block type, and merge
-	// default values for missing attributes.
-	const sanitizedAttributes = reduce(
-		blockType.attributes,
-		( accumulator, schema, key ) => {
-			const value = attributes[ key ];
-
-			if ( undefined !== value ) {
-				accumulator[ key ] = value;
-			} else if ( schema.hasOwnProperty( 'default' ) ) {
-				accumulator[ key ] = schema.default;
-			}
-
-			if ( [ 'node', 'children' ].indexOf( schema.source ) !== -1 ) {
-				// Ensure value passed is always an array, which we're expecting in
-				// the RichText component to handle the deprecated value.
-				if ( typeof accumulator[ key ] === 'string' ) {
-					accumulator[ key ] = [ accumulator[ key ] ];
-				} else if ( ! Array.isArray( accumulator[ key ] ) ) {
-					accumulator[ key ] = [];
-				}
-			}
-
-			return accumulator;
-		},
-		{}
-	);
+	const sanitizedAttributes = sanitizeBlockAttributes( name, attributes );
 
 	const clientId = uuid();
 
@@ -134,13 +100,15 @@ export function createBlocksFromInnerBlocksTemplate(
 export function cloneBlock( block, mergeAttributes = {}, newInnerBlocks ) {
 	const clientId = uuid();
 
+	const sanitizedAttributes = sanitizeBlockAttributes( block.name, {
+		...block.attributes,
+		...mergeAttributes,
+	} );
+
 	return {
 		...block,
 		clientId,
-		attributes: {
-			...block.attributes,
-			...mergeAttributes,
-		},
+		attributes: sanitizedAttributes,
 		innerBlocks:
 			newInnerBlocks ||
 			block.innerBlocks.map( ( innerBlock ) => cloneBlock( innerBlock ) ),

diff --git a/packages/blocks/src/api/test/factory.js b/packages/blocks/src/api/test/factory.js
@@ -159,6 +159,23 @@ describe( 'block factory', () => {
 				content: 'test',
 			} );
 		} );
+
+		it( 'should sanitize attributes not defined in the block type', () => {
+			registerBlockType( 'core/test-block', {
+				...defaultBlockSettings,
+				attributes: {
+					align: {
+						type: 'string',
+					},
+				},
+			} );
+
+			const block = createBlock( 'core/test-block', {
+				notDefined: 'not-defined',
+			} );
+
+			expect( block.attributes ).toEqual( {} );
+		} );
 	} );
 
 	describe( 'createBlocksFromInnerBlocksTemplate', () => {
@@ -274,6 +291,31 @@ describe( 'block factory', () => {
 						type: 'boolean',
 						default: false,
 					},
+					includesDefault: {
+						type: 'boolean',
+						default: true,
+					},
+					includesFalseyDefault: {
+						type: 'number',
+						default: 0,
+					},
+					content: {
+						type: 'array',
+						source: 'children',
+					},
+					defaultContent: {
+						type: 'array',
+						source: 'children',
+						default: 'test',
+					},
+					unknownDefaultContent: {
+						type: 'array',
+						source: 'children',
+						default: 1,
+					},
+					htmlContent: {
+						source: 'html',
+					},
 				},
 				save: noop,
 				category: 'text',
@@ -287,12 +329,19 @@ describe( 'block factory', () => {
 
 			const clonedBlock = cloneBlock( block, {
 				isDifferent: true,
+				htmlContent: 'test',
 			} );
 
 			expect( clonedBlock.name ).toEqual( block.name );
 			expect( clonedBlock.attributes ).toEqual( {
+				includesDefault: true,
+				includesFalseyDefault: 0,
 				align: 'left',
 				isDifferent: true,
+				content: [],
+				defaultContent: [ 'test' ],
+				unknownDefaultContent: [],
+				htmlContent: 'test',
 			} );
 			expect( clonedBlock.innerBlocks ).toHaveLength( 1 );
 			expect( typeof clonedBlock.clientId ).toBe( 'string' );
@@ -375,6 +424,27 @@ describe( 'block factory', () => {
 				block.innerBlocks[ 1 ].attributes
 			);
 		} );
+
+		it( 'should sanitize attributes not defined in the block type', () => {
+			registerBlockType( 'core/test-block', {
+				...defaultBlockSettings,
+				attributes: {
+					align: {
+						type: 'string',
+					},
+				},
+			} );
+
+			const block = createBlock( 'core/test-block', {
+				notDefined: 'not-defined',
+			} );
+
+			const clonedBlock = cloneBlock( block, {
+				notDefined2: 'not-defined-2',
+			} );
+
+			expect( clonedBlock.attributes ).toEqual( {} );
+		} );
 	} );
 
 	describe( 'getPossibleBlockTransformations()', () => {