Merge pull request #1146 from Yoast/P2-786-only-render-ok-when-block-…

…is-valid

P2 786 only render ok when block is valid

packages/schema-blocks/jest.config.js

@@ -9,6 +9,5 @@ module.exports = {
 	testURL: "http://localhost",
 	transform: {
 		"^.+\\.(t|j)sx?$": [ "ts-jest", "babel-jest" ],
-		"^.+\\.tsx?$": "ts-jest",
 	},
 };

packages/schema-blocks/src/core/Instruction.ts

@@ -2,6 +2,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[];
@@ -100,6 +101,7 @@ export default abstract class Instruction {
 
 		if ( ! klass ) {
 			logger.error( "Invalid instruction: ", name );
+			throw new Error( "Invalid block instruction type: " + name );
 		}
 
 		return new klass( id, options );

packages/schema-blocks/src/core/blocks/BlockInstruction.ts

@@ -1,17 +1,9 @@
 import BlockLeaf from "./BlockLeaf";
-import { RenderEditProps, RenderSaveProps } from "./BlockDefinition";
 import { ReactElement } from "@wordpress/element";
+import { RenderEditProps, RenderSaveProps } from "./BlockDefinition";
 import { BlockConfiguration, BlockInstance } from "@wordpress/blocks";
-import { BlockValidation, BlockValidationResult } from "../validation";
-import Instruction, { InstructionOptions } from "../Instruction";
-import { attributeExists, attributeNotEmpty } from "../../functions/validators";
-import { BlockPresence } from "../validation/BlockValidationResult";
-import { maxBy } from "lodash";
-
-export type BlockInstructionClass = {
-	new( id: number, options: InstructionOptions ): BlockInstruction;
-	options: InstructionOptions;
-};
+import { BlockValidation, BlockValidationResult, BlockPresence } from "../validation";
+import Instruction from "../Instruction";
 
 /**
  * BlockInstruction class.
@@ -64,10 +56,16 @@ export default abstract class BlockInstruction extends Instruction {
 	/**
 	 * Returns the configuration of this instruction.
 	 *
-	 * @returns {Partial<BlockConfiguration>} The configuration.
+	 * @returns The block configuration.
 	 */
 	configuration(): Partial<BlockConfiguration> {
-		return {};
+		return {
+			attributes: {
+				[ this.options.name ]: {
+					required: this.options.required === true,
+				},
+			},
+		};
 	}
 
 	/**
@@ -78,32 +76,6 @@ export default abstract class BlockInstruction extends Instruction {
 	 * @returns {BlockValidationResult} The validation result.
 	 */
 	validate( blockInstance: BlockInstance ): BlockValidationResult {
-		const issues: BlockValidationResult[] = [];
-
-		if ( this.options ) {
-			const presence = this.options.required ? BlockPresence.Required : BlockPresence.Recommended;
-			const attributeValid = attributeExists( blockInstance, this.options.name as string ) &&
-								attributeNotEmpty( blockInstance, this.options.name as string );
-			if ( ! attributeValid ) {
-				issues.push( BlockValidationResult.MissingAttribute( blockInstance, this.constructor.name, presence ) );
-			}
-		}
-
-		if ( blockInstance.name.startsWith( "core/" ) && ! blockInstance.isValid ) {
-			issues.push( new BlockValidationResult( blockInstance.clientId, this.constructor.name, BlockValidation.Invalid, BlockPresence.Unknown ) );
-		}
-
-		// No issues found? That means the block is valid.
-		if ( issues.length < 1 ) {
-			return BlockValidationResult.Valid( blockInstance, this.constructor.name );
-		}
-
-		// Make sure to report the worst case scenario as the final validation result.
-		const worstCase: BlockValidationResult = maxBy( issues, issue => issue.result );
-
-		const validation = new BlockValidationResult( blockInstance.clientId, this.constructor.name, worstCase.result, worstCase.blockPresence );
-		validation.issues = issues;
-
-		return validation;
+		return new BlockValidationResult( blockInstance.clientId, this.constructor.name, BlockValidation.Unknown, BlockPresence.Unknown );
 	}
 }

packages/schema-blocks/src/core/blocks/BlockInstructionClass.ts

@@ -0,0 +1,7 @@
+import { InstructionOptions } from "../Instruction";
+import BlockInstruction from "./BlockInstruction";
+
+export type BlockInstructionClass = {
+	new( id: number, options: InstructionOptions ): BlockInstruction;
+	options: InstructionOptions;
+};

packages/schema-blocks/src/core/blocks/index.ts

@@ -0,0 +1,13 @@
+import BlockDefinition from "./BlockDefinition";
+import BlockInstruction from "./BlockInstruction";
+import { BlockInstructionClass } from "./BlockInstructionClass";
+import BlockLeaf from "./BlockLeaf";
+import * as BlockDefinitionRepository from "./BlockDefinitionRepository";
+
+export {
+	BlockDefinition,
+	BlockInstruction,
+	BlockInstructionClass,
+	BlockLeaf,
+	BlockDefinitionRepository,
+};

packages/schema-blocks/src/core/validation/BlockValidation.ts

@@ -30,5 +30,5 @@ export enum BlockValidation {
 	/** This block contains a Variationpicker to choose between subblocks, but no choice has been made yet for this recommended block. */
 	MissingRequiredVariation = 303,
 	/** There may be only one of this type of block, but we found more than one. */
-	TooMany = 303,
+	TooMany = 304,
 }

packages/schema-blocks/src/core/validation/BlockValidationResult.ts

@@ -53,7 +53,6 @@ export class BlockValidationResult {
 	constructor( clientId: string, name: string, result: BlockValidation, blockPresence: BlockPresence, message?: string ) {
 		this.name = name;
 		this.clientId = clientId;
-		this.name = name;
 		this.result = result;
 		this.blockPresence = blockPresence;
 		this.issues = [];
@@ -70,28 +69,49 @@ export class BlockValidationResult {
 	 * @constructor
 	 */
 	static MissingAttribute( blockInstance: BlockInstance, name?: string, blockPresence?: BlockPresence ) {
-		const blockValidation: BlockValidation = ( blockPresence === BlockPresence.Required )
-			? BlockValidation.MissingRequiredAttribute : BlockValidation.MissingRecommendedAttribute;
+		let blockValidation: BlockValidation = BlockValidation.Unknown;
+		let message = "";
+
+		switch ( blockPresence ) {
+			case BlockPresence.Required :
+				blockValidation = BlockValidation.MissingRequiredAttribute;
+				message = sprintf(
+					/* Translators: %1$s expands to the block name. */
+					__( "The `%1$s` attribute is required but missing.", "yoast-schema-blocks" ),
+					name,
+				);
+				break;
+
+			case BlockPresence.Recommended :
+				blockValidation = BlockValidation.MissingRecommendedAttribute;
+				message = sprintf(
+					/* Translators: %1$s expands to the block name. */
+					__( "The `%1$s` attribute is recommended but missing.", "yoast-schema-blocks" ),
+					name,
+				);
+				break;
+		}
 
 		return new BlockValidationResult(
 			blockInstance.clientId,
 			name || blockInstance.name,
 			blockValidation,
 			blockPresence || BlockPresence.Unknown,
+			message,
 		);
 	}
 
 	/**
 	 * Named constructor for a 'missing recommended / required block' validation result.
 	 *
 	 * @param name The name of the missing block.
-	 * @param [blockPresence] The block presence.
+	 * @param blockPresence The block presence.
 	 *
 	 * @constructor
 	 */
 	static MissingBlock( name: string, blockPresence?: BlockPresence ) {
 		if ( blockPresence === BlockPresence.Recommended ) {
-			return BlockValidationResult.MissingRecommendedBlock( name );
+			return BlockValidationResult.MissingRecommendedBlock( name, blockPresence === BlockPresence.Recommended );
 		}
 
 		return new BlockValidationResult(
@@ -111,15 +131,16 @@ export class BlockValidationResult {
 	 * Named constructor for a 'missing recommended block' validation result.
 	 *
 	 * @param name The name of the missing block.
+	 * @param recommended Wether the block is recommended or optional.
 	 *
 	 * @constructor
 	 */
-	private static MissingRecommendedBlock( name: string ) {
+	private static MissingRecommendedBlock( name: string, recommended: boolean ) {
 		return new BlockValidationResult(
 			null,
 			name,
 			BlockValidation.MissingRecommendedBlock,
-			BlockPresence.Recommended,
+			recommended ? BlockPresence.Recommended : BlockPresence.Unknown,
 			sprintf(
 				/* Translators: %1$s expands to the block name. */
 				__( "The `%1$s` block is recommended but missing.", "yoast-schema-blocks" ),

packages/schema-blocks/src/core/validation/index.ts

@@ -5,4 +5,12 @@ import { RequiredBlock } from "./RequiredBlock";
 import { RecommendedBlock } from "./RecommendedBlock";
 import { SuggestedBlockProperties } from "./SuggestedBlockProperties";
 
-export { BlockPresence, BlockValidation, BlockValidationResult, RequiredBlockOption, RequiredBlock, RecommendedBlock, SuggestedBlockProperties };
+export {
+	BlockPresence,
+	BlockValidation,
+	BlockValidationResult,
+	RecommendedBlock,
+	RequiredBlock,
+	RequiredBlockOption,
+	SuggestedBlockProperties,
+};

packages/schema-blocks/src/functions/gutenberg/storeBlockValidation.ts

@@ -12,8 +12,6 @@ export default function storeBlockValidation( validations: BlockValidationResult
 		return;
 	}
 
-	logger.debug( "Updating the store with the validation results." );
-
 	const store = dispatch( YOAST_SCHEMA_BLOCKS_STORE_NAME );
 	if ( store ) {
 		store.resetBlockValidation();
@@ -22,6 +20,6 @@ export default function storeBlockValidation( validations: BlockValidationResult
 			store.addBlockValidation( blockValidation );
 		} );
 	} else {
-		logger.debug( "No Store!" );
+		logger.debug( "No Store! Cannot store validations." );
 	}
 }