feat: allow deleting card type fields with values via two-step confirmationclaude/allow-delete-card-fields-auoeV

Previously, fields with existing values could not be deleted at all.
Now, when a user tries to delete such a field, a two-step confirmation
dialog is shown. The first step warns the user with the number of cards
using that card type. The second step asks for final confirmation since
the action cannot be undone. The backend now accepts a `force=true` query
parameter to bypass the field-has-values check, and returns the card
count in the 409 response for the frontend to display.

https://claude.ai/code/session_017S5QP6SnC5GFNJwQFyT4as

7 files changed, 396 insertions, 31 deletions

diff --git a/src/client/components/NoteTypeEditor.test.tsx b/src/client/components/NoteTypeEditor.test.tsx
index a628859..a8e8c51 100644
--- a/src/client/components/NoteTypeEditor.test.tsx
+++ b/src/client/components/NoteTypeEditor.test.tsx
@@ -12,6 +12,7 @@ const mockFieldPut = vi.fn();
 const mockFieldDelete = vi.fn();
 const mockFieldsReorder = vi.fn();
 const mockHandleResponse = vi.fn();
+const mockAuthenticatedFetch = vi.fn();
 
 vi.mock("../api/client", () => ({
 	apiClient: {
@@ -36,6 +37,8 @@ vi.mock("../api/client", () => ({
 			},
 		},
 		handleResponse: (res: unknown) => mockHandleResponse(res),
+		authenticatedFetch: (input: unknown, init: unknown) =>
+			mockAuthenticatedFetch(input, init),
 	},
 	ApiClientError: class ApiClientError extends Error {
 		constructor(
@@ -91,7 +94,7 @@ describe("NoteTypeEditor", () => {
 		mockNoteTypePut.mockResolvedValue({ ok: true });
 		mockFieldPost.mockResolvedValue({ ok: true });
 		mockFieldPut.mockResolvedValue({ ok: true });
-		mockFieldDelete.mockResolvedValue({ ok: true });
+		mockFieldDelete.mockResolvedValue({ ok: true, status: 200 });
 		mockFieldsReorder.mockResolvedValue({ ok: true });
 	});
 
@@ -305,9 +308,9 @@ describe("NoteTypeEditor", () => {
 	it("deletes a field", async () => {
 		const user = userEvent.setup();
 
-		mockHandleResponse
-			.mockResolvedValueOnce({ noteType: mockNoteTypeWithFields })
-			.mockResolvedValueOnce({ success: true });
+		mockHandleResponse.mockResolvedValueOnce({
+			noteType: mockNoteTypeWithFields,
+		});
 
 		render(<NoteTypeEditor {...defaultProps} />);
 
@@ -330,14 +333,24 @@ describe("NoteTypeEditor", () => {
 		});
 	});
 
-	it("displays error when field deletion fails", async () => {
+	it("shows confirmation when deleting field with values", async () => {
 		const user = userEvent.setup();
 
-		mockHandleResponse
-			.mockResolvedValueOnce({ noteType: mockNoteTypeWithFields })
-			.mockRejectedValueOnce(
-				new ApiClientError("Cannot delete field with existing values", 409),
-			);
+		mockHandleResponse.mockResolvedValueOnce({
+			noteType: mockNoteTypeWithFields,
+		});
+		mockFieldDelete.mockResolvedValueOnce({
+			ok: false,
+			status: 409,
+			json: () =>
+				Promise.resolve({
+					error: {
+						message: "Cannot delete field with existing values",
+						code: "FIELD_HAS_VALUES",
+					},
+					cardCount: 5,
+				}),
+		});
 
 		render(<NoteTypeEditor {...defaultProps} />);
 
@@ -345,9 +358,7 @@ describe("NoteTypeEditor", () => {
 			expect(screen.getByText("Front")).toBeDefined();
 		});
 
-		// Find the delete button for the "Front" field (first delete button)
 		const deleteButtons = screen.getAllByTitle("Delete field");
-		expect(deleteButtons.length).toBeGreaterThan(0);
 		const deleteButton = deleteButtons.at(0);
 		if (!deleteButton) throw new Error("Delete button not found");
 
@@ -356,13 +367,197 @@ describe("NoteTypeEditor", () => {
 		await waitFor(() => {
 			const alerts = screen.getAllByRole("alert");
 			expect(
-				alerts.some((alert) =>
-					alert.textContent?.includes(
-						"Cannot delete field with existing values",
-					),
-				),
+				alerts.some((alert) => alert.textContent?.includes("5 card(s)")),
+			).toBe(true);
+			expect(alerts.some((alert) => alert.textContent?.includes("Front"))).toBe(
+				true,
+			);
+		});
+	});
+
+	it("shows second confirmation after first is accepted", async () => {
+		const user = userEvent.setup();
+
+		mockHandleResponse.mockResolvedValueOnce({
+			noteType: mockNoteTypeWithFields,
+		});
+		mockFieldDelete.mockResolvedValueOnce({
+			ok: false,
+			status: 409,
+			json: () =>
+				Promise.resolve({
+					error: {
+						message: "Cannot delete field with existing values",
+						code: "FIELD_HAS_VALUES",
+					},
+					cardCount: 3,
+				}),
+		});
+
+		render(<NoteTypeEditor {...defaultProps} />);
+
+		await waitFor(() => {
+			expect(screen.getByText("Front")).toBeDefined();
+		});
+
+		// Click delete
+		const deleteButtons = screen.getAllByTitle("Delete field");
+		const deleteButton = deleteButtons.at(0);
+		if (!deleteButton) throw new Error("Delete button not found");
+		await user.click(deleteButton);
+
+		// First confirmation should appear
+		await waitFor(() => {
+			expect(
+				screen
+					.getAllByRole("alert")
+					.some((a) => a.textContent?.includes("3 card(s)")),
+			).toBe(true);
+		});
+
+		// Click Delete on first confirmation
+		const confirmDeleteButton = screen.getByRole("button", { name: "Delete" });
+		await user.click(confirmDeleteButton);
+
+		// Second confirmation should appear
+		await waitFor(() => {
+			expect(
+				screen
+					.getAllByRole("alert")
+					.some((a) => a.textContent?.includes("cannot be undone")),
+			).toBe(true);
+		});
+	});
+
+	it("force deletes field after two confirmations", async () => {
+		const user = userEvent.setup();
+
+		mockHandleResponse.mockResolvedValueOnce({
+			noteType: mockNoteTypeWithFields,
+		});
+		mockFieldDelete.mockResolvedValueOnce({
+			ok: false,
+			status: 409,
+			json: () =>
+				Promise.resolve({
+					error: {
+						message: "Cannot delete field with existing values",
+						code: "FIELD_HAS_VALUES",
+					},
+					cardCount: 2,
+				}),
+		});
+		mockAuthenticatedFetch.mockResolvedValueOnce({
+			ok: true,
+			status: 200,
+			json: () => Promise.resolve({ success: true }),
+		});
+
+		render(<NoteTypeEditor {...defaultProps} />);
+
+		await waitFor(() => {
+			expect(screen.getByText("Front")).toBeDefined();
+		});
+
+		// Click delete
+		const deleteButtons = screen.getAllByTitle("Delete field");
+		const deleteButton = deleteButtons.at(0);
+		if (!deleteButton) throw new Error("Delete button not found");
+		await user.click(deleteButton);
+
+		// First confirmation
+		await waitFor(() => {
+			expect(
+				screen
+					.getAllByRole("alert")
+					.some((a) => a.textContent?.includes("2 card(s)")),
 			).toBe(true);
 		});
+		await user.click(screen.getByRole("button", { name: "Delete" }));
+
+		// Second confirmation
+		await waitFor(() => {
+			expect(
+				screen
+					.getAllByRole("alert")
+					.some((a) => a.textContent?.includes("cannot be undone")),
+			).toBe(true);
+		});
+		await user.click(screen.getByRole("button", { name: "Delete" }));
+
+		// Force delete should be called
+		await waitFor(() => {
+			expect(mockAuthenticatedFetch).toHaveBeenCalledWith(
+				"/api/note-types/note-type-123/fields/field-1?force=true",
+				{ method: "DELETE" },
+			);
+		});
+
+		// Field should be removed from the list
+		await waitFor(() => {
+			expect(screen.queryByText("Front")).toBeNull();
+		});
+	});
+
+	it("cancels deletion when Cancel is clicked on confirmation", async () => {
+		const user = userEvent.setup();
+
+		mockHandleResponse.mockResolvedValueOnce({
+			noteType: mockNoteTypeWithFields,
+		});
+		mockFieldDelete.mockResolvedValueOnce({
+			ok: false,
+			status: 409,
+			json: () =>
+				Promise.resolve({
+					error: {
+						message: "Cannot delete field with existing values",
+						code: "FIELD_HAS_VALUES",
+					},
+					cardCount: 5,
+				}),
+		});
+
+		render(<NoteTypeEditor {...defaultProps} />);
+
+		await waitFor(() => {
+			expect(screen.getByText("Front")).toBeDefined();
+		});
+
+		// Click delete
+		const deleteButtons = screen.getAllByTitle("Delete field");
+		const deleteButton = deleteButtons.at(0);
+		if (!deleteButton) throw new Error("Delete button not found");
+		await user.click(deleteButton);
+
+		// Confirmation should appear
+		await waitFor(() => {
+			expect(
+				screen
+					.getAllByRole("alert")
+					.some((a) => a.textContent?.includes("5 card(s)")),
+			).toBe(true);
+		});
+
+		// Click Cancel
+		const cancelButtons = screen.getAllByRole("button", { name: "Cancel" });
+		// The confirmation Cancel button (not the main dialog Cancel)
+		const confirmCancelButton = cancelButtons.find((btn) =>
+			btn.closest(".bg-amber-50"),
+		);
+		if (!confirmCancelButton) throw new Error("Cancel button not found");
+		await user.click(confirmCancelButton);
+
+		// Confirmation should disappear
+		await waitFor(() => {
+			const alerts = screen.queryAllByRole("alert");
+			expect(alerts.some((a) => a.textContent?.includes("5 card(s)"))).toBe(
+				false,
+			);
+		});
+
+		// Field should still be there
+		expect(screen.getByText("Front")).toBeDefined();
 	});
 
 	it("moves a field up", async () => {
diff --git a/src/client/components/NoteTypeEditor.tsx b/src/client/components/NoteTypeEditor.tsx
index 2487c62..f514653 100644
--- a/src/client/components/NoteTypeEditor.tsx
+++ b/src/client/components/NoteTypeEditor.tsx
@@ -42,6 +42,12 @@ interface NoteTypeEditorProps {
 	onNoteTypeUpdated: () => void;
 }
 
+interface DeleteConfirmation {
+	fieldId: string;
+	cardCount: number;
+	step: 1 | 2;
+}
+
 export function NoteTypeEditor({
 	isOpen,
 	noteTypeId,
@@ -62,6 +68,9 @@ export function NoteTypeEditor({
 	const [editingFieldId, setEditingFieldId] = useState<string | null>(null);
 	const [editingFieldName, setEditingFieldName] = useState("");
 	const [fieldError, setFieldError] = useState<string | null>(null);
+	const [deleteConfirm, setDeleteConfirm] = useState<DeleteConfirmation | null>(
+		null,
+	);
 	const editInputRef = useRef<HTMLInputElement>(null);
 
 	const fetchNoteType = useCallback(async () => {
@@ -114,6 +123,7 @@ export function NoteTypeEditor({
 		setIsAddingField(false);
 		setEditingFieldId(null);
 		setNoteType(null);
+		setDeleteConfirm(null);
 		onClose();
 	};
 
@@ -212,19 +222,50 @@ export function NoteTypeEditor({
 		}
 	};
 
-	const handleDeleteField = async (fieldId: string) => {
+	const handleDeleteField = async (fieldId: string, force = false) => {
 		if (!noteType) return;
 
 		setFieldError(null);
 
 		try {
-			const res = await apiClient.rpc.api["note-types"][":id"].fields[
-				":fieldId"
-			].$delete({
-				param: { id: noteType.id, fieldId },
-			});
-			await apiClient.handleResponse(res);
-			setFields(fields.filter((f) => f.id !== fieldId));
+			let res: Response;
+			if (force) {
+				res = await apiClient.authenticatedFetch(
+					`/api/note-types/${noteType.id}/fields/${fieldId}?force=true`,
+					{ method: "DELETE" },
+				);
+			} else {
+				res = await apiClient.rpc.api["note-types"][":id"].fields[
+					":fieldId"
+				].$delete({
+					param: { id: noteType.id, fieldId },
+				});
+			}
+
+			if (res.ok) {
+				setFields(fields.filter((f) => f.id !== fieldId));
+				setDeleteConfirm(null);
+				return;
+			}
+
+			const body = (await res.json()) as {
+				error?: { message?: string; code?: string };
+				cardCount?: number;
+			};
+			if (
+				res.status === 409 &&
+				body.error?.code === "FIELD_HAS_VALUES" &&
+				body.cardCount !== undefined
+			) {
+				setDeleteConfirm({ fieldId, cardCount: body.cardCount, step: 1 });
+				return;
+			}
+
+			throw new ApiClientError(
+				body.error?.message || `Request failed with status ${res.status}`,
+				res.status,
+				body.error?.code,
+			);
 		} catch (err) {
 			if (err instanceof ApiClientError) {
 				setFieldError(err.message);
@@ -290,6 +331,10 @@ export function NoteTypeEditor({
 		return null;
 	}
 
+	const confirmFieldName = deleteConfirm
+		? fields.find((f) => f.id === deleteConfirm.fieldId)?.name
+		: null;
+
 	return (
 		<div
 			role="dialog"
@@ -390,6 +435,70 @@ export function NoteTypeEditor({
 									</div>
 								)}
 
+								{deleteConfirm && (
+									<div
+										role="alert"
+										className="bg-amber-50 text-amber-800 text-sm px-4 py-3 rounded-lg border border-amber-200 mb-3"
+									>
+										{deleteConfirm.step === 1 ? (
+											<>
+												<p>
+													This note type has{" "}
+													<strong>{deleteConfirm.cardCount} card(s)</strong>.
+													Deleting the field &ldquo;{confirmFieldName}
+													&rdquo; will remove its values from all cards.
+												</p>
+												<div className="flex gap-2 mt-2">
+													<button
+														type="button"
+														onClick={() =>
+															setDeleteConfirm({
+																...deleteConfirm,
+																step: 2,
+															})
+														}
+														className="px-3 py-1.5 bg-error hover:bg-error/90 text-white text-xs font-medium rounded-lg transition-colors"
+													>
+														Delete
+													</button>
+													<button
+														type="button"
+														onClick={() => setDeleteConfirm(null)}
+														className="px-3 py-1.5 text-slate hover:bg-ivory text-xs font-medium rounded-lg transition-colors"
+													>
+														Cancel
+													</button>
+												</div>
+											</>
+										) : (
+											<>
+												<p>
+													This action cannot be undone. Are you sure you want to
+													delete the field &ldquo;{confirmFieldName}&rdquo;?
+												</p>
+												<div className="flex gap-2 mt-2">
+													<button
+														type="button"
+														onClick={() =>
+															handleDeleteField(deleteConfirm.fieldId, true)
+														}
+														className="px-3 py-1.5 bg-error hover:bg-error/90 text-white text-xs font-medium rounded-lg transition-colors"
+													>
+														Delete
+													</button>
+													<button
+														type="button"
+														onClick={() => setDeleteConfirm(null)}
+														className="px-3 py-1.5 text-slate hover:bg-ivory text-xs font-medium rounded-lg transition-colors"
+													>
+														Cancel
+													</button>
+												</div>
+											</>
+										)}
+									</div>
+								)}
+
 								<div className="space-y-2 mb-4">
 									{fields.map((field, index) => (
 										<div
diff --git a/src/server/repositories/noteType.test.ts b/src/server/repositories/noteType.test.ts
index fdb9d5c..3fd9e7d 100644
--- a/src/server/repositories/noteType.test.ts
+++ b/src/server/repositories/noteType.test.ts
@@ -66,6 +66,7 @@ function createMockNoteTypeRepo(): NoteTypeRepository {
 		update: vi.fn(),
 		softDelete: vi.fn(),
 		hasNotes: vi.fn(),
+		countCards: vi.fn(),
 	};
 }
 
diff --git a/src/server/repositories/noteType.ts b/src/server/repositories/noteType.ts
index 06c8834..89cf6fd 100644
--- a/src/server/repositories/noteType.ts
+++ b/src/server/repositories/noteType.ts
@@ -143,6 +143,24 @@ export const noteTypeRepository: NoteTypeRepository = {
 
 		return result.length > 0;
 	},
+
+	async countCards(noteTypeId: string): Promise<number> {
+		const { cards } = await import("../db/schema.js");
+
+		const result = await db
+			.select({ count: sql<number>`cast(count(*) as int)` })
+			.from(cards)
+			.innerJoin(notes, eq(cards.noteId, notes.id))
+			.where(
+				and(
+					eq(notes.noteTypeId, noteTypeId),
+					isNull(notes.deletedAt),
+					isNull(cards.deletedAt),
+				),
+			);
+
+		return Number(result[0]?.count ?? 0);
+	},
 };
 
 export const noteFieldTypeRepository: NoteFieldTypeRepository = {
diff --git a/src/server/repositories/types.ts b/src/server/repositories/types.ts
index 7e0819a..27a12b4 100644
--- a/src/server/repositories/types.ts
+++ b/src/server/repositories/types.ts
@@ -250,6 +250,7 @@ export interface NoteTypeRepository {
 	): Promise<NoteType | undefined>;
 	softDelete(id: string, userId: string): Promise<boolean>;
 	hasNotes(id: string, userId: string): Promise<boolean>;
+	countCards(noteTypeId: string): Promise<number>;
 }
 
 export interface NoteFieldTypeRepository {
diff --git a/src/server/routes/noteTypes.test.ts b/src/server/routes/noteTypes.test.ts
index ccc29af..edfce59 100644
--- a/src/server/routes/noteTypes.test.ts
+++ b/src/server/routes/noteTypes.test.ts
@@ -20,6 +20,7 @@ function createMockNoteTypeRepo(): NoteTypeRepository {
 		update: vi.fn(),
 		softDelete: vi.fn(),
 		hasNotes: vi.fn(),
+		countCards: vi.fn(),
 	};
 }
 
@@ -835,13 +836,14 @@ describe("DELETE /api/note-types/:id/fields/:fieldId", () => {
 		);
 	});
 
-	it("returns 409 when field has values", async () => {
+	it("returns 409 with card count when field has values", async () => {
 		const noteTypeId = "a0000000-0000-4000-8000-000000000001";
 		const fieldId = "b0000000-0000-4000-8000-000000000002";
 		const mockNoteType = createMockNoteType({ id: noteTypeId });
 
 		vi.mocked(mockNoteTypeRepo.findById).mockResolvedValue(mockNoteType);
 		vi.mocked(mockNoteFieldTypeRepo.hasNoteFieldValues).mockResolvedValue(true);
+		vi.mocked(mockNoteTypeRepo.countCards).mockResolvedValue(5);
 
 		const res = await app.request(
 			`/api/note-types/${noteTypeId}/fields/${fieldId}`,
@@ -852,8 +854,39 @@ describe("DELETE /api/note-types/:id/fields/:fieldId", () => {
 		);
 
 		expect(res.status).toBe(409);
-		const body = (await res.json()) as NoteTypeResponse;
+		const body = (await res.json()) as NoteTypeResponse & {
+			cardCount?: number;
+		};
 		expect(body.error?.code).toBe("FIELD_HAS_VALUES");
+		expect(body.cardCount).toBe(5);
+		expect(mockNoteTypeRepo.countCards).toHaveBeenCalledWith(noteTypeId);
+	});
+
+	it("deletes field with values when force=true", async () => {
+		const noteTypeId = "a0000000-0000-4000-8000-000000000001";
+		const fieldId = "b0000000-0000-4000-8000-000000000002";
+		const mockNoteType = createMockNoteType({ id: noteTypeId });
+
+		vi.mocked(mockNoteTypeRepo.findById).mockResolvedValue(mockNoteType);
+		vi.mocked(mockNoteFieldTypeRepo.hasNoteFieldValues).mockResolvedValue(true);
+		vi.mocked(mockNoteFieldTypeRepo.softDelete).mockResolvedValue(true);
+
+		const res = await app.request(
+			`/api/note-types/${noteTypeId}/fields/${fieldId}?force=true`,
+			{
+				method: "DELETE",
+				headers: { Authorization: `Bearer ${authToken}` },
+			},
+		);
+
+		expect(res.status).toBe(200);
+		const body = (await res.json()) as NoteTypeResponse;
+		expect(body.success).toBe(true);
+		expect(mockNoteFieldTypeRepo.softDelete).toHaveBeenCalledWith(
+			fieldId,
+			noteTypeId,
+		);
+		expect(mockNoteTypeRepo.countCards).not.toHaveBeenCalled();
 	});
 
 	it("returns 404 when note type not found", async () => {
diff --git a/src/server/routes/noteTypes.ts b/src/server/routes/noteTypes.ts
index 7ab5aa0..ce051d4 100644
--- a/src/server/routes/noteTypes.ts
+++ b/src/server/routes/noteTypes.ts
@@ -188,6 +188,7 @@ export function createNoteTypesRouter(deps: NoteTypeDependencies) {
 				async (c) => {
 					const user = getAuthUser(c);
 					const { id, fieldId } = c.req.valid("param");
+					const force = c.req.query("force") === "true";
 
 					// Verify note type exists and belongs to user
 					const noteType = await noteTypeRepo.findById(id, user.id);
@@ -197,10 +198,17 @@ export function createNoteTypesRouter(deps: NoteTypeDependencies) {
 
 					// Check if there are note field values referencing this field
 					const hasValues = await noteFieldTypeRepo.hasNoteFieldValues(fieldId);
-					if (hasValues) {
-						throw Errors.conflict(
-							"Cannot delete field with existing values",
-							"FIELD_HAS_VALUES",
+					if (hasValues && !force) {
+						const cardCount = await noteTypeRepo.countCards(id);
+						return c.json(
+							{
+								error: {
+									message: "Cannot delete field with existing values",
+									code: "FIELD_HAS_VALUES",
+								},
+								cardCount,
+							},
+							409,
 						);
 					}