RTC: Fix error when entity record doesn't have 'meta' property

packages/core-data/src/entities.js

@@ -419,7 +419,7 @@ async function loadPostTypeEntities() {
 			 */
 			getPersistedCRDTDoc: ( record ) => {
 				return (
-					record?.meta[ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ] ||
+					record?.meta?.[ POST_META_KEY_FOR_CRDT_DOC_PERSISTENCE ] ||
 					null
 				);
 			},

packages/core-data/src/resolvers.js

@@ -229,18 +229,26 @@ export const getEntityRecord =
 								query
 							);
 						},
-						// Save the current entity record, whether or not it has unsaved
-						// edits. This is used to trigger a persisted CRDT document.
-						saveRecord: () => {
+						// Persist the CRDT document.
+						//
+						// TODO: Currently, persisted CRDT documents are stored in post meta.
+						// This effectively means that only post entities support CRDT
+						// persistence. As we add support for syncing additional entity,
+						// we'll need to revisit where persisted CRDT documents are stored.
+						persistCRDTDoc: () => {
 							resolveSelect
 								.getEditedEntityRecord( kind, name, key )
 								.then( ( editedRecord ) => {
-									// Don't trigger a save if the record is still an auto-draft.
-									const { status } = editedRecord;
-									if ( 'auto-draft' === status ) {
+									// Don't persist the CRDT document if the record is still an
+									// auto-draft or if the entity does not support meta.
+									const { meta, status } = editedRecord;
+									if ( 'auto-draft' === status || ! meta ) {
 										return;
 									}
 
+									// Trigger a save to persist the CRDT document. The entity's
+									// pre-persist hooks will create the persisted CRDT document
+									// and apply it to the record's meta.
 									dispatch.saveEntityRecord(
 										kind,
 										name,

packages/core-data/src/test/resolvers.js

@@ -172,16 +172,69 @@ describe( 'getEntityRecord', () => {
 				editRecord: expect.any( Function ),
 				getEditedRecord: expect.any( Function ),
 				onStatusChange: expect.any( Function ),
+				persistCRDTDoc: expect.any( Function ),
 				refetchRecord: expect.any( Function ),
 				restoreUndoMeta: expect.any( Function ),
-				saveRecord: expect.any( Function ),
 			}
 		);
 	} );
 
-	it( 'saveRecord fetches edited record and saves full entity record', async () => {
-		const POST_RECORD = { id: 1, title: 'Test Post' };
-		const EDITED_RECORD = { id: 1, title: 'Edited Post' };
+	it( 'persistCRDTDoc fetches edited record and does not save full entity record when the entity does not support meta', async () => {
+		const ENTITY_RECORD = { id: 1, title: 'Test Record' };
+		const EDITED_RECORD = { id: 1, title: 'Edited Record' };
+		const ENTITY_RESPONSE = {
+			json: () => Promise.resolve( ENTITY_RECORD ),
+		};
+		const ENTITIES_WITH_SYNC = [
+			{
+				name: 'bar',
+				kind: 'foo',
+				baseURL: '/wp/v2/foo',
+				baseURLParams: { context: 'edit' },
+				syncConfig: {},
+			},
+		];
+
+		dispatch.saveEntityRecord = jest.fn();
+
+		const resolveSelectWithSync = {
+			getEntitiesConfig: jest.fn( () => ENTITIES_WITH_SYNC ),
+			getEditedEntityRecord: jest.fn( () =>
+				Promise.resolve( EDITED_RECORD )
+			),
+		};
+
+		triggerFetch.mockImplementation( () => ENTITY_RESPONSE );
+
+		await getEntityRecord(
+			'foo',
+			'bar',
+			1
+		)( {
+			dispatch,
+			registry,
+			resolveSelect: resolveSelectWithSync,
+		} );
+
+		// Extract the handlers passed to syncManager.load.
+		const handlers = syncManager.load.mock.calls[ 0 ][ 4 ];
+
+		// Call persistCRDTDoc and wait for the internal promise chain.
+		handlers.persistCRDTDoc();
+		await resolveSelectWithSync.getEditedEntityRecord();
+
+		// Should have fetched the full edited entity record.
+		expect(
+			resolveSelectWithSync.getEditedEntityRecord
+		).toHaveBeenCalledWith( 'foo', 'bar', 1 );
+
+		// Should not have called saveEntityRecord.
+		expect( dispatch.saveEntityRecord ).not.toHaveBeenCalled();
+	} );
+
+	it( 'persistCRDTDoc fetches edited record and saves full entity record', async () => {
+		const POST_RECORD = { id: 1, title: 'Test Post', meta: {} };
+		const EDITED_RECORD = { id: 1, title: 'Edited Post', meta: {} };
 		const POST_RESPONSE = {
 			json: () => Promise.resolve( POST_RECORD ),
 		};
@@ -219,8 +272,8 @@ describe( 'getEntityRecord', () => {
 		// Extract the handlers passed to syncManager.load.
 		const handlers = syncManager.load.mock.calls[ 0 ][ 4 ];
 
-		// Call saveRecord and wait for the internal promise chain.
-		handlers.saveRecord();
+		// Call persistCRDTDoc and wait for the internal promise chain.
+		handlers.persistCRDTDoc();
 		await resolveSelectWithSync.getEditedEntityRecord();
 
 		// Should have fetched the full edited entity record.
@@ -236,8 +289,8 @@ describe( 'getEntityRecord', () => {
 		);
 	} );
 
-	it( 'saveRecord saves even when there are no unsaved edits', async () => {
-		const POST_RECORD = { id: 1, title: 'Test Post' };
+	it( 'persistCRDTDoc saves even when there are no unsaved edits', async () => {
+		const POST_RECORD = { id: 1, title: 'Test Post', meta: {} };
 		const POST_RESPONSE = {
 			json: () => Promise.resolve( POST_RECORD ),
 		};
@@ -275,8 +328,8 @@ describe( 'getEntityRecord', () => {
 
 		const handlers = syncManager.load.mock.calls[ 0 ][ 4 ];
 
-		// Call saveRecord and wait for the internal promise chain.
-		handlers.saveRecord();
+		// Call persistCRDTDoc and wait for the internal promise chain.
+		handlers.persistCRDTDoc();
 		await resolveSelectWithSync.getEditedEntityRecord();
 
 		// Should save the record even with no edits (the whole point of the fix).
@@ -336,9 +389,9 @@ describe( 'getEntityRecord', () => {
 				editRecord: expect.any( Function ),
 				getEditedRecord: expect.any( Function ),
 				onStatusChange: expect.any( Function ),
+				persistCRDTDoc: expect.any( Function ),
 				refetchRecord: expect.any( Function ),
 				restoreUndoMeta: expect.any( Function ),
-				saveRecord: expect.any( Function ),
 			}
 		);
 	} );

packages/sync/src/manager.ts

@@ -177,9 +177,9 @@ export function createSyncManager( debug = false ): SyncManager {
 			editRecord: debugWrap( handlers.editRecord ),
 			getEditedRecord: debugWrap( handlers.getEditedRecord ),
 			onStatusChange: debugWrap( handlers.onStatusChange ),
+			persistCRDTDoc: debugWrap( handlers.persistCRDTDoc ),
 			refetchRecord: debugWrap( handlers.refetchRecord ),
 			restoreUndoMeta: debugWrap( handlers.restoreUndoMeta ),
-			saveRecord: debugWrap( handlers.saveRecord ),
 		};
 
 		const ydoc = createYjsDoc( { objectType } );
@@ -467,12 +467,12 @@ export function createSyncManager( debug = false ): SyncManager {
 
 		if ( ! tempDoc ) {
 			log( 'applyPersistedCrdtDoc', 'no persisted doc', entityId );
-			// Apply the current record as changes and trigger a save, which will
-			// persist the CRDT document. (The entity should call `createPersistedCRDTDoc`
-			// via its pre-persist hook.)
+			// Apply the current record as changes and request that the CRDT doc be
+			// persisted with the entity. The persisted CRDT doc can be created by
+			// calling `syncManager.createPersistedCRDTDoc`.
 			targetDoc.transact( () => {
 				applyChangesToCRDTDoc( targetDoc, record );
-				handlers.saveRecord();
+				handlers.persistCRDTDoc();
 			}, LOCAL_SYNC_MANAGER_ORIGIN );
 			return;
 		}
@@ -524,12 +524,12 @@ export function createSyncManager( debug = false ): SyncManager {
 			{}
 		);
 
-		// Apply the changes and trigger a save, which will persist the CRDT
-		// document. (The entity should call `createPersistedCRDTDoc` via its
-		// pre-persist hook.)
+		// Apply the changes and request that the updated CRDT doc be persisted with
+		// the entity. The persisted CRDT doc can be created by calling
+		// `syncManager.createPersistedCRDTDoc`.
 		targetDoc.transact( () => {
 			applyChangesToCRDTDoc( targetDoc, changes );
-			handlers.saveRecord();
+			handlers.persistCRDTDoc();
 		}, LOCAL_SYNC_MANAGER_ORIGIN );
 	}
 

packages/sync/src/test/manager.ts

@@ -54,6 +54,7 @@ describe( 'SyncManager', () => {
 		mockRecord = {
 			id: '123',
 			title: 'Test Post',
+			meta: {},
 		};
 
 		mockProviderResult = {
@@ -96,9 +97,9 @@ describe( 'SyncManager', () => {
 				Promise.resolve( mockRecord )
 			),
 			onStatusChange: jest.fn(),
+			persistCRDTDoc: jest.fn(),
 			refetchRecord: jest.fn( async () => Promise.resolve() ),
 			restoreUndoMeta: jest.fn(),
-			saveRecord: jest.fn(),
 		};
 	} );
 
@@ -265,8 +266,10 @@ describe( 'SyncManager', () => {
 					mockSyncConfig.getChangesFromCRDTDoc
 				).not.toHaveBeenCalled();
 
-				// Verify a save operation occurred.
-				expect( mockHandlers.saveRecord ).toHaveBeenCalledTimes( 1 );
+				// Verify that the CRDT doc was persisted.
+				expect( mockHandlers.persistCRDTDoc ).toHaveBeenCalledTimes(
+					1
+				);
 			} );
 
 			it( 'accepts a valid persisted CRDT doc without applying changes', async () => {
@@ -300,9 +303,9 @@ describe( 'SyncManager', () => {
 					mockSyncConfig.getChangesFromCRDTDoc
 				).toHaveBeenCalledWith( expect.any( Y.Doc ), mockRecord );
 
-				// Verify no save operation occurred
+				// Verify that the CRDT doc was persisted.
 				expect( mockHandlers.editRecord ).not.toHaveBeenCalled();
-				expect( mockHandlers.saveRecord ).not.toHaveBeenCalled();
+				expect( mockHandlers.persistCRDTDoc ).not.toHaveBeenCalled();
 			} );
 
 			it( 'applies a persisted CRDT doc with invalidated fields, then applies changes', async () => {
@@ -346,8 +349,10 @@ describe( 'SyncManager', () => {
 					mockSyncConfig.getChangesFromCRDTDoc
 				).toHaveBeenCalledWith( expect.any( Y.Doc ), mockRecord );
 
-				// Verify a save operation occurred.
-				expect( mockHandlers.saveRecord ).toHaveBeenCalledTimes( 1 );
+				// Verify that the CRDT doc was persisted.
+				expect( mockHandlers.persistCRDTDoc ).toHaveBeenCalledTimes(
+					1
+				);
 			} );
 		} );
 	} );

packages/sync/src/types.ts

@@ -134,9 +134,9 @@ export interface RecordHandlers {
 	) => void;
 	getEditedRecord: () => Promise< ObjectData >;
 	onStatusChange: OnStatusChangeCallback;
+	persistCRDTDoc: () => void;
 	refetchRecord: () => Promise< void >;
 	restoreUndoMeta: ( ydoc: Y.Doc, meta: Map< string, any > ) => void;
-	saveRecord: () => void;
 }
 
 export interface SyncConfig {