feat(documents): folder service · rename + move + soft-rescue delete
renameFolder + moveFolder enforce sibling-name uniqueness via the shared isSiblingNameConflict helper and reject cross-port leakage at the service boundary. moveFolder walks the destination's ancestor chain to refuse cycles before the write. deleteFolderSoftRescue re-parents every child folder and document up to the deleted folder's parent (or to root) inside a transaction, then drops the folder row. Children never disappear silently — a wrong click moves work up the tree, never deletes it. Audit-logged with rescuedTo metadata. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,7 +1,8 @@
|
|||||||
import { and, asc, eq } from 'drizzle-orm';
|
import { and, asc, eq } from 'drizzle-orm';
|
||||||
|
|
||||||
import { db } from '@/lib/db';
|
import { db } from '@/lib/db';
|
||||||
import { documentFolders, type DocumentFolder } from '@/lib/db/schema/documents';
|
import { documentFolders, documents, type DocumentFolder } from '@/lib/db/schema/documents';
|
||||||
|
import { createAuditLog } from '@/lib/audit';
|
||||||
import { ConflictError, NotFoundError, ValidationError } from '@/lib/errors';
|
import { ConflictError, NotFoundError, ValidationError } from '@/lib/errors';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -110,3 +111,147 @@ export async function createFolder(
|
|||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Renames a folder. Throws ConflictError if a sibling with the same
|
||||||
|
* case-insensitive name already exists. Throws NotFoundError if the
|
||||||
|
* folder doesn't belong to the given port (cross-port leakage guard).
|
||||||
|
*/
|
||||||
|
export async function renameFolder(
|
||||||
|
portId: string,
|
||||||
|
folderId: string,
|
||||||
|
newName: string,
|
||||||
|
): Promise<DocumentFolder> {
|
||||||
|
const trimmed = newName.trim();
|
||||||
|
if (!trimmed) throw new ValidationError('Folder name cannot be empty');
|
||||||
|
if (trimmed.length > 200) throw new ValidationError('Folder name cannot exceed 200 chars');
|
||||||
|
|
||||||
|
const existing = await db.query.documentFolders.findFirst({
|
||||||
|
where: and(eq(documentFolders.id, folderId), eq(documentFolders.portId, portId)),
|
||||||
|
});
|
||||||
|
if (!existing) throw new NotFoundError('Folder');
|
||||||
|
|
||||||
|
try {
|
||||||
|
const [updated] = await db
|
||||||
|
.update(documentFolders)
|
||||||
|
.set({ name: trimmed, updatedAt: new Date() })
|
||||||
|
.where(eq(documentFolders.id, folderId))
|
||||||
|
.returning();
|
||||||
|
if (!updated) throw new NotFoundError('Folder');
|
||||||
|
return updated;
|
||||||
|
} catch (err) {
|
||||||
|
if (isSiblingNameConflict(err)) {
|
||||||
|
throw new ConflictError(`A folder named "${trimmed}" already exists here`);
|
||||||
|
}
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Moves a folder to a new parent (or to root with newParentId=null).
|
||||||
|
* Walks the destination's ancestor chain to detect cycles before
|
||||||
|
* writing. Throws ValidationError for cycle/invalid-parent, NotFoundError
|
||||||
|
* for cross-port access.
|
||||||
|
*/
|
||||||
|
export async function moveFolder(
|
||||||
|
portId: string,
|
||||||
|
folderId: string,
|
||||||
|
newParentId: string | null,
|
||||||
|
): Promise<DocumentFolder> {
|
||||||
|
if (newParentId === folderId) {
|
||||||
|
throw new ValidationError('Cannot move a folder under itself (cycle)');
|
||||||
|
}
|
||||||
|
|
||||||
|
const folder = await db.query.documentFolders.findFirst({
|
||||||
|
where: and(eq(documentFolders.id, folderId), eq(documentFolders.portId, portId)),
|
||||||
|
});
|
||||||
|
if (!folder) throw new NotFoundError('Folder');
|
||||||
|
|
||||||
|
if (newParentId !== null) {
|
||||||
|
const newParent = await db.query.documentFolders.findFirst({
|
||||||
|
where: and(eq(documentFolders.id, newParentId), eq(documentFolders.portId, portId)),
|
||||||
|
});
|
||||||
|
if (!newParent) throw new ValidationError('Invalid parent folder');
|
||||||
|
|
||||||
|
// Cycle check: walk the destination's ancestor chain. If we hit
|
||||||
|
// folderId, the destination is a descendant of the folder being
|
||||||
|
// moved — moving would create a cycle.
|
||||||
|
let cursor: string | null = newParent.parentId;
|
||||||
|
const seen = new Set<string>([newParent.id]);
|
||||||
|
while (cursor) {
|
||||||
|
if (cursor === folderId) {
|
||||||
|
throw new ValidationError('Cannot move a folder under one of its descendants (cycle)');
|
||||||
|
}
|
||||||
|
if (seen.has(cursor)) break; // defensive — pre-existing cycle, bail
|
||||||
|
seen.add(cursor);
|
||||||
|
const next = await db.query.documentFolders.findFirst({
|
||||||
|
where: eq(documentFolders.id, cursor),
|
||||||
|
columns: { parentId: true },
|
||||||
|
});
|
||||||
|
cursor = next?.parentId ?? null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
const [updated] = await db
|
||||||
|
.update(documentFolders)
|
||||||
|
.set({ parentId: newParentId, updatedAt: new Date() })
|
||||||
|
.where(eq(documentFolders.id, folderId))
|
||||||
|
.returning();
|
||||||
|
if (!updated) throw new NotFoundError('Folder');
|
||||||
|
return updated;
|
||||||
|
} catch (err) {
|
||||||
|
if (isSiblingNameConflict(err)) {
|
||||||
|
throw new ConflictError('A folder with that name already exists in the destination');
|
||||||
|
}
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Soft-rescue delete: re-parent every child folder + every linked
|
||||||
|
* document to the deleted folder's parent (or to root if the deleted
|
||||||
|
* folder is at root). Audit-logged. Wrapped in a transaction so
|
||||||
|
* partial failures don't leave dangling rows.
|
||||||
|
*/
|
||||||
|
export async function deleteFolderSoftRescue(
|
||||||
|
portId: string,
|
||||||
|
folderId: string,
|
||||||
|
userId: string,
|
||||||
|
): Promise<void> {
|
||||||
|
await db.transaction(async (tx) => {
|
||||||
|
const folder = await tx.query.documentFolders.findFirst({
|
||||||
|
where: and(eq(documentFolders.id, folderId), eq(documentFolders.portId, portId)),
|
||||||
|
});
|
||||||
|
if (!folder) throw new NotFoundError('Folder');
|
||||||
|
|
||||||
|
const newParent = folder.parentId; // null = re-parent to root
|
||||||
|
|
||||||
|
await tx
|
||||||
|
.update(documentFolders)
|
||||||
|
.set({ parentId: newParent, updatedAt: new Date() })
|
||||||
|
.where(
|
||||||
|
and(
|
||||||
|
eq(documentFolders.parentId, folderId),
|
||||||
|
eq(documentFolders.portId, portId),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
await tx
|
||||||
|
.update(documents)
|
||||||
|
.set({ folderId: newParent, updatedAt: new Date() })
|
||||||
|
.where(and(eq(documents.folderId, folderId), eq(documents.portId, portId)));
|
||||||
|
|
||||||
|
await tx.delete(documentFolders).where(eq(documentFolders.id, folderId));
|
||||||
|
|
||||||
|
void createAuditLog({
|
||||||
|
userId,
|
||||||
|
portId,
|
||||||
|
action: 'delete',
|
||||||
|
entityType: 'document_folder',
|
||||||
|
entityId: folderId,
|
||||||
|
oldValue: { name: folder.name, parentId: folder.parentId },
|
||||||
|
metadata: { rescuedTo: newParent },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
/**
|
/**
|
||||||
* Task 3 — document-folders service: listTree + createFolder (TDD).
|
* Task 3 — document-folders service: listTree + createFolder (TDD).
|
||||||
|
* Task 4 — renameFolder + moveFolder (TDD).
|
||||||
*
|
*
|
||||||
* Uses the makePort factory (not a "setupTestPort" helper — that name
|
* Uses the makePort factory (not a "setupTestPort" helper — that name
|
||||||
* doesn't exist in this codebase). TEST_USER_ID is resolved once via
|
* doesn't exist in this codebase). TEST_USER_ID is resolved once via
|
||||||
@@ -14,7 +15,12 @@ import { eq } from 'drizzle-orm';
|
|||||||
import { db } from '@/lib/db';
|
import { db } from '@/lib/db';
|
||||||
import { documentFolders } from '@/lib/db/schema/documents';
|
import { documentFolders } from '@/lib/db/schema/documents';
|
||||||
import { user } from '@/lib/db/schema/users';
|
import { user } from '@/lib/db/schema/users';
|
||||||
import { listTree, createFolder } from '@/lib/services/document-folders.service';
|
import {
|
||||||
|
listTree,
|
||||||
|
createFolder,
|
||||||
|
renameFolder,
|
||||||
|
moveFolder,
|
||||||
|
} from '@/lib/services/document-folders.service';
|
||||||
import { makePort } from '../helpers/factories';
|
import { makePort } from '../helpers/factories';
|
||||||
|
|
||||||
let TEST_USER_ID = '';
|
let TEST_USER_ID = '';
|
||||||
@@ -96,3 +102,71 @@ describe('document-folders service · createFolder unique-sibling guard', () =>
|
|||||||
).rejects.toThrow(/invalid parent/i);
|
).rejects.toThrow(/invalid parent/i);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('document-folders service · renameFolder', () => {
|
||||||
|
let portId: string;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
const port = await makePort();
|
||||||
|
portId = port.id;
|
||||||
|
await db.delete(documentFolders).where(eq(documentFolders.portId, portId));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('renames a folder and bumps updatedAt', async () => {
|
||||||
|
const folder = await createFolder(portId, TEST_USER_ID, { name: 'Old', parentId: null });
|
||||||
|
const before = folder.updatedAt.getTime();
|
||||||
|
await new Promise((r) => setTimeout(r, 10));
|
||||||
|
const renamed = await renameFolder(portId, folder.id, 'New');
|
||||||
|
expect(renamed.name).toBe('New');
|
||||||
|
expect(renamed.updatedAt.getTime()).toBeGreaterThan(before);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects rename to an existing sibling name', async () => {
|
||||||
|
await createFolder(portId, TEST_USER_ID, { name: 'Existing', parentId: null });
|
||||||
|
const folder = await createFolder(portId, TEST_USER_ID, { name: 'Mine', parentId: null });
|
||||||
|
await expect(renameFolder(portId, folder.id, 'Existing')).rejects.toThrow(/already exists/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('throws NotFound when the folder belongs to another port', async () => {
|
||||||
|
const otherPort = await makePort();
|
||||||
|
const folder = await createFolder(otherPort.id, TEST_USER_ID, { name: 'X', parentId: null });
|
||||||
|
await expect(renameFolder(portId, folder.id, 'Y')).rejects.toThrow(/couldn't find/i);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('document-folders service · moveFolder', () => {
|
||||||
|
let portId: string;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
const port = await makePort();
|
||||||
|
portId = port.id;
|
||||||
|
await db.delete(documentFolders).where(eq(documentFolders.portId, portId));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('moves a folder under a new parent', async () => {
|
||||||
|
const root = await createFolder(portId, TEST_USER_ID, { name: 'Root', parentId: null });
|
||||||
|
const orphan = await createFolder(portId, TEST_USER_ID, { name: 'Orphan', parentId: null });
|
||||||
|
const moved = await moveFolder(portId, orphan.id, root.id);
|
||||||
|
expect(moved.parentId).toBe(root.id);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('moves a folder back to root with parentId=null', async () => {
|
||||||
|
const root = await createFolder(portId, TEST_USER_ID, { name: 'Root', parentId: null });
|
||||||
|
const child = await createFolder(portId, TEST_USER_ID, { name: 'Child', parentId: root.id });
|
||||||
|
const moved = await moveFolder(portId, child.id, null);
|
||||||
|
expect(moved.parentId).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects a move that would create a cycle', async () => {
|
||||||
|
const a = await createFolder(portId, TEST_USER_ID, { name: 'A', parentId: null });
|
||||||
|
const b = await createFolder(portId, TEST_USER_ID, { name: 'B', parentId: a.id });
|
||||||
|
const c = await createFolder(portId, TEST_USER_ID, { name: 'C', parentId: b.id });
|
||||||
|
// moving A under C would create A → B → C → A
|
||||||
|
await expect(moveFolder(portId, a.id, c.id)).rejects.toThrow(/cycle/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects moving a folder under itself', async () => {
|
||||||
|
const a = await createFolder(portId, TEST_USER_ID, { name: 'A', parentId: null });
|
||||||
|
await expect(moveFolder(portId, a.id, a.id)).rejects.toThrow(/cycle/i);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
84
tests/integration/document-folders-soft-delete.test.ts
Normal file
84
tests/integration/document-folders-soft-delete.test.ts
Normal file
@@ -0,0 +1,84 @@
|
|||||||
|
import { describe, it, expect, beforeEach, beforeAll } from 'vitest';
|
||||||
|
import { eq } from 'drizzle-orm';
|
||||||
|
|
||||||
|
import { db } from '@/lib/db';
|
||||||
|
import { documentFolders, documents } from '@/lib/db/schema/documents';
|
||||||
|
import { user } from '@/lib/db/schema/users';
|
||||||
|
import {
|
||||||
|
createFolder,
|
||||||
|
deleteFolderSoftRescue,
|
||||||
|
} from '@/lib/services/document-folders.service';
|
||||||
|
import { makePort } from '../helpers/factories';
|
||||||
|
|
||||||
|
describe('document-folders · deleteFolderSoftRescue', () => {
|
||||||
|
let portId: string;
|
||||||
|
let testUserId: string;
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
const [u] = await db.select({ id: user.id }).from(user).limit(1);
|
||||||
|
testUserId = u!.id;
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
const port = await makePort();
|
||||||
|
portId = port.id;
|
||||||
|
await db.delete(documentFolders).where(eq(documentFolders.portId, portId));
|
||||||
|
});
|
||||||
|
|
||||||
|
it('moves child subfolders up to the deleted folder\'s parent', async () => {
|
||||||
|
const root = await createFolder(portId, testUserId, { name: 'Root', parentId: null });
|
||||||
|
const middle = await createFolder(portId, testUserId, { name: 'Middle', parentId: root.id });
|
||||||
|
const leaf = await createFolder(portId, testUserId, { name: 'Leaf', parentId: middle.id });
|
||||||
|
|
||||||
|
await deleteFolderSoftRescue(portId, middle.id, testUserId);
|
||||||
|
|
||||||
|
const survivor = await db.query.documentFolders.findFirst({
|
||||||
|
where: eq(documentFolders.id, leaf.id),
|
||||||
|
});
|
||||||
|
expect(survivor?.parentId).toBe(root.id);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('moves child documents to the deleted folder\'s parent', async () => {
|
||||||
|
const root = await createFolder(portId, testUserId, { name: 'Root', parentId: null });
|
||||||
|
const child = await createFolder(portId, testUserId, { name: 'Child', parentId: root.id });
|
||||||
|
|
||||||
|
const [doc] = await db
|
||||||
|
.insert(documents)
|
||||||
|
.values({
|
||||||
|
portId,
|
||||||
|
documentType: 'other',
|
||||||
|
title: 'Orphan-rescue test',
|
||||||
|
createdBy: testUserId,
|
||||||
|
folderId: child.id,
|
||||||
|
})
|
||||||
|
.returning();
|
||||||
|
|
||||||
|
await deleteFolderSoftRescue(portId, child.id, testUserId);
|
||||||
|
|
||||||
|
const updatedDoc = await db.query.documents.findFirst({
|
||||||
|
where: eq(documents.id, doc!.id),
|
||||||
|
});
|
||||||
|
expect(updatedDoc?.folderId).toBe(root.id);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('moves root-folder children to root (folderId=null) when the deleted folder is at root', async () => {
|
||||||
|
const folder = await createFolder(portId, testUserId, { name: 'TopLevel', parentId: null });
|
||||||
|
const child = await createFolder(portId, testUserId, {
|
||||||
|
name: 'Survivor',
|
||||||
|
parentId: folder.id,
|
||||||
|
});
|
||||||
|
await deleteFolderSoftRescue(portId, folder.id, testUserId);
|
||||||
|
const survivor = await db.query.documentFolders.findFirst({
|
||||||
|
where: eq(documentFolders.id, child.id),
|
||||||
|
});
|
||||||
|
expect(survivor?.parentId).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('throws NotFound for a folder in another port', async () => {
|
||||||
|
const otherPort = await makePort();
|
||||||
|
const folder = await createFolder(otherPort.id, testUserId, { name: 'X', parentId: null });
|
||||||
|
await expect(deleteFolderSoftRescue(portId, folder.id, testUserId)).rejects.toThrow(
|
||||||
|
/couldn't find/i,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user