Skip to content

Commit 3da7412

Browse files
authored
Fix duplicate row creating empty row page (#339)
* fix: avoid duplicating empty row documents * refactor: simplify row document duplicate flags * test: fix duplicate row dispatch lint * fix: ensure row doc before cell updates * test: wait for duplicated inline grid content
1 parent 53b075e commit 3da7412

7 files changed

Lines changed: 552 additions & 266 deletions

File tree

playwright/e2e/embeded/database/cloud-duplicate-doc-with-linked-and-inline-db.spec.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,20 @@ import { currentViewIdFromUrl } from '../../../support/page-utils';
2020
const DUPLICATE_USER_EMAIL = 'duplicate@appflowy.io';
2121
const DUPLICATE_USER_PASSWORD = 'AppFlowy!@123';
2222
const DOCUMENT_NAME = 'Document with linked database';
23+
const INLINE_GRID_TEXT = 'This is inline Grid';
24+
25+
async function expectFirstGridCellToContain(
26+
gridBlock: ReturnType<typeof databaseBlocks>,
27+
text: string,
28+
timeout = 30000
29+
): Promise<void> {
30+
await expect
31+
.poll(async () => firstGridCellText(gridBlock), {
32+
timeout,
33+
message: `Expected first grid cell to contain "${text}"`,
34+
})
35+
.toContain(text);
36+
}
2337

2438
test.describe('Cloud Duplicate Document With Linked And Inline Database', () => {
2539
test.beforeEach(async ({ page }) => {
@@ -68,11 +82,11 @@ test.describe('Cloud Duplicate Document With Linked And Inline Database', () =>
6882
// Restore the expected cell content in case a previous test run corrupted it
6983
const currentCellText = await firstGridCellText(originalBlocks.nth(0));
7084

71-
if (!currentCellText.includes('This is inline Grid')) {
72-
await editFirstGridCell(page, originalBlocks.nth(0), 'This is inline Grid');
85+
if (!currentCellText.includes(INLINE_GRID_TEXT)) {
86+
await editFirstGridCell(page, originalBlocks.nth(0), INLINE_GRID_TEXT);
7387
}
7488

75-
await expect(await firstGridCellText(originalBlocks.nth(0))).toContain('This is inline Grid');
89+
await expectFirstGridCellToContain(originalBlocks.nth(0), INLINE_GRID_TEXT);
7690

7791
const previousCopyCount = await pageNamesByCopyText(page, DOCUMENT_NAME).count();
7892
await duplicateCurrentPageViaHeader(page);
@@ -84,19 +98,20 @@ test.describe('Cloud Duplicate Document With Linked And Inline Database', () =>
8498
const copiedBlocks = databaseBlocks(copiedEditor);
8599
await expect(copiedBlocks).toHaveCount(originalBlockCount, { timeout: 30000 });
86100
await expectNoActiveFilters(copiedBlocks.nth(0));
87-
await expect(await firstGridCellText(copiedBlocks.nth(0))).toContain('This is inline Grid');
101+
await expectFirstGridCellToContain(copiedBlocks.nth(0), INLINE_GRID_TEXT, 60000);
88102

89103
await editFirstGridCell(page, copiedBlocks.nth(0), 'edited in copy');
90104
await openPageByExactText(page, DOCUMENT_NAME);
91-
await expect(
92-
await firstGridCellText(databaseBlocks(editorForView(page, currentViewIdFromUrl(page))).nth(0))
93-
).toContain('This is inline Grid');
105+
await expectFirstGridCellToContain(
106+
databaseBlocks(editorForView(page, currentViewIdFromUrl(page))).nth(0),
107+
INLINE_GRID_TEXT
108+
);
94109

95110
await openPageByExactText(page, copyName);
96111
await editFirstGridCell(
97112
page,
98113
databaseBlocks(editorForView(page, currentViewIdFromUrl(page))).nth(0),
99-
'This is inline Grid'
114+
INLINE_GRID_TEXT
100115
);
101116

102117
await openPageByExactText(page, DOCUMENT_NAME);

playwright/support/duplicate-test-helpers.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,15 @@ export async function duplicateCurrentPageViaHeader(page: Page): Promise<void> {
172172
await dupBtn.click();
173173

174174
const blockingLoader = page.getByTestId('blocking-loader');
175-
await blockingLoader.waitFor({ state: 'visible', timeout: 5000 }).catch(() => undefined);
176-
177-
if ((await blockingLoader.count()) > 0) {
178-
await expect(blockingLoader)
179-
.toBeHidden({ timeout: 10000 })
180-
.catch(() => undefined);
175+
const loaderAppeared = await blockingLoader
176+
.waitFor({ state: 'visible', timeout: 5000 })
177+
.then(() => true)
178+
.catch(() => false);
179+
180+
if (loaderAppeared || (await blockingLoader.count()) > 0) {
181+
await expect(blockingLoader, 'Expected duplicate blocking loader to finish before opening the copy').toBeHidden({
182+
timeout: 60000,
183+
});
181184
}
182185

183186
await page.waitForTimeout(2000);
@@ -442,7 +445,12 @@ export async function editFirstGridCell(page: Page, gridBlock: Locator, text: st
442445
await page.keyboard.press(process.platform === 'darwin' ? 'Meta+A' : 'Control+A');
443446
await page.keyboard.type(text);
444447
await page.keyboard.press('Enter');
445-
await page.waitForTimeout(1000);
448+
await expect
449+
.poll(async () => firstGridCellText(gridBlock), {
450+
timeout: 15000,
451+
message: `Expected first grid cell to contain "${text}" after editing`,
452+
})
453+
.toContain(text);
446454
}
447455

448456
export async function firstGridCellText(gridBlock: Locator): Promise<string> {
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
import { expect } from '@jest/globals';
2+
import { act, renderHook } from '@testing-library/react';
3+
import { ReactNode } from 'react';
4+
import * as Y from 'yjs';
5+
6+
import { DatabaseContext, DatabaseContextState } from '@/application/database-yjs';
7+
import { FieldType, RowMetaKey } from '@/application/database-yjs/database.type';
8+
import { useDuplicateRowDispatch } from '@/application/database-yjs/dispatch/row';
9+
import { getMetaIdMap, getRowKey } from '@/application/database-yjs/row_meta';
10+
import { RowId, YDatabase, YDatabaseField, YDatabaseView, YDoc, YjsDatabaseKey, YjsEditorKey } from '@/application/types';
11+
12+
import { createCell, createRowDoc } from './test-helpers';
13+
14+
jest.mock('@/utils/runtime-config', () => ({
15+
getConfigValue: (_key: string, fallback: string) => fallback,
16+
}));
17+
18+
jest.mock('@/application/db', () => {
19+
// eslint-disable-next-line @typescript-eslint/no-var-requires
20+
const Yjs = require('yjs');
21+
22+
return {
23+
deleteCollabDB: jest.fn(),
24+
getCachedProviderDoc: jest.fn(),
25+
openCollabDB: jest.fn(async () => new Yjs.Doc()),
26+
};
27+
});
28+
29+
const databaseId = 'database-id';
30+
const databaseDocId = '00000000-0000-4000-8000-000000000001';
31+
const viewId = 'view-id';
32+
const sourceRowId = 'source-row-id';
33+
const fieldId = 'name-field-id';
34+
35+
function createField(fieldId: string): YDatabaseField {
36+
const field = new Y.Map() as YDatabaseField;
37+
38+
field.set(YjsDatabaseKey.id, fieldId);
39+
field.set(YjsDatabaseKey.name, 'Name');
40+
field.set(YjsDatabaseKey.type, FieldType.RichText);
41+
42+
return field;
43+
}
44+
45+
function createDatabaseDoc(): YDoc {
46+
const doc = new Y.Doc({ guid: databaseDocId }) as YDoc;
47+
const sharedRoot = doc.getMap(YjsEditorKey.data_section);
48+
const database = new Y.Map() as YDatabase;
49+
const fields = new Y.Map<YDatabaseField>();
50+
const views = new Y.Map<YDatabaseView>();
51+
const view = new Y.Map() as YDatabaseView;
52+
const rowOrders = new Y.Array<{ id: RowId; height: number }>();
53+
54+
fields.set(fieldId, createField(fieldId));
55+
rowOrders.push([{ id: sourceRowId, height: 36 }]);
56+
view.set(YjsDatabaseKey.row_orders, rowOrders);
57+
views.set(viewId, view);
58+
59+
database.set(YjsDatabaseKey.id, databaseId);
60+
database.set(YjsDatabaseKey.fields, fields);
61+
database.set(YjsDatabaseKey.views, views);
62+
sharedRoot.set(YjsEditorKey.database, database);
63+
64+
return doc;
65+
}
66+
67+
function createReferenceRowDoc(options: {
68+
documentId?: string;
69+
isEmptyDocument?: boolean;
70+
} = {}): YDoc {
71+
const rowDoc = createRowDoc(sourceRowId, databaseId, {
72+
[fieldId]: createCell(FieldType.RichText, 'Source row'),
73+
});
74+
const meta = new Y.Map<unknown>();
75+
const metaKeys = getMetaIdMap(sourceRowId);
76+
77+
if (options.documentId !== undefined) {
78+
meta.set(metaKeys.get(RowMetaKey.DocumentId) ?? '', options.documentId);
79+
}
80+
81+
if (options.isEmptyDocument !== undefined) {
82+
meta.set(metaKeys.get(RowMetaKey.IsDocumentEmpty) ?? '', options.isEmptyDocument);
83+
}
84+
85+
rowDoc.getMap(YjsEditorKey.data_section).set(YjsEditorKey.meta, meta);
86+
return rowDoc;
87+
}
88+
89+
function getRowMetaValue(rowDoc: YDoc, rowId: string, key: RowMetaKey) {
90+
const metaKey = getMetaIdMap(rowId).get(key) ?? '';
91+
const meta = rowDoc.getMap(YjsEditorKey.data_section).get(YjsEditorKey.meta) as Y.Map<unknown>;
92+
93+
return meta.get(metaKey);
94+
}
95+
96+
function createWrapper({
97+
databaseDoc,
98+
referenceRowDoc,
99+
createdRows,
100+
duplicateRowDocument,
101+
}: {
102+
databaseDoc: YDoc;
103+
referenceRowDoc: YDoc;
104+
createdRows: Map<string, YDoc>;
105+
duplicateRowDocument: NonNullable<DatabaseContextState['duplicateRowDocument']>;
106+
}) {
107+
const contextValue: DatabaseContextState = {
108+
readOnly: false,
109+
databaseDoc,
110+
databasePageId: 'database-page-id',
111+
activeViewId: viewId,
112+
rowMap: {
113+
[sourceRowId]: referenceRowDoc,
114+
},
115+
workspaceId: 'workspace-id',
116+
createRow: async (rowKey: string) => {
117+
const rowDoc = new Y.Doc({ guid: rowKey }) as YDoc;
118+
119+
createdRows.set(rowKey, rowDoc);
120+
return rowDoc;
121+
},
122+
duplicateRowDocument,
123+
};
124+
125+
return ({ children }: { children: ReactNode }) => (
126+
<DatabaseContext.Provider value={contextValue}>{children}</DatabaseContext.Provider>
127+
);
128+
}
129+
130+
describe('useDuplicateRowDispatch', () => {
131+
it('does not create or duplicate a row page when the source row is document-empty', async () => {
132+
const databaseDoc = createDatabaseDoc();
133+
const referenceRowDoc = createReferenceRowDoc({ isEmptyDocument: true });
134+
const createdRows = new Map<string, YDoc>();
135+
const duplicateRowDocument = jest.fn().mockResolvedValue(undefined);
136+
const { result } = renderHook(() => useDuplicateRowDispatch(), {
137+
wrapper: createWrapper({ databaseDoc, referenceRowDoc, createdRows, duplicateRowDocument }),
138+
});
139+
let duplicatedRowId = '';
140+
141+
await act(async () => {
142+
duplicatedRowId = await result.current(sourceRowId);
143+
});
144+
145+
const createdRowDoc = createdRows.get(getRowKey(databaseDocId, duplicatedRowId));
146+
147+
expect(createdRowDoc).toBeDefined();
148+
expect(getRowMetaValue(createdRowDoc as YDoc, duplicatedRowId, RowMetaKey.IsDocumentEmpty)).toBe(true);
149+
expect(duplicateRowDocument).not.toHaveBeenCalled();
150+
});
151+
152+
it('keeps duplicated rows with unknown document state marked empty until content is confirmed', async () => {
153+
const databaseDoc = createDatabaseDoc();
154+
const referenceRowDoc = createReferenceRowDoc();
155+
const createdRows = new Map<string, YDoc>();
156+
const duplicateRowDocument = jest.fn().mockResolvedValue(undefined);
157+
const { result } = renderHook(() => useDuplicateRowDispatch(), {
158+
wrapper: createWrapper({ databaseDoc, referenceRowDoc, createdRows, duplicateRowDocument }),
159+
});
160+
let duplicatedRowId = '';
161+
162+
await act(async () => {
163+
duplicatedRowId = await result.current(sourceRowId);
164+
});
165+
166+
const createdRowDoc = createdRows.get(getRowKey(databaseDocId, duplicatedRowId));
167+
168+
expect(createdRowDoc).toBeDefined();
169+
expect(getRowMetaValue(createdRowDoc as YDoc, duplicatedRowId, RowMetaKey.IsDocumentEmpty)).toBe(true);
170+
expect(duplicateRowDocument).toHaveBeenCalledWith(databaseId, sourceRowId, duplicatedRowId, undefined);
171+
});
172+
173+
it('still requests row page duplication for a known non-empty source document', async () => {
174+
const sourceDocumentId = 'source-document-id';
175+
const databaseDoc = createDatabaseDoc();
176+
const referenceRowDoc = createReferenceRowDoc({
177+
documentId: sourceDocumentId,
178+
isEmptyDocument: false,
179+
});
180+
const createdRows = new Map<string, YDoc>();
181+
const duplicateRowDocument = jest.fn().mockResolvedValue(undefined);
182+
const { result } = renderHook(() => useDuplicateRowDispatch(), {
183+
wrapper: createWrapper({ databaseDoc, referenceRowDoc, createdRows, duplicateRowDocument }),
184+
});
185+
let duplicatedRowId = '';
186+
187+
await act(async () => {
188+
duplicatedRowId = await result.current(sourceRowId);
189+
});
190+
191+
const createdRowDoc = createdRows.get(getRowKey(databaseDocId, duplicatedRowId));
192+
193+
expect(createdRowDoc).toBeDefined();
194+
expect(getRowMetaValue(createdRowDoc as YDoc, duplicatedRowId, RowMetaKey.IsDocumentEmpty)).toBe(false);
195+
expect(duplicateRowDocument).toHaveBeenCalledWith(databaseId, sourceRowId, duplicatedRowId, undefined);
196+
});
197+
});
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { renderHook, waitFor } from '@testing-library/react';
2+
import type React from 'react';
3+
import * as Y from 'yjs';
4+
5+
import { DatabaseContext, DatabaseContextState, FieldType } from '@/application/database-yjs';
6+
import { useUpdateCellDispatch } from '@/application/database-yjs/dispatch';
7+
import {
8+
RowId,
9+
YDatabase,
10+
YDatabaseField,
11+
YDatabaseFields,
12+
YDatabaseView,
13+
YDatabaseViews,
14+
YDoc,
15+
YjsDatabaseKey,
16+
YjsEditorKey,
17+
} from '@/application/types';
18+
19+
import { createRowDoc } from './test-helpers';
20+
21+
jest.mock('@/utils/runtime-config', () => ({
22+
getConfigValue: (_key: string, fallback: string) => fallback,
23+
}));
24+
25+
const databaseId = 'database-id';
26+
const viewId = 'view-id';
27+
const rowId = 'row-id';
28+
const fieldId = 'name-field-id';
29+
30+
function createTextField(): YDatabaseField {
31+
const field = new Y.Map() as YDatabaseField;
32+
33+
field.set(YjsDatabaseKey.id, fieldId);
34+
field.set(YjsDatabaseKey.name, 'Name');
35+
field.set(YjsDatabaseKey.type, FieldType.RichText);
36+
37+
return field;
38+
}
39+
40+
function createDatabaseDoc(): YDoc {
41+
const doc = new Y.Doc({ guid: databaseId }) as YDoc;
42+
const sharedRoot = doc.getMap(YjsEditorKey.data_section);
43+
const database = new Y.Map() as YDatabase;
44+
const fields = new Y.Map<YDatabaseField>() as YDatabaseFields;
45+
const views = new Y.Map<YDatabaseView>() as YDatabaseViews;
46+
const view = new Y.Map() as YDatabaseView;
47+
48+
fields.set(fieldId, createTextField());
49+
views.set(viewId, view);
50+
database.set(YjsDatabaseKey.id, databaseId);
51+
database.set(YjsDatabaseKey.fields, fields);
52+
database.set(YjsDatabaseKey.views, views);
53+
sharedRoot.set(YjsEditorKey.database, database);
54+
55+
return doc;
56+
}
57+
58+
function getCellData(rowDoc: YDoc) {
59+
const row = rowDoc.getMap(YjsEditorKey.data_section).get(YjsEditorKey.database_row);
60+
const cells = row?.get(YjsDatabaseKey.cells);
61+
62+
return cells?.get(fieldId)?.get(YjsDatabaseKey.data);
63+
}
64+
65+
function createWrapper(contextValue: DatabaseContextState) {
66+
return ({ children }: { children: React.ReactNode }) => (
67+
<DatabaseContext.Provider value={contextValue}>{children}</DatabaseContext.Provider>
68+
);
69+
}
70+
71+
describe('useUpdateCellDispatch', () => {
72+
it('ensures a missing row doc before committing the cell update', async () => {
73+
const databaseDoc = createDatabaseDoc();
74+
const rowDoc = createRowDoc(rowId, databaseId, {});
75+
const ensureRow = jest.fn<Promise<YDoc>, [RowId]>().mockResolvedValue(rowDoc);
76+
const contextValue: DatabaseContextState = {
77+
readOnly: false,
78+
databaseDoc,
79+
databasePageId: viewId,
80+
activeViewId: viewId,
81+
rowMap: {},
82+
ensureRow,
83+
workspaceId: 'workspace-id',
84+
};
85+
const { result } = renderHook(() => useUpdateCellDispatch(rowId, fieldId), {
86+
wrapper: createWrapper(contextValue),
87+
});
88+
89+
result.current('Recovered value');
90+
91+
await waitFor(() => {
92+
expect(getCellData(rowDoc)).toBe('Recovered value');
93+
});
94+
expect(ensureRow).toHaveBeenCalledWith(rowId);
95+
});
96+
});

0 commit comments

Comments
 (0)