fix: offer to delete permanently when a Files delete can't go to the trash

This commit is contained in:
Linus Rath
2026-10-29 03:07:40 +01:00
parent 8be9743b80
commit e1e6af3cca
4 changed files with 67 additions and 8 deletions
+1
View File
@@ -515,6 +515,7 @@
"shared_with": "Shared with",
"sharing_access": "Sharing & access",
"symlink": "Symbolic link",
"trash_unavailable_message": "{count, plural, one {This item} other {These # items}} can't be moved to the trash. Delete permanently instead? You can't undo this.",
"upload": "Upload",
"upload_failed": "{count, plural, one {1 file could not be uploaded} other {# files could not be uploaded}}",
"upload_unavailable": "The file picker is not installed in this build. Rebuild the app to enable uploads.",
+26 -1
View File
@@ -21,7 +21,9 @@ vi.mock('../jmap-client', () => ({
import { jmapClient } from '../jmap-client';
import type { FileNode } from '../types';
import { emptyTrash, moveToTrash, purgeFromTrash, restoreFromTrash, undoMoveToTrash } from '../files-trash';
import {
TrashUnavailableError, emptyTrash, moveToTrash, purgeFromTrash, restoreFromTrash, undoMoveToTrash,
} from '../files-trash';
import { trashedItems } from '../../lib/files-trash';
// A Stalwart-like FileNode store for the user's own account: destroy without
@@ -171,6 +173,29 @@ describe('moveToTrash', () => {
expect(destroy?.onDestroyRemoveChildren).toBeUndefined();
});
it('says the trash cannot be had when a file at the root is named .Trash', async () => {
store.push({ id: 'f', parentId: null, name: '.Trash', blobId: 'bf' });
const nodes = listed();
const err = await moveToTrash(nodes, nodes.filter((n) => n.id === 'a'), 1730112000005).catch((e) => e);
expect(err).toBeInstanceOf(TrashUnavailableError);
expect(calls.some((c) => c.update)).toBe(false);
expect(byId('a')!.parentId).toBe('docs');
});
it('says the trash cannot be had when no folder can be made in it (quota)', async () => {
store.push({ id: 'T', parentId: null, name: '.Trash', blobId: null });
const real = (jmapClient.request as ReturnType<typeof vi.fn>).getMockImplementation() as (c: any[]) => Promise<unknown>;
(jmapClient.request as ReturnType<typeof vi.fn>).mockImplementation(async (methodCalls: any[]) => {
if (methodCalls[0][1].create) {
return { methodResponses: [['FileNode/set', { notCreated: { 'new-dir': { type: 'overQuota' } } }, '0']] };
}
return real(methodCalls);
});
const nodes = listed();
await expect(moveToTrash(nodes, nodes.filter((n) => n.id === 'a'), 1730112000005)).rejects.toBeInstanceOf(TrashUnavailableError);
expect(byId('a')!.parentId).toBe('docs');
});
it('reports a partial refusal and keeps what went', async () => {
refuseUpdate.add('b');
const nodes = listed();
+27 -3
View File
@@ -62,6 +62,18 @@ export interface TrashMoveResult extends TrashWriteResult {
drive: TrashDrive;
}
/**
* The trash, or a delete's folder in it, can't be made: a file is named
* `.Trash` at the root, the user may not create folders there, or the quota
* is used up. Nothing was moved; the items can only be deleted for good.
*/
export class TrashUnavailableError extends Error {
constructor(readonly reason: unknown) {
super(reason instanceof Error ? reason.message : 'The trash cannot be used');
this.name = 'TrashUnavailableError';
}
}
/** The trash folder, created on the first delete. */
async function ensureTrashFolder(nodes: FileNode[]): Promise<FileNode> {
const known = findTrashFolder(nodes);
@@ -72,7 +84,7 @@ async function ensureTrashFolder(nodes: FileNode[]): Promise<FileNode> {
// Another client may have made it since the drive was listed.
const fresh = findTrashFolder(await getAllFileNodes());
if (fresh) return fresh;
throw err;
throw new TrashUnavailableError(err);
}
}
@@ -116,13 +128,25 @@ export async function moveToTrash(
// the drive again and try once more.
trash = await ensureTrashFolder(await getAllFileNodes());
assertDrive(drive);
return createFolder(name, trash.id);
try {
return await createFolder(name, trash.id);
} catch (err) {
throw new TrashUnavailableError(err);
}
}
};
for (const [parentId, group] of groups) {
if (!isCurrentDrive(drive)) break;
const batch = await createBatch(parentId);
let batch: FileNode;
try {
batch = await createBatch(parentId);
} catch (err) {
// What went already stays in the trash; the rest is reported.
if (!(err instanceof TrashUnavailableError) || result.done.length === 0) throw err;
result.refused.push(new JMAPSetError('setError', err.message));
break;
}
if (!isCurrentDrive(drive)) break;
const update = await updateFileNodes(Object.fromEntries(group.map((n) => [n.id, { parentId: batch.id }])));
const moved = new Set(update.updated);
+13 -4
View File
@@ -52,7 +52,8 @@ import {
isSymlink, listFolder, moveFileNode, renameFileNode, supportsSharing, uploadFileBatch,
} from '../api/files';
import {
emptyTrash, moveToTrash, purgeFromTrash, restoreFromTrash, undoMoveToTrash, type TrashMoveResult,
TrashUnavailableError, emptyTrash, moveToTrash, purgeFromTrash, restoreFromTrash, undoMoveToTrash,
type TrashMoveResult,
} from '../api/files-trash';
import { trashOriginPath, trashedItems, withoutTrash, type TrashedItem } from '../lib/files-trash';
import { toast } from '../stores/toast-store';
@@ -232,7 +233,8 @@ export default function FilesScreen() {
// The trash (lib/files-trash) is a place of its own, entered from the root.
const [inTrash, setInTrash] = useState(false);
const [confirmDelete, setConfirmDelete] = useState<{ rows: FileRow[] } | null>(null);
// `noTrash`: the rows were meant for the trash, which could not be made.
const [confirmDelete, setConfirmDelete] = useState<{ rows: FileRow[]; noTrash?: boolean } | null>(null);
const [confirmEmptyTrash, setConfirmEmptyTrash] = useState(false);
const [newFolderOpen, setNewFolderOpen] = useState(false);
const [newFolderName, setNewFolderName] = useState('');
@@ -783,7 +785,10 @@ export default function FilesScreen() {
{ action: { label: t('files.undo', 'Undo'), onPress: () => void undoTrash(moved) } },
);
} catch (e) {
Alert.alert(t('files.delete_error', 'Failed to delete'), fileErrorMessage(e, t));
// No trash to be had here (a file named .Trash at the root, no right
// to create folders there, a full quota): offer to delete for good.
if (e instanceof TrashUnavailableError) setConfirmDelete({ rows, noTrash: true });
else Alert.alert(t('files.delete_error', 'Failed to delete'), fileErrorMessage(e, t));
await loadFiles('refresh');
}
}, [allNodes, clearSelection, loadFiles, undoTrash, t]);
@@ -1393,7 +1398,11 @@ export default function FilesScreen() {
count: confirmDelete?.rows.length ?? 1,
})}
message={
confirmDelete && confirmDelete.rows.length > 1
confirmDelete?.noTrash
? t('files.trash_unavailable_message', "{count, plural, one {This item} other {These # items}} can't be moved to the trash. Delete permanently instead? You can't undo this.", {
count: confirmDelete.rows.length,
})
: confirmDelete && confirmDelete.rows.length > 1
? t('files.delete_forever_message_count', "{count, plural, one {1 item} other {# items}} will be deleted. You can't undo this.", {
count: confirmDelete.rows.length,
})