fix: Correctly parse nested attribute and tag sources. (wpengine#293)

* fix: php error for null $attribute_value in parse_query_source() * fix: resolve missing bool/rich-text attributes * tests: backfill tests for `CoreTable` attributes * tests: CoreTableAttributes.hasFixedLayout defaults true in 6.6 * fix: Correctly parse nested attribute and tag sources. * test: make CoreTableTest compatible with WP6.1 --------- Co-authored-by: DDEV User <[email protected]>
rtCamp · Sep 26, 2024 · 3a1157b · 3a1157b
1 parent 96bad40
commit 3a1157b
Show file tree

Hide file tree

Showing 7 changed files with 750 additions and 70 deletions.
diff --git a/.changeset/neat-books-kick.md b/.changeset/neat-books-kick.md
@@ -0,0 +1,5 @@
+---
+"@wpengine/wp-graphql-content-blocks": patch
+---
+
+fix: Correctly parse nested attribute and tag sources.
diff --git a/.changeset/pink-months-flash.md b/.changeset/pink-months-flash.md
@@ -0,0 +1,5 @@
+---
+"@wpengine/wp-graphql-content-blocks": patch
+---
+
+tests: backfill tests for `CoreTable` attributes.
diff --git a/includes/Blocks/Block.php b/includes/Blocks/Block.php
@@ -279,9 +279,10 @@ private function create_attributes_fields( $attributes, $prefix ): array {
 						$name,
 						$prefix
 					),
-					'resolve'     => function ( $attributes ) use ( $name, $default_value ) {
+					'resolve'     => function ( $attributes ) use ( $name, $default_value, $type ) {
 						$value = $attributes[ $name ] ?? $default_value;
-						return $this->normalize_attribute_value( $value, $attributes['__type'][ $name ]['type'] );
+
+						return $this->normalize_attribute_value( $value, $type );
 					},
 				];
 			}
@@ -305,10 +306,11 @@ private function normalize_attribute_value( $value, $type ) {
 		}
 
 		switch ( $type ) {
-			case 'rich-text':
 			case 'array':
 				// If we're here, we want an array type, even though the value is not an array.
-				return isset( $value ) ? [ $value ] : [];
+				// @todo This should return null if the value is empty.
+				return ! empty( $value ) ? [ $value ] : [];
+			case 'rich-text':
 			case 'string':
 				return (string) $value;
 			case 'number':

diff --git a/includes/Data/BlockAttributeResolver.php b/includes/Data/BlockAttributeResolver.php
@@ -28,15 +28,16 @@ public static function resolve_block_attribute( $attribute, string $html, $attri
 		$value = null;
 
 		if ( isset( $attribute['source'] ) ) {
+			// @todo parse remaining sources: https://github.com/WordPress/gutenberg/blob/trunk/packages/blocks/src/api/parser/get-block-attributes.js#L198
 			switch ( $attribute['source'] ) {
 				case 'attribute':
 					$value = self::parse_attribute_source( $html, $attribute );
 					break;
 				case 'html':
 				case 'rich-text':
-					// If there is no selector, we are dealing with single source.
+					// If there is no selector, the source is the node's innerHTML.
 					if ( ! isset( $attribute['selector'] ) ) {
-						$value = self::parse_single_source( $html, $attribute['source'] );
+						$value = ! empty( $html ) ? DOMHelpers::find_nodes( $html )->innerHTML() : null;
 						break;
 					}
 					$value = self::parse_html_source( $html, $attribute );
@@ -47,6 +48,9 @@ public static function resolve_block_attribute( $attribute, string $html, $attri
 				case 'query':
 					$value = self::parse_query_source( $html, $attribute, $attribute_value );
 					break;
+				case 'tag':
+					$value = self::parse_tag_source( $html );
+					break;
 				case 'meta':
 					$value = self::parse_meta_source( $attribute );
 					break;
@@ -59,14 +63,15 @@ public static function resolve_block_attribute( $attribute, string $html, $attri
 						$value = intval( $value );
 						break;
 					case 'boolean':
-						$value = ! empty( $value );
+						// Boolean attributes can be an empty string.
+						$value = ( ! is_array( $value ) && isset( $value ) ) || ! empty( $value );
 						break;
 				}
 			}
 		}
 
 		// Fallback to the attributes or default value if the result is empty.
-		if ( empty( $value ) ) {
+		if ( null === $value ) {
 			$default = $attribute['default'] ?? null;
 
 			$value = $attribute_value ?? $default;
@@ -75,25 +80,6 @@ public static function resolve_block_attribute( $attribute, string $html, $attri
 		return $value;
 	}
 
-	/**
-	 * Parses the block content of a source only block type
-	 *
-	 * @param string $html The html value
-	 * @param string $source The source type
-	 */
-	private static function parse_single_source( string $html, $source ): ?string {
-		if ( empty( $html ) ) {
-			return null;
-		}
-
-		switch ( $source ) {
-			case 'html':
-				return DOMHelpers::find_nodes( $html )->innerHTML();
-		}
-
-		return null;
-	}
-
 	/**
 	 * Parses the block content of an HTML source block type.
 	 *
@@ -124,11 +110,11 @@ private static function parse_html_source( string $html, array $config ): ?strin
 	 * @param array<string,mixed> $config The value configuration.
 	 */
 	private static function parse_attribute_source( string $html, array $config ): ?string {
-		if ( empty( $html ) || ! isset( $config['selector'] ) || ! isset( $config['attribute'] ) ) {
+		if ( empty( $html ) || ! isset( $config['attribute'] ) ) {
 			return null;
 		}
 
-		return DOMHelpers::parse_attribute( $html, $config['selector'], $config['attribute'] );
+		return DOMHelpers::parse_attribute( $html, $config['selector'] ?? '', $config['attribute'] );
 	}
 
 	/**
@@ -148,13 +134,13 @@ private static function parse_text_source( string $html, $config ): ?string {
 	/**
 	 * Parses a query source block type.
 	 *
-	 * @param string              $html The html value.
-	 * @param array<string,mixed> $config The value configuration.
-	 * @param array<string,mixed> $attribute_values The attribute values for the block.
+	 * @param string               $html The html value.
+	 * @param array<string,mixed>  $config The value configuration.
+	 * @param ?array<string,mixed> $attribute_values The attribute values for the block.
 	 *
 	 * @return ?mixed[]
 	 */
-	private static function parse_query_source( string $html, array $config, array $attribute_values ): ?array {
+	private static function parse_query_source( string $html, array $config, ?array $attribute_values ): ?array {
 		if ( ! isset( $config['selector'] ) || ! isset( $config['query'] ) ) {
 			return null;
 		}
@@ -203,4 +189,13 @@ private static function parse_meta_source( array $config ): ?string {
 
 		return get_post_meta( $post_id, $config['meta'], true );
 	}
+
+	/**
+	 * Parses a tag source.
+	 *
+	 * @param string $html The html value.
+	 */
+	private static function parse_tag_source( string $html ): ?string {
+		return DOMHelpers::get_first_node_tag_name( $html );
+	}
 }
diff --git a/includes/Utilities/DOMHelpers.php b/includes/Utilities/DOMHelpers.php
@@ -164,6 +164,33 @@ public static function parse_text( string $html, string $selector ): ?string {
 		return $nodes[0]->text();
 	}
 
+	/**
+	 * Gets the tag name of the first node.
+	 *
+	 * @internal This method should only be used internally. There are no guarantees for backwards compatibility.
+	 *
+	 * @param string $html The HTML string to parse.
+	 */
+	public static function get_first_node_tag_name( string $html ): ?string {
+		// Bail early if there's no html to parse.
+		if ( empty( trim( $html ) ) ) {
+			return null;
+		}
+
+		$doc = new Document();
+		$doc->loadHtml( $html, LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD );
+
+		/** @var \DiDom\Element[] $nodes */
+		$nodes = $doc->find( '*' );
+
+		if ( count( $nodes ) === 0 ) {
+			return null;
+		}
+
+		// Lowercase the tag name.
+		return strtolower( $nodes[0]->tagName() );
+	}
+
 	/**
 	 * Parses the html into DOMElement and searches the DOM tree for a given XPath expression or CSS selector.
 	 *