Skip to content

Commit b75f68a

Browse files
AaronPlaveAaronPlave
authored
Simultaneous-edit protection for workspace files (#1949)
* simultaneous-edit protection for workspace files
1 parent 6f94953 commit b75f68a

28 files changed

Lines changed: 1596 additions & 104 deletions

‎e2e-tests/fixtures/WorkspaceSaveConflict.ts‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import type { Locator, Page } from '@playwright/test';
2+
import { expect } from '@playwright/test';
3+
4+
/**
5+
* Page Object Model for the workspace save-conflict modal (`WorkspaceSaveConflictModal`),
6+
* shown when a save is rejected by the optimistic-concurrency check (`412`). Covers both
7+
* the "conflict" variant (the file changed underneath) and the "deleted" variant (the file
8+
* was deleted/moved underneath). Selectors are scoped to the modal root.
9+
*/
10+
export class WorkspaceSaveConflict {
11+
cancelButton!: Locator;
12+
conflictTitle!: Locator;
13+
deletedTitle!: Locator;
14+
discardButton!: Locator;
15+
mineHeader!: Locator;
16+
modal!: Locator;
17+
recreateButton!: Locator;
18+
takeMineButton!: Locator;
19+
takeTheirsButton!: Locator;
20+
theirsHeader!: Locator;
21+
22+
constructor(public page: Page) {
23+
this.updatePage(page);
24+
}
25+
26+
async cancel(): Promise<void> {
27+
await this.cancelButton.click();
28+
await expect(this.modal).toBeHidden();
29+
}
30+
31+
async discardAndClose(): Promise<void> {
32+
await this.discardButton.click();
33+
await expect(this.modal).toBeHidden();
34+
}
35+
36+
async recreate(): Promise<void> {
37+
await this.recreateButton.click();
38+
await expect(this.modal).toBeHidden();
39+
}
40+
41+
async takeMine(): Promise<void> {
42+
await this.takeMineButton.click();
43+
await expect(this.modal).toBeHidden();
44+
}
45+
46+
async takeTheirs(): Promise<void> {
47+
await this.takeTheirsButton.click();
48+
await expect(this.modal).toBeHidden();
49+
}
50+
51+
updatePage(page: Page): void {
52+
this.page = page;
53+
this.modal = page.locator('#modal-container');
54+
this.conflictTitle = this.modal.getByText('This file was changed by someone else');
55+
this.deletedTitle = this.modal.getByText('File deleted or moved');
56+
this.theirsHeader = this.modal.getByText('Theirs (server)');
57+
this.mineHeader = this.modal.getByText('Mine (your edits)');
58+
this.takeTheirsButton = this.modal.getByRole('button', { name: 'Keep theirs' });
59+
this.takeMineButton = this.modal.getByRole('button', { name: 'Keep mine' });
60+
this.cancelButton = this.modal.getByRole('button', { name: 'Keep editing' });
61+
this.recreateButton = this.modal.getByRole('button', { name: 'Recreate file' });
62+
this.discardButton = this.modal.getByRole('button', { name: 'Discard & close' });
63+
}
64+
65+
/** Wait for the "conflict" variant (read-only diff of theirs vs mine). */
66+
async waitForConflict(): Promise<void> {
67+
await expect(this.conflictTitle).toBeVisible({ timeout: 15000 });
68+
await expect(this.theirsHeader).toBeVisible();
69+
await expect(this.mineHeader).toBeVisible();
70+
}
71+
72+
/** Wait for the "deleted/moved" variant (single read-only pane of mine). */
73+
async waitForDeleted(): Promise<void> {
74+
await expect(this.deletedTitle).toBeVisible({ timeout: 15000 });
75+
}
76+
}

‎e2e-tests/tests/workspace-edit-protection.test.ts‎

Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
import test, { expect } from '@playwright/test';
2+
import { Dictionaries } from '../fixtures/Dictionaries.js';
3+
import { Parcels } from '../fixtures/Parcels.js';
4+
import { Workspace } from '../fixtures/Workspace.js';
5+
import { WorkspaceSaveConflict } from '../fixtures/WorkspaceSaveConflict.js';
6+
import { Workspaces } from '../fixtures/Workspaces.js';
7+
import { setupTest, teardownTest, type BrowserSetupResult } from '../utilities/api.js';
8+
import { generateRandomName } from '../utilities/helpers.js';
9+
10+
// Simultaneous-edit protection: a save is rejected (412) when the file changed underneath the
11+
// editor since it was opened. We simulate the "other editor" with a second browser context for
12+
// the same 'test' user editing the same workspace file.
13+
14+
let setup: BrowserSetupResult; // primary editor — the one under test
15+
let setupOther: BrowserSetupResult; // concurrent editor — changes the file first
16+
17+
let dictionaries: Dictionaries;
18+
let parcels: Parcels;
19+
let workspaces: Workspaces;
20+
let workspace: Workspace; // primary page
21+
let otherWorkspace: Workspace; // concurrent page (same workspace)
22+
let conflict: WorkspaceSaveConflict;
23+
let workspaceId: string;
24+
let workspaceName: string;
25+
26+
test.beforeAll(async ({ baseURL, browser }) => {
27+
test.setTimeout(120000);
28+
29+
setup = await setupTest(browser, { model: false });
30+
dictionaries = new Dictionaries(setup.page);
31+
parcels = new Parcels(setup.page);
32+
workspaces = new Workspaces(setup.page, parcels, baseURL);
33+
34+
await dictionaries.goto();
35+
await dictionaries.createCommandDictionary();
36+
await parcels.goto();
37+
await parcels.createParcel(dictionaries.commandDictionaryName, baseURL);
38+
39+
await workspaces.goto();
40+
workspaceId = await workspaces.createWorkspace();
41+
workspaceName = workspaces.workspaceName;
42+
workspace = new Workspace(setup.page, workspaceId, workspaceName, baseURL);
43+
conflict = new WorkspaceSaveConflict(setup.page);
44+
45+
// Second context (same 'test' user) editing the same workspace concurrently.
46+
setupOther = await setupTest(browser, { model: false });
47+
otherWorkspace = new Workspace(setupOther.page, workspaceId, workspaceName, baseURL);
48+
49+
await workspace.goto();
50+
});
51+
52+
test.afterAll(async () => {
53+
await workspaces.goto();
54+
await workspaces.deleteWorkspace(workspaceName);
55+
await parcels.goto();
56+
await parcels.deleteParcel();
57+
await dictionaries.goto();
58+
await dictionaries.deleteCommandDictionary();
59+
60+
await teardownTest(setup);
61+
await teardownTest(setupOther);
62+
});
63+
64+
/** Create a sequence file on the primary page and open it (capturing its base version). */
65+
async function createAndOpenSequence(): Promise<string> {
66+
const { sequenceName } = await workspace.createSequence(undefined, `${generateRandomName()}.seq`);
67+
await workspace.searchForFileAndWait(sequenceName);
68+
await workspace.clickFile(sequenceName);
69+
await expect(workspace.saveSequenceButton).toBeVisible({ timeout: 10000 });
70+
return sequenceName;
71+
}
72+
73+
/** The concurrent editor opens the same file and saves a change, advancing the server version. */
74+
async function otherEditorSaves(sequenceName: string, content: string): Promise<void> {
75+
await otherWorkspace.goto();
76+
await otherWorkspace.searchForFileAndWait(sequenceName);
77+
await otherWorkspace.clickFile(sequenceName);
78+
await expect(otherWorkspace.saveSequenceButton).toBeVisible({ timeout: 10000 });
79+
await otherWorkspace.fillSequenceContent(content);
80+
await otherWorkspace.saveSequence();
81+
}
82+
83+
/** Edit on the primary page and click Save, expecting the concurrency check to reject it. */
84+
async function editAndSaveExpectingConflict(content: string): Promise<void> {
85+
await workspace.fillSequenceContent(content);
86+
await expect(workspace.saveSequenceButton).toBeEnabled();
87+
await workspace.saveSequenceButton.click();
88+
}
89+
90+
test.describe.serial('Workspace simultaneous-edit protection', () => {
91+
test('consecutive saves of the same file succeed (token round-trips on PUT)', async () => {
92+
// No concurrent editor here: this confirms a save advances the stored token so the next
93+
// save against the same editor does not spuriously conflict.
94+
await createAndOpenSequence();
95+
96+
await workspace.fillSequenceContent('// first change');
97+
await workspace.saveSequence();
98+
await expect(workspace.saveSequenceButton).toBeDisabled();
99+
100+
await workspace.fillSequenceContent('// second change');
101+
await workspace.saveSequence();
102+
await expect(workspace.saveSequenceButton).toBeDisabled();
103+
});
104+
105+
test('a stale save shows the conflict modal with a diff', async () => {
106+
const sequenceName = await createAndOpenSequence();
107+
await otherEditorSaves(sequenceName, '// their change');
108+
109+
await editAndSaveExpectingConflict('// my change');
110+
111+
await conflict.waitForConflict();
112+
113+
// Cancel leaves the document dirty (Save still enabled) and keeps the stale token.
114+
await conflict.cancel();
115+
await expect(workspace.saveSequenceButton).toBeEnabled();
116+
117+
// Resolve the still-dirty document so the next test starts from a clean editor:
118+
// saving again re-checks the (still stale) token, then take theirs to rebase clean.
119+
await workspace.saveSequenceButton.click();
120+
await conflict.waitForConflict();
121+
await conflict.takeTheirs();
122+
await expect(workspace.saveSequenceButton).toBeDisabled();
123+
});
124+
125+
test('"Keep theirs" rebases the editor and the next save succeeds', async () => {
126+
const sequenceName = await createAndOpenSequence();
127+
await otherEditorSaves(sequenceName, '// their change');
128+
129+
await editAndSaveExpectingConflict('// my change');
130+
await conflict.waitForConflict();
131+
await conflict.takeTheirs();
132+
133+
// Editor now reflects the server version; saving a fresh edit succeeds (token advanced,
134+
// no immediate re-conflict).
135+
await workspace.fillSequenceContent('// after taking theirs');
136+
await workspace.saveSequence();
137+
await expect(workspace.saveSequenceButton).toBeDisabled();
138+
});
139+
140+
test('"Keep theirs" is undoable — Ctrl/Cmd+Z restores the discarded edits', async () => {
141+
const sequenceName = await createAndOpenSequence();
142+
await otherEditorSaves(sequenceName, '// their change');
143+
144+
await editAndSaveExpectingConflict('// my change');
145+
await conflict.waitForConflict();
146+
await conflict.takeTheirs();
147+
148+
// Right after taking theirs the editor shows the server version.
149+
const editor = setup.page.locator('.cm-content').first();
150+
await expect(editor).toContainText('// their change');
151+
152+
// The rebase is recorded in the undo history, so a single undo restores the discarded edits
153+
// and the document goes dirty again (restored content differs from the new server baseline).
154+
await editor.click();
155+
await setup.page.keyboard.press('ControlOrMeta+z');
156+
await expect(editor).toContainText('// my change');
157+
await expect(workspace.saveSequenceButton).toBeEnabled();
158+
159+
// Leave a clean editor for the next serial test. "Keep theirs" already advanced the stored
160+
// token to the server version, so this save succeeds without re-conflicting. Without it the
161+
// dirty doc leaks: the next test's createAndOpenSequence auto-selects its new file, which pops
162+
// a "Navigate Away" modal whose backdrop blocks the file-row click.
163+
await workspace.saveSequence();
164+
await expect(workspace.saveSequenceButton).toBeDisabled();
165+
});
166+
167+
test('"Keep mine" saves the local version and the next save succeeds', async () => {
168+
const sequenceName = await createAndOpenSequence();
169+
await otherEditorSaves(sequenceName, '// their change');
170+
171+
await editAndSaveExpectingConflict('// my change');
172+
await conflict.waitForConflict();
173+
await conflict.takeMine();
174+
175+
// Force-overwrite saved successfully and advanced the token.
176+
await workspace.waitForToast('Workspace File Saved Successfully');
177+
await expect(workspace.saveSequenceButton).toBeDisabled();
178+
179+
await workspace.fillSequenceContent('// after taking mine');
180+
await workspace.saveSequence();
181+
await expect(workspace.saveSequenceButton).toBeDisabled();
182+
});
183+
184+
test('Ctrl/Cmd+S is ignored while the conflict modal is open (no stacked save)', async () => {
185+
const sequenceName = await createAndOpenSequence();
186+
await otherEditorSaves(sequenceName, '// their change');
187+
188+
await editAndSaveExpectingConflict('// my change');
189+
await conflict.waitForConflict();
190+
191+
// The global save shortcut must not fire another save / stack a second modal.
192+
await setup.page.keyboard.press('ControlOrMeta+s');
193+
await expect(conflict.takeMineButton).toHaveCount(1);
194+
await expect(conflict.conflictTitle).toBeVisible();
195+
196+
await conflict.takeTheirs();
197+
});
198+
199+
test('a file deleted underneath shows the deleted/moved variant — Recreate restores it', async () => {
200+
const sequenceName = await createAndOpenSequence();
201+
202+
// Concurrent editor deletes the file out from under the open editor.
203+
await otherWorkspace.goto();
204+
await otherWorkspace.searchForFileAndWait(sequenceName);
205+
await otherWorkspace.deleteFile(sequenceName);
206+
207+
await editAndSaveExpectingConflict('// my change after delete');
208+
await conflict.waitForDeleted();
209+
await conflict.recreate();
210+
211+
await workspace.waitForToast('Workspace File Saved Successfully');
212+
// The recreated file is back in the tree.
213+
await workspace.searchForFileAndWait(sequenceName);
214+
await expect(workspace.getFileRow(sequenceName)).toBeVisible();
215+
});
216+
217+
test('the deleted/moved variant — "Discard & close" closes the document', async () => {
218+
const sequenceName = await createAndOpenSequence();
219+
220+
await otherWorkspace.goto();
221+
await otherWorkspace.searchForFileAndWait(sequenceName);
222+
await otherWorkspace.deleteFile(sequenceName);
223+
224+
await editAndSaveExpectingConflict('// my change after delete');
225+
await conflict.waitForDeleted();
226+
await conflict.discardAndClose();
227+
228+
// The unsaved edits are discarded — the editor is no longer dirty (Save disabled) and
229+
// the discarded content is gone.
230+
await expect(workspace.saveSequenceButton).toBeDisabled();
231+
await expect(setup.page.locator('.cm-content').first()).not.toContainText('my change after delete');
232+
});
233+
});

‎e2e-tests/tests/workspace.test.ts‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,17 +603,31 @@ test.describe.serial('Workspace', () => {
603603
});
604604

605605
test('Toggle file read-only and verify editor is locked', async () => {
606-
// Create a sequence file to test with
607-
const { sequenceName } = await workspace.createSequence(undefined, `${generateRandomName()}.seq`);
606+
// Use the adaptation's input-sequence extension so the file opens as a sequence with the
607+
// Selected Command panel (a plain `.seq` would open as generic text — no command panel).
608+
const { sequenceName } = await workspace.createSequence(undefined, `${generateRandomName()}.seqN.txt`);
608609
await workspace.searchForFileAndWait(sequenceName);
609610
await workspace.clickFile(sequenceName);
610611

611612
// Wait for the editor to load and the file metadata banner to appear
612613
await expect(workspace.readOnlyCheckbox).toBeVisible({ timeout: 10000 });
613614

615+
// Add a command so the Selected Command panel renders editable argument inputs.
616+
await workspace.fillSequenceContent('C FSW_CMD_0 "ON" true 0.5');
617+
618+
// The Selected Command panel (right side) shows the command's argument editors;
619+
// float_arg_0 renders as a numeric input (spinbutton) labeled by its arg name.
620+
const commandArgInput = setup.page.getByRole('spinbutton', { name: 'float_arg_0' });
621+
await expect(commandArgInput).toBeVisible({ timeout: 10000 });
622+
614623
// Verify the file is initially editable — title should NOT contain "(Read only)"
615624
await expect(setup.page.getByText('(Read only)')).not.toBeVisible();
616625
await expect(workspace.saveSequenceButton).toBeVisible();
626+
// ...and the command argument inputs are interactive.
627+
await expect(commandArgInput).toBeEnabled();
628+
629+
// Persist so the document is clean before toggling read-only / deleting later.
630+
await workspace.saveSequence();
617631

618632
// Toggle read-only ON
619633
await workspace.readOnlyCheckbox.click();
@@ -625,6 +639,10 @@ test.describe.serial('Workspace', () => {
625639
// Verify the Save button is hidden (read-only files can't be saved)
626640
await expect(workspace.saveSequenceButton).not.toBeVisible();
627641

642+
// Verify the Selected Command panel argument inputs are disabled while read-only —
643+
// `EditorState.readOnly` alone wouldn't stop the form-builder from editing the document.
644+
await expect(commandArgInput).toBeDisabled();
645+
628646
// Verify the editor rejects input — type something and confirm content didn't change
629647
await workspace.sequenceEditor.click();
630648
await setup.page.keyboard.type('this should not appear');
@@ -639,6 +657,8 @@ test.describe.serial('Workspace', () => {
639657

640658
// Verify the Save button reappears
641659
await expect(workspace.saveSequenceButton).toBeVisible();
660+
// ...and the command argument inputs are interactive again.
661+
await expect(commandArgInput).toBeEnabled();
642662

643663
// Cleanup
644664
await workspace.searchForFileAndWait(sequenceName);

‎package-lock.json‎

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)