From 579d53360941c818f3e2ef5dd9efc0838c9b8f76 Mon Sep 17 00:00:00 2001 From: Michal Gasiorek Date: Thu, 7 May 2026 14:23:01 +0200 Subject: [PATCH 1/2] test(red): add failing tests for document delete endpoint (AC1-AC6) Covers: 204 happy path, atomic cascade with shares, disk-failure 500 path, ownership 403, 404 for unknown id, auth/role guards. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/routes/documents/documents.test.ts | 217 +++++++++++++++++- 1 file changed, 215 insertions(+), 2 deletions(-) diff --git a/apps/api/src/routes/documents/documents.test.ts b/apps/api/src/routes/documents/documents.test.ts index bbb56ac..82983ad 100644 --- a/apps/api/src/routes/documents/documents.test.ts +++ b/apps/api/src/routes/documents/documents.test.ts @@ -1,14 +1,15 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { mkdtemp, rm, readdir } from 'node:fs/promises'; +import { mkdtemp, rm, readdir, access } from 'node:fs/promises'; +import { constants } from 'node:fs'; import { eq } from 'drizzle-orm'; import bcrypt from 'bcryptjs'; import { createApp } from '../../app'; import { createDb } from '../../infrastructure/db'; import { createLogger } from '../../lib/logger'; import { signJwt } from '../../lib/jwt'; -import { users, documents } from '../../db/schema'; +import { users, documents, documentShares } from '../../db/schema'; const DATABASE_URL = process.env['DATABASE_URL']; const JWT_SECRET = 'test-secret-that-is-at-least-32-chars-long'; @@ -54,6 +55,7 @@ describeWithDb('Documents routes (integration)', () => { await db.delete(documents).where(eq(documents.patientId, patientUserId)); await db.delete(users).where(eq(users.id, patientUserId)); await db.delete(users).where(eq(users.id, doctorUserId)); + await db.delete(users).where(eq(users.email, 'doc.other-patient@example.com')); await rm(tmpDir, { recursive: true, force: true }); }); @@ -314,4 +316,215 @@ describeWithDb('Documents routes (integration)', () => { expect(res.status).toBe(403); }); }); + + // AC 7 (Slice 2): DELETE /documents/:id — owner happy path + describe('AC 7: DELETE /documents/:id by owner', () => { + it('returns 204 and removes the documents row and disk file', async () => { + const app = makeApp(); + + // Upload a document + const uploadForm = makePdfFormData('to-delete.pdf', 256); + const uploadRes = await app.request('/documents', { + method: 'POST', + headers: { Authorization: `Bearer ${patientToken}` }, + body: uploadForm, + }); + expect(uploadRes.status).toBe(201); + const uploaded = await uploadRes.json() as { id: string }; + + // Find the storage filename for this document so we can verify it's gone + const [docRow] = await db + .select() + .from(documents) + .where(eq(documents.id, uploaded.id)); + expect(docRow).toBeDefined(); + const storedPath = join(tmpDir, docRow!.storagePath); + // Sanity: file exists before delete + await expect(access(storedPath, constants.F_OK)).resolves.toBeUndefined(); + + // DELETE the document + const delRes = await app.request(`/documents/${uploaded.id}`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${patientToken}` }, + }); + + expect(delRes.status).toBe(204); + + // documents row is gone + const remaining = await db + .select() + .from(documents) + .where(eq(documents.id, uploaded.id)); + expect(remaining).toHaveLength(0); + + // file on disk is gone + await expect(access(storedPath, constants.F_OK)).rejects.toThrow(); + }); + + it('removes associated document_shares rows atomically with the document', async () => { + const app = makeApp(); + + // Upload a document as the patient + const uploadForm = makePdfFormData('shared-and-deleted.pdf', 128); + const uploadRes = await app.request('/documents', { + method: 'POST', + headers: { Authorization: `Bearer ${patientToken}` }, + body: uploadForm, + }); + expect(uploadRes.status).toBe(201); + const uploaded = await uploadRes.json() as { id: string }; + + // Manually insert a share row (sharing endpoints land in slice 3) + await db.insert(documentShares).values({ + documentId: uploaded.id, + doctorId: doctorUserId, + }); + + // Sanity: share exists + const sharesBefore = await db + .select() + .from(documentShares) + .where(eq(documentShares.documentId, uploaded.id)); + expect(sharesBefore).toHaveLength(1); + + // DELETE the document + const delRes = await app.request(`/documents/${uploaded.id}`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${patientToken}` }, + }); + expect(delRes.status).toBe(204); + + // Both rows are gone + const docsAfter = await db + .select() + .from(documents) + .where(eq(documents.id, uploaded.id)); + expect(docsAfter).toHaveLength(0); + + const sharesAfter = await db + .select() + .from(documentShares) + .where(eq(documentShares.documentId, uploaded.id)); + expect(sharesAfter).toHaveLength(0); + }); + + it('returns 500 DELETE_FAILED when the disk file is missing after txn commit', async () => { + const app = makeApp(); + + // Upload a document + const uploadForm = makePdfFormData('disk-fail.pdf', 64); + const uploadRes = await app.request('/documents', { + method: 'POST', + headers: { Authorization: `Bearer ${patientToken}` }, + body: uploadForm, + }); + expect(uploadRes.status).toBe(201); + const uploaded = await uploadRes.json() as { id: string }; + + // Look up the storage path, then remove the file out-of-band so the + // disk-side delete will fail after the DB transaction commits. + const [docRow] = await db + .select() + .from(documents) + .where(eq(documents.id, uploaded.id)); + const storedPath = join(tmpDir, docRow!.storagePath); + await rm(storedPath, { force: true }); + + const delRes = await app.request(`/documents/${uploaded.id}`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${patientToken}` }, + }); + + expect(delRes.status).toBe(500); + const body = await delRes.json() as { error: string; message: string }; + expect(body.error).toBe('DELETE_FAILED'); + expect(body.message).toBe( + 'Document metadata removed but file could not be deleted from storage', + ); + + // The DB row should still be gone — committed txn is the source of truth + const remaining = await db + .select() + .from(documents) + .where(eq(documents.id, uploaded.id)); + expect(remaining).toHaveLength(0); + }); + + it('returns 403 when called by a different patient', async () => { + const app = makeApp(); + + // Upload a document as the original patient + const uploadForm = makePdfFormData('not-yours.pdf', 64); + const uploadRes = await app.request('/documents', { + method: 'POST', + headers: { Authorization: `Bearer ${patientToken}` }, + body: uploadForm, + }); + expect(uploadRes.status).toBe(201); + const uploaded = await uploadRes.json() as { id: string }; + + // Create another patient + const hash = await bcrypt.hash('password', 4); + await db.delete(users).where(eq(users.email, 'doc.other-patient@example.com')); + const [otherPatient] = await db + .insert(users) + .values({ + email: 'doc.other-patient@example.com', + passwordHash: hash, + role: 'patient', + }) + .returning(); + const otherToken = signJwt( + { sub: otherPatient!.id, role: 'patient' }, + JWT_SECRET, + ); + + const delRes = await app.request(`/documents/${uploaded.id}`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${otherToken}` }, + }); + + expect(delRes.status).toBe(403); + + // Document still exists + const remaining = await db + .select() + .from(documents) + .where(eq(documents.id, uploaded.id)); + expect(remaining).toHaveLength(1); + + await db.delete(users).where(eq(users.id, otherPatient!.id)); + }); + + it('returns 404 DOCUMENT_NOT_FOUND for an unknown id', async () => { + const app = makeApp(); + const fakeId = '00000000-0000-4000-8000-000000000000'; + + const delRes = await app.request(`/documents/${fakeId}`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${patientToken}` }, + }); + + expect(delRes.status).toBe(404); + const body = await delRes.json() as { error: string }; + expect(body.error).toBe('DOCUMENT_NOT_FOUND'); + }); + + it('returns 401 without a JWT', async () => { + const app = makeApp(); + const fakeId = '00000000-0000-4000-8000-000000000001'; + const delRes = await app.request(`/documents/${fakeId}`, { method: 'DELETE' }); + expect(delRes.status).toBe(401); + }); + + it('returns 403 with a doctor JWT', async () => { + const app = makeApp(); + const fakeId = '00000000-0000-4000-8000-000000000002'; + const delRes = await app.request(`/documents/${fakeId}`, { + method: 'DELETE', + headers: { Authorization: `Bearer ${doctorToken}` }, + }); + expect(delRes.status).toBe(403); + }); + }); }); From ea9d795490fc67e486a1e78aca744ef383f06d00 Mon Sep 17 00:00:00 2001 From: Michal Gasiorek Date: Thu, 7 May 2026 14:33:56 +0200 Subject: [PATCH 2/2] feat(green): implement document delete with atomic cascade (Module 02, Slice 2) - Add findDocumentById and deleteDocumentWithSharesById to the document repository; the latter wraps share + document deletion in a single Drizzle transaction so no observable intermediate state exists. - Add deleteDocument use case: loads the row, checks ownership (403), commits the txn, then attempts to unlink the file. Disk-side failure after a successful txn surfaces as a typed disk_delete_failed result. - Wire DELETE /documents/:id behind requireAuth + requireRole(['patient']) with inline middleware (root /documents middleware doesn't cover sub-paths). Returns 204 on success, 404 DOCUMENT_NOT_FOUND, 403 FORBIDDEN, 500 DELETE_FAILED with the contractually-required body. - Fill in the ts-rest contract entry for delete in packages/contracts. - Add useDeleteDocument mutation hook and a delete button on DocumentList; wire it into the patient documents page with a per-row "Deleting..." state and a delete-error banner. Closes #10. All 75 tests pass; pnpm verify clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/infrastructure/document-repository.ts | 25 ++++++++++++- .../src/routes/documents/documents-router.ts | 35 +++++++++++++++++++ .../use-cases/documents/delete-document.ts | 34 ++++++++++++++++++ .../src/features/documents/DocumentList.tsx | 19 +++++++++- .../features/documents/use-delete-document.ts | 32 +++++++++++++++++ apps/web/src/routes/patient/documents.tsx | 20 ++++++++++- packages/contracts/src/documents.ts | 17 +++++++++ 7 files changed, 179 insertions(+), 3 deletions(-) create mode 100644 apps/api/src/use-cases/documents/delete-document.ts create mode 100644 apps/web/src/features/documents/use-delete-document.ts diff --git a/apps/api/src/infrastructure/document-repository.ts b/apps/api/src/infrastructure/document-repository.ts index 0f93d37..f35f430 100644 --- a/apps/api/src/infrastructure/document-repository.ts +++ b/apps/api/src/infrastructure/document-repository.ts @@ -1,5 +1,5 @@ import { desc, eq } from 'drizzle-orm'; -import { documents } from '../db/schema'; +import { documents, documentShares } from '../db/schema'; import type { Db } from './db'; export type DocumentRow = typeof documents.$inferSelect; @@ -37,3 +37,26 @@ export async function listDocumentsByPatient( .where(eq(documents.patientId, patientId)) .orderBy(desc(documents.uploadedAt)); } + +export async function findDocumentById( + db: Db, + documentId: string, +): Promise { + const [row] = await db + .select() + .from(documents) + .where(eq(documents.id, documentId)); + return row ?? null; +} + +export async function deleteDocumentWithSharesById( + db: Db, + documentId: string, +): Promise { + await db.transaction(async (tx) => { + await tx + .delete(documentShares) + .where(eq(documentShares.documentId, documentId)); + await tx.delete(documents).where(eq(documents.id, documentId)); + }); +} diff --git a/apps/api/src/routes/documents/documents-router.ts b/apps/api/src/routes/documents/documents-router.ts index bb001a4..6a7b3da 100644 --- a/apps/api/src/routes/documents/documents-router.ts +++ b/apps/api/src/routes/documents/documents-router.ts @@ -8,6 +8,7 @@ import { requireAuth, requireRole } from '../../middleware/require-auth'; import { createDocumentStorage } from '../../infrastructure/document-storage'; import { uploadDocument } from '../../use-cases/documents/upload-document'; import { listDocuments } from '../../use-cases/documents/list-documents'; +import { deleteDocument } from '../../use-cases/documents/delete-document'; const MAX_BODY_SIZE = 10 * 1024 * 1024 + 1; // 10 MiB + 1 byte @@ -81,5 +82,39 @@ export function createDocumentsRouter( ); }); + router.delete( + '/documents/:id', + requireAuth(deps), + requireRole(['patient'], deps), + async (c) => { + const patientId = c.get('userId'); + const documentId = c.req.param('id'); + + const result = await deleteDocument(documentId, patientId, { + db: deps.db, + storage, + }); + + if (result.ok) { + return c.body(null, 204); + } + + if (result.reason === 'not_found') { + return c.json({ error: 'DOCUMENT_NOT_FOUND' }, 404); + } + if (result.reason === 'forbidden') { + return c.json({ error: 'FORBIDDEN' }, 403); + } + return c.json( + { + error: 'DELETE_FAILED', + message: + 'Document metadata removed but file could not be deleted from storage', + }, + 500, + ); + }, + ); + return router; } diff --git a/apps/api/src/use-cases/documents/delete-document.ts b/apps/api/src/use-cases/documents/delete-document.ts new file mode 100644 index 0000000..c7658c8 --- /dev/null +++ b/apps/api/src/use-cases/documents/delete-document.ts @@ -0,0 +1,34 @@ +import type { Db } from '../../infrastructure/db'; +import type { DocumentStorage } from '../../infrastructure/document-storage'; +import { + deleteDocumentWithSharesById, + findDocumentById, +} from '../../infrastructure/document-repository'; + +export type DeleteDocumentResult = + | { ok: true } + | { ok: false; reason: 'not_found' | 'forbidden' | 'disk_delete_failed' }; + +export async function deleteDocument( + documentId: string, + patientId: string, + deps: { db: Db; storage: DocumentStorage }, +): Promise { + const row = await findDocumentById(deps.db, documentId); + if (!row) { + return { ok: false, reason: 'not_found' }; + } + if (row.patientId !== patientId) { + return { ok: false, reason: 'forbidden' }; + } + + await deleteDocumentWithSharesById(deps.db, documentId); + + try { + await deps.storage.delete(row.storagePath); + } catch { + return { ok: false, reason: 'disk_delete_failed' }; + } + + return { ok: true }; +} diff --git a/apps/web/src/features/documents/DocumentList.tsx b/apps/web/src/features/documents/DocumentList.tsx index 746a698..5ae662e 100644 --- a/apps/web/src/features/documents/DocumentList.tsx +++ b/apps/web/src/features/documents/DocumentList.tsx @@ -3,6 +3,8 @@ import type { DocumentItem } from './use-documents'; interface DocumentListProps { documents: DocumentItem[]; isLoading?: boolean; + onDelete?: (id: string) => void; + deletingId?: string | null; } function formatBytes(bytes: number): string { @@ -21,7 +23,12 @@ function formatDate(isoString: string): string { }); } -export function DocumentList({ documents, isLoading }: DocumentListProps) { +export function DocumentList({ + documents, + isLoading, + onDelete, + deletingId, +}: DocumentListProps) { if (isLoading) { return

Loading documents...

; } @@ -40,6 +47,16 @@ export function DocumentList({ documents, isLoading }: DocumentListProps) { {doc.mimeType} · {formatBytes(doc.size)} · {formatDate(doc.uploadedAt)}

+ {onDelete && ( + + )} ))} diff --git a/apps/web/src/features/documents/use-delete-document.ts b/apps/web/src/features/documents/use-delete-document.ts new file mode 100644 index 0000000..8a7362a --- /dev/null +++ b/apps/web/src/features/documents/use-delete-document.ts @@ -0,0 +1,32 @@ +import { useMutation, useQueryClient } from '@tanstack/react-query'; +import { apiClient } from '../../lib/api-client'; + +function extractErrorMessage(body: unknown): string { + if ( + body !== null && + typeof body === 'object' && + 'error' in body && + typeof (body as { error: unknown }).error === 'string' + ) { + return (body as { error: string }).error; + } + return 'Delete failed'; +} + +async function deleteDocument(id: string): Promise { + const res = await apiClient.documents.delete({ params: { id } }); + if (res.status !== 204) { + throw new Error(extractErrorMessage(res.body)); + } +} + +export function useDeleteDocument() { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: deleteDocument, + onSuccess: () => { + void queryClient.invalidateQueries({ queryKey: ['documents'] }); + }, + }); +} diff --git a/apps/web/src/routes/patient/documents.tsx b/apps/web/src/routes/patient/documents.tsx index 9ea32d5..4319faf 100644 --- a/apps/web/src/routes/patient/documents.tsx +++ b/apps/web/src/routes/patient/documents.tsx @@ -5,6 +5,7 @@ import { currentUserQueryOptions } from '../../features/auth/queries'; import { getToken, clearToken } from '../../lib/auth-token'; import { useDocuments } from '../../features/documents/use-documents'; import { useUploadDocument } from '../../features/documents/use-upload-document'; +import { useDeleteDocument } from '../../features/documents/use-delete-document'; import { DocumentList } from '../../features/documents/DocumentList'; export const patientDocumentsRoute = createRoute({ @@ -32,6 +33,12 @@ export const patientDocumentsRoute = createRoute({ function PatientDocumentsPage() { const { data: documents = [], isLoading } = useDocuments(); const { mutate: upload, isPending, error } = useUploadDocument(); + const { + mutate: remove, + variables: deletingId, + isPending: isDeleting, + error: deleteError, + } = useDeleteDocument(); const inputRef = useRef(null); function handleFileChange(e: React.ChangeEvent) { @@ -66,8 +73,19 @@ function PatientDocumentsPage() {

)} + {deleteError && ( +

+ Delete failed: {deleteError instanceof Error ? deleteError.message : 'Unknown error'} +

+ )} +
- + remove(id)} + deletingId={isDeleting ? deletingId ?? null : null} + />
); diff --git a/packages/contracts/src/documents.ts b/packages/contracts/src/documents.ts index 4f30969..f07f6c1 100644 --- a/packages/contracts/src/documents.ts +++ b/packages/contracts/src/documents.ts @@ -15,6 +15,11 @@ export const documentErrorSchema = z.object({ error: z.string(), }); +export const documentDeleteFailedSchema = z.object({ + error: z.literal('DELETE_FAILED'), + message: z.string(), +}); + export const documentsContract = c.router({ upload: { method: 'POST', @@ -36,4 +41,16 @@ export const documentsContract = c.router({ 200: z.array(documentMetadataSchema), }, }, + delete: { + method: 'DELETE', + path: '/documents/:id', + pathParams: z.object({ id: z.string().uuid() }), + body: c.type(), + responses: { + 204: c.noBody(), + 403: documentErrorSchema, + 404: documentErrorSchema, + 500: documentDeleteFailedSchema, + }, + }, });