diff --git a/src/index.js b/src/index.js index b738e27..03048c2 100644 --- a/src/index.js +++ b/src/index.js @@ -64,8 +64,15 @@ addFilter( * Ensure each Query Loop block has a unique queryId attribute. * * WordPress core does not deduplicate queryId when blocks are duplicated. - * This HOC generates a unique queryId from the current post ID and the - * block's index among all query blocks on the page. + * This HOC only assigns a new id when a block's queryId is missing or is + * already claimed by an earlier query block, so genuinely duplicated ids are + * fixed once while already-unique ids are left untouched. + * + * It deliberately does not force a strictly index-based value on every load: + * doing so rewrites valid, unique ids whenever they don't line up with the + * current block order (e.g. after a block is deleted or reordered), which marks + * the post as changed the instant the editor opens and surfaces a false + * "unsaved changes" / "Leave site?" warning. */ const withUniqueQueryId = createHigherOrderComponent( ( BlockEdit ) => { return ( props ) => { @@ -75,29 +82,61 @@ const withUniqueQueryId = createHigherOrderComponent( ( BlockEdit ) => { return ; } - const expectedQueryId = useSelect( + const uniqueQueryId = useSelect( ( select ) => { + const blockEditor = select( 'core/block-editor' ); + const allQueryBlocks = + blockEditor.getBlocksByName( 'core/query' ); + const index = allQueryBlocks.indexOf( clientId ); + + // Bail if the block is not (yet) part of the tree; a transient + // state where reassigning would be unsafe. + if ( index === -1 ) { + return null; + } + + const currentId = attributes.queryId; + const idOf = ( id ) => + blockEditor.getBlockAttributes( id )?.queryId; + + // Keep the existing id if it is set and not already used by an + // earlier query block. Only missing ids or later duplicates get + // reassigned, so unique ids are never rewritten on load. This + // mirrors the server-side deduplicate_query_ids() behaviour. + const earlierIds = allQueryBlocks.slice( 0, index ).map( idOf ); + if ( currentId && ! earlierIds.includes( currentId ) ) { + return null; + } + let postId = 0; try { postId = select( 'core/editor' )?.getCurrentPostId?.() || 0; } catch { postId = 0; } - const allQueryBlocks = - select( 'core/block-editor' ).getBlocksByName( - 'core/query' - ); - const index = allQueryBlocks.indexOf( clientId ); - return postId * 1000 + index + 1; + + // Generate a unique id, skipping any already used by another + // query block on the page. + const otherIds = allQueryBlocks + .filter( ( id ) => id !== clientId ) + .map( idOf ); + let candidate = postId * 1000 + index + 1; + while ( otherIds.includes( candidate ) ) { + candidate++; + } + return candidate; }, - [ clientId ] + [ clientId, attributes.queryId ] ); useEffect( () => { - if ( attributes.queryId !== expectedQueryId ) { - setAttributes( { queryId: expectedQueryId } ); + if ( + uniqueQueryId !== null && + attributes.queryId !== uniqueQueryId + ) { + setAttributes( { queryId: uniqueQueryId } ); } - }, [ expectedQueryId, attributes.queryId, setAttributes ] ); + }, [ uniqueQueryId, attributes.queryId, setAttributes ] ); return ; }; diff --git a/tests/e2e/unique-query-id.spec.js b/tests/e2e/unique-query-id.spec.js index 9082a54..60b1003 100644 --- a/tests/e2e/unique-query-id.spec.js +++ b/tests/e2e/unique-query-id.spec.js @@ -118,8 +118,6 @@ test.describe( 'Unique Query IDs', () => { } ); } ); - console.log( 'Query IDs in editor:', queryIds ); - // Verify we have 3 query loops expect( queryIds ).toHaveLength( 3 ); @@ -131,10 +129,135 @@ test.describe( 'Unique Query IDs', () => { // Verify all queryIds are unique const uniqueIds = [ ...new Set( queryIds ) ]; - console.log( 'Unique query IDs:', uniqueIds ); expect( uniqueIds ).toHaveLength( 3 ); } ); + test( 'should not mark post as dirty when editor reloads with already-unique queryIds', async ( { + page, + admin, + } ) => { + // Create a new page with a Query Loop block + await admin.createNewPost( { postType: 'page' } ); + await page.waitForTimeout( 1500 ); + + const closeButton = page.getByRole( 'button', { name: 'Close' } ); + if ( + await closeButton + .isVisible( { timeout: 1000 } ) + .catch( () => false ) + ) { + await closeButton.click(); + } + + const canvas = page + .locator( 'iframe[name="editor-canvas"]' ) + .contentFrame(); + + await canvas.getByRole( 'textbox', { name: 'Add title' } ).click(); + await canvas + .getByRole( 'textbox', { name: 'Add title' } ) + .fill( 'Test no dirty on load' ); + + await canvas + .getByRole( 'button', { name: 'Add default block' } ) + .click(); + await canvas + .getByRole( 'document', { + name: 'Empty block; start writing or', + } ) + .fill( '/query' ); + await page + .getByRole( 'option', { name: 'Query Loop' } ) + .first() + .click(); + await page.waitForTimeout( 1000 ); + + const startBlankButton = canvas.getByRole( 'button', { + name: 'Start blank', + } ); + if ( + await startBlankButton + .isVisible( { timeout: 2000 } ) + .catch( () => false ) + ) { + await startBlankButton.click(); + await page.waitForTimeout( 500 ); + } + + const patternButton = canvas.getByRole( 'button', { + name: 'Title & Date', + } ); + if ( + await patternButton + .isVisible( { timeout: 2000 } ) + .catch( () => false ) + ) { + await patternButton.click(); + await page.waitForTimeout( 500 ); + } + + // Give the HOC time to assign a queryId + await page.waitForTimeout( 1000 ); + + const initialQueryId = await page.evaluate( () => { + const blocks = window.wp.data + .select( 'core/block-editor' ) + .getBlocksByName( 'core/query' ); + const block = window.wp.data + .select( 'core/block-editor' ) + .getBlock( blocks[ 0 ] ); + return block.attributes.queryId; + } ); + + expect( typeof initialQueryId ).toBe( 'number' ); + + // Publish the page + await page + .getByRole( 'button', { name: 'Publish', exact: true } ) + .click(); + await page + .getByLabel( 'Editor publish' ) + .getByRole( 'button', { name: 'Publish', exact: true } ) + .click(); + await page.waitForTimeout( 1500 ); + + const postId = await page.evaluate( () => { + return window.wp.data.select( 'core/editor' ).getCurrentPostId(); + } ); + + // Reopen the editor — this is the scenario where the false dirty + // warning appeared before the fix: the HOC was rewriting the + // queryId on every load, marking the post as changed immediately. + await admin.visitAdminPage( `post.php?post=${ postId }&action=edit` ); + await page.waitForSelector( 'iframe[name="editor-canvas"]', { + timeout: 15000, + } ); + // Wait for Gutenberg to fully initialise and for all useEffect + // callbacks (including our withUniqueQueryId HOC) to run. + await page.waitForTimeout( 2000 ); + + const reloadedQueryId = await page.evaluate( () => { + const blocks = window.wp.data + .select( 'core/block-editor' ) + .getBlocksByName( 'core/query' ); + const block = window.wp.data + .select( 'core/block-editor' ) + .getBlock( blocks[ 0 ] ); + return block.attributes.queryId; + } ); + + // The queryId must not have changed — stable ids prevent rewriting. + expect( reloadedQueryId ).toBe( initialQueryId ); + + // The editor must not be dirty; any attribute rewrite on load would + // have triggered the "unsaved changes / Leave site?" warning. + const isDirty = await page.evaluate( () => { + return window.wp.data.select( 'core/editor' ).isEditedPostDirty(); + } ); + + expect( isDirty ).toBe( false ); + } ); + test( 'should have unique queryId in saved content and exclude posts correctly on front end', async ( { page, admin, @@ -238,7 +361,6 @@ test.describe( 'Unique Query IDs', () => { } ); } ); - console.log( 'Query IDs before publish:', queryIds ); expect( queryIds ).toHaveLength( 2 ); const uniqueIds = [ ...new Set( queryIds ) ]; expect( uniqueIds ).toHaveLength( 2 ); @@ -251,9 +373,6 @@ test.describe( 'Unique Query IDs', () => { .locator( '.wp-block-post-template .wp-block-post-title' ) .allTextContents(); - console.log( 'All post titles on page:', allPostTitles ); - console.log( 'Total posts:', allPostTitles.length ); - // Both query loops should have posts (each shows 10 by default) expect( allPostTitles.length ).toBe( 20 ); @@ -261,7 +380,6 @@ test.describe( 'Unique Query IDs', () => { // the second loop should exclude posts from the first. // All 20 titles should be unique. const uniqueTitles = [ ...new Set( allPostTitles ) ]; - console.log( 'Unique posts:', uniqueTitles.length ); expect( uniqueTitles ).toHaveLength( 20 ); } ); } );