diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/client/components/NoteTypeEditor.test.tsx | 229 | ||||
| -rw-r--r-- | src/client/components/NoteTypeEditor.tsx | 125 | ||||
| -rw-r--r-- | src/server/repositories/noteType.test.ts | 1 | ||||
| -rw-r--r-- | src/server/repositories/noteType.ts | 18 | ||||
| -rw-r--r-- | src/server/repositories/types.ts | 1 | ||||
| -rw-r--r-- | src/server/routes/noteTypes.test.ts | 37 | ||||
| -rw-r--r-- | src/server/routes/noteTypes.ts | 16 |
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 “{confirmFieldName} + ” 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 “{confirmFieldName}”? + </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, ); } |
