diff --git a/packages/block-library/src/table/state.js b/packages/block-library/src/table/state.js index 961e0bd8415a46..7b9f5ba0317610 100644 --- a/packages/block-library/src/table/state.js +++ b/packages/block-library/src/table/state.js @@ -94,6 +94,7 @@ export function updateSelectedCell( state, selection, updateCell ) { } return { + ...row, cells: row.cells.map( ( cellAttributes, columnIndex ) => { const cellLocation = { @@ -248,6 +249,7 @@ export function insertColumn( state, { columnIndex } ) { } return { + ...row, cells: [ ...row.cells.slice( 0, columnIndex ), { @@ -290,6 +292,7 @@ export function deleteColumn( state, { columnIndex } ) { sectionName, section .map( ( row ) => ( { + ...row, cells: row.cells.length >= columnIndex ? row.cells.filter( diff --git a/packages/block-library/src/table/test/state.js b/packages/block-library/src/table/test/state.js index 1daa8e06919606..e62407d7a3d972 100644 --- a/packages/block-library/src/table/test/state.js +++ b/packages/block-library/src/table/test/state.js @@ -466,6 +466,30 @@ describe( 'insertColumn', () => { expect( state ).toEqual( expected ); } ); + it( 'preserves symbol row and cell properties when inserting a column', () => { + const syncId = Symbol( 'syncId' ); + const tableWithSymbolIdentity = deepFreeze( { + body: [ + { + [ syncId ]: 'row-1', + cells: [ + { + [ syncId ]: 'cell-1', + content: 'test', + tag: 'td', + }, + ], + }, + ], + } ); + const state = insertColumn( tableWithSymbolIdentity, { + columnIndex: 0, + } ); + + expect( state.body[ 0 ][ syncId ] ).toBe( 'row-1' ); + expect( state.body[ 0 ].cells[ 1 ][ syncId ] ).toBe( 'cell-1' ); + } ); + it( 'adds `th` cells to the head', () => { const state = insertColumn( tableWithHead, { columnIndex: 1, @@ -774,6 +798,34 @@ describe( 'deleteColumn', () => { expect( state ).toEqual( expected ); } ); + it( 'preserves symbol row and cell properties when deleting a column', () => { + const syncId = Symbol( 'syncId' ); + const tableWithSymbolIdentity = deepFreeze( { + body: [ + { + [ syncId ]: 'row-1', + cells: [ + { + content: 'remove', + tag: 'td', + }, + { + [ syncId ]: 'cell-2', + content: 'keep', + tag: 'td', + }, + ], + }, + ], + } ); + const state = deleteColumn( tableWithSymbolIdentity, { + columnIndex: 0, + } ); + + expect( state.body[ 0 ][ syncId ] ).toBe( 'row-1' ); + expect( state.body[ 0 ].cells[ 0 ][ syncId ] ).toBe( 'cell-2' ); + } ); + it( 'should delete all rows when only one column present', () => { const tableWithOneColumn = { body: [ @@ -1306,6 +1358,73 @@ describe( 'updateSelectedCell', () => { } ); } ); + it( 'preserves unknown row properties when updating a cell', () => { + const tableWithRowIdentity = deepFreeze( { + body: [ + { + __unstableSyncId: 'row-1', + cells: [ + { + content: '', + tag: 'td', + }, + ], + }, + ], + } ); + const cellSelection = { + type: 'cell', + sectionName: 'body', + rowIndex: 0, + columnIndex: 0, + }; + const updated = updateSelectedCell( + tableWithRowIdentity, + cellSelection, + ( cell ) => ( { + ...cell, + content: 'test', + } ) + ); + + expect( updated.body[ 0 ].__unstableSyncId ).toBe( 'row-1' ); + } ); + + it( 'preserves symbol row and cell properties when updating a cell', () => { + const syncId = Symbol( 'syncId' ); + const tableWithSymbolIdentity = deepFreeze( { + body: [ + { + [ syncId ]: 'row-1', + cells: [ + { + [ syncId ]: 'cell-1', + content: '', + tag: 'td', + }, + ], + }, + ], + } ); + const cellSelection = { + type: 'cell', + sectionName: 'body', + rowIndex: 0, + columnIndex: 0, + }; + const updated = updateSelectedCell( + tableWithSymbolIdentity, + cellSelection, + ( cell ) => ( { + ...cell, + content: 'test', + } ) + ); + + expect( updated.body[ 0 ][ syncId ] ).toBe( 'row-1' ); + expect( updated.body[ 0 ].cells[ 0 ][ syncId ] ).toBe( 'cell-1' ); + } ); + it( 'updates every cell in the column when the selection type is `column`', () => { const cellSelection = { type: 'column', columnIndex: 1 }; const updated = updateSelectedCell( diff --git a/packages/core-data/src/utils/crdt-blocks.ts b/packages/core-data/src/utils/crdt-blocks.ts index 268fc95afe03bf..cded84df7fc4ef 100644 --- a/packages/core-data/src/utils/crdt-blocks.ts +++ b/packages/core-data/src/utils/crdt-blocks.ts @@ -61,6 +61,8 @@ export type YBlocks = Y.Array< YBlock >; // Attribute values will be typed as the union of `Y.Text` and `unknown`. export type YBlockAttributes = Y.Map< Y.Text | unknown >; +const ARRAY_ELEMENT_ID_KEY = '__unstableSyncId'; +const ARRAY_ELEMENT_ID_SYMBOL = Symbol( 'wpSyncArrayElementId' ); const serializableBlocksCache = new WeakMap< WeakKey, Block[] >(); /** @@ -84,10 +86,20 @@ function serializeAttributeValue( value: unknown ): unknown { // e.g. a single row inside core/table `body`: { cells: [ ... ] } if ( value && typeof value === 'object' ) { const result: Record< string, unknown > = {}; + const arrayElementId = getArrayElementId( value ); for ( const [ k, v ] of Object.entries( value ) ) { + if ( k === ARRAY_ELEMENT_ID_KEY ) { + continue; + } + result[ k ] = serializeAttributeValue( v ); } + + if ( arrayElementId ) { + result[ ARRAY_ELEMENT_ID_KEY ] = arrayElementId; + } + return result; } @@ -150,16 +162,25 @@ function deserializeAttributeValue( // e.g. a single row inside core/table `body`: { cells: [ ... ] } if ( value && typeof value === 'object' ) { const result: Record< string, unknown > = {}; + const arrayElementId = getArrayElementId( value ); for ( const [ key, innerValue ] of Object.entries( value as Record< string, unknown > ) ) { + if ( key === ARRAY_ELEMENT_ID_KEY ) { + continue; + } + result[ key ] = deserializeAttributeValue( schema?.query?.[ key ], innerValue ); } + if ( arrayElementId ) { + defineArrayElementId( result, arrayElementId ); + } + return result; } @@ -329,12 +350,15 @@ function createYMapFromQuery( return new Y.Map(); } - const entries: [ string, unknown ][] = Object.entries( obj ).map( - ( [ key, val ] ): [ string, unknown ] => { + const arrayElementId = getArrayElementId( obj ) ?? uuidv4(); + const entries: [ string, unknown ][] = Object.entries( obj ) + .filter( ( [ key ] ) => key !== ARRAY_ELEMENT_ID_KEY ) + .map( ( [ key, val ] ): [ string, unknown ] => { const subSchema = query[ key ]; return [ key, createYValueFromSchema( subSchema, val ) ]; - } - ); + } ); + + entries.push( [ ARRAY_ELEMENT_ID_KEY, arrayElementId ] ); return new Y.Map( entries ); } @@ -594,10 +618,122 @@ function areArrayElementsEqual( yElement: unknown ): boolean { if ( yElement instanceof Y.Map && isRecord( newElement ) ) { - return fastDeepEqual( newElement, yElement.toJSON() ); + return fastDeepEqual( + stripArrayElementIds( newElement ), + stripArrayElementIds( yElement.toJSON() ) + ); + } + + return fastDeepEqual( + stripArrayElementIds( newElement ), + stripArrayElementIds( yElement ) + ); +} + +function getArrayElementId( value: unknown ): string | undefined { + if ( value instanceof Y.Map ) { + const id = value.get( ARRAY_ELEMENT_ID_KEY ); + return typeof id === 'string' ? id : undefined; + } + + if ( isRecord( value ) ) { + const id = value[ ARRAY_ELEMENT_ID_KEY ]; + if ( typeof id === 'string' ) { + return id; + } + + const symbolId = ( value as Record< symbol, unknown > )[ + ARRAY_ELEMENT_ID_SYMBOL + ]; + return typeof symbolId === 'string' ? symbolId : undefined; + } + + return undefined; +} + +function defineArrayElementId( + value: Record< string, unknown >, + id: string +): void { + Object.defineProperty( value, ARRAY_ELEMENT_ID_SYMBOL, { + configurable: true, + enumerable: true, + value: id, + } ); +} + +function stripArrayElementIds( value: unknown ): unknown { + if ( Array.isArray( value ) ) { + return value.map( stripArrayElementIds ); + } + + if ( isRecord( value ) ) { + return Object.fromEntries( + Object.entries( value ) + .filter( ( [ key ] ) => key !== ARRAY_ELEMENT_ID_KEY ) + .map( ( [ key, innerValue ] ) => [ + key, + stripArrayElementIds( innerValue ), + ] ) + ); + } + + return value; +} + +function mergeYArrayByElementIds( + yArray: Y.Array< unknown >, + newValue: unknown[], + query: Record< string, BlockAttributeSchema >, + cursorPosition: number | null +): boolean { + if ( ! newValue.some( getArrayElementId ) ) { + return false; + } + + let index = 0; + + for ( const newElement of newValue ) { + const newId = getArrayElementId( newElement ); + let currentIndex = -1; + + if ( newId ) { + for ( let i = index; i < yArray.length; i++ ) { + if ( getArrayElementId( yArray.get( i ) ) === newId ) { + currentIndex = i; + break; + } + } + } + + if ( currentIndex > index ) { + yArray.delete( index, currentIndex - index ); + } + + if ( currentIndex >= index ) { + const currentElement = yArray.get( index ); + if ( currentElement instanceof Y.Map && isRecord( newElement ) ) { + mergeYMapValues( + currentElement, + newElement, + query, + cursorPosition + ); + } + } else { + yArray.insert( index, [ + createYMapFromQuery( query, newElement ), + ] ); + } + + index++; + } + + if ( yArray.length > index ) { + yArray.delete( index, yArray.length - index ); } - return fastDeepEqual( newElement, yElement ); + return true; } /** @@ -625,6 +761,11 @@ function mergeYArray( } const query = schema.query; + + if ( mergeYArrayByElementIds( yArray, newValue, query, cursorPosition ) ) { + return; + } + const numOfCommonEntries = Math.min( newValue.length, yArray.length ); let left = 0; @@ -786,7 +927,7 @@ function mergeYMapValues( // Delete properties absent from the incoming object. for ( const key of yMap.keys() ) { - if ( ! Object.hasOwn( newObj, key ) ) { + if ( key !== ARRAY_ELEMENT_ID_KEY && ! Object.hasOwn( newObj, key ) ) { yMap.delete( key ); } } diff --git a/packages/core-data/src/utils/test/crdt-table-duplicates-repro.test.ts b/packages/core-data/src/utils/test/crdt-table-duplicates-repro.test.ts new file mode 100644 index 00000000000000..e2f22186f27b2c --- /dev/null +++ b/packages/core-data/src/utils/test/crdt-table-duplicates-repro.test.ts @@ -0,0 +1,124 @@ +/** + * External dependencies + */ +import { describe, expect, it, jest } from '@jest/globals'; + +/** + * WordPress dependencies + */ +import { Y } from '@wordpress/sync'; + +/** + * Internal dependencies + */ +import { + deserializeBlockAttributes, + mergeCrdtBlocks, + type Block, + type YBlock, +} from '../crdt-blocks'; + +jest.mock( '@wordpress/blocks', () => ( { + getBlockTypes: () => [ + { + name: 'core/table', + attributes: { + body: { + type: 'array', + query: { + cells: { + type: 'array', + query: { + content: { type: 'rich-text' }, + tag: { type: 'string' }, + }, + }, + }, + }, + }, + }, + ], +} ) ); + +function createTableBlock( values: string[] ): Block { + return { + name: 'core/table', + clientId: 'table', + attributes: { + body: values.map( ( value ) => ( { + cells: [ + { + content: value, + tag: 'td', + }, + ], + } ) ), + }, + innerBlocks: [], + }; +} + +function getRuntimeTableBody( blocks: Block[] ) { + return blocks[ 0 ].attributes.body as Array< { + cells: Array< Record< string, unknown > >; + } >; +} + +function getCellContentText( content: unknown ) { + return typeof content === 'object' && content && 'valueOf' in content + ? String( content.valueOf() ) + : content; +} + +function getTableBodyCellContents( yblocks: Y.Array< YBlock > ) { + const blocks = deserializeBlockAttributes( yblocks.toJSON() as Block[] ); + return getRuntimeTableBody( blocks ).map( ( row ) => + getCellContentText( row.cells[ 0 ].content ) + ); +} + +describe( 'CRDT duplicate table row repro', () => { + it( 'converges when one user edits the later duplicate row and another deletes the earlier duplicate row', () => { + const docA = new Y.Doc(); + const docB = new Y.Doc(); + const yblocksA = docA.getArray< YBlock >( 'blocks' ); + const yblocksB = docB.getArray< YBlock >( 'blocks' ); + + mergeCrdtBlocks( + yblocksA, + [ createTableBlock( [ 'anchor', 'same', 'same' ] ) ], + null + ); + Y.applyUpdate( docB, Y.encodeStateAsUpdate( docA ) ); + + const stateVectorA = Y.encodeStateVector( docA ); + const stateVectorB = Y.encodeStateVector( docB ); + const runtimeBlocksA = deserializeBlockAttributes( + yblocksA.toJSON() as Block[] + ); + const runtimeBlocksB = deserializeBlockAttributes( + yblocksB.toJSON() as Block[] + ); + + getRuntimeTableBody( runtimeBlocksA )[ 2 ].cells[ 0 ].content = + 'edited-second-duplicate'; + getRuntimeTableBody( runtimeBlocksB ).splice( 1, 1 ); + + mergeCrdtBlocks( yblocksA, runtimeBlocksA, null ); + mergeCrdtBlocks( yblocksB, runtimeBlocksB, null ); + + const updateA = Y.encodeStateAsUpdate( docA, stateVectorB ); + const updateB = Y.encodeStateAsUpdate( docB, stateVectorA ); + Y.applyUpdate( docA, updateB ); + Y.applyUpdate( docB, updateA ); + + expect( getTableBodyCellContents( yblocksA ) ).toEqual( [ + 'anchor', + 'edited-second-duplicate', + ] ); + expect( getTableBodyCellContents( yblocksB ) ).toEqual( [ + 'anchor', + 'edited-second-duplicate', + ] ); + } ); +} ); diff --git a/packages/core-data/src/utils/test/crdt-table-query-identity.test.ts b/packages/core-data/src/utils/test/crdt-table-query-identity.test.ts new file mode 100644 index 00000000000000..1a177ca7c8abaa --- /dev/null +++ b/packages/core-data/src/utils/test/crdt-table-query-identity.test.ts @@ -0,0 +1,211 @@ +/** + * External dependencies + */ +import { describe, expect, it, jest } from '@jest/globals'; + +/** + * WordPress dependencies + */ +import { Y } from '@wordpress/sync'; + +/** + * Internal dependencies + */ +import { + deserializeBlockAttributes, + mergeCrdtBlocks, + type Block, + type YBlock, +} from '../crdt-blocks'; + +jest.mock( '@wordpress/blocks', () => ( { + getBlockTypes: () => [ + { + name: 'core/table', + attributes: { + body: { + type: 'array', + query: { + cells: { + type: 'array', + query: { + content: { type: 'rich-text' }, + tag: { type: 'string' }, + }, + }, + }, + }, + }, + }, + ], +} ) ); + +const INTERNAL_ID_KEY = '__unstableSyncId'; + +function createTableBlock( values: string[] ): Block { + return { + name: 'core/table', + clientId: 'table', + attributes: { + body: values.map( ( value ) => ( { + cells: [ + { + content: value, + tag: 'td', + }, + ], + } ) ), + }, + innerBlocks: [], + }; +} + +describe( 'CRDT table query array identity', () => { + function getRuntimeBody( yblocks: Y.Array< YBlock > ) { + const [ table ] = deserializeBlockAttributes( + yblocks.toJSON() as Block[] + ); + return table.attributes.body as Array< + Record< string, unknown > & { + cells: Array< Record< string, unknown > >; + } + >; + } + + function getRuntimeCellValues( yblocks: Y.Array< YBlock > ) { + return getRuntimeBody( yblocks ).map( ( row ) => { + const content = row.cells[ 0 ].content; + return typeof content === 'object' && + content && + 'valueOf' in content + ? String( content.valueOf() ) + : content; + } ); + } + + function getYBody( yblocks: Y.Array< YBlock > ) { + const [ table ] = yblocks.toJSON() as Block[]; + return table.attributes.body as Array< + Record< string, unknown > & { + cells: Array< Record< string, unknown > >; + } + >; + } + + it( 'stores internal identities in the CRDT document only', () => { + const doc = new Y.Doc(); + const yblocks = doc.getArray< YBlock >( 'blocks' ); + + mergeCrdtBlocks( + yblocks, + [ createTableBlock( [ 'anchor', 'same', 'same' ] ) ], + null + ); + + const yBody = getYBody( yblocks ); + expect( yBody[ 0 ][ INTERNAL_ID_KEY ] ).toEqual( expect.any( String ) ); + expect( yBody[ 1 ][ INTERNAL_ID_KEY ] ).toEqual( expect.any( String ) ); + expect( yBody[ 2 ][ INTERNAL_ID_KEY ] ).toEqual( expect.any( String ) ); + expect( yBody[ 2 ].cells[ 0 ][ INTERNAL_ID_KEY ] ).toEqual( + expect.any( String ) + ); + + const [ table ] = deserializeBlockAttributes( + yblocks.toJSON() as Block[] + ); + const body = table.attributes.body as Array< + Record< string, unknown > & { + cells: Array< Record< string, unknown > >; + } + >; + + expect( body[ 0 ] ).not.toHaveProperty( INTERNAL_ID_KEY ); + expect( body[ 1 ] ).not.toHaveProperty( INTERNAL_ID_KEY ); + expect( body[ 2 ] ).not.toHaveProperty( INTERNAL_ID_KEY ); + expect( body[ 2 ].cells[ 0 ] ).not.toHaveProperty( INTERNAL_ID_KEY ); + expect( JSON.stringify( table.attributes ) ).not.toContain( + INTERNAL_ID_KEY + ); + } ); + + it( 'preserves CRDT identities when deserialized blocks are merged back', () => { + const doc = new Y.Doc(); + const yblocks = doc.getArray< YBlock >( 'blocks' ); + + mergeCrdtBlocks( + yblocks, + [ createTableBlock( [ 'anchor', 'same', 'same' ] ) ], + null + ); + + const beforeIds = getYBody( yblocks ).map( + ( row ) => row[ INTERNAL_ID_KEY ] + ); + const beforeCellIds = getYBody( yblocks ).map( + ( row ) => row.cells[ 0 ][ INTERNAL_ID_KEY ] + ); + const runtimeBlocks = deserializeBlockAttributes( + yblocks.toJSON() as Block[] + ); + + mergeCrdtBlocks( yblocks, runtimeBlocks, null ); + + expect( + getYBody( yblocks ).map( ( row ) => row[ INTERNAL_ID_KEY ] ) + ).toEqual( beforeIds ); + expect( + getYBody( yblocks ).map( + ( row ) => row.cells[ 0 ][ INTERNAL_ID_KEY ] + ) + ).toEqual( beforeCellIds ); + } ); + + it( 'preserves a later duplicate row edit when the earlier duplicate is deleted', () => { + const docA = new Y.Doc(); + const docB = new Y.Doc(); + const yblocksA = docA.getArray< YBlock >( 'blocks' ); + const yblocksB = docB.getArray< YBlock >( 'blocks' ); + + mergeCrdtBlocks( + yblocksA, + [ createTableBlock( [ 'anchor', 'same', 'same' ] ) ], + null + ); + Y.applyUpdate( docB, Y.encodeStateAsUpdate( docA ) ); + + const stateVectorA = Y.encodeStateVector( docA ); + const stateVectorB = Y.encodeStateVector( docB ); + const runtimeBlocksA = deserializeBlockAttributes( + yblocksA.toJSON() as Block[] + ); + const runtimeBlocksB = deserializeBlockAttributes( + yblocksB.toJSON() as Block[] + ); + const bodyA = runtimeBlocksA[ 0 ].attributes.body as Array< { + cells: Array< Record< string, unknown > >; + } >; + const bodyB = runtimeBlocksB[ 0 ].attributes.body as Array< { + cells: Array< Record< string, unknown > >; + } >; + + bodyA[ 2 ].cells[ 0 ].content = 'edited-second-duplicate'; + bodyB.splice( 1, 1 ); + + mergeCrdtBlocks( yblocksA, runtimeBlocksA, null ); + mergeCrdtBlocks( yblocksB, runtimeBlocksB, null ); + + const updateA = Y.encodeStateAsUpdate( docA, stateVectorB ); + const updateB = Y.encodeStateAsUpdate( docB, stateVectorA ); + Y.applyUpdate( docA, updateB ); + Y.applyUpdate( docB, updateA ); + + expect( getRuntimeCellValues( yblocksA ) ).toEqual( [ + 'anchor', + 'edited-second-duplicate', + ] ); + expect( getRuntimeCellValues( yblocksB ) ).toEqual( [ + 'anchor', + 'edited-second-duplicate', + ] ); + } ); +} ); diff --git a/packages/core-data/src/utils/test/crdt.ts b/packages/core-data/src/utils/test/crdt.ts index e5aef7a2c9560d..e06c18e85e7920 100644 --- a/packages/core-data/src/utils/test/crdt.ts +++ b/packages/core-data/src/utils/test/crdt.ts @@ -63,7 +63,7 @@ import { type PostChanges, type YPostRecord, } from '../crdt'; -import type { YBlock, YBlockRecord, YBlocks } from '../crdt-blocks'; +import type { Block, YBlock, YBlockRecord, YBlocks } from '../crdt-blocks'; import { updateSelectionHistory } from '../crdt-selection'; import { createYMap, getRootMap, type YMapWrap } from '../crdt-utils'; import type { Post } from '../../entity-types'; @@ -275,6 +275,59 @@ describe( 'crdt', () => { ); } ); + it( 'converges duplicate table row edit/delete through the post changes wrapper', () => { + const docB = new Y.Doc(); + + try { + applyPostChangesToCRDTDoc( + doc, + { + blocks: [ + createTableBlock( [ 'anchor', 'same', 'same' ] ), + ], + }, + defaultSyncedProperties + ); + Y.applyUpdate( docB, Y.encodeStateAsUpdate( doc ) ); + + const stateVectorA = Y.encodeStateVector( doc ); + const stateVectorB = Y.encodeStateVector( docB ); + const runtimeBlocksA = getRuntimeBlocksFromDoc( doc ); + const runtimeBlocksB = getRuntimeBlocksFromDoc( docB ); + + getRuntimeTableBody( runtimeBlocksA )[ 2 ].cells[ 0 ].content = + 'edited-second-duplicate'; + getRuntimeTableBody( runtimeBlocksB ).splice( 1, 1 ); + + applyPostChangesToCRDTDoc( + doc, + { blocks: runtimeBlocksA }, + defaultSyncedProperties + ); + applyPostChangesToCRDTDoc( + docB, + { blocks: runtimeBlocksB }, + defaultSyncedProperties + ); + + const updateA = Y.encodeStateAsUpdate( doc, stateVectorB ); + const updateB = Y.encodeStateAsUpdate( docB, stateVectorA ); + Y.applyUpdate( doc, updateB ); + Y.applyUpdate( docB, updateA ); + + expect( getTableBodyCellContentsFromDoc( doc ) ).toEqual( [ + 'anchor', + 'edited-second-duplicate', + ] ); + expect( getTableBodyCellContentsFromDoc( docB ) ).toEqual( [ + 'anchor', + 'edited-second-duplicate', + ] ); + } finally { + docB.destroy(); + } + } ); + it( 'initializes blocks as Y.Array when not present', () => { const changes = { blocks: [], @@ -994,3 +1047,47 @@ function addBlockToDoc( return ytext; } + +function createTableBlock( values: string[] ): Block { + return { + name: 'core/table', + clientId: 'table', + attributes: { + body: values.map( ( value ) => ( { + cells: [ + { + content: value, + tag: 'td', + }, + ], + } ) ), + }, + innerBlocks: [], + }; +} + +function getRuntimeBlocksFromDoc( ydoc: Y.Doc ): Block[] { + return getPostChangesFromCRDTDoc( + ydoc, + { blocks: [] } as unknown as Post, + defaultSyncedProperties + ).blocks as Block[]; +} + +function getRuntimeTableBody( blocks: Block[] ) { + return blocks[ 0 ].attributes.body as Array< { + cells: Array< Record< string, unknown > >; + } >; +} + +function getCellContentText( content: unknown ) { + return typeof content === 'object' && content && 'valueOf' in content + ? String( content.valueOf() ) + : content; +} + +function getTableBodyCellContentsFromDoc( ydoc: Y.Doc ) { + return getRuntimeTableBody( getRuntimeBlocksFromDoc( ydoc ) ).map( + ( row ) => getCellContentText( row.cells[ 0 ].content ) + ); +} diff --git a/test/e2e/specs/editor/collaboration/collaboration-table-duplicates.spec.ts b/test/e2e/specs/editor/collaboration/collaboration-table-duplicates.spec.ts new file mode 100644 index 00000000000000..f3e6638830d4da --- /dev/null +++ b/test/e2e/specs/editor/collaboration/collaboration-table-duplicates.spec.ts @@ -0,0 +1,117 @@ +/** + * Internal dependencies + */ +import { test, expect } from './fixtures'; + +type Editor = import('@wordpress/e2e-test-utils-playwright').Editor; +type Page = import('@playwright/test').Page; + +const TABLE_POST_CONTENT = ` +
anchor
same
same
+`; + +async function getTableBodyCellContents( editor: Editor ) { + return editor.canvas + .getByRole( 'textbox', { name: 'Body cell text' } ) + .evaluateAll( ( cells ) => + cells.map( ( cell ) => cell.textContent?.trim() ) + ); +} + +async function editTableCell( { + editor, + page, + index, + content, +}: { + editor: Editor; + page: Page; + index: number; + content: string; +} ) { + await editor.canvas + .getByRole( 'textbox', { name: 'Body cell text' } ) + .nth( index ) + .click(); + await page.keyboard.press( 'ControlOrMeta+a' ); + await page.keyboard.type( content ); +} + +async function deleteTableRow( { + editor, + page, + index, +}: { + editor: Editor; + page: Page; + index: number; +} ) { + await editor.canvas + .getByRole( 'textbox', { name: 'Body cell text' } ) + .nth( index ) + .click(); + await editor.clickBlockToolbarButton( 'Edit table' ); + await page.getByRole( 'menuitem', { name: 'Delete row' } ).click(); +} + +test.describe( 'Collaboration - duplicate table rows', () => { + test( 'preserves a later duplicate row edit when the earlier duplicate row is deleted', async ( { + collaborationUtils, + requestUtils, + editor, + page, + } ) => { + test.setTimeout( 45_000 ); + + const post = await requestUtils.createPost( { + title: 'Duplicate table row collaboration repro', + status: 'draft', + content: TABLE_POST_CONTENT, + date_gmt: new Date().toISOString(), + } ); + + await collaborationUtils.openCollaborativeSession( post.id ); + const { editor2, page2 } = collaborationUtils; + + await expect + .poll( () => getTableBodyCellContents( editor ), { + timeout: 10_000, + } ) + .toEqual( [ 'anchor', 'same', 'same' ] ); + await expect + .poll( () => getTableBodyCellContents( editor2 ), { + timeout: 10_000, + } ) + .toEqual( [ 'anchor', 'same', 'same' ] ); + + await Promise.all( [ + editTableCell( { + content: 'edited-second-duplicate', + editor, + index: 2, + page, + } ), + deleteTableRow( { + editor: editor2, + index: 1, + page: page2, + } ), + ] ); + + await Promise.all( [ + collaborationUtils.waitForSyncCycle( page, 5 ), + collaborationUtils.waitForSyncCycle( page2, 5 ), + ] ); + + await expect + .poll( () => getTableBodyCellContents( editor ), { + timeout: 10_000, + } ) + .toEqual( [ 'anchor', 'edited-second-duplicate' ] ); + await expect + .poll( () => getTableBodyCellContents( editor2 ), { + timeout: 10_000, + } ) + .toEqual( [ 'anchor', 'edited-second-duplicate' ] ); + } ); +} ); diff --git a/test/e2e/specs/editor/collaboration/fixtures/collaboration-utils.ts b/test/e2e/specs/editor/collaboration/fixtures/collaboration-utils.ts index c85db8bd786211..11f432530e5285 100644 --- a/test/e2e/specs/editor/collaboration/fixtures/collaboration-utils.ts +++ b/test/e2e/specs/editor/collaboration/fixtures/collaboration-utils.ts @@ -149,18 +149,89 @@ export default class CollaborationUtils { async waitForMutualDiscovery( { timeout }: { timeout?: number } = {} ) { const pages = this.allPages; const resolvedTimeout = timeout ?? 10000 + pages.length * 2500; + const roomName = await this.getCurrentPostRoomName( this.primaryPage ); await Promise.all( pages.map( ( pg ) => - pg - .getByRole( 'button', { name: /Collaborators list/ } ) - .waitFor( { timeout: resolvedTimeout } ) + this.waitForAwarenessPeerCount( + pg, + pages.length, + resolvedTimeout, + roomName + ) ) ); await Promise.all( pages.map( ( pg ) => this.waitForSyncCycle( pg ) ) ); } + /** + * Wait until the sync transport reports the expected number of clients in + * the requested room's awareness payload. + * + * Some repros exercise lower-level sync behavior before the rendered + * collaborator presence UI has enough display metadata to show the + * "Collaborators list" button. The transport-level awareness count is the + * synchronization gate these repros actually need. + * + * @param page The Playwright page to wait on. + * @param expectedPeerCount Expected number of awareness clients. + * @param timeout Maximum wait time in ms. + * @param roomName Optional room name to require. + */ + async waitForAwarenessPeerCount( + page: Page, + expectedPeerCount: number, + timeout: number, + roomName?: string + ) { + await page.waitForResponse( + async ( response ) => { + if ( + ! response.url().includes( 'wp-sync' ) || + response.status() !== 200 + ) { + return false; + } + + const body = await response.json().catch( () => null ); + return ( + body?.rooms?.some( + ( room: { + room?: string; + awareness?: Record< string, unknown >; + } ) => + ( ! roomName || room.room === roomName ) && + room.awareness && + Object.keys( room.awareness ).length >= + expectedPeerCount + ) ?? false + ); + }, + { timeout } + ); + } + + /** + * Return the collaboration room name for the current post. + * + * @param page The Playwright page to read from. + */ + async getCurrentPostRoomName( page: Page ): Promise< string > { + const postId = await page.evaluate( + () => + ( window as any ).wp?.data + ?.select( 'core/editor' ) + ?.getCurrentPostId?.() + ); + + if ( ! postId ) { + throw new Error( 'Current post ID is unavailable.' ); + } + + return `postType/post:${ postId }`; + } + /** * Convenience method: open a two-user collaborative session. *