From a94aeb6c35a97410ddcb14bb589ce2ab1e3fc125 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 21 Feb 2026 06:57:15 +0000 Subject: feat: allow deleting card type fields with values via two-step confirmation 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 --- src/client/components/NoteTypeEditor.test.tsx | 229 ++++++++++++++++++++++++-- src/client/components/NoteTypeEditor.tsx | 125 +++++++++++++- 2 files changed, 329 insertions(+), 25 deletions(-) (limited to 'src/client') 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(); @@ -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(); @@ -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(); + + 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(); + + 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(); + + 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(null); const [editingFieldName, setEditingFieldName] = useState(""); const [fieldError, setFieldError] = useState(null); + const [deleteConfirm, setDeleteConfirm] = useState( + null, + ); const editInputRef = useRef(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 (
)} + {deleteConfirm && ( +
+ {deleteConfirm.step === 1 ? ( + <> +

+ This note type has{" "} + {deleteConfirm.cardCount} card(s). + Deleting the field “{confirmFieldName} + ” will remove its values from all cards. +

+
+ + +
+ + ) : ( + <> +

+ This action cannot be undone. Are you sure you want to + delete the field “{confirmFieldName}”? +

+
+ + +
+ + )} +
+ )} +
{fields.map((field, index) => (