mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-09-01 21:27:58 +00:00
fix: surface NOT_EMPTY error code so DeleteFileConfirm can offer recursive delete (#1765)
* fix: surface NOT_EMPTY error code so DeleteFileConfirm can offer recursive retry The parseApiError helper discards the backend's machine-readable code field, returning only the human-readable message. DeleteFileConfirm tried to detect non-empty directory refusals by matching the substring NOT_EMPTY against the message text, but the actual server message is "Directory is not empty" (with spaces, not underscores), so the two-step "Delete all" confirmation flow was dead code. Add a NotEmptyError class (mirroring the existing UploadConflictError pattern) and intercept HTTP 409 responses in deleteStackPath so the code is preserved. Replace the fragile string match in DeleteFileConfirm with instanceof. * fix: extend NOT_EMPTY fix to volume roots and fix stale viewer after recursive delete P0-1: sendFsError's helper/ExecError branch (volume-browser deletes) never attached a code field to 409 responses, so the frontend NotEmptyError was never thrown for named-volume non-empty directories. Map ExecError 409s whose message matches 'not empty' to code: NOT_EMPTY. P0-2: The context-menu delete onDeleted callback used an exact-match check (ctxDeletePath === selectedPath) to decide whether to clear the viewer. When deleting a folder containing the open file, the viewer stayed open showing now-deleted content. Use the existing openFileAffectedBy helper (which checks ancestor paths) instead, matching bulk-delete behavior.
This commit is contained in:
@@ -14,6 +14,8 @@ import {
|
||||
nextDuplicateName,
|
||||
createEmptyStackFile,
|
||||
UploadConflictError,
|
||||
deleteStackPath,
|
||||
NotEmptyError,
|
||||
} from '../stackFilesApi';
|
||||
|
||||
describe('isClientSafeRelPath', () => {
|
||||
@@ -193,3 +195,77 @@ describe('createEmptyStackFile', () => {
|
||||
expect(err).not.toBeInstanceOf(UploadConflictError);
|
||||
});
|
||||
});
|
||||
|
||||
describe('deleteStackPath', () => {
|
||||
function stubFetch(status: number, body?: object) {
|
||||
const res = {
|
||||
status,
|
||||
ok: status >= 200 && status < 300,
|
||||
headers: { get: () => null },
|
||||
clone() { return this; },
|
||||
json: async () => body ?? {},
|
||||
};
|
||||
const fetchMock = vi.fn().mockResolvedValue(res);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
return fetchMock;
|
||||
}
|
||||
|
||||
beforeEach(() => localStorage.clear());
|
||||
afterEach(() => vi.unstubAllGlobals());
|
||||
|
||||
it('resolves on success without throwing', async () => {
|
||||
const fetchMock = stubFetch(204);
|
||||
await expect(deleteStackPath('my-stack', 'nonempty')).resolves.toBeUndefined();
|
||||
const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit];
|
||||
expect(new URL(url, 'http://x').searchParams.get('path')).toBe('nonempty');
|
||||
expect(init).toMatchObject({ method: 'DELETE' });
|
||||
});
|
||||
|
||||
it('throws NotEmptyError when the server reports the directory is not empty', async () => {
|
||||
stubFetch(409, { code: 'NOT_EMPTY', error: 'Directory is not empty' });
|
||||
await expect(deleteStackPath('my-stack', 'nonempty')).rejects.toBeInstanceOf(NotEmptyError);
|
||||
});
|
||||
|
||||
it('includes the server error message in NotEmptyError', async () => {
|
||||
stubFetch(409, { code: 'NOT_EMPTY', error: 'Directory is not empty' });
|
||||
const err = await deleteStackPath('my-stack', 'nonempty').catch((e) => e);
|
||||
expect(err).toBeInstanceOf(NotEmptyError);
|
||||
expect((err as Error).message).toBe('Directory is not empty');
|
||||
});
|
||||
|
||||
it('uses a fallback message when the server omits the error field', async () => {
|
||||
stubFetch(409, { code: 'NOT_EMPTY' });
|
||||
const err = await deleteStackPath('my-stack', 'nonempty').catch((e) => e);
|
||||
expect(err).toBeInstanceOf(NotEmptyError);
|
||||
expect((err as Error).message).toBe('Directory is not empty.');
|
||||
});
|
||||
|
||||
it('throws a generic error (not NotEmptyError) for other 409 codes', async () => {
|
||||
stubFetch(409, { code: 'PROTECTED_FILE', error: 'This is a protected stack file.' });
|
||||
const err = await deleteStackPath('my-stack', 'compose.yaml').catch((e) => e);
|
||||
expect(err).toBeInstanceOf(Error);
|
||||
expect(err).not.toBeInstanceOf(NotEmptyError);
|
||||
expect((err as Error).message).toBe('This is a protected stack file.');
|
||||
});
|
||||
|
||||
it('passes recursive=1 in the query string when recursive is true', async () => {
|
||||
const fetchMock = stubFetch(204);
|
||||
await deleteStackPath('my-stack', 'nonempty', true);
|
||||
const [url] = fetchMock.mock.calls[0] as [string];
|
||||
expect(new URL(url, 'http://x').searchParams.get('recursive')).toBe('1');
|
||||
});
|
||||
|
||||
it('omits the recursive query param when recursive is false', async () => {
|
||||
const fetchMock = stubFetch(204);
|
||||
await deleteStackPath('my-stack', 'nonempty', false);
|
||||
const [url] = fetchMock.mock.calls[0] as [string];
|
||||
expect(new URL(url, 'http://x').searchParams.get('recursive')).toBeNull();
|
||||
});
|
||||
|
||||
it('includes rootId in the query string when provided', async () => {
|
||||
const fetchMock = stubFetch(204);
|
||||
await deleteStackPath('my-stack', 'nonempty', undefined, 'vol-abc');
|
||||
const [url] = fetchMock.mock.calls[0] as [string];
|
||||
expect(new URL(url, 'http://x').searchParams.get('rootId')).toBe('vol-abc');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -380,6 +380,20 @@ export async function writeStackFile(
|
||||
return { version, mtimeMs };
|
||||
}
|
||||
|
||||
/**
|
||||
* Thrown by deleteStackPath when the server refuses to delete a non-empty
|
||||
* directory without the recursive flag (HTTP 409, code NOT_EMPTY). The
|
||||
* DeleteFileConfirm dialog promotes its confirm button to "Delete all" on
|
||||
* this signal and retries with recursive=true.
|
||||
*/
|
||||
export class NotEmptyError extends Error {
|
||||
readonly code = 'NOT_EMPTY' as const;
|
||||
constructor(message: string) {
|
||||
super(message);
|
||||
this.name = 'NotEmptyError';
|
||||
}
|
||||
}
|
||||
|
||||
export async function deleteStackPath(
|
||||
stackName: string,
|
||||
relPath: string,
|
||||
@@ -392,6 +406,15 @@ export async function deleteStackPath(
|
||||
stackFilesUrl(stackName, `?path=${encodeURIComponent(relPath)}${recursiveSuffix}${rootParam(rootId)}`),
|
||||
{ method: 'DELETE' },
|
||||
);
|
||||
if (res.status === 409) {
|
||||
let body: { code?: string; error?: string } = {};
|
||||
try { body = await res.clone().json(); } catch { /* ignore */ }
|
||||
if (body.code === 'NOT_EMPTY') {
|
||||
throw new NotEmptyError(body.error ?? 'Directory is not empty.');
|
||||
}
|
||||
// PROTECTED_FILE and any other 409 fall through to the generic Error path
|
||||
// so the caller surfaces the server message as a toast.
|
||||
}
|
||||
if (!res.ok) throw new Error(await parseApiError(res));
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user