Skip to content

Fix Page List to Navigation Link conversion with entity bindings #72287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
scruffian merged 4 commits into from
Oct 17, 2025
Merged

Fix Page List to Navigation Link conversion with entity bindings #72287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f472b30
Fix Page List to Navigation Link conversion with entity bindings
getdave Oct 13, 2025
f90bf64
Refactor to accommodate changes from trunk
getdave Oct 17, 2025
385e823
Fix expected binding in tests and rename for clarity
getdave Oct 17, 2025
1bfd369
Improve invalid binding kind
getdave Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/block-library/src/navigation-link/shared/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
215 changes: 48 additions & 167 deletions packages/block-library/src/navigation-link/shared/test/use-entity-binding.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ( {
Expand Down Expand Up @@ -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( () =>
Expand All @@ -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();
} );
} );
} );
64 changes: 51 additions & 13 deletions packages/block-library/src/navigation-link/shared/use-entity-binding.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

@scruffian scruffian Oct 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we only support post-type or taxonomy here? Right now I could pass some CPT and instead of erroring it would just use the post data binding.

@getdave getdave Oct 17, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we throw an error if the wrong type is used? I guess it's a severe problem. But we'd need to bail out of any entity creation as well.

@scruffian scruffian Oct 17, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should throw an error yeah. I'm not sure how gracefully we need to handle it, since it should never happen to a user - only a developer who is messing with things.

@getdave getdave Oct 17, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to throw error and NOT create the binding if error is encountered.


return {
url: {
source,
args: {
key: 'link',
},
},
};
}

/**
* Shared hook for entity binding functionality in Navigation blocks.
*
Expand Down Expand Up @@ -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 ]
);
Expand Down
Loading
Loading