diff --git a/assets/src/js/bindings/block-editor.js b/assets/src/js/bindings/block-editor.js index 8b8464b2..792719e3 100644 --- a/assets/src/js/bindings/block-editor.js +++ b/assets/src/js/bindings/block-editor.js @@ -9,6 +9,8 @@ import { useBlockBindingsUtils, } from '@wordpress/block-editor'; import { + Button, + Notice, ComboboxControl, __experimentalToolsPanel as ToolsPanel, __experimentalToolsPanelItem as ToolsPanelItem, @@ -48,8 +50,12 @@ const withCustomControls = createHigherOrderComponent( ( BlockEdit ) => { // Get fields based on editor context const postEditorFields = usePostEditorFields(); - const { fields: siteEditorFields } = - useSiteEditorFields( templatePostType ); + const { + fields: siteEditorFields, + isLoading: isLoadingSiteEditorFields, + error: siteEditorFieldsError, + refresh: refreshSiteEditorFields, + } = useSiteEditorFields( templatePostType ); // Use appropriate fields based on context const activeFields = isSiteEditor ? siteEditorFields : postEditorFields; @@ -156,6 +162,24 @@ const withCustomControls = createHigherOrderComponent( ( BlockEdit ) => { ) } resetAll={ handleReset } > + { isSiteEditor && siteEditorFieldsError && ( + +

+ { __( + 'Unable to load custom fields. Please try again.', + 'secure-custom-fields' + ) } +

+ + + ) } { canUseAllAttributesMode ? ( diff --git a/assets/src/js/bindings/hooks.js b/assets/src/js/bindings/hooks.js index 62232f57..8f134529 100644 --- a/assets/src/js/bindings/hooks.js +++ b/assets/src/js/bindings/hooks.js @@ -2,12 +2,10 @@ * Custom hooks for block bindings */ -import { useState, useEffect } from '@wordpress/element'; -import { useSelect } from '@wordpress/data'; +import { useState, useEffect, useMemo, useCallback } from '@wordpress/element'; +import { useSelect, useDispatch } from '@wordpress/data'; import { store as coreDataStore } from '@wordpress/core-data'; import { store as editorStore } from '@wordpress/editor'; -import apiFetch from '@wordpress/api-fetch'; -import { addQueryArgs } from '@wordpress/url'; import { extractPostTypeFromTemplate, formatFieldGroupsData } from './utils'; import { addFieldMetadata } from './fieldMetadataCache'; @@ -94,63 +92,55 @@ export function usePostEditorFields() { * * @since 6.7.0 * @param {string|null} postType The post type to fetch fields for. - * @return {Object} Object containing fields, isLoading, and error. + * @return {Object} Fields, loading/error state, and an explicit refresh callback. */ export function useSiteEditorFields( postType ) { - const [ fields, setFields ] = useState( {} ); - const [ isLoading, setIsLoading ] = useState( false ); - const [ error, setError ] = useState( null ); - - useEffect( () => { - if ( ! postType ) { - setFields( {} ); - setIsLoading( false ); - setError( null ); - return; - } - - let isCancelled = false; - setIsLoading( true ); - setError( null ); - - const fetchFields = async () => { - try { - const path = addQueryArgs( `/wp/v2/types/${ postType }`, { - context: 'edit', - } ); - - const postTypeData = await apiFetch( { path } ); - - if ( isCancelled ) { - return; - } + const { invalidateResolution } = useDispatch( coreDataStore ); + // Share one getPostType resolution per type within this registry. + // root/postType records default to context=edit, which includes + // scf_field_groups; use the same selector key for status and refresh. + const { record, isLoading, error } = useSelect( + ( select ) => { + if ( ! postType ) { + return { record: null, isLoading: false, error: null }; + } - const fieldsMap = formatFieldGroupsData( - postTypeData.scf_field_groups - ); + const core = select( coreDataStore ); - // Store field metadata in the data store - addFieldMetadata( fieldsMap ); + return { + record: core.getPostType( postType ), + isLoading: ! core.hasFinishedResolution( 'getPostType', [ + postType, + ] ), + error: + core.getResolutionError( 'getPostType', [ postType ] ) || + null, + }; + }, + [ postType ] + ); - setFields( fieldsMap ); - setIsLoading( false ); - } catch ( err ) { - if ( ! isCancelled ) { - setError( err ); - setIsLoading( false ); - } - } - }; + // Keep the stored fields visible while a refetch is in progress. + const fields = useMemo( + () => formatFieldGroupsData( record?.scf_field_groups ), + [ record ] + ); - fetchFields(); + useEffect( () => { + if ( record ) { + addFieldMetadata( fields ); + } + }, [ record, fields ] ); - // Cleanup function to prevent state updates after unmount - return () => { - isCancelled = true; - }; - }, [ postType ] ); + // One consumer must not cancel work shared by the rest of the registry. + // Explicit invalidation also permits retry without a per-mount retry storm. + const refresh = useCallback( () => { + if ( postType ) { + invalidateResolution( 'getPostType', [ postType ] ); + } + }, [ postType, invalidateResolution ] ); - return { fields, isLoading, error }; + return { fields, isLoading, error, refresh }; } /** diff --git a/assets/src/js/bindings/utils.js b/assets/src/js/bindings/utils.js index 1b10b3aa..2aeb96ea 100644 --- a/assets/src/js/bindings/utils.js +++ b/assets/src/js/bindings/utils.js @@ -43,9 +43,9 @@ export function getAllowedFieldTypes( blockName, attribute = null ) { * Filters field options based on allowed field types. * * @since 6.7.0 - * @param {Array} fieldOptions Array of field option objects with value, label, and type. - * @param {string} blockName The name of the block. - * @param {string|null} attribute The attribute name, or null for all types. + * @param {Array} fieldOptions Array of field option objects with value, label, and type. + * @param {string} blockName The name of the block. + * @param {string|null} attribute The attribute name, or null for all types. * @return {Array} Filtered array of field options. */ export function getFilteredFieldOptions( @@ -142,8 +142,11 @@ export function formatFieldGroupsData( fieldGroups ) { } fieldGroups.forEach( ( group ) => { - if ( Array.isArray( group.fields ) ) { + if ( Array.isArray( group?.fields ) ) { group.fields.forEach( ( field ) => { + if ( ! field?.name ) { + return; + } fieldsMap[ field.name ] = { label: field.label, type: field.type, diff --git a/tests/js/bindings/block-editor-recovery.test.js b/tests/js/bindings/block-editor-recovery.test.js new file mode 100644 index 00000000..4dc474c1 --- /dev/null +++ b/tests/js/bindings/block-editor-recovery.test.js @@ -0,0 +1,120 @@ +/** + * Inspector recovery from shared Site Editor schema failures. + */ +import { fireEvent, render, screen } from '@testing-library/react'; +import { addFilter } from '@wordpress/hooks'; +import { + useSiteEditorContext, + useSiteEditorFields, +} from '../../../assets/src/js/bindings/hooks'; + +jest.mock( '@wordpress/hooks', () => ( { addFilter: jest.fn() } ) ); +jest.mock( '@wordpress/compose', () => ( { + createHigherOrderComponent: ( callback ) => callback, +} ) ); +jest.mock( '@wordpress/block-editor', () => ( { + InspectorControls: ( { children } ) =>
{ children }
, + useBlockBindingsUtils: () => ( { + updateBlockBindings: jest.fn(), + removeAllBlockBindings: jest.fn(), + } ), +} ) ); +jest.mock( '@wordpress/components', () => ( { + Notice: ( { children } ) =>
{ children }
, + Button: ( { children, onClick, disabled, isBusy } ) => ( + + ), + ComboboxControl: ( { options } ) => ( + + ), + __experimentalToolsPanel: ( { children } ) =>
{ children }
, + __experimentalToolsPanelItem: ( { children } ) =>
{ children }
, +} ) ); +jest.mock( '../../../assets/src/js/bindings/hooks', () => ( { + useSiteEditorContext: jest.fn(), + useSiteEditorFields: jest.fn(), + usePostEditorFields: () => ( { + field1: { label: 'Field 1', type: 'text' }, + } ), + useBoundFields: () => ( { boundFields: {}, setBoundFields: jest.fn() } ), +} ) ); + +require( '../../../assets/src/js/bindings/block-editor' ); +const withControls = addFilter.mock.calls[ 0 ][ 2 ]; +const BlockEdit = () =>
Block content
; +const EnhancedBlock = withControls( BlockEdit ); +const renderBlock = () => ( + +); + +describe( 'Site Editor field recovery', () => { + let state; + + beforeEach( () => { + state = { + fields: {}, + isLoading: false, + error: new Error( 'Private REST error details' ), + refresh: jest.fn(), + }; + useSiteEditorFields.mockImplementation( () => state ); + useSiteEditorContext.mockReturnValue( { + isSiteEditor: true, + templatePostType: 'product', + } ); + } ); + + it( 'shows a generic error and retries only on user action', () => { + const { rerender } = render( renderBlock() ); + expect( screen.getByRole( 'alert' ).textContent ).toContain( + 'Unable to load custom fields. Please try again.' + ); + expect( screen.queryByText( 'Private REST error details' ) ).toBeNull(); + rerender( renderBlock() ); + expect( state.refresh ).not.toHaveBeenCalled(); + fireEvent.click( screen.getByRole( 'button', { name: 'Retry' } ) ); + expect( state.refresh ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'disables retry while loading and keeps cached field options', () => { + state.isLoading = true; + state.fields = { field1: { label: 'Field 1', type: 'text' } }; + render( renderBlock() ); + const retry = screen.getByRole( 'button', { name: 'Retry' } ); + expect( retry.disabled ).toBe( true ); + expect( retry.getAttribute( 'aria-busy' ) ).toBe( 'true' ); + fireEvent.click( retry ); + expect( state.refresh ).not.toHaveBeenCalled(); + expect( screen.getByText( 'Field 1' ) ).toBeTruthy(); + } ); + + it( 'removes the notice and shows field options after successful recovery', () => { + const { rerender } = render( renderBlock() ); + fireEvent.click( screen.getByRole( 'button', { name: 'Retry' } ) ); + state.error = null; + state.fields = { field1: { label: 'Field 1', type: 'text' } }; + rerender( renderBlock() ); + expect( screen.queryByRole( 'alert' ) ).toBeNull(); + expect( screen.queryByRole( 'button', { name: 'Retry' } ) ).toBeNull(); + expect( screen.getByText( 'Field 1' ) ).toBeTruthy(); + expect( state.refresh ).toHaveBeenCalledTimes( 1 ); + } ); + + it( 'does not show the Site Editor error in the Post Editor', () => { + useSiteEditorContext.mockReturnValue( { + isSiteEditor: false, + templatePostType: null, + } ); + render( renderBlock() ); + expect( screen.queryByRole( 'alert' ) ).toBeNull(); + expect( screen.queryByRole( 'button', { name: 'Retry' } ) ).toBeNull(); + expect( screen.getByText( 'Field 1' ) ).toBeTruthy(); + expect( state.refresh ).not.toHaveBeenCalled(); + } ); +} ); diff --git a/tests/js/bindings/hooks.test.js b/tests/js/bindings/hooks.test.js index ffe75e52..d6c65150 100644 --- a/tests/js/bindings/hooks.test.js +++ b/tests/js/bindings/hooks.test.js @@ -2,8 +2,8 @@ * Unit tests for custom hooks */ -import { renderHook, waitFor } from '@testing-library/react'; -import { useSelect } from '@wordpress/data'; +import { renderHook } from '@testing-library/react'; +import { useSelect, useDispatch } from '@wordpress/data'; import apiFetch from '@wordpress/api-fetch'; // Mock the fieldMetadataCache module @@ -265,209 +265,191 @@ describe( 'Custom Hooks', () => { } ); describe( 'useSiteEditorFields', () => { - const mockFieldGroups = [ + const groups = [ { - title: 'Group 1', - fields: [ - { name: 'field1', label: 'Field 1', type: 'text' }, - { name: 'field2', label: 'Field 2', type: 'textarea' }, - ], + title: 'Group', + fields: [ { name: 'field1', label: 'Field 1', type: 'text' } ], }, ]; + let core, invalidateResolution; + + beforeEach( () => { + core = { + getPostType: jest.fn( () => ( { + scf_field_groups: groups, + } ) ), + getResolutionError: jest.fn( () => null ), + hasFinishedResolution: jest.fn( () => true ), + }; + invalidateResolution = jest.fn(); + useDispatch.mockReturnValue( { invalidateResolution } ); + useSelect.mockImplementation( ( callback ) => + callback( () => core ) + ); + } ); - it( 'should fetch fields for a post type', async () => { - apiFetch.mockResolvedValue( { - scf_field_groups: mockFieldGroups, - } ); - + it( 'should fetch fields for a post type', () => { const { result } = renderHook( () => useSiteEditorFields( 'product' ) ); - - expect( result.current.isLoading ).toBe( true ); - - await waitFor( () => { - expect( result.current.isLoading ).toBe( false ); - } ); - + expect( core.getPostType ).toHaveBeenCalledWith( 'product' ); expect( result.current.fields ).toEqual( { field1: { label: 'Field 1', type: 'text' }, - field2: { label: 'Field 2', type: 'textarea' }, } ); - + expect( result.current.isLoading ).toBe( false ); expect( result.current.error ).toBeNull(); - expect( fieldMetadataCache.addFieldMetadata ).toHaveBeenCalled(); + expect( fieldMetadataCache.addFieldMetadata ).toHaveBeenCalledWith( + result.current.fields + ); + expect( apiFetch ).not.toHaveBeenCalled(); } ); - it( 'should handle API errors', async () => { - const error = new Error( 'API Error' ); - apiFetch.mockRejectedValue( error ); - + it( 'should store field metadata in cache on successful fetch', () => { const { result } = renderHook( () => useSiteEditorFields( 'product' ) ); - - await waitFor( () => { - expect( result.current.isLoading ).toBe( false ); - } ); - - expect( result.current.fields ).toEqual( {} ); - expect( result.current.error ).toBe( error ); + expect( fieldMetadataCache.addFieldMetadata ).toHaveBeenCalledWith( + result.current.fields + ); + expect( result.current.fields ).toHaveProperty( 'field1' ); } ); + it.each( [ 1, 10, 50, 200 ] )( + 'never starts a private fetch for %i consumers', + ( count ) => { + const consumers = Array.from( { length: count }, () => + renderHook( () => useSiteEditorFields( 'product' ) ) + ); + expect( apiFetch ).not.toHaveBeenCalled(); + consumers.forEach( ( { unmount } ) => unmount() ); + expect( invalidateResolution ).not.toHaveBeenCalled(); + } + ); + it( 'should reset when post type is null', () => { const { result } = renderHook( () => useSiteEditorFields( null ) ); - + result.current.refresh(); + expect( core.getPostType ).not.toHaveBeenCalled(); expect( result.current.fields ).toEqual( {} ); expect( result.current.isLoading ).toBe( false ); expect( result.current.error ).toBeNull(); + expect( invalidateResolution ).not.toHaveBeenCalled(); } ); - it( 'should call apiFetch with correct parameters', async () => { - apiFetch.mockResolvedValue( { - scf_field_groups: mockFieldGroups, - } ); - - renderHook( () => useSiteEditorFields( 'product' ) ); - - await waitFor( () => { - expect( apiFetch ).toHaveBeenCalledWith( { - path: '/wp/v2/types/product?context=edit', - } ); - } ); - } ); - - it( 'should update when post type changes', async () => { - apiFetch.mockResolvedValue( { - scf_field_groups: mockFieldGroups, - } ); - - const { rerender } = renderHook( - ( { postType } ) => useSiteEditorFields( postType ), - { initialProps: { postType: 'product' } } + it( 'keeps stored fields while a resolution is pending', () => { + core.hasFinishedResolution.mockReturnValue( false ); + const { result } = renderHook( () => + useSiteEditorFields( 'product' ) ); - - await waitFor( () => { - expect( apiFetch ).toHaveBeenCalledWith( { - path: '/wp/v2/types/product?context=edit', - } ); - } ); - - apiFetch.mockClear(); - - rerender( { postType: 'book' } ); - - await waitFor( () => { - expect( apiFetch ).toHaveBeenCalledWith( { - path: '/wp/v2/types/book?context=edit', - } ); + expect( result.current.isLoading ).toBe( true ); + expect( result.current.fields ).toEqual( { + field1: { label: 'Field 1', type: 'text' }, } ); + expect( result.current.error ).toBeNull(); } ); - it( 'should handle empty scf_field_groups', async () => { - apiFetch.mockResolvedValue( { - scf_field_groups: [], - } ); - + it( 'has no fields until a record is available', () => { + core.getPostType.mockReturnValue( undefined ); + core.hasFinishedResolution.mockReturnValue( false ); const { result } = renderHook( () => useSiteEditorFields( 'product' ) ); - - await waitFor( () => { - expect( result.current.isLoading ).toBe( false ); - } ); - expect( result.current.fields ).toEqual( {} ); - expect( result.current.error ).toBeNull(); + expect( result.current.isLoading ).toBe( true ); + expect( + fieldMetadataCache.addFieldMetadata + ).not.toHaveBeenCalled(); } ); - it( 'should handle missing scf_field_groups in response', async () => { - apiFetch.mockResolvedValue( {} ); - + it( 'should handle API errors', () => { + const error = new Error( 'Temporary failure' ); + core.getPostType.mockReturnValue( undefined ); + core.hasFinishedResolution.mockReturnValue( true ); + core.getResolutionError.mockReturnValue( error ); const { result } = renderHook( () => useSiteEditorFields( 'product' ) ); - - await waitFor( () => { - expect( result.current.isLoading ).toBe( false ); - } ); - + expect( result.current.isLoading ).toBe( false ); + expect( result.current.error ).toBe( error ); expect( result.current.fields ).toEqual( {} ); + expect( + fieldMetadataCache.addFieldMetadata + ).not.toHaveBeenCalled(); } ); - it( 'should cancel fetch on unmount', async () => { - // Use a promise that we can control - let resolvePromise; - const fetchPromise = new Promise( ( resolve ) => { - resolvePromise = resolve; - } ); - apiFetch.mockReturnValue( fetchPromise ); - - const { unmount } = renderHook( () => + it( 'refreshes exactly the shared resolution without automatic retries', () => { + core.getResolutionError.mockReturnValue( new Error( 'Failure' ) ); + const { result, rerender } = renderHook( () => useSiteEditorFields( 'product' ) ); - - // Unmount before the fetch resolves - unmount(); - - // Resolve the fetch after unmount - resolvePromise( { scf_field_groups: mockFieldGroups } ); - - // Wait a tick to ensure any state updates would have happened - await new Promise( ( resolve ) => setTimeout( resolve, 0 ) ); - - // If cleanup worked correctly, no state update errors should occur - // Verify test completed by checking unmount didn't throw - expect( true ).toBe( true ); + rerender(); + expect( invalidateResolution ).not.toHaveBeenCalled(); + result.current.refresh(); + expect( invalidateResolution ).toHaveBeenCalledTimes( 1 ); + expect( invalidateResolution ).toHaveBeenCalledWith( + 'getPostType', + [ 'product' ] + ); } ); - it( 'should handle post type changing from value to null', async () => { - apiFetch.mockResolvedValue( { - scf_field_groups: mockFieldGroups, - } ); - + it( 'should update when post type changes', () => { + core.getPostType.mockImplementation( ( type ) => + type === 'product' ? { scf_field_groups: groups } : undefined + ); + core.hasFinishedResolution.mockImplementation( + ( selector, args ) => args[ 0 ] === 'product' + ); const { result, rerender } = renderHook( - ( { postType } ) => useSiteEditorFields( postType ), - { initialProps: { postType: 'product' } } + ( { type } ) => useSiteEditorFields( type ), + { initialProps: { type: 'product' } } ); + expect( result.current.fields ).toHaveProperty( 'field1' ); + rerender( { type: 'book' } ); + expect( result.current.fields ).toEqual( {} ); + expect( result.current.isLoading ).toBe( true ); + } ); - await waitFor( () => { - expect( result.current.isLoading ).toBe( false ); - } ); - - expect( result.current.fields ).toEqual( { - field1: { label: 'Field 1', type: 'text' }, - field2: { label: 'Field 2', type: 'textarea' }, - } ); - - // Change to null - rerender( { postType: null } ); - + it( 'should handle post type changing from value to null', () => { + const { result, rerender } = renderHook( + ( { type } ) => useSiteEditorFields( type ), + { initialProps: { type: 'product' } } + ); + expect( result.current.fields ).toHaveProperty( 'field1' ); + rerender( { type: null } ); expect( result.current.fields ).toEqual( {} ); expect( result.current.isLoading ).toBe( false ); expect( result.current.error ).toBeNull(); } ); - it( 'should store field metadata in cache on successful fetch', async () => { - apiFetch.mockResolvedValue( { - scf_field_groups: mockFieldGroups, + it( 'keeps valid fields alongside malformed groups without a fetch error', () => { + core.getPostType.mockReturnValue( { + scf_field_groups: [ null, groups[ 0 ] ], } ); - const { result } = renderHook( () => useSiteEditorFields( 'product' ) ); - - await waitFor( () => { - expect( result.current.isLoading ).toBe( false ); + expect( result.current.fields ).toEqual( { + field1: { label: 'Field 1', type: 'text' }, } ); - + expect( result.current.error ).toBeNull(); + expect( result.current.isLoading ).toBe( false ); expect( fieldMetadataCache.addFieldMetadata ).toHaveBeenCalledWith( - { - field1: { label: 'Field 1', type: 'text' }, - field2: { label: 'Field 2', type: 'textarea' }, - } + result.current.fields ); } ); + + it.each( [ + [ 'empty scf_field_groups', { scf_field_groups: [] } ], + [ 'missing scf_field_groups in response', {} ], + ] )( 'should handle %s', ( description, record ) => { + core.getPostType.mockReturnValue( record ); + const { result } = renderHook( () => + useSiteEditorFields( 'product' ) + ); + expect( result.current.fields ).toEqual( {} ); + expect( result.current.isLoading ).toBe( false ); + expect( result.current.error ).toBeNull(); + } ); } ); describe( 'useBoundFields', () => { diff --git a/tests/js/bindings/utils.test.js b/tests/js/bindings/utils.test.js index 63fcebf8..83a18edc 100644 --- a/tests/js/bindings/utils.test.js +++ b/tests/js/bindings/utils.test.js @@ -272,6 +272,41 @@ describe( 'Block Bindings Utils', () => { } ); } ); + it( 'keeps valid fields alongside a null group', () => { + expect( + formatFieldGroupsData( [ null, mockFieldGroups[ 0 ] ] ) + ).toEqual( { + field1: { label: 'Field 1', type: 'text' }, + field2: { label: 'Field 2', type: 'textarea' }, + } ); + } ); + + it( 'ignores invalid groups', () => { + expect( + formatFieldGroupsData( [ + undefined, + 'string', + 42, + { fields: null }, + ] ) + ).toEqual( {} ); + } ); + + it( 'ignores fields without a name but keeps valid siblings', () => { + expect( + formatFieldGroupsData( [ + { + fields: [ + null, + {}, + { label: 'x' }, + mockFieldGroups[ 0 ].fields[ 0 ], + ], + }, + ] ) + ).toEqual( { field1: { label: 'Field 1', type: 'text' } } ); + } ); + it( 'should return empty object for empty array', () => { const result = formatFieldGroupsData( [] ); expect( result ).toEqual( {} );