diff --git a/packages/block-library/src/navigation-link/shared/index.js b/packages/block-library/src/navigation-link/shared/index.js index f51728c8b7bfcd..9e1d52a73c2b09 100644 --- a/packages/block-library/src/navigation-link/shared/index.js +++ b/packages/block-library/src/navigation-link/shared/index.js @@ -7,5 +7,8 @@ export { Controls } from './controls'; export { updateAttributes } from './update-attributes'; -export { useEntityBinding } from './use-entity-binding'; +export { + useEntityBinding, + buildNavigationLinkEntityBinding, +} from './use-entity-binding'; export { LinkUI } from '../link-ui'; diff --git a/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js b/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js index 31636c9e8cb8ae..060e3eb715cdd5 100644 --- a/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js +++ b/packages/block-library/src/navigation-link/shared/test/use-entity-binding.js @@ -10,7 +10,10 @@ import { renderHook, act } from '@testing-library/react'; /** * Internal dependencies */ -import { useEntityBinding } from '../use-entity-binding'; +import { + useEntityBinding, + buildNavigationLinkEntityBinding, +} from '../use-entity-binding'; // Mock the entire @wordpress/block-editor module jest.mock( '@wordpress/block-editor', () => ( { @@ -276,160 +279,62 @@ describe( 'useEntityBinding', () => { } ); } ); - it( 'should create core/term-data binding when createBinding is called for taxonomy', () => { - const attributes = { - metadata: {}, - id: null, - kind: 'taxonomy', - }; - - const { result } = renderHook( () => - useEntityBinding( { - clientId: 'test-client-id', - attributes, - } ) - ); - - act( () => { - result.current.createBinding(); - } ); - - expect( mockUpdateBlockBindings ).toHaveBeenCalledWith( { - url: { - source: 'core/term-data', - args: { - key: 'link', - }, - }, - } ); - } ); - - describe( 'clearBinding behavior', () => { - it( 'should call updateBlockBindings when clearBinding is called and valid binding exists', () => { - const attributes = { - metadata: { - bindings: { - url: { - source: 'core/post-data', - args: { key: 'link' }, - }, - }, + describe( 'buildNavigationLinkEntityBinding', () => { + it( 'returns correct binding for post-type', () => { + const binding = buildNavigationLinkEntityBinding( 'post-type' ); + expect( binding ).toEqual( { + url: { + source: 'core/post-data', + args: { key: 'link' }, }, - id: 123, - kind: 'post-type', - }; - - const { result } = renderHook( () => - useEntityBinding( { - clientId: 'test-client-id', - attributes, - } ) - ); - - act( () => { - result.current.clearBinding(); - } ); - - expect( mockUpdateBlockBindings ).toHaveBeenCalledWith( { - url: undefined, } ); } ); - it( 'should call updateBlockBindings when clearBinding is called and valid taxonomy binding exists', () => { - const attributes = { - metadata: { - bindings: { - url: { - source: 'core/term-data', - args: { key: 'link' }, - }, - }, + it( 'returns correct binding for taxonomy', () => { + const binding = buildNavigationLinkEntityBinding( 'taxonomy' ); + expect( binding ).toEqual( { + url: { + source: 'core/term-data', + args: { key: 'link' }, }, - id: 456, - kind: 'taxonomy', - }; - - const { result } = renderHook( () => - useEntityBinding( { - clientId: 'test-client-id', - attributes, - } ) - ); - - act( () => { - result.current.clearBinding(); - } ); - - expect( mockUpdateBlockBindings ).toHaveBeenCalledWith( { - url: undefined, } ); } ); - it( 'should NOT call updateBlockBindings when clearBinding is called and binding exists but no id', () => { - const attributes = { - metadata: { - bindings: { - url: { - source: 'core/post-data', - args: { key: 'link' }, - }, - }, - }, - id: null, - kind: 'post-type', - }; - - const { result } = renderHook( () => - useEntityBinding( { - clientId: 'test-client-id', - attributes, - } ) + it( 'throws error when called without parameter', () => { + expect( () => { + buildNavigationLinkEntityBinding(); + } ).toThrow( + 'buildNavigationLinkEntityBinding requires a kind parameter' ); - - act( () => { - result.current.clearBinding(); - } ); - - expect( mockUpdateBlockBindings ).not.toHaveBeenCalled(); } ); - it( 'should call updateBlockBindings when clearBinding is called and binding exists with any source', () => { - const attributes = { - metadata: { - bindings: { - url: { - source: 'core/post-data', - args: { key: 'link' }, - }, - }, - }, - id: 123, - kind: 'post-type', // Correct kind for post-data source - }; - - const { result } = renderHook( () => - useEntityBinding( { - clientId: 'test-client-id', - attributes, - } ) - ); + it( 'throws error for invalid kind', () => { + expect( () => { + buildNavigationLinkEntityBinding( 'invalid-kind' ); + } ).toThrow( 'Invalid kind "invalid-kind"' ); + } ); - act( () => { - result.current.clearBinding(); - } ); + it( 'throws error for null kind', () => { + expect( () => { + buildNavigationLinkEntityBinding( null ); + } ).toThrow( 'Invalid kind "null"' ); + } ); - expect( mockUpdateBlockBindings ).toHaveBeenCalledWith( { - url: undefined, - } ); + it( 'throws error for empty string', () => { + expect( () => { + buildNavigationLinkEntityBinding( '' ); + } ).toThrow( 'Invalid kind ""' ); } ); - } ); - describe( 'createBinding behavior', () => { - it( 'should not create binding when createBinding is called without kind', () => { + it( 'handles invalid kind gracefully in createBinding', () => { + const consoleSpy = jest + .spyOn( console, 'warn' ) + .mockImplementation(); + const attributes = { metadata: {}, id: null, - kind: null, + kind: 'invalid-kind', }; const { result } = renderHook( () => @@ -443,39 +348,15 @@ describe( 'useEntityBinding', () => { result.current.createBinding(); } ); - expect( mockUpdateBlockBindings ).not.toHaveBeenCalled(); - } ); - - it( 'should create binding with updated attributes when createBinding is called with updatedAttributes', () => { - const attributes = { - metadata: {}, - id: null, - kind: 'post-type', - }; - - const updatedAttributes = { - kind: 'taxonomy', - }; - - const { result } = renderHook( () => - useEntityBinding( { - clientId: 'test-client-id', - attributes, - } ) + expect( consoleSpy ).toHaveBeenCalledWith( + 'Failed to create entity binding:', + expect.stringContaining( 'Invalid kind "invalid-kind"' ) ); - act( () => { - result.current.createBinding( updatedAttributes ); - } ); + // Should not call updateBlockBindings when validation fails + expect( mockUpdateBlockBindings ).not.toHaveBeenCalled(); - expect( mockUpdateBlockBindings ).toHaveBeenCalledWith( { - url: { - source: 'core/term-data', - args: { - key: 'link', - }, - }, - } ); + consoleSpy.mockRestore(); } ); } ); } ); diff --git a/packages/block-library/src/navigation-link/shared/use-entity-binding.js b/packages/block-library/src/navigation-link/shared/use-entity-binding.js index 6e164f24955e71..6dbbe8652bf8c2 100644 --- a/packages/block-library/src/navigation-link/shared/use-entity-binding.js +++ b/packages/block-library/src/navigation-link/shared/use-entity-binding.js @@ -4,6 +4,46 @@ import { useCallback } from '@wordpress/element'; import { useBlockBindingsUtils } from '@wordpress/block-editor'; +/** + * 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). + * + * @param {('post-type'|'taxonomy')} kind - The kind of entity. Only 'post-type' and 'taxonomy' are supported. + * @return {Object} Entity binding configuration object + * @throws {Error} If kind is not 'post-type' or 'taxonomy' + */ +export function buildNavigationLinkEntityBinding( kind ) { + // Validate kind parameter exists + if ( kind === undefined ) { + throw new Error( + 'buildNavigationLinkEntityBinding requires a kind parameter. ' + + 'Only "post-type" and "taxonomy" are supported.' + ); + } + + // Validate kind parameter value + if ( kind !== 'post-type' && kind !== 'taxonomy' ) { + throw new Error( + `Invalid kind "${ kind }" provided to buildNavigationLinkEntityBinding. ` + + `Only 'post-type' and 'taxonomy' are supported.` + ); + } + + const source = kind === 'taxonomy' ? 'core/term-data' : 'core/post-data'; + + return { + url: { + source, + args: { + key: 'link', + }, + }, + }; +} + /** * Shared hook for entity binding functionality in Navigation blocks. * @@ -42,19 +82,17 @@ export function useEntityBinding( { clientId, attributes } ) { return; } - // Default to post-type in case there is a need to support dynamic kinds - // in the future. - const source = - kindToUse === 'taxonomy' ? 'core/term-data' : 'core/post-data'; - - updateBlockBindings( { - url: { - source, - args: { - key: 'link', - }, - }, - } ); + try { + const binding = buildNavigationLinkEntityBinding( kindToUse ); + updateBlockBindings( binding ); + } catch ( error ) { + // eslint-disable-next-line no-console + console.warn( + 'Failed to create entity binding:', + error.message + ); + // Don't create binding if validation fails + } }, [ updateBlockBindings, kind, id ] ); diff --git a/packages/block-library/src/page-list/test/convert-to-links-modal.js b/packages/block-library/src/page-list/test/convert-to-navigation-links.js similarity index 87% rename from packages/block-library/src/page-list/test/convert-to-links-modal.js rename to packages/block-library/src/page-list/test/convert-to-navigation-links.js index 985bed8381cfaf..a3cbdc8f184ad5 100644 --- a/packages/block-library/src/page-list/test/convert-to-links-modal.js +++ b/packages/block-library/src/page-list/test/convert-to-navigation-links.js @@ -4,6 +4,16 @@ import { convertToNavigationLinks } from '../use-convert-to-navigation-links'; +// Expected entity binding structure for navigation links +const EXPECTED_ENTITY_BINDING = { + url: { + source: 'core/post-data', + args: { + key: 'link', + }, + }, +}; + // Mock createBlock to avoid creating the blocks in test environment // as convertToNavigationLinks calls this method internally. jest.mock( '@wordpress/blocks', () => { @@ -117,6 +127,9 @@ describe( 'page list convert to links', () => { label: 'Sample Page', type: 'page', url: 'http://wordpress.local/sample-page/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -128,6 +141,9 @@ describe( 'page list convert to links', () => { label: 'About', type: 'page', url: 'http://wordpress.local/about/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [ { @@ -137,6 +153,9 @@ describe( 'page list convert to links', () => { label: 'About Sub 1', type: 'page', url: 'http://wordpress.local/about/about-sub-1/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -148,6 +167,9 @@ describe( 'page list convert to links', () => { label: 'About Sub 2', type: 'page', url: 'http://wordpress.local/about/about-sub-2/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -162,6 +184,9 @@ describe( 'page list convert to links', () => { label: 'Contact Page', type: 'page', url: 'http://wordpress.local/contact-page/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -173,6 +198,9 @@ describe( 'page list convert to links', () => { label: 'Test', type: 'page', url: 'http://wordpress.local/test/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [ { @@ -182,6 +210,9 @@ describe( 'page list convert to links', () => { label: 'Test Sub', type: 'page', url: 'http://wordpress.local/test/test-sub/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [ { @@ -191,6 +222,9 @@ describe( 'page list convert to links', () => { label: 'Test Sub Sub', type: 'page', url: 'http://wordpress.local/test/test-sub/test-sub-sub/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -297,6 +331,9 @@ describe( 'page list convert to links', () => { label: 'Sample Page', type: 'page', url: 'http://wordpress.local/sample-page/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -308,6 +345,9 @@ describe( 'page list convert to links', () => { label: 'About', type: 'page', url: 'http://wordpress.local/about/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [ { @@ -317,6 +357,9 @@ describe( 'page list convert to links', () => { label: 'About Sub 1', type: 'page', url: 'http://wordpress.local/about/about-sub-1/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -328,6 +371,9 @@ describe( 'page list convert to links', () => { label: 'About Sub 2', type: 'page', url: 'http://wordpress.local/about/about-sub-2/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -342,6 +388,9 @@ describe( 'page list convert to links', () => { label: 'Contact Page', type: 'page', url: 'http://wordpress.local/contact-page/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -353,6 +402,9 @@ describe( 'page list convert to links', () => { label: 'Test', type: 'page', url: 'http://wordpress.local/test/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [ { @@ -362,6 +414,9 @@ describe( 'page list convert to links', () => { label: 'Test Sub', type: 'page', url: 'http://wordpress.local/test/test-sub/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [ { @@ -371,6 +426,9 @@ describe( 'page list convert to links', () => { label: 'Test Sub Sub', type: 'page', url: 'http://wordpress.local/test/test-sub/test-sub-sub/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -481,6 +539,9 @@ describe( 'page list convert to links', () => { label: 'About Sub 1', type: 'page', url: 'http://wordpress.local/about/about-sub-1/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -492,6 +553,9 @@ describe( 'page list convert to links', () => { label: 'About Sub 2', type: 'page', url: 'http://wordpress.local/about/about-sub-2/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', @@ -511,6 +575,9 @@ describe( 'page list convert to links', () => { label: 'Test Sub Sub', type: 'page', url: 'http://wordpress.local/test/test-sub/test-sub-sub/', + metadata: { + bindings: EXPECTED_ENTITY_BINDING, + }, }, innerBlocks: [], name: 'core/navigation-link', diff --git a/packages/block-library/src/page-list/use-convert-to-navigation-links.js b/packages/block-library/src/page-list/use-convert-to-navigation-links.js index 4cbc69d6e6de66..a32a334c29ef2c 100644 --- a/packages/block-library/src/page-list/use-convert-to-navigation-links.js +++ b/packages/block-library/src/page-list/use-convert-to-navigation-links.js @@ -5,6 +5,11 @@ import { createBlock } from '@wordpress/blocks'; import { useDispatch } from '@wordpress/data'; import { store as blockEditorStore } from '@wordpress/block-editor'; +/** + * Internal dependencies + */ +import { buildNavigationLinkEntityBinding } from '../navigation-link/shared'; + /** * Converts an array of pages into a nested array of navigation link blocks. * @@ -13,6 +18,7 @@ import { store as blockEditorStore } from '@wordpress/block-editor'; * @return {Array} A nested array of navigation link blocks. */ function createNavigationLinks( pages = [] ) { + const POST_TYPE_KIND = 'post-type'; const linkMap = {}; const navigationLinks = []; pages.forEach( ( { id, title, link: url, type, parent } ) => { @@ -25,7 +31,11 @@ function createNavigationLinks( pages = [] ) { label: title.rendered, url, type, - kind: 'post-type', + kind: POST_TYPE_KIND, + metadata: { + bindings: + buildNavigationLinkEntityBinding( POST_TYPE_KIND ), + }, }, innerBlocks ); diff --git a/test/e2e/specs/editor/blocks/navigation.spec.js b/test/e2e/specs/editor/blocks/navigation.spec.js index 01ee0223d88d66..da035949422dcd 100644 --- a/test/e2e/specs/editor/blocks/navigation.spec.js +++ b/test/e2e/specs/editor/blocks/navigation.spec.js @@ -1125,6 +1125,105 @@ test.describe( 'Navigation block', () => { await expect( linkInput ).toBeEnabled(); await expect( linkInput ).toHaveValue( testPage1.link ); } ); + + test( 'Page List converts to Navigation Links with entity bindings', async ( { + editor, + page, + admin, + requestUtils, + } ) => { + // Step 1: Create menu with Page List block + const menu = await requestUtils.createNavigationMenu( { + title: 'Test Menu with Page List', + content: '', + } ); + + // Step 2: Insert Navigation block + await admin.createNewPost(); + + await editor.insertBlock( { + name: 'core/navigation', + attributes: { + ref: menu.id, + }, + } ); + + // Step 3: Verify Page List is present + const pageListBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Page List', + } ); + await expect( pageListBlock ).toBeVisible(); + + // Verify pages are shown in the list + const pageItems = pageListBlock.locator( 'li' ); + + // Wait for Page List to load pages + await pageItems.first().waitFor( { state: 'visible' } ); + const itemCount = await pageItems.count(); + expect( itemCount ).toBeGreaterThan( 0 ); + + // Step 4: Convert Page List using Edit button + // Select the Page List block + await editor.selectBlocks( pageListBlock ); + + // Try using the toolbar Edit button instead + const editButton = page + .getByRole( 'button', { name: 'Edit' } ) + .first(); + await expect( editButton ).toBeVisible(); + + await editButton.click(); + + // Wait for modal and approve conversion + await expect( + page.getByRole( 'dialog', { name: 'Edit Page List' } ) + ).toBeVisible(); + + await page.getByRole( 'button', { name: 'Edit' } ).last().click(); + + // Wait for conversion - check that Page List is gone + await expect( pageListBlock ).toBeHidden(); + + // Step 5: Verify conversion to entity links + + // Get Navigation block + const navBlock = editor.canvas.getByRole( 'document', { + name: 'Block: Navigation', + } ); + + // Should have Navigation Link blocks + const navLinkBlocks = navBlock.getByRole( 'document', { + name: 'Block: Page Link', + } ); + + const linkCount = await navLinkBlocks.count(); + expect( linkCount ).toBeGreaterThan( 0 ); + + // Select first link and verify binding + const navLinkBlock = navLinkBlocks.first(); + await editor.selectBlocks( navLinkBlock ); + + // Open sidebar to check Link field + await editor.openDocumentSettingsSidebar(); + const settingsControls = page + .getByRole( 'region', { name: 'Editor settings' } ) + .getByRole( 'tabpanel', { name: 'Settings' } ); + + await expect( settingsControls ).toBeVisible(); + + // Verify Link field is disabled (indicating binding is active) + const linkInput = settingsControls.getByRole( 'textbox', { + name: 'Link', + description: 'Synced with the selected page', + } ); + + await expect( linkInput ).toBeDisabled(); + + // Verify help text is present + await expect( + settingsControls.getByText( 'Synced with the selected page.' ) + ).toBeVisible(); + } ); } ); } );