Skip to content

Commit 01b95a4

Browse files
authored
Fix stale trash state after remote restore (#351)
* Fix stale trash state after remote restore * Increase embedded database height * Fix row document image content indicator * Stabilize row document BDD card creation * Harden trash refresh and row icon BDD coverage
1 parent 4a15d4b commit 01b95a4

8 files changed

Lines changed: 509 additions & 76 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Feature: Database row document
2+
Row document content should be reflected in the database primary cell.
3+
4+
Scenario: Image link content shows the row document indicator
5+
Given a board database with a card is open
6+
When I add image link "https://example.com/row-page-image.png" to the card row page
7+
And I close the card row page
8+
Then the card primary cell shows a row document icon
9+
When I switch the database to a new Grid view
10+
Then the grid primary cell shows a row document icon
Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
import { expect, Page } from '@playwright/test';
2+
import { createBdd } from 'playwright-bdd';
3+
import { v4 as uuidv4 } from 'uuid';
4+
5+
import { signInAndCreateDatabaseView, waitForGridReady } from '../../support/database-ui-helpers';
6+
import { closeRowDetailWithEscape } from '../../support/row-detail-helpers';
7+
import { BoardSelectors, DatabaseGridSelectors, DatabaseViewSelectors, RowDetailSelectors } from '../../support/selectors';
8+
import { generateRandomEmail } from '../../support/test-config';
9+
10+
const { Given, When, Then } = createBdd();
11+
12+
const cardNamesByPage = new WeakMap<Page, string>();
13+
14+
Given('a board database with a card is open', async ({ page, request }) => {
15+
const currentCardName = `ImageLink-${uuidv4().slice(0, 6)}`;
16+
17+
cardNamesByPage.set(page, currentCardName);
18+
19+
await signInAndCreateDatabaseView(page, request, generateRandomEmail(), 'Board', {
20+
createWaitMs: 7000,
21+
verify: async (p) => {
22+
await expect(BoardSelectors.boardContainer(p)).toBeVisible({ timeout: 15000 });
23+
await expect(BoardSelectors.cards(p).first()).toBeVisible({ timeout: 15000 });
24+
},
25+
});
26+
27+
await addNewCard(page, currentCardName);
28+
await expect(cardByName(page, currentCardName)).toBeVisible({ timeout: 15000 });
29+
});
30+
31+
When('I add image link {string} to the card row page', async ({ page }, imageUrl: string) => {
32+
const currentCardName = getCurrentCardName(page);
33+
34+
await cardByName(page, currentCardName).click({ force: true });
35+
await expect(RowDetailSelectors.modal(page)).toBeVisible({ timeout: 15000 });
36+
37+
await focusRowDocumentEditor(page);
38+
await page.keyboard.type('/image', { delay: 30 });
39+
40+
const imageCommand = page.getByTestId('slash-menu-image');
41+
42+
await expect(imageCommand).toBeVisible({ timeout: 10000 });
43+
await imageCommand.click({ force: true });
44+
45+
const popover = page.locator('.MuiPopover-root:visible').last();
46+
47+
await expect(popover).toBeVisible({ timeout: 10000 });
48+
await popover.getByText('Embed link', { exact: true }).click({ force: true });
49+
50+
const input = popover.getByPlaceholder('Paste or type an image link');
51+
52+
await expect(input).toBeVisible({ timeout: 10000 });
53+
await input.fill(imageUrl);
54+
await input.press('Enter');
55+
await expect(popover).toBeHidden({ timeout: 10000 });
56+
});
57+
58+
When('I close the card row page', async ({ page }) => {
59+
await closeRowDetailWithEscape(page);
60+
await page.waitForTimeout(2000);
61+
});
62+
63+
When('I switch the database to a new Grid view', async ({ page }) => {
64+
await DatabaseViewSelectors.addViewButton(page).click({ force: true });
65+
await DatabaseViewSelectors.viewTypeOption(page, 'Grid').click({ force: true });
66+
await expect(DatabaseViewSelectors.viewTab(page).filter({ hasText: 'Grid' })).toBeVisible({ timeout: 15000 });
67+
await expect(DatabaseViewSelectors.viewTab(page).filter({ hasText: 'Grid' })).toHaveAttribute('data-state', 'active', {
68+
timeout: 15000,
69+
});
70+
await waitForGridReady(page);
71+
});
72+
73+
Then('the card primary cell shows a row document icon', async ({ page }) => {
74+
const currentCardName = getCurrentCardName(page);
75+
76+
await expect(cardByName(page, currentCardName).locator('.custom-icon')).toBeVisible({ timeout: 15000 });
77+
});
78+
79+
Then('the grid primary cell shows a row document icon', async ({ page }) => {
80+
const currentCardName = getCurrentCardName(page);
81+
const row = DatabaseGridSelectors.dataRows(page).filter({ hasText: currentCardName }).first();
82+
83+
await expect(row).toBeVisible({ timeout: 15000 });
84+
await expect(row.locator('.custom-icon')).toBeVisible({ timeout: 15000 });
85+
});
86+
87+
async function addNewCard(page: Page, cardName: string) {
88+
const todoColumn = BoardSelectors.boardContainer(page)
89+
.locator('[data-column-id]')
90+
.filter({ hasText: 'To Do' });
91+
92+
await todoColumn.getByText('New').click({ force: true });
93+
await page.waitForTimeout(500);
94+
await page.keyboard.type(cardName, { delay: 30 });
95+
await page.keyboard.press('Enter');
96+
await page.waitForTimeout(2000);
97+
}
98+
99+
async function focusRowDocumentEditor(page: Page) {
100+
const dialog = page.locator('[role="dialog"]');
101+
const scrollContainer = dialog.locator('.appflowy-scroll-container');
102+
103+
if ((await scrollContainer.count()) > 0) {
104+
await scrollContainer.evaluate((el) => el.scrollTo(0, 9999));
105+
await page.waitForTimeout(500);
106+
}
107+
108+
const editor = dialog.locator('[data-testid="editor-content"], [role="textbox"][contenteditable="true"]').first();
109+
110+
await expect(editor).toBeVisible({ timeout: 15000 });
111+
await editor.click({ force: true });
112+
}
113+
114+
function cardByName(page: Page, cardName: string) {
115+
return BoardSelectors.boardContainer(page).locator('.board-card').filter({ hasText: cardName }).first();
116+
}
117+
118+
function getCurrentCardName(page: Page) {
119+
const cardName = cardNamesByPage.get(page);
120+
121+
if (!cardName) {
122+
throw new Error('No current card name is available for this scenario');
123+
}
124+
125+
return cardName;
126+
}

src/application/slate-yjs/utils/editor.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,15 @@ import {
4646
turnToBlock,
4747
updateBlockParent,
4848
} from '@/application/slate-yjs/utils/yjs';
49-
import { BlockData, BlockType, ToggleListBlockData, YBlock, YjsEditorKey, YSharedRoot } from '@/application/types';
49+
import {
50+
BlockData,
51+
BlockType,
52+
CollabOrigin,
53+
ToggleListBlockData,
54+
YBlock,
55+
YjsEditorKey,
56+
YSharedRoot,
57+
} from '@/application/types';
5058

5159
import { YjsEditor } from '../plugins/withYjs';
5260

@@ -1174,7 +1182,7 @@ export function addBlock(
11741182
extendNextSiblingsToToggleHeading(sharedRoot, newBlock);
11751183
});
11761184

1177-
executeOperations(sharedRoot, operations, 'addBlock');
1185+
executeOperations(sharedRoot, operations, 'addBlock', CollabOrigin.LocalManual);
11781186

11791187
return newBlockId;
11801188
}
Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
import EventEmitter from 'events';
2+
3+
import { act, renderHook, waitFor } from '@testing-library/react';
4+
import { type ReactNode } from 'react';
5+
import { MemoryRouter } from 'react-router-dom';
6+
7+
import { APP_EVENTS } from '@/application/constants';
8+
import { AccessService, ViewService } from '@/application/services/domains';
9+
import { Role, View, ViewLayout } from '@/application/types';
10+
import { AuthInternalContext, AuthInternalContextType } from '@/components/app/contexts/AuthInternalContext';
11+
import { SyncInternalContext, SyncInternalContextType } from '@/components/app/contexts/SyncInternalContext';
12+
13+
import { useWorkspaceData } from '../useWorkspaceData';
14+
15+
jest.mock('lodash-es', () => ({
16+
sortBy: (items: Record<string, unknown>[], key: string) =>
17+
[...items].sort((a, b) => String(a[key] ?? '').localeCompare(String(b[key] ?? ''))),
18+
uniqBy: (items: Record<string, unknown>[], key: string) => {
19+
const seen = new Set<unknown>();
20+
21+
return items.filter((item) => {
22+
const value = item[key];
23+
24+
if (seen.has(value)) return false;
25+
seen.add(value);
26+
return true;
27+
});
28+
},
29+
}));
30+
31+
jest.mock('@/application/services/domains', () => ({
32+
AccessService: {
33+
getShareWithMe: jest.fn(),
34+
},
35+
ViewService: {
36+
get: jest.fn(),
37+
getDatabaseRelations: jest.fn(),
38+
getMultiple: jest.fn(),
39+
getOutline: jest.fn(),
40+
getTrash: jest.fn(),
41+
invalidateCache: jest.fn(),
42+
},
43+
WorkspaceService: {
44+
getMentionableUsers: jest.fn(),
45+
},
46+
}));
47+
48+
const workspaceId = 'workspace-id';
49+
const restoredViewId = 'restored-view-id';
50+
51+
function createDeferred<T>() {
52+
let resolve!: (value: T) => void;
53+
let reject!: (reason?: unknown) => void;
54+
const promise = new Promise<T>((res, rej) => {
55+
resolve = res;
56+
reject = rej;
57+
});
58+
59+
return { promise, resolve, reject };
60+
}
61+
62+
const createView = (viewId: string, overrides: Partial<View> = {}): View => ({
63+
view_id: viewId,
64+
name: overrides.name ?? viewId,
65+
icon: overrides.icon ?? null,
66+
layout: overrides.layout ?? ViewLayout.Document,
67+
extra: overrides.extra ?? null,
68+
children: overrides.children ?? [],
69+
has_children: overrides.has_children,
70+
is_published: overrides.is_published ?? false,
71+
is_private: overrides.is_private ?? false,
72+
...overrides,
73+
});
74+
75+
function createWrapper(eventEmitter: EventEmitter) {
76+
const authContext: AuthInternalContextType = {
77+
currentWorkspaceId: workspaceId,
78+
isAuthenticated: true,
79+
onChangeWorkspace: jest.fn(),
80+
userWorkspaceInfo: {
81+
userId: 'user-id',
82+
selectedWorkspace: {
83+
id: workspaceId,
84+
databaseStorageId: 'database-storage-id',
85+
role: Role.Owner,
86+
},
87+
} as AuthInternalContextType['userWorkspaceInfo'],
88+
};
89+
90+
const syncContext = {
91+
eventEmitter,
92+
awarenessMap: {},
93+
broadcastChannel: {},
94+
flushAllSync: jest.fn(),
95+
registerSyncContext: jest.fn(),
96+
revertCollabVersion: jest.fn(),
97+
scheduleDeferredCleanup: jest.fn(),
98+
syncAllToServer: jest.fn(),
99+
webSocket: {},
100+
} as unknown as SyncInternalContextType;
101+
102+
return function Wrapper({ children }: { children: ReactNode }) {
103+
return (
104+
<MemoryRouter future={{ v7_relativeSplatPath: true, v7_startTransition: true }}>
105+
<AuthInternalContext.Provider value={authContext}>
106+
<SyncInternalContext.Provider value={syncContext}>
107+
{children}
108+
</SyncInternalContext.Provider>
109+
</AuthInternalContext.Provider>
110+
</MemoryRouter>
111+
);
112+
};
113+
}
114+
115+
describe('useWorkspaceData trash refresh', () => {
116+
beforeEach(() => {
117+
jest.clearAllMocks();
118+
119+
(AccessService.getShareWithMe as jest.Mock).mockResolvedValue(null);
120+
(ViewService.getDatabaseRelations as jest.Mock).mockResolvedValue({});
121+
(ViewService.getMultiple as jest.Mock).mockResolvedValue([]);
122+
(ViewService.getOutline as jest.Mock).mockResolvedValue({
123+
outline: [],
124+
folderRid: '1-1',
125+
});
126+
});
127+
128+
it('refreshes stale trash state when a remote restore adds the view back to the folder', async () => {
129+
const eventEmitter = new EventEmitter();
130+
const restoredView = createView(restoredViewId);
131+
let trashResponse: View[] = [restoredView];
132+
133+
(ViewService.getTrash as jest.Mock).mockImplementation(async () => trashResponse);
134+
135+
const { result } = renderHook(() => useWorkspaceData(), {
136+
wrapper: createWrapper(eventEmitter),
137+
});
138+
139+
await waitFor(() => {
140+
expect(result.current.trashList?.map((view) => view.view_id)).toEqual([restoredViewId]);
141+
});
142+
143+
const initialTrashRequestCount = (ViewService.getTrash as jest.Mock).mock.calls.length;
144+
145+
trashResponse = [];
146+
147+
await act(async () => {
148+
eventEmitter.emit(APP_EVENTS.FOLDER_VIEW_CHANGED, {
149+
changeType: 1,
150+
folderRid: '2-1',
151+
parentViewId: 'space-id',
152+
viewJson: JSON.stringify(restoredView),
153+
});
154+
});
155+
156+
await waitFor(() => {
157+
expect(ViewService.getTrash).toHaveBeenCalledTimes(initialTrashRequestCount + 1);
158+
expect(result.current.trashList).toEqual([]);
159+
});
160+
});
161+
162+
it('ignores stale trash refresh responses from overlapping folder notifications', async () => {
163+
const eventEmitter = new EventEmitter();
164+
const restoredView = createView(restoredViewId);
165+
const trashRequests: Array<ReturnType<typeof createDeferred<View[]>>> = [];
166+
167+
(ViewService.getTrash as jest.Mock).mockImplementation(() => {
168+
const deferred = createDeferred<View[]>();
169+
170+
trashRequests.push(deferred);
171+
return deferred.promise;
172+
});
173+
174+
const { result } = renderHook(() => useWorkspaceData(), {
175+
wrapper: createWrapper(eventEmitter),
176+
});
177+
178+
await waitFor(() => {
179+
expect(trashRequests).toHaveLength(1);
180+
});
181+
182+
await act(async () => {
183+
trashRequests[0].resolve([restoredView]);
184+
});
185+
186+
await waitFor(() => {
187+
expect(result.current.trashList?.map((view) => view.view_id)).toEqual([restoredViewId]);
188+
});
189+
190+
await act(async () => {
191+
eventEmitter.emit(APP_EVENTS.FOLDER_VIEW_CHANGED, {
192+
changeType: 1,
193+
folderRid: '2-1',
194+
parentViewId: 'space-id',
195+
viewJson: JSON.stringify(restoredView),
196+
});
197+
eventEmitter.emit(APP_EVENTS.FOLDER_OUTLINE_CHANGED, {
198+
folderRid: '3-1',
199+
outlineDiffJson: JSON.stringify([{ op: 'replace', path: '/outline', value: [] }]),
200+
});
201+
});
202+
203+
await waitFor(() => {
204+
expect(trashRequests).toHaveLength(3);
205+
});
206+
207+
await act(async () => {
208+
trashRequests[2].resolve([]);
209+
});
210+
211+
await waitFor(() => {
212+
expect(result.current.trashList).toEqual([]);
213+
});
214+
215+
await act(async () => {
216+
trashRequests[1].resolve([restoredView]);
217+
});
218+
219+
await waitFor(() => {
220+
expect(result.current.trashList).toEqual([]);
221+
});
222+
});
223+
});

0 commit comments

Comments
 (0)