diff --git a/packages/schema-blocks/src/core/Definition.ts b/packages/schema-blocks/src/core/Definition.ts index 5d4e65bc59..4433e2e702 100644 --- a/packages/schema-blocks/src/core/Definition.ts +++ b/packages/schema-blocks/src/core/Definition.ts @@ -4,6 +4,7 @@ import { isArray, mergeWith } from "lodash"; import Instruction from "./Instruction"; import Leaf from "./Leaf"; import logger from "../functions/logger"; +import { BlockPresence } from "./validation/BlockValidationResult"; export type DefinitionClass = { new( separator: string, template?: string, instructions?: Record, tree?: Leaf ): T; @@ -80,7 +81,7 @@ export default abstract class Definition { return null; } - const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown ); + const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown, BlockPresence.Unknown ); logger.startGroup( `Validation results: ${ blockInstance.name }` ); diff --git a/packages/schema-blocks/src/core/Instruction.ts b/packages/schema-blocks/src/core/Instruction.ts index 9f7ffb8448..88af349cea 100644 --- a/packages/schema-blocks/src/core/Instruction.ts +++ b/packages/schema-blocks/src/core/Instruction.ts @@ -1,6 +1,7 @@ import { BlockInstance } from "@wordpress/blocks"; import logger from "../functions/logger"; import { BlockValidationResult, BlockValidation } from "./validation"; +import { BlockPresence } from "./validation/BlockValidationResult"; export type InstructionPrimitive = string | number | boolean; export type InstructionValue = InstructionPrimitive | InstructionObject | InstructionArray; export type InstructionArray = readonly InstructionValue[]; @@ -60,7 +61,7 @@ export default abstract class Instruction { * @returns {BlockValidationResult} The validation result. */ validate( blockInstance: BlockInstance ): BlockValidationResult { - return new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown ); + return new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown, BlockPresence.Unknown ); } /** diff --git a/packages/schema-blocks/src/core/blocks/BlockInstruction.ts b/packages/schema-blocks/src/core/blocks/BlockInstruction.ts index 4df76a2dc9..e6229bb282 100644 --- a/packages/schema-blocks/src/core/blocks/BlockInstruction.ts +++ b/packages/schema-blocks/src/core/blocks/BlockInstruction.ts @@ -1,12 +1,13 @@ import BlockLeaf from "./BlockLeaf"; -import { RenderSaveProps, RenderEditProps } from "./BlockDefinition"; +import { RenderEditProps, RenderSaveProps } from "./BlockDefinition"; import { ReactElement } from "@wordpress/element"; import { BlockConfiguration, BlockInstance } from "@wordpress/blocks"; -import { BlockValidationResult, BlockValidation } from "../validation"; +import { BlockValidation, BlockValidationResult } from "../validation"; import Instruction, { InstructionOptions } from "../Instruction"; import { attributeExists, attributeNotEmpty } from "../../functions/validators"; import validateMany from "../../functions/validators/validateMany"; import logger from "../../functions/logger"; +import { BlockPresence } from "../validation/BlockValidationResult"; export type BlockInstructionClass = { new( id: number, options: InstructionOptions ): BlockInstruction; @@ -78,16 +79,18 @@ export default abstract class BlockInstruction extends Instruction { * @returns {BlockValidationResult} The validation result. */ validate( blockInstance: BlockInstance ): BlockValidationResult { - const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Skipped ); + const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Skipped, BlockPresence.Unknown ); if ( this.options && this.options.required === true ) { const attributeValid = attributeExists( blockInstance, this.options.name as string ) && attributeNotEmpty( blockInstance, this.options.name as string ); if ( attributeValid ) { - validation.issues.push( new BlockValidationResult( blockInstance.clientId, this.options.name, BlockValidation.Valid ) ); + validation.issues.push( new BlockValidationResult( blockInstance.clientId, this.options.name, + BlockValidation.Valid, BlockPresence.Unknown ) ); } else { logger.warning( "block " + blockInstance.name + " has a required attributes " + this.options.name + " but it is missing or empty" ); - validation.issues.push( new BlockValidationResult( blockInstance.clientId, this.options.name, BlockValidation.MissingAttribute ) ); + validation.issues.push( new BlockValidationResult( blockInstance.clientId, this.options.name, + BlockValidation.MissingAttribute, BlockPresence.Unknown ) ); validation.result = BlockValidation.Invalid; } } else { diff --git a/packages/schema-blocks/src/core/schema/SchemaInstruction.ts b/packages/schema-blocks/src/core/schema/SchemaInstruction.ts index 1392184c57..2ab609e801 100644 --- a/packages/schema-blocks/src/core/schema/SchemaInstruction.ts +++ b/packages/schema-blocks/src/core/schema/SchemaInstruction.ts @@ -1,7 +1,8 @@ -import { SchemaValue, SchemaDefinitionConfiguration } from "./SchemaDefinition"; +import { SchemaDefinitionConfiguration, SchemaValue } from "./SchemaDefinition"; import Instruction, { InstructionOptions } from "../Instruction"; import { BlockInstance } from "@wordpress/blocks"; import { BlockValidation, BlockValidationResult } from "../validation"; +import { BlockPresence } from "../validation/BlockValidationResult"; export type SchemaInstructionClass = { new( id: number, options: InstructionOptions ): SchemaInstruction }; @@ -39,6 +40,6 @@ export default abstract class SchemaInstruction extends Instruction { * @returns {BlockValidationResult} The validation results. */ validate( blockInstance: BlockInstance ): BlockValidationResult { - return new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Valid ); + return new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Valid, BlockPresence.Unknown ); } } diff --git a/packages/schema-blocks/src/core/validation/BlockValidation.ts b/packages/schema-blocks/src/core/validation/BlockValidation.ts index e1283453f6..cfbab0f6e5 100644 --- a/packages/schema-blocks/src/core/validation/BlockValidation.ts +++ b/packages/schema-blocks/src/core/validation/BlockValidation.ts @@ -1,4 +1,3 @@ - export enum BlockValidation { /** This block was skipped during validation, on purpose. If you ever see this value, that's a bug. */ Skipped = -2, diff --git a/packages/schema-blocks/src/core/validation/BlockValidationResult.ts b/packages/schema-blocks/src/core/validation/BlockValidationResult.ts index c2e14c5776..b2528eab64 100644 --- a/packages/schema-blocks/src/core/validation/BlockValidationResult.ts +++ b/packages/schema-blocks/src/core/validation/BlockValidationResult.ts @@ -1,6 +1,13 @@ import { BlockValidation } from "."; import { BlockInstance } from "@wordpress/blocks"; +export enum BlockPresence { + Required = "required", + Recommended = "recommended", + Optional = "optional", + Unknown = "unknown" +} + /** * Contains the result of a block validation. */ @@ -16,24 +23,31 @@ export class BlockValidationResult { public clientId: string; /** - * The validation result; + * The validation result. */ public result: BlockValidation; + /** + * The block presence. + */ + public blockPresence: BlockPresence; + /** * The validation issues for this block's innerblocks or attributes, if any. */ public issues: BlockValidationResult[] /** - * @param clientId The clientId of the validated block. - * @param name The name of the validated block. - * @param result The validation result. + * @param clientId The clientId of the validated block. + * @param name The name of the validated block. + * @param result The validation result. + * @param blockPresence The block type. */ - constructor( clientId: string, name: string, result: BlockValidation ) { - this.name = name; + constructor( clientId: string, name: string, result: BlockValidation, blockPresence: BlockPresence ) { this.clientId = clientId; + this.name = name; this.result = result; + this.blockPresence = blockPresence; this.issues = []; } @@ -50,6 +64,7 @@ export class BlockValidationResult { blockInstance.clientId, name || blockInstance.name, BlockValidation.MissingAttribute, + BlockPresence.Unknown, ); } @@ -66,6 +81,7 @@ export class BlockValidationResult { blockInstance.clientId, name || blockInstance.name, BlockValidation.Valid, + BlockPresence.Unknown, ); } } diff --git a/packages/schema-blocks/src/core/validation/RequiredBlock.ts b/packages/schema-blocks/src/core/validation/RequiredBlock.ts index e34e589b25..23fdf9b910 100644 --- a/packages/schema-blocks/src/core/validation/RequiredBlock.ts +++ b/packages/schema-blocks/src/core/validation/RequiredBlock.ts @@ -1,5 +1,5 @@ -import { RequiredBlockOption } from "."; import { InstructionOptions } from "../Instruction"; +import { RequiredBlockOption } from "."; import { SuggestedBlockProperties } from "./SuggestedBlockProperties"; /** diff --git a/packages/schema-blocks/src/functions/gutenberg/watch.ts b/packages/schema-blocks/src/functions/gutenberg/watch.ts index bcee7635ef..d11a7a133f 100644 --- a/packages/schema-blocks/src/functions/gutenberg/watch.ts +++ b/packages/schema-blocks/src/functions/gutenberg/watch.ts @@ -7,6 +7,7 @@ import { getBlockDefinition } from "../../core/blocks/BlockDefinitionRepository" import { BlockValidation, BlockValidationResult } from "../../core/validation"; import storeBlockValidation from "./storeBlockValidation"; import logger from "../logger"; +import { BlockPresence } from "../../core/validation/BlockValidationResult"; let updatingSchema = false; let previousRootBlocks: BlockInstance[]; @@ -104,7 +105,7 @@ export function validateBlocks( blocks: BlockInstance[] ): BlockValidationResult validations.push( definition.validate( block ) ); } else { logger.warning( "Unable to validate block of type [" + block.name + "] " + block.clientId ); - validations.push( new BlockValidationResult( block.clientId, block.name, BlockValidation.Unknown ) ); + validations.push( new BlockValidationResult( block.clientId, block.name, BlockValidation.Unknown, BlockPresence.Unknown ) ); // Recursively validate all blocks' innerblocks. if ( block.innerBlocks && block.innerBlocks.length > 0 ) { diff --git a/packages/schema-blocks/src/functions/presenters/InnerBlocksSidebarPresenter.tsx b/packages/schema-blocks/src/functions/presenters/InnerBlocksSidebarPresenter.tsx index c362769ea0..1e6e506d66 100644 --- a/packages/schema-blocks/src/functions/presenters/InnerBlocksSidebarPresenter.tsx +++ b/packages/schema-blocks/src/functions/presenters/InnerBlocksSidebarPresenter.tsx @@ -11,7 +11,7 @@ import { SvgIcon } from "@yoast/components"; * Renders warnings and appenders for any block's InnerBlocks. * * @param currentBlock The innerblocks instance. - * @param options The InnerBlocksInstructionOptions of the innerblock. + * @param options The InnerBlocksInstructionOptions of the innerblock. * * @returns {ReactElement[]} React Elements representing the sidebar elements for these innerblocks. */ diff --git a/packages/schema-blocks/src/functions/presenters/SidebarWarningPresenter.ts b/packages/schema-blocks/src/functions/presenters/SidebarWarningPresenter.ts index c8f9a615b8..c0be37c76d 100644 --- a/packages/schema-blocks/src/functions/presenters/SidebarWarningPresenter.ts +++ b/packages/schema-blocks/src/functions/presenters/SidebarWarningPresenter.ts @@ -4,9 +4,10 @@ import { __ } from "@wordpress/i18n"; import { get } from "lodash"; import { BlockValidationResult } from "../../core/validation"; import { BlockValidation } from "../../core/validation"; +import { BlockPresence } from "../../core/validation/BlockValidationResult"; const analysisMessageTemplates: Record = { - [ BlockValidation.MissingBlock ]: "The '{child}' block is {status} but missing.", + [ BlockValidation.MissingBlock ]: "The '{child}' block is {presence} but missing.", [ BlockValidation.MissingAttribute ]: "The '{child}' block is empty.", }; @@ -16,7 +17,7 @@ type analysisIssue = { name: string; parent: string; result: BlockValidation; - status: string; + presence: string; }; export type sidebarWarning = { @@ -51,7 +52,7 @@ export function replaceVariables( issue: analysisIssue ): string { const warning = get( analysisMessageTemplates, issue.result, "" ); return warning.replace( "{parent}", __( issue.parent, "wpseo-schema-blocks" ) ) .replace( "{child}", __( issue.name, "wpseo-schema-blocks" ) ) - .replace( "{status}", __( issue.status, "wpseo-schema-blocks" ) ); + .replace( "{presence}", __( issue.presence, "wpseo-schema-blocks" ) ); } /** @@ -63,20 +64,26 @@ export function replaceVariables( issue: analysisIssue ): string { * @returns {sidebarWarning} Any analysis conclusions that should be in the footer. */ function getAnalysisConclusion( validation: BlockValidation, issues: analysisIssue[] ): sidebarWarning { - if ( issues.some( issue => issue.result === BlockValidation.MissingBlock || + // Show a red bullet when not all required blocks have been completed. + if ( issues.some( issue => issue.result === BlockValidation.MissingBlock && issue.presence === BlockPresence.Required || issue.result === BlockValidation.MissingAttribute ) ) { return { - text: __( "Not all required blocks are completed! No " + issues[ 0 ].parent + + text: __( "Not all required blocks have been completed! No " + issues[ 0 ].parent + " schema will be generated for your page.", "wpseo-schema-blocks" ), color: "red", } as sidebarWarning; } + // Show a green bullet when all required blocks have been completed. + const requiredBlockIssues = issues.filter( issue => { + return issue.presence === BlockPresence.Required; + } ); + if ( validation === BlockValidation.Valid || - issues.every( issue => issue.result !== BlockValidation.MissingAttribute && + requiredBlockIssues.every( issue => issue.result !== BlockValidation.MissingAttribute && issue.result !== BlockValidation.MissingBlock ) ) { return { - text: __( "Good job! All required blocks are completed.", "wpseo-schema-blocks" ), + text: __( "Good job! All required blocks have been completed.", "wpseo-schema-blocks" ), color: "green", } as sidebarWarning; } @@ -87,7 +94,7 @@ function getAnalysisConclusion( validation: BlockValidation, issues: analysisIss * * @param validation The root validation result. * - * @return all validation results. + * @return All validation results. */ function getAllDescendantIssues( validation: BlockValidationResult ): BlockValidationResult[] { let results = [ validation ]; @@ -114,10 +121,11 @@ export function createAnalysisMessages( validation: BlockValidationResult ): sid name: sanitizeBlockName( issue.name ), parent: sanitizeParentName( parent ), result: issue.result, - status: "required", + presence: issue.blockPresence, } ) ); + const messages = messageData.map( msg => { - return { text: replaceVariables( msg ), color: "red" } as sidebarWarning; + return { text: replaceVariables( msg ), color: ( msg.presence === BlockPresence.Recommended ? "orange" : "red" ) } as sidebarWarning; } ); const conclusion = getAnalysisConclusion( validation.result, messageData ); diff --git a/packages/schema-blocks/src/functions/validators/innerBlocksValid.ts b/packages/schema-blocks/src/functions/validators/innerBlocksValid.ts index 3da4a6fa90..15bf1697b2 100644 --- a/packages/schema-blocks/src/functions/validators/innerBlocksValid.ts +++ b/packages/schema-blocks/src/functions/validators/innerBlocksValid.ts @@ -1,46 +1,57 @@ import { BlockInstance } from "@wordpress/blocks"; import { countBy } from "lodash"; import { getBlockDefinition } from "../../core/blocks/BlockDefinitionRepository"; -import { RequiredBlockOption, BlockValidation, RequiredBlock, BlockValidationResult } from "../../core/validation"; +import { + BlockValidation, + BlockValidationResult, + RecommendedBlock, + RequiredBlock, + RequiredBlockOption, +} from "../../core/validation"; import recurseOverBlocks from "../blocks/recurseOverBlocks"; import { getInnerblocksByName } from "../innerBlocksHelper"; import logger from "../logger"; import isValidResult from "./isValidResult"; +import { BlockPresence } from "../../core/validation/BlockValidationResult"; /** - * Finds all blocks that should be in the inner blocks, but aren't. + * Finds all blocks that should/could be in the inner blocks, but aren't. * - * @param existingRequiredBlocks The actual array of all inner blocks. - * @param requiredBlocks All of the blocks that should occur in the inner blocks. + * @param existingBlocks The actual array of all inner blocks. + * @param allBlocks All of the blocks that should occur (required), or could occur (recommended) in the inner blocks. + * @param blockPresence The block presence. * - * @returns {BlockValidationResult[]} The names of blocks that should occur but don't, with reason 'MissingBlock'. + * @returns {BlockValidationResult[]} The names of blocks that should/could occur but don't, with reason 'MissingBlock'. */ -function findMissingBlocks( existingRequiredBlocks: BlockInstance[], requiredBlocks: RequiredBlock[] ): BlockValidationResult[] { - const missingRequiredBlocks = requiredBlocks.filter( requiredBlock => { - // If there are not any blocks with the name of a required block, that required block is missing. - return ! existingRequiredBlocks.some( block => block.name === requiredBlock.name ); +function findMissingBlocks( existingBlocks: BlockInstance[], allBlocks: RequiredBlock[] | RecommendedBlock[], + blockPresence: BlockPresence ): BlockValidationResult[] { + const missingBlocks = allBlocks.filter( block => { + // If, in the existing blocks, there are not any blocks with the name of block, that block is missing. + return ! existingBlocks.some( existingBlock => existingBlock.name === block.name ); } ); // These blocks should've existed, but they don't. - return missingRequiredBlocks.map( missingBlock => - new BlockValidationResult( null, missingBlock.name, BlockValidation.MissingBlock ) ); + return missingBlocks.map( missingBlock => + new BlockValidationResult( null, missingBlock.name, BlockValidation.MissingBlock, blockPresence ) ); } /** * Finds all blocks that occur more than once in the inner blocks. * - * @param existingRequiredBlocks The actual array of all inner blocks. - * @param requiredBlocks Requirements of the blocks that should occur only once in the inner blocks. + * @param existingBlocks The actual array of all inner blocks. + * @param allBlocks All of the blocks that should occur (required), or could occur (recommended) in the inner blocks. + * @param blockPresence The block presence. * * @returns {BlockValidationResult[]} The names of blocks that occur more than once in the inner blocks with reason 'TooMany'. */ -function findRedundantBlocks( existingRequiredBlocks: BlockInstance[], requiredBlocks: RequiredBlock[] ): BlockValidationResult[] { +function findRedundantBlocks( existingBlocks: BlockInstance[], allBlocks: RequiredBlock[] | RecommendedBlock[], + blockPresence: BlockPresence ): BlockValidationResult[] { const validationResults: BlockValidationResult[] = []; - const singletons = requiredBlocks.filter( block => block.option === RequiredBlockOption.One ); + const singletons = allBlocks.filter( block => block.option !== RequiredBlockOption.Multiple ); if ( singletons.length > 0 ) { // Count the occurrences of each block so we can find all keys that have too many occurrences. - const existingSingletons = existingRequiredBlocks.filter( block => singletons.some( s => s.name === block.name ) ); + const existingSingletons = existingBlocks.filter( block => singletons.some( s => s.name === block.name ) ); const countPerBlockType = countBy( existingSingletons, ( block: BlockInstance ) => block.name ); for ( const blockName in countPerBlockType ) { @@ -50,7 +61,7 @@ function findRedundantBlocks( existingRequiredBlocks: BlockInstance[], requiredB existingSingletons.forEach( ( block: BlockInstance ) => { if ( block.name === blockName ) { - validationResults.push( new BlockValidationResult( block.clientId, blockName, BlockValidation.TooMany ) ); + validationResults.push( new BlockValidationResult( block.clientId, blockName, BlockValidation.TooMany, blockPresence ) ); } } ); } @@ -76,7 +87,7 @@ function validateInnerblockTree( blockInstance: BlockInstance ): BlockValidation validations.push( definition.validate( block ) ); } else { logger.warning( "Block definition for '" + block.name + "' is not registered." ); - validations.push( new BlockValidationResult( block.clientId, block.name, BlockValidation.Unknown ) ); + validations.push( new BlockValidationResult( block.clientId, block.name, BlockValidation.Unknown, BlockPresence.Unknown ) ); } } ); return validations; @@ -85,23 +96,36 @@ function validateInnerblockTree( blockInstance: BlockInstance ): BlockValidation /** * Validates all inner blocks recursively and returns all invalid blocks. * - * @param blockInstance The block whose inner blocks need to be validated. - * @param requiredBlocks Requirements of the blocks that should occur in the inner blocks. + * @param blockInstance The block whose inner blocks need to be validated. + * @param requiredBlocks The inner blocks that are required. + * @param recommendedBlocks The inner blocks that are recommended. * * @returns {BlockValidationResult[]} The names and reasons of the inner blocks that are invalid. */ -function validateInnerBlocks( blockInstance: BlockInstance, requiredBlocks: RequiredBlock[] = [] ): BlockValidationResult[] { +function validateInnerBlocks( blockInstance: BlockInstance, requiredBlocks: RequiredBlock[] = [], + recommendedBlocks: RecommendedBlock[] = [] ): BlockValidationResult[] { const requiredBlockKeys = requiredBlocks.map( rblock => rblock.name ); + const recommendedBlockKeys = recommendedBlocks.map( rblock => rblock.name ); + let validationResults: BlockValidationResult[] = []; // Find all instances of required block types. const existingRequiredBlocks = getInnerblocksByName( blockInstance, requiredBlockKeys ); - // Find all block types that do not occur in existingBlocks. - validationResults.push( ...findMissingBlocks( existingRequiredBlocks, requiredBlocks ) ); + // Find all required block types that do not occur in existingRequiredBlocks. + validationResults.push( ...findMissingBlocks( existingRequiredBlocks, requiredBlocks, BlockPresence.Required ) ); + + // Find all required block types that allow only one occurrence. + validationResults.push( ...findRedundantBlocks( existingRequiredBlocks, requiredBlocks, BlockPresence.Required ) ); + + // Find all instances of recommended block types. + const existingRecommendedBlocks = getInnerblocksByName( blockInstance, recommendedBlockKeys ); + + // Find all recommended block types that do not occur in existingRecommendedBlocks. + validationResults.push( ...findMissingBlocks( existingRecommendedBlocks, recommendedBlocks, BlockPresence.Recommended ) ); - // Find all block types that allow only one occurrence. - validationResults.push( ...findRedundantBlocks( existingRequiredBlocks, requiredBlocks ) ); + // Find all recommended block types that allow only one occurrence. + validationResults.push( ...findRedundantBlocks( existingRecommendedBlocks, recommendedBlocks, BlockPresence.Recommended ) ); // Let all innerblocks validate themselves. // We differentiate between blocks that are internally valid but are not valid in the context of the innerblock. diff --git a/packages/schema-blocks/src/instructions/blocks/InnerBlocks.tsx b/packages/schema-blocks/src/instructions/blocks/InnerBlocks.tsx index 68f9a7ae0a..d9693c3f13 100644 --- a/packages/schema-blocks/src/instructions/blocks/InnerBlocks.tsx +++ b/packages/schema-blocks/src/instructions/blocks/InnerBlocks.tsx @@ -11,6 +11,7 @@ import validateMany from "../../functions/validators/validateMany"; import { innerBlocksSidebar } from "../../functions/presenters/InnerBlocksSidebarPresenter"; import { InnerBlocksInstructionOptions } from "./InnerBlocksInstructionOptions"; import BlockLeaf from "../../core/blocks/BlockLeaf"; +import { BlockPresence } from "../../core/validation/BlockValidationResult"; /** * Custom props for InnerBlocks. @@ -152,8 +153,9 @@ export default class InnerBlocks extends BlockInstruction { * @returns {BlockValidationResult} The validation result. */ validate( blockInstance: BlockInstance ): BlockValidationResult { - const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown ); - validation.issues = validateInnerBlocks( blockInstance, this.options.requiredBlocks ); + const validation = new BlockValidationResult( blockInstance.clientId, blockInstance.name, BlockValidation.Unknown, BlockPresence.Unknown ); + validation.issues = validateInnerBlocks( blockInstance, this.options.requiredBlocks, this.options.recommendedBlocks ); + return validateMany( validation ); } } diff --git a/packages/schema-blocks/tests/core/Definition.test.ts b/packages/schema-blocks/tests/core/Definition.test.ts index 34c1fb4873..2b735bad3b 100644 --- a/packages/schema-blocks/tests/core/Definition.test.ts +++ b/packages/schema-blocks/tests/core/Definition.test.ts @@ -2,6 +2,8 @@ import BlockInstruction from "../../src/core/blocks/BlockInstruction"; import Definition from "../../src/core/Definition"; import { BlockValidation, BlockValidationResult } from "../../src/core/validation"; import { BlockConfiguration, BlockInstance } from "@wordpress/blocks"; +import { BlockPresence } from "../../src/core/validation/BlockValidationResult"; + /** * Test class, to be able to test the methods in the abstract Definition class. */ @@ -50,7 +52,7 @@ class TestInstruction extends BlockInstruction { * @returns {BlockValidationResult[]} The constructor parameter wrapped in an array. */ validate( blockInstance: BlockInstance ): BlockValidationResult { - return new BlockValidationResult( "id" + this.id, "test", this.result ); + return new BlockValidationResult( "id" + this.id, "test", this.result, BlockPresence.Required ); } /* eslint-enable @typescript-eslint/no-unused-vars */ } diff --git a/packages/schema-blocks/tests/functions/presenters/SidebarWarningPresenter.test.ts b/packages/schema-blocks/tests/functions/presenters/SidebarWarningPresenter.test.ts index 5b01b25119..2ecda75316 100644 --- a/packages/schema-blocks/tests/functions/presenters/SidebarWarningPresenter.test.ts +++ b/packages/schema-blocks/tests/functions/presenters/SidebarWarningPresenter.test.ts @@ -1,5 +1,6 @@ import { BlockValidation, BlockValidationResult } from "../../../src/core/validation"; import getWarnings, { createAnalysisMessages, sanitizeBlockName } from "../../../src/functions/presenters/SidebarWarningPresenter"; +import { BlockPresence } from "../../../src/core/validation/BlockValidationResult"; const validations: Record = {}; const blockTypes: Record = {}; @@ -25,24 +26,24 @@ jest.mock( "@wordpress/data", () => { describe( "The createAnalysisMessages method ", () => { it( "creates a compliment for valid blocks.", () => { - const testcase = new BlockValidationResult( "1", "mijnblock", BlockValidation.Valid ); + const testcase = new BlockValidationResult( "1", "mijnblock", BlockValidation.Valid, BlockPresence.Required ); const result = createAnalysisMessages( testcase ); - expect( result ).toEqual( [ { text: "Good job! All required blocks are completed.", color: "green" } ] ); + expect( result ).toEqual( [ { text: "Good job! All required blocks have been completed.", color: "green" } ] ); } ); it( "creates a compliment for validation results we have no copy for.", () => { - const testcase = new BlockValidationResult( "1", "mijnblock", BlockValidation.Skipped ); + const testcase = new BlockValidationResult( "1", "mijnblock", BlockValidation.Skipped, BlockPresence.Required ); const result = createAnalysisMessages( testcase ); - expect( result ).toEqual( [ { text: "Good job! All required blocks are completed.", color: "green" } ] ); + expect( result ).toEqual( [ { text: "Good job! All required blocks have been completed.", color: "green" } ] ); } ); it( "creates warning messages for missing attributes, with a footer message.", () => { - const testcase = new BlockValidationResult( "1", "mijnblock", BlockValidation.Invalid ); - testcase.issues.push( new BlockValidationResult( null, "missingblockattribute", BlockValidation.MissingAttribute ) ); + const testcase = new BlockValidationResult( "1", "mijnblock", BlockValidation.Invalid, BlockPresence.Required ); + testcase.issues.push( new BlockValidationResult( null, "missingblockattribute", BlockValidation.MissingAttribute, BlockPresence.Required ) ); const result = createAnalysisMessages( testcase ); @@ -50,15 +51,15 @@ describe( "The createAnalysisMessages method ", () => { expect( result[ 0 ] ).toEqual( { text: "The 'missingblockattribute' block is empty.", color: "red" } ); expect( result[ 1 ] ).toEqual( { - text: "Not all required blocks are completed! No mijnblock schema will be generated for your page.", + text: "Not all required blocks have been completed! No mijnblock schema will be generated for your page.", color: "red", }, ); } ); - it( "creates warning messages for missing blocks, with a footer message.", () => { - const testcase = new BlockValidationResult( "1", "mijnblock", BlockValidation.Invalid ); - testcase.issues.push( new BlockValidationResult( null, "missingblock", BlockValidation.MissingBlock ) ); + it( "creates warning messages for missing required blocks, with a footer message.", () => { + const testcase = new BlockValidationResult( "1", "mijnblock", BlockValidation.Invalid, BlockPresence.Required ); + testcase.issues.push( new BlockValidationResult( null, "missingblock", BlockValidation.MissingBlock, BlockPresence.Required ) ); const result = createAnalysisMessages( testcase ); @@ -66,11 +67,25 @@ describe( "The createAnalysisMessages method ", () => { expect( result[ 0 ] ).toEqual( { text: "The 'missingblock' block is required but missing.", color: "red" } ); expect( result[ 1 ] ).toEqual( { - text: "Not all required blocks are completed! No mijnblock schema will be generated for your page.", + text: "Not all required blocks have been completed! No mijnblock schema will be generated for your page.", color: "red", }, ); } ); + + it( "creates a warning for missing recommended blocks, but when all the required blocks are present " + + "the conclusion should still be green.", () => { + const testcase = new BlockValidationResult( "1", "mijnblock", BlockValidation.Invalid, BlockPresence.Recommended ); + testcase.issues.push( new BlockValidationResult( null, "missing recommended block", BlockValidation.MissingBlock, BlockPresence.Recommended ) ); + testcase.issues.push( new BlockValidationResult( null, "missing recommended block 2", BlockValidation.MissingBlock, BlockPresence.Recommended ) ); + + const result = createAnalysisMessages( testcase ); + + expect( result.length ).toEqual( 3 ); + expect( result[ 0 ] ).toEqual( { text: "The 'missing recommended block' block is recommended but missing.", color: "orange" } ); + expect( result[ 1 ] ).toEqual( { text: "The 'missing recommended block 2' block is recommended but missing.", color: "orange" } ); + expect( result[ 2 ] ).toEqual( { text: "Good job! All required blocks have been completed.", color: "green" } ); + } ); } ); describe( "The sanitizeBlockName method ", () => { @@ -98,29 +113,29 @@ describe( "The sanitizeBlockName method ", () => { } ); describe( "The getWarnings method ", () => { - it( "creates a compliment for valid blocks.", () => { - validations[ "1" ] = new BlockValidationResult( "1", "myBlock", BlockValidation.Valid ); + it( "creates a compliment for required valid blocks.", () => { + validations[ "1" ] = new BlockValidationResult( "1", "myBlock", BlockValidation.Valid, BlockPresence.Required ); const result = getWarnings( "1" ); - expect( result ).toEqual( [ { text: "Good job! All required blocks are completed.", color: "green" } ] ); + expect( result ).toEqual( [ { text: "Good job! All required blocks have been completed.", color: "green" } ] ); } ); - it( "creates a compliment if we do not have copy for any of the validations.", () => { - const testcase = new BlockValidationResult( "1", "myBlock", BlockValidation.Invalid ); - testcase.issues.push( new BlockValidationResult( "2", "innerblock1", BlockValidation.Skipped ) ); - testcase.issues.push( new BlockValidationResult( "3", "anotherinnerblock", BlockValidation.TooMany ) ); - testcase.issues.push( new BlockValidationResult( "4", "anotherinnerblock", BlockValidation.Unknown ) ); + it( "creates a compliment if we do not have copy for any of the validations of the required blocks.", () => { + const testcase = new BlockValidationResult( "1", "myBlock", BlockValidation.Invalid, BlockPresence.Required ); + testcase.issues.push( new BlockValidationResult( "2", "innerblock1", BlockValidation.Skipped, BlockPresence.Required ) ); + testcase.issues.push( new BlockValidationResult( "3", "anotherinnerblock", BlockValidation.TooMany, BlockPresence.Required ) ); + testcase.issues.push( new BlockValidationResult( "4", "anotherinnerblock", BlockValidation.Unknown, BlockPresence.Required ) ); validations[ "1" ] = testcase; const result = getWarnings( "1" ); - expect( result ).toEqual( [ { text: "Good job! All required blocks are completed.", color: "green" } ] ); + expect( result ).toEqual( [ { text: "Good job! All required blocks have been completed.", color: "green" } ] ); } ); - it( "creates a warning for a block with validation problems.", () => { - const testcase = new BlockValidationResult( "1", "myBlock", BlockValidation.Invalid ); - testcase.issues.push( new BlockValidationResult( "2", "innerblock1", BlockValidation.MissingBlock ) ); + it( "creates a warning for a required block with validation problems.", () => { + const testcase = new BlockValidationResult( "1", "myBlock", BlockValidation.Invalid, BlockPresence.Required ); + testcase.issues.push( new BlockValidationResult( "2", "innerblock1", BlockValidation.MissingBlock, BlockPresence.Required ) ); validations[ "1" ] = testcase; const result = getWarnings( "1" ); @@ -131,8 +146,14 @@ describe( "The getWarnings method ", () => { color: "red", } ); expect( result[ 1 ] ).toEqual( { - text: "Not all required blocks are completed! No myblock schema will be generated for your page.", + text: "Not all required blocks have been completed! No myblock schema will be generated for your page.", color: "red", } ); } ); + + it( "creates no output when the validation results cannot be retrieved.", () => { + const result = getWarnings( "123" ); + + expect( result ).toBeNull(); + } ); } ); diff --git a/packages/schema-blocks/tests/functions/validators/innerBlocksValid.test.ts b/packages/schema-blocks/tests/functions/validators/innerBlocksValid.test.ts index a62debe4ef..d0ca50c8ab 100644 --- a/packages/schema-blocks/tests/functions/validators/innerBlocksValid.test.ts +++ b/packages/schema-blocks/tests/functions/validators/innerBlocksValid.test.ts @@ -1,9 +1,16 @@ import "../../matchMedia.mock"; import { BlockInstance } from "@wordpress/blocks"; -import { BlockValidationResult, BlockValidation, RequiredBlock, RequiredBlockOption } from "../../../src/core/validation"; +import { + BlockValidation, + BlockValidationResult, + RequiredBlock, + RequiredBlockOption, + RecommendedBlock, +} from "../../../src/core/validation"; import BlockDefinition from "../../../src/core/blocks/BlockDefinition"; import * as innerBlocksValid from "../../../src/functions/validators/innerBlocksValid"; import * as blockDefinitionRepository from "../../../src/core/blocks/BlockDefinitionRepository"; +import { BlockPresence } from "../../../src/core/validation/BlockValidationResult"; const mockBlockRegistry: Record = {}; const getBlockDefinitionMock = jest.spyOn( blockDefinitionRepository, "getBlockDefinition" ); @@ -16,28 +23,32 @@ getBlockDefinitionMock.mockImplementation( ( name: string ) => { /** * Provides a fake definition. + * * @param clientId ClientId of the mocked Definition. * @param name Name of the mocked Definition. * @param expectedValue Returned value for this clientId. + * * @returns {BlockDefinition} The fake definition. */ function FakeBlockDefinition( clientId: string, name: string, expectedValue: BlockValidation ): BlockDefinition { return { name: name, validate: () => { - const output = new BlockValidationResult( clientId, name, expectedValue ); + const output = new BlockValidationResult( clientId, name, expectedValue, BlockPresence.Required ); if ( expectedValue === BlockValidation.Valid || expectedValue === BlockValidation.Unknown ) { return output; } output.result = BlockValidation.Invalid; - output.issues.push( new BlockValidationResult( "child_or_attribute_id_" + name, "child_or_attribute_name_" + name, expectedValue ) ); + output.issues.push( new BlockValidationResult( "child_or_attribute_id_" + name, "child_or_attribute_name_" + + name, expectedValue, BlockPresence.Required ) ); return output; }, } as unknown as BlockDefinition; } /** - * Add a definition for a block to the mocked block definition. + * Adds a definition for a block to the mocked block definition. + * * @param clientId The clientId of the block to mock. * @param name The name of the block to mock. * @param expectedValue The validation output of the block. @@ -59,7 +70,7 @@ describe( "The BlockValidationResult constructor", () => { it( "creates a valid BlockValidationResult", () => { for ( let i = 0; i < createBlockValidationResultTestArrangement.length; i++ ) { const input = createBlockValidationResultTestArrangement[ i ]; - const result = new BlockValidationResult( "clientId", input.name, input.reason ); + const result = new BlockValidationResult( "clientId", input.name, input.reason, BlockPresence.Required ); expect( result.name ).toEqual( input.name ); expect( result.result ).toEqual( input.reason ); } @@ -67,11 +78,11 @@ describe( "The BlockValidationResult constructor", () => { } ); describe( "The findMissingBlocks function", () => { - it( "creates an BlockValidationResult with and reason 'missing' when a required block is missing.", () => { + it( "creates a BlockValidationResult with reason 'MissingBlock' when a required block is missing.", () => { // Arrange. const requiredBlocks: RequiredBlock[] = [ { - name: "existingBlock", + name: "existingRequiredBlock", option: RequiredBlockOption.Multiple, } as RequiredBlock, { @@ -81,12 +92,12 @@ describe( "The findMissingBlocks function", () => { ]; const existingRequiredBlocks: BlockInstance[] = [ { - name: "existingBlock", + name: "existingRequiredBlock", } as BlockInstance, ]; // Act. - const result: BlockValidationResult[] = innerBlocksValid.findMissingBlocks( existingRequiredBlocks, requiredBlocks ); + const result: BlockValidationResult[] = innerBlocksValid.findMissingBlocks( existingRequiredBlocks, requiredBlocks, BlockPresence.Required ); // Assert. expect( result.length ).toEqual( 1 ); @@ -94,11 +105,37 @@ describe( "The findMissingBlocks function", () => { expect( result[ 0 ].result ).toEqual( BlockValidation.MissingBlock ); } ); - it( "creates no missing blocks for blocks that are present.", () => { + it( "creates a BlockValidationResult with reason 'MissingBlock' when a recommended block is missing.", () => { + // Arrange. + const recommendedBlocks: RecommendedBlock[] = [ + { + name: "existingRecommendedBlock", + } as RecommendedBlock, + { + name: "missingblock", + } as RecommendedBlock, + ]; + const existingRecommendedBlocks: BlockInstance[] = [ + { + name: "existingRecommendedBlock", + } as BlockInstance, + ]; + + // Act. + const result: BlockValidationResult[] = innerBlocksValid.findMissingBlocks( existingRecommendedBlocks, recommendedBlocks, + BlockPresence.Recommended ); + + // Assert. + expect( result.length ).toEqual( 1 ); + expect( result[ 0 ].name ).toEqual( "missingblock" ); + expect( result[ 0 ].result ).toEqual( BlockValidation.MissingBlock ); + } ); + + it( "creates no missing blocks for required blocks that are present.", () => { // Arrange. const requiredBlocks: RequiredBlock[] = [ { - name: "existingBlock", + name: "existingRequiredBlock", option: RequiredBlockOption.Multiple, } as RequiredBlock, { @@ -108,7 +145,7 @@ describe( "The findMissingBlocks function", () => { ]; const existingRequiredBlocks: BlockInstance[] = [ { - name: "existingBlock", + name: "existingRequiredBlock", clientId: "existingBlock1", } as BlockInstance, { @@ -118,7 +155,7 @@ describe( "The findMissingBlocks function", () => { ]; // Act. - const result: BlockValidationResult[] = innerBlocksValid.findMissingBlocks( existingRequiredBlocks, requiredBlocks ); + const result: BlockValidationResult[] = innerBlocksValid.findMissingBlocks( existingRequiredBlocks, requiredBlocks, BlockPresence.Required ); // Assert. expect( result.length ).toEqual( 0 ); @@ -146,7 +183,7 @@ describe( "The findRedundantBlocks function", () => { ]; // Act. - const result = innerBlocksValid.findRedundantBlocks( existingRequiredBlocks, requiredBlocks ); + const result = innerBlocksValid.findRedundantBlocks( existingRequiredBlocks, requiredBlocks, BlockPresence.Required ); // Assert. expect( result.length ).toEqual( 2 ); @@ -181,7 +218,7 @@ describe( "The findRedundantBlocks function", () => { ]; // Act. - const result: BlockValidationResult[] = innerBlocksValid.findRedundantBlocks( existingBlocks, requiredBlocks ); + const result: BlockValidationResult[] = innerBlocksValid.findRedundantBlocks( existingBlocks, requiredBlocks, BlockPresence.Required ); // Assert. expect( result.length ).toEqual( 0 ); diff --git a/packages/schema-blocks/tests/functions/validators/validateMany.test.ts b/packages/schema-blocks/tests/functions/validators/validateMany.test.ts index cdeecf3fd7..afe3c8a484 100644 --- a/packages/schema-blocks/tests/functions/validators/validateMany.test.ts +++ b/packages/schema-blocks/tests/functions/validators/validateMany.test.ts @@ -1,14 +1,15 @@ import "../../matchMedia.mock"; import { BlockValidation, BlockValidationResult } from "../../../src/core/validation"; import validateMany from "../../../src/functions/validators/validateMany"; +import { BlockPresence } from "../../../src/core/validation/BlockValidationResult"; describe( "The validateMany function", () => { it( "considers a group of Valid and Unknown blocks to be Valid.", () => { // Arrange. - const validation = new BlockValidationResult( "validatedId", "validatedBlock", BlockValidation.Unknown ); + const validation = new BlockValidationResult( "validatedId", "validatedBlock", BlockValidation.Unknown, BlockPresence.Required ); validation.issues = [ - new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Valid ), - new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.Unknown ), + new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Valid, BlockPresence.Required ), + new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.Unknown, BlockPresence.Required ), ]; // Act. @@ -22,12 +23,12 @@ describe( "The validateMany function", () => { it( "considers a group of non-Valid blocks to be Invalid.", () => { // Arrange. - const validation = new BlockValidationResult( "validatedId", "validatedBlock", BlockValidation.Unknown ); + const validation = new BlockValidationResult( "validatedId", "validatedBlock", BlockValidation.Unknown, BlockPresence.Required ); validation.issues = [ - new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Invalid ), - new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.MissingAttribute ), - new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.MissingBlock ), - new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.TooMany ), + new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Invalid, BlockPresence.Required ), + new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.MissingAttribute, BlockPresence.Required ), + new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.MissingBlock, BlockPresence.Required ), + new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.TooMany, BlockPresence.Required ), ]; // Act. @@ -41,14 +42,14 @@ describe( "The validateMany function", () => { it( "considers a mix of Valid and non-Valid blocks to be Invalid.", () => { // Arrange. - const validation = new BlockValidationResult( "validatedId", "validatedBlock", BlockValidation.Unknown ); + const validation = new BlockValidationResult( "validatedId", "validatedBlock", BlockValidation.Unknown, BlockPresence.Required ); validation.issues = [ - new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Valid ), - new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Unknown ), - new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Invalid ), - new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.MissingAttribute ), - new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.MissingBlock ), - new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.TooMany ), + new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Valid, BlockPresence.Required ), + new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Unknown, BlockPresence.Required ), + new BlockValidationResult( "innerblock1", "innerblock1", BlockValidation.Invalid, BlockPresence.Required ), + new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.MissingAttribute, BlockPresence.Required ), + new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.MissingBlock, BlockPresence.Required ), + new BlockValidationResult( "innerblock2", "innerblock2", BlockValidation.TooMany, BlockPresence.Required ), ]; // Act.