From 853a92a0ec7479cd74688bc5d4ef37fdcff12945 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 13 Oct 2025 12:59:28 +0100 Subject: [PATCH 1/4] Fix Classic Menu to Navigation Link conversion with entity bindings - Add buildNavigationLinkEntityBinding function to menu-items-to-blocks.js - Update menuItemToBlockAttributes to include entity bindings for non-custom items - Add comprehensive test coverage for entity binding behavior - Ensure classic menu conversions maintain dynamic URL resolution - Follow same pattern as PR #72287 for consistency Fixes classic menu items losing entity relationships during conversion. --- .../src/navigation/menu-items-to-blocks.js | 23 ++++ .../navigation/test/menu-items-to-blocks.js | 112 ++++++++++++++++++ 2 files changed, 135 insertions(+) diff --git a/packages/block-library/src/navigation/menu-items-to-blocks.js b/packages/block-library/src/navigation/menu-items-to-blocks.js index 00bd4dbbe2a98a..12669e0a991d9c 100644 --- a/packages/block-library/src/navigation/menu-items-to-blocks.js +++ b/packages/block-library/src/navigation/menu-items-to-blocks.js @@ -4,6 +4,26 @@ import { createBlock, parse } from '@wordpress/blocks'; import { applyFilters } from '@wordpress/hooks'; +/** + * Builds entity binding configuration for navigation link URLs. + * This function generates the structure used to bind navigation link URLs to their entity sources. + * + * Using a function instead of a constant allows for future enhancements where the binding + * might need dynamic data (e.g., entity ID, context-specific arguments). + * + * @return {Object} Entity binding configuration object + */ +function buildNavigationLinkEntityBinding() { + return { + url: { + source: 'core/entity', + args: { + key: 'url', + }, + }, + }; +} + /** * Convert a flat menu item structure to a nested blocks structure. * @@ -167,6 +187,9 @@ function menuItemToBlockAttributes( ...( object_id && 'custom' !== object && { id: object_id, + metadata: { + bindings: buildNavigationLinkEntityBinding(), + }, } ), /* eslint-enable camelcase */ ...( description?.length && { diff --git a/packages/block-library/src/navigation/test/menu-items-to-blocks.js b/packages/block-library/src/navigation/test/menu-items-to-blocks.js index 088880868022ae..4aba8b3eaba6a4 100644 --- a/packages/block-library/src/navigation/test/menu-items-to-blocks.js +++ b/packages/block-library/src/navigation/test/menu-items-to-blocks.js @@ -371,4 +371,116 @@ describe( 'converting menu items to blocks', () => { const { innerBlocks: actual } = menuItemsToBlocks( [] ); expect( actual ).toEqual( [] ); } ); + + it( 'adds entity bindings for non-custom menu items', () => { + const { innerBlocks: actual } = menuItemsToBlocks( [ + { + id: 1, + title: { + raw: 'Page Item', + rendered: 'Page Item', + }, + url: 'http://localhost:8889/page-item/', + attr_title: '', + description: '', + type: 'post_type', + type_label: 'Page', + object: 'page', + object_id: 123, + parent: 0, + menu_order: 1, + target: '', + classes: [ '' ], + xfn: [ '' ], + }, + { + id: 2, + title: { + raw: 'Category Item', + rendered: 'Category Item', + }, + url: 'http://localhost:8889/category/category-item/', + attr_title: '', + description: '', + type: 'taxonomy', + type_label: 'Category', + object: 'category', + object_id: 456, + parent: 0, + menu_order: 2, + target: '', + classes: [ '' ], + xfn: [ '' ], + }, + { + id: 3, + title: { + raw: 'Custom Item', + rendered: 'Custom Item', + }, + url: 'http://localhost:8889/custom-link/', + attr_title: '', + description: '', + type: 'custom', + type_label: 'Custom Link', + object: 'custom', + parent: 0, + menu_order: 3, + target: '', + classes: [ '' ], + xfn: [ '' ], + }, + ] ); + + expect( actual ).toEqual( [ + expect.objectContaining( { + name: 'core/navigation-link', + attributes: expect.objectContaining( { + label: 'Page Item', + id: 123, + metadata: { + bindings: { + url: { + source: 'core/entity', + args: { + key: 'url', + }, + }, + }, + }, + } ), + innerBlocks: [], + } ), + expect.objectContaining( { + name: 'core/navigation-link', + attributes: expect.objectContaining( { + label: 'Category Item', + id: 456, + metadata: { + bindings: { + url: { + source: 'core/entity', + args: { + key: 'url', + }, + }, + }, + }, + } ), + innerBlocks: [], + } ), + expect.objectContaining( { + name: 'core/navigation-link', + attributes: expect.objectContaining( { + label: 'Custom Item', + // Custom items should NOT have id, metadata, or bindings + } ), + innerBlocks: [], + } ), + ] ); + + // Verify custom item does NOT have bindings + expect( actual[ 2 ].attributes ).not.toHaveProperty( 'id' ); + expect( actual[ 2 ].attributes ).not.toHaveProperty( 'metadata' ); + } ); } ); From c5b5a387b783532bf913bca0615e345b46c58a9f Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 20 Oct 2025 14:02:18 +0100 Subject: [PATCH 2/4] Use canonical building of bindings --- .../src/navigation/menu-items-to-blocks.js | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/packages/block-library/src/navigation/menu-items-to-blocks.js b/packages/block-library/src/navigation/menu-items-to-blocks.js index 12669e0a991d9c..d3bb052132e453 100644 --- a/packages/block-library/src/navigation/menu-items-to-blocks.js +++ b/packages/block-library/src/navigation/menu-items-to-blocks.js @@ -5,24 +5,9 @@ import { createBlock, parse } from '@wordpress/blocks'; import { applyFilters } from '@wordpress/hooks'; /** - * Builds entity binding configuration for navigation link URLs. - * This function generates the structure used to bind navigation link URLs to their entity sources. - * - * Using a function instead of a constant allows for future enhancements where the binding - * might need dynamic data (e.g., entity ID, context-specific arguments). - * - * @return {Object} Entity binding configuration object + * Internal dependencies */ -function buildNavigationLinkEntityBinding() { - return { - url: { - source: 'core/entity', - args: { - key: 'url', - }, - }, - }; -} +import { buildNavigationLinkEntityBinding } from '../navigation-link/shared/use-entity-binding'; /** * Convert a flat menu item structure to a nested blocks structure. @@ -165,12 +150,14 @@ function menuItemToBlockAttributes( object = 'tag'; } + const inferredKind = menuItemTypeField?.replace( '_', '-' ) || 'custom'; + return { label: menuItemTitleField?.rendered || '', ...( object?.length && { type: object, } ), - kind: menuItemTypeField?.replace( '_', '-' ) || 'custom', + kind: inferredKind, url: url || '', ...( xfn?.length && xfn.join( ' ' ).trim() && { @@ -185,10 +172,10 @@ function menuItemToBlockAttributes( title: attr_title, } ), ...( object_id && - 'custom' !== object && { + ( inferredKind === 'post-type' || inferredKind === 'taxonomy' ) && { id: object_id, metadata: { - bindings: buildNavigationLinkEntityBinding(), + bindings: buildNavigationLinkEntityBinding( inferredKind ), }, } ), /* eslint-enable camelcase */ From 470d4c012ab593489d4c8faa60c7400136bdc825 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 20 Oct 2025 14:10:14 +0100 Subject: [PATCH 3/4] Update test expectations --- .../src/navigation/test/menu-items-to-blocks.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/navigation/test/menu-items-to-blocks.js b/packages/block-library/src/navigation/test/menu-items-to-blocks.js index 4aba8b3eaba6a4..6a4dc7ee665d5b 100644 --- a/packages/block-library/src/navigation/test/menu-items-to-blocks.js +++ b/packages/block-library/src/navigation/test/menu-items-to-blocks.js @@ -441,9 +441,9 @@ describe( 'converting menu items to blocks', () => { metadata: { bindings: { url: { - source: 'core/entity', + source: 'core/post-data', args: { - key: 'url', + field: 'link', }, }, }, @@ -459,9 +459,9 @@ describe( 'converting menu items to blocks', () => { metadata: { bindings: { url: { - source: 'core/entity', + source: 'core/term-data', args: { - key: 'url', + field: 'link', }, }, }, From 4992f162f29d2c53558dccdda16fdd1ccbb29b08 Mon Sep 17 00:00:00 2001 From: Dave Smith Date: Mon, 20 Oct 2025 14:13:35 +0100 Subject: [PATCH 4/4] Add edge case coverage for unsupported kinds --- .../navigation/test/menu-items-to-blocks.js | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/block-library/src/navigation/test/menu-items-to-blocks.js b/packages/block-library/src/navigation/test/menu-items-to-blocks.js index 6a4dc7ee665d5b..12eb19af858caa 100644 --- a/packages/block-library/src/navigation/test/menu-items-to-blocks.js +++ b/packages/block-library/src/navigation/test/menu-items-to-blocks.js @@ -483,4 +483,36 @@ describe( 'converting menu items to blocks', () => { expect( actual[ 2 ].attributes ).not.toHaveProperty( 'id' ); expect( actual[ 2 ].attributes ).not.toHaveProperty( 'metadata' ); } ); + + it( 'does not add bindings for invalid kinds even when object_id is present', () => { + const { innerBlocks: actual } = menuItemsToBlocks( [ + { + id: 10, + title: { + raw: 'Invalid Kind Item', + rendered: 'Invalid Kind Item', + }, + url: 'http://localhost:8889/invalid-kind-item/', + attr_title: '', + description: '', + type: 'invalid', // becomes inferred kind 'invalid' + type_label: 'Invalid', + object: 'page', + object_id: 999, + parent: 0, + menu_order: 1, + target: '', + classes: [ '' ], + xfn: [ '' ], + }, + ] ); + + expect( actual ).toHaveLength( 1 ); + expect( actual[ 0 ].name ).toBe( 'core/navigation-link' ); + // Should not set id or metadata when kind is not supported + expect( actual[ 0 ].attributes ).not.toHaveProperty( 'id' ); + expect( actual[ 0 ].attributes ).not.toHaveProperty( 'metadata' ); + // Label should still be set correctly + expect( actual[ 0 ].attributes.label ).toBe( 'Invalid Kind Item' ); + } ); } );