From 76360c03380925a940abe132a7e23aad8181d3f1 Mon Sep 17 00:00:00 2001 From: BE-Webdesign Date: Tue, 25 Apr 2017 15:33:05 -0400 Subject: [PATCH] Validate block settings for edit() and save() Fixes #362. Validates that a block has a `save()` and `edit()`. This is probably a temporary solution. The API should probably be cleaned up a bit so everything is not quite as coupled and tests can be done differently. --- blocks/api/registration.js | 38 ++++++++ blocks/api/test/factory.js | 29 +++++-- blocks/api/test/function-refs.js | 13 +++ blocks/api/test/parser.js | 51 ++++++++--- blocks/api/test/registration.js | 144 ++++++++++++++++++++++++++----- blocks/api/test/serializer.js | 6 +- editor/test/state.js | 9 +- 7 files changed, 251 insertions(+), 39 deletions(-) create mode 100644 blocks/api/test/function-refs.js diff --git a/blocks/api/registration.js b/blocks/api/registration.js index 3dcd1f86b356c..6450195559db6 100644 --- a/blocks/api/registration.js +++ b/blocks/api/registration.js @@ -43,6 +43,13 @@ export function registerBlock( slug, settings ) { ); return; } + if ( true !== validateBlockSettings( settings ) ) { + console.error( + 'Block not registered.' + ); + // Return the block even though it was not registered. + return Object.assign( { slug }, settings ); + } const block = Object.assign( { slug }, settings ); blocks[ slug ] = block; return block; @@ -104,3 +111,34 @@ export function getBlockSettings( slug ) { export function getBlocks() { return Object.values( blocks ); } + +/** + * Validates the block settings. + * + * @param {Object} settings Block settings. + * @return {Boolean} Whether the block settings are valid. + */ +export function validateBlockSettings( settings ) { + if ( ! settings ) { + console.error( + 'Block settings must specify a save() and edit() method for each block.' + ); + return false; + } + + if ( ! settings.save || 'function' !== typeof settings.save ) { + console.error( + 'Block settings must specify a save() method for each block.' + ); + return false; + } + + if ( ! settings.edit || 'function' !== typeof settings.edit ) { + console.error( + 'Block settings must specify a edit() method for each block.' + ); + return false; + } + + return true; +} diff --git a/blocks/api/test/factory.js b/blocks/api/test/factory.js index cb1a0265738d5..f0e5275b51b3b 100644 --- a/blocks/api/test/factory.js +++ b/blocks/api/test/factory.js @@ -8,6 +8,7 @@ import { expect } from 'chai'; */ import { createBlock, switchToBlockType } from '../factory'; import { getBlocks, unregisterBlock, setUnknownTypeHandler, registerBlock } from '../registration'; +import { edit, save } from './function-refs'; describe( 'block factory', () => { afterEach( () => { @@ -43,9 +44,14 @@ describe( 'block factory', () => { }; } } ] - } + }, + edit: edit, + save: save + } ); + registerBlock( 'core/text-block', { + edit: edit, + save: save } ); - registerBlock( 'core/text-block', {} ); const block = { uid: 1, @@ -67,7 +73,10 @@ describe( 'block factory', () => { } ); it( 'should switch the blockType of a block using the "transform to"', () => { - registerBlock( 'core/updated-text-block', {} ); + registerBlock( 'core/updated-text-block', { + edit: edit, + save: save + } ); registerBlock( 'core/text-block', { transforms: { to: [ { @@ -78,7 +87,9 @@ describe( 'block factory', () => { }; } } ] - } + }, + edit: edit, + save: save } ); const block = { @@ -101,8 +112,14 @@ describe( 'block factory', () => { } ); it( 'should return null if no transformation is found', () => { - registerBlock( 'core/updated-text-block', {} ); - registerBlock( 'core/text-block', {} ); + registerBlock( 'core/updated-text-block', { + edit: edit, + save: save + } ); + registerBlock( 'core/text-block', { + edit: edit, + save: save + } ); const block = { uid: 1, diff --git a/blocks/api/test/function-refs.js b/blocks/api/test/function-refs.js new file mode 100644 index 0000000000000..45a12bd8b8531 --- /dev/null +++ b/blocks/api/test/function-refs.js @@ -0,0 +1,13 @@ +/** + * Function references used so that Chai doesn't complain about functions + * not being equal. + * + * These are methods that should be assigned to a valid block. + */ +export const edit = function edit() { + 'tacos'; +}; + +export const save = function save() { + 'delicious'; +}; diff --git a/blocks/api/test/parser.js b/blocks/api/test/parser.js index d483366a252dc..81fbd24dddab3 100644 --- a/blocks/api/test/parser.js +++ b/blocks/api/test/parser.js @@ -19,6 +19,10 @@ import { getBlocks, setUnknownTypeHandler, } from '../registration'; +import { + edit, + save +} from './function-refs.js'; describe( 'block parser', () => { afterEach( () => { @@ -87,7 +91,10 @@ describe( 'block parser', () => { describe( 'createBlockWithFallback', () => { it( 'should create the requested block if it exists', () => { - registerBlock( 'core/test-block', {} ); + registerBlock( 'core/test-block', { + edit: edit, + save: save + } ); const block = createBlockWithFallback( 'core/test-block', @@ -99,7 +106,10 @@ describe( 'block parser', () => { } ); it( 'should create the requested block with no attributes if it exists', () => { - registerBlock( 'core/test-block', {} ); + registerBlock( 'core/test-block', { + edit: edit, + save: save + } ); const block = createBlockWithFallback( 'core/test-block', 'content' ); expect( block.blockType ).to.eql( 'core/test-block' ); @@ -107,7 +117,10 @@ describe( 'block parser', () => { } ); it( 'should fall back to the unknown type handler for unknown blocks if present', () => { - registerBlock( 'core/unknown-block', {} ); + registerBlock( 'core/unknown-block', { + edit: edit, + save: save + } ); setUnknownTypeHandler( 'core/unknown-block' ); const block = createBlockWithFallback( @@ -120,7 +133,10 @@ describe( 'block parser', () => { } ); it( 'should fall back to the unknown type handler if block type not specified', () => { - registerBlock( 'core/unknown-block', {} ); + registerBlock( 'core/unknown-block', { + edit: edit, + save: save + } ); setUnknownTypeHandler( 'core/unknown-block' ); const block = createBlockWithFallback( null, 'content' ); @@ -142,7 +158,9 @@ describe( 'block parser', () => { return { content: rawContent, }; - } + }, + edit: edit, + save: save } ); const parsed = parse( @@ -166,7 +184,9 @@ describe( 'block parser', () => { return { content: rawContent + ' & Chicken' }; - } + }, + edit: edit, + save: save } ); const parsed = parse( @@ -184,8 +204,14 @@ describe( 'block parser', () => { } ); it( 'should parse the post content, using unknown block handler', () => { - registerBlock( 'core/test-block', {} ); - registerBlock( 'core/unknown-block', {} ); + registerBlock( 'core/test-block', { + edit: edit, + save: save + } ); + registerBlock( 'core/unknown-block', { + edit: edit, + save: save + } ); setUnknownTypeHandler( 'core/unknown-block' ); @@ -204,14 +230,19 @@ describe( 'block parser', () => { } ); it( 'should parse the post content, including raw HTML at each end', () => { - registerBlock( 'core/test-block', {} ); + registerBlock( 'core/test-block', { + edit: edit, + save: save + } ); registerBlock( 'core/unknown-block', { // Currently this is the only way to test block content parsing? attributes: function( rawContent ) { return { content: rawContent, }; - } + }, + edit: edit, + save: save } ); setUnknownTypeHandler( 'core/unknown-block' ); diff --git a/blocks/api/test/registration.js b/blocks/api/test/registration.js index 3af2d10dfe6c1..9bccb6772f39d 100644 --- a/blocks/api/test/registration.js +++ b/blocks/api/test/registration.js @@ -15,8 +15,13 @@ import { setUnknownTypeHandler, getUnknownTypeHandler, getBlockSettings, - getBlocks + getBlocks, + validateBlockSettings } from '../registration'; +import { + edit, + save +} from './function-refs'; describe( 'blocks', () => { // Reset block state before each test. @@ -52,25 +57,44 @@ describe( 'blocks', () => { } ); it( 'should accept valid block names', () => { - const block = registerBlock( 'my-plugin/fancy-block-4' ); + const block = registerBlock( 'my-plugin/fancy-block-4', { + settingName: 'settingValue', + edit: edit, + save: save + } ); expect( console.error ).to.not.have.been.called(); - expect( block ).to.eql( { slug: 'my-plugin/fancy-block-4' } ); + expect( block ).to.eql( { + slug: 'my-plugin/fancy-block-4', + settingName: 'settingValue', + edit: edit, + save: save + } ); } ); it( 'should prohibit registering the same block twice', () => { - registerBlock( 'core/test-block' ); + registerBlock( 'core/test-block', { + settingName: 'settingValue', + edit: edit, + save: save + } ); const block = registerBlock( 'core/test-block' ); expect( console.error ).to.have.been.calledWith( 'Block "core/test-block" is already registered.' ); expect( block ).to.be.undefined(); } ); it( 'should store a copy of block settings', () => { - const blockSettings = { settingName: 'settingValue' }; + const blockSettings = { + settingName: 'settingValue', + edit: edit, + save: save + }; registerBlock( 'core/test-block-with-settings', blockSettings ); blockSettings.mutated = true; expect( getBlockSettings( 'core/test-block-with-settings' ) ).to.eql( { slug: 'core/test-block-with-settings', settingName: 'settingValue', + edit: edit, + save: save } ); } ); } ); @@ -83,13 +107,27 @@ describe( 'blocks', () => { } ); it( 'should unregister existing blocks', () => { - registerBlock( 'core/test-block' ); - expect( getBlocks() ).to.eql( [ - { slug: 'core/test-block' }, + registerBlock( 'core/test-block', { + settingName: 'settingValue', + edit: edit, + save: save + } ); + expect( getBlocks() ).to.deep.equal( [ + { + slug: 'core/test-block', + settingName: 'settingValue', + edit: edit, + save: save + } ] ); const oldBlock = unregisterBlock( 'core/test-block' ); expect( console.error ).to.not.have.been.called(); - expect( oldBlock ).to.eql( { slug: 'core/test-block' } ); + expect( oldBlock ).to.deep.eql( { + slug: 'core/test-block', + settingName: 'settingValue', + edit: edit, + save: save + } ); expect( getBlocks() ).to.eql( [] ); } ); } ); @@ -109,19 +147,18 @@ describe( 'blocks', () => { } ); describe( 'getBlockSettings()', () => { - it( 'should return { slug } for blocks with no settings', () => { - registerBlock( 'core/test-block' ); - expect( getBlockSettings( 'core/test-block' ) ).to.eql( { - slug: 'core/test-block', - } ); - } ); - it( 'should return all block settings', () => { - const blockSettings = { settingName: 'settingValue' }; + const blockSettings = { + settingName: 'settingValue', + edit: edit, + save: save + }; registerBlock( 'core/test-block-with-settings', blockSettings ); expect( getBlockSettings( 'core/test-block-with-settings' ) ).to.eql( { slug: 'core/test-block-with-settings', settingName: 'settingValue', + edit: edit, + save: save } ); } ); } ); @@ -132,13 +169,78 @@ describe( 'blocks', () => { } ); it( 'should return all registered blocks', () => { - registerBlock( 'core/test-block' ); - const blockSettings = { settingName: 'settingValue' }; + registerBlock( 'core/test-block', { + edit: edit, + save: save + } ); + const blockSettings = { + settingName: 'settingValue', + edit: edit, + save: save + }; registerBlock( 'core/test-block-with-settings', blockSettings ); expect( getBlocks() ).to.eql( [ - { slug: 'core/test-block' }, - { slug: 'core/test-block-with-settings', settingName: 'settingValue' }, + { + slug: 'core/test-block', + edit: edit, + save: save + }, + { + slug: 'core/test-block-with-settings', + settingName: 'settingValue', + edit: edit, + save: save + }, ] ); } ); } ); + + describe( 'validateBlockSettings()', () => { + it( 'should return true when edit() and save() are present', () => { + const blockSettings = { + settingName: 'settingValue', + edit: edit, + save: save + }; + expect( validateBlockSettings( blockSettings ) ).to.eql( true ); + } ); + it( 'should return false when settings is falsy', () => { + const blockSettings = {}; + expect( validateBlockSettings( blockSettings ) ).to.eql( false ); + } ); + it( 'should return false when settings does not contain save()', () => { + const blockSettings = { + setting: 'settingValue', + edit: edit + }; + expect( validateBlockSettings( blockSettings ) ).to.eql( false ); + expect( console.error ).to.have.been.called(); + } ); + it( 'should return false when settings does not contain edit()', () => { + const blockSettings = { + setting: 'settingValue', + save: save + }; + expect( validateBlockSettings( blockSettings ) ).to.eql( false ); + expect( console.error ).to.have.been.called(); + } ); + it( 'should return false when save is not a function', () => { + const blockSettings = { + setting: 'settingValue', + edit: edit, + save: 'notAFunction' + }; + expect( validateBlockSettings( blockSettings ) ).to.eql( false ); + expect( console.error ).to.have.been.called(); + } ); + it( 'should return false when edit is not a function', () => { + const blockSettings = { + setting: 'settingValue', + save: save, + edit: 'notAFunction' + }; + expect( validateBlockSettings( blockSettings ) ).to.eql( false ); + expect( console.error ).to.have.been.called(); + } ); + } ); } ); diff --git a/blocks/api/test/serializer.js b/blocks/api/test/serializer.js index 89790ba6af5a6..b73c70092de7f 100644 --- a/blocks/api/test/serializer.js +++ b/blocks/api/test/serializer.js @@ -8,6 +8,9 @@ import { expect } from 'chai'; */ import serialize, { getCommentAttributes, getSaveContent } from '../serializer'; import { getBlocks, registerBlock, unregisterBlock } from '../registration'; +import { + edit +} from './function-refs'; describe( 'block serializer', () => { afterEach( () => { @@ -97,7 +100,8 @@ describe( 'block serializer', () => { }, save( { attributes } ) { return

; - } + }, + edit }; registerBlock( 'core/test-block', blockSettings ); const blockList = [ diff --git a/editor/test/state.js b/editor/test/state.js index 5c7e89c25ef1f..7c9551a180454 100644 --- a/editor/test/state.js +++ b/editor/test/state.js @@ -16,11 +16,18 @@ import { isSidebarOpened, createReduxStore } from '../state'; +import { + edit, + save +} from '../../blocks/api/test/function-refs'; describe( 'state', () => { describe( 'blocks()', () => { before( () => { - wp.blocks.registerBlock( 'core/test-block', {} ); + wp.blocks.registerBlock( 'core/test-block', { + edit: edit, + save: save + } ); } ); after( () => {