mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-01 19:42:03 +00:00
fix(studio): expect what deleting a clip already wrote, not what it read
Deleting a clip refused its own save. The flow reads the file, POSTs `remove-element` — which rewrites the file server-side — then saves the duration shrink on top. That second write carried the content read at the start as its optimistic-concurrency expectation, but the server had already moved the file on, so it came back 409 "file conflict". The save queue pauses on a conflict rather than retry stale work, so the error surfaced and nothing persisted afterwards; the clip stayed on the timeline until a reload, since the store update runs after the save and the throw skipped it. The undo baseline and "what is on disk" were one value in saveProjectFilesWithHistory, which is right until a server-side mutation has already written part of the edit. They are now separable: `diskContent` says what to expect on disk, `readFile` still says what undo restores. Both delete paths pass what remove-element left behind. Measured in the studio, before and after: PUT /files/index.html 409 -> 200, no error toast, the clip leaves the timeline immediately, and undo still brings it back. Pre-existing — reproduced identically on the pre-session studio source.
This commit is contained in:
@@ -141,6 +141,9 @@ export function useElementLifecycleOps({
|
||||
kind: "timeline",
|
||||
files: { [targetPath]: patchedContent },
|
||||
readFile: async () => originalContent,
|
||||
// remove-element already wrote the removal, so disk holds THAT — not
|
||||
// the content read at the top. Undo still goes back to the original.
|
||||
diskContent: { [targetPath]: patchedContent },
|
||||
writeFile: writeProjectFile,
|
||||
recordEdit: editHistory.recordEdit,
|
||||
});
|
||||
|
||||
@@ -448,6 +448,9 @@ export function useTimelineEditing({
|
||||
kind: "timeline",
|
||||
files: { [targetPath]: patchedContent },
|
||||
readFile: async () => originalContent,
|
||||
// remove-element already wrote the removal, so disk holds THAT — not the
|
||||
// content read at the top. Undo still goes back to the original.
|
||||
diskContent: { [targetPath]: removedContent },
|
||||
writeFile: writeProjectFile,
|
||||
recordEdit,
|
||||
});
|
||||
|
||||
@@ -29,6 +29,55 @@ describe("saveProjectFilesWithHistory", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Deleting a clip POSTs `remove-element`, which rewrites the file server-side,
|
||||
* and only then saves the duration shrink. Expecting the content read before
|
||||
* the mutation made the server refuse that write as a conflict: the save queue
|
||||
* paused on the 409 and the clip stayed on the timeline until a reload.
|
||||
*/
|
||||
it("expects what is on disk, not the undo baseline, when they differ", async () => {
|
||||
const expectations: Record<string, string | undefined> = {};
|
||||
const recordEdit = vi.fn();
|
||||
|
||||
await saveProjectFilesWithHistory({
|
||||
projectId: "project-1",
|
||||
label: "Delete timeline clip",
|
||||
kind: "timeline",
|
||||
files: { "index.html": "removed+shrunk" },
|
||||
readFile: async () => "original",
|
||||
diskContent: { "index.html": "removed" },
|
||||
writeFile: async (path, _content, expectedContent) => {
|
||||
expectations[path] = expectedContent;
|
||||
},
|
||||
recordEdit,
|
||||
});
|
||||
|
||||
expect(expectations["index.html"]).toBe("removed");
|
||||
// Undo still goes all the way back, which is the whole reason the two are
|
||||
// allowed to differ.
|
||||
expect(recordEdit).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
files: { "index.html": { before: "original", after: "removed+shrunk" } },
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("still expects the undo baseline when nothing says otherwise", async () => {
|
||||
const expectations: Record<string, string | undefined> = {};
|
||||
await saveProjectFilesWithHistory({
|
||||
projectId: "project-1",
|
||||
label: "Move layer",
|
||||
kind: "manual",
|
||||
files: { "index.html": "after" },
|
||||
readFile: async () => "before",
|
||||
writeFile: async (path, _content, expectedContent) => {
|
||||
expectations[path] = expectedContent;
|
||||
},
|
||||
recordEdit: vi.fn(),
|
||||
});
|
||||
expect(expectations["index.html"]).toBe("before");
|
||||
});
|
||||
|
||||
it("skips writes and history for unchanged content", async () => {
|
||||
const writeFile = vi.fn();
|
||||
const recordEdit = vi.fn();
|
||||
|
||||
@@ -34,6 +34,21 @@ interface SaveProjectFilesWithHistoryInput {
|
||||
readFile: (path: string) => Promise<string>;
|
||||
writeFile: ProjectFileWriter;
|
||||
recordEdit: (entry: RecordEditInput) => Promise<void>;
|
||||
/**
|
||||
* What a path holds ON DISK right now, when that is not the same as the
|
||||
* history's "before".
|
||||
*
|
||||
* The two are normally one value, so the write's optimistic-concurrency
|
||||
* expectation was taken straight from the undo baseline. They come apart when
|
||||
* a server-side mutation has already written part of the edit: deleting a clip
|
||||
* POSTs `remove-element`, which rewrites the file, and only then saves the
|
||||
* duration shrink — expecting the pre-delete content it read at the start. The
|
||||
* server had moved the file on, so the write was refused as a conflict, the
|
||||
* save queue paused, and the clip stayed on the timeline until a reload.
|
||||
*
|
||||
* Undo still restores `before`; this only says what to expect on disk.
|
||||
*/
|
||||
diskContent?: Record<string, string>;
|
||||
}
|
||||
|
||||
export async function readProjectFileContent(pid: string, path: string): Promise<string> {
|
||||
@@ -57,6 +72,7 @@ export async function saveProjectFilesWithHistory({
|
||||
readFile,
|
||||
writeFile,
|
||||
recordEdit,
|
||||
diskContent,
|
||||
}: SaveProjectFilesWithHistoryInput): Promise<string[]> {
|
||||
return serializeStudioFileMutations(writeFile, Object.keys(files), async () => {
|
||||
const snapshots: Record<string, { before: string; after: string }> = {};
|
||||
@@ -73,7 +89,7 @@ export async function saveProjectFilesWithHistory({
|
||||
const writtenPaths: string[] = [];
|
||||
try {
|
||||
for (const path of changedPaths) {
|
||||
await writeFile(path, snapshots[path].after, snapshots[path].before);
|
||||
await writeFile(path, snapshots[path].after, diskContent?.[path] ?? snapshots[path].before);
|
||||
writtenPaths.push(path);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user