Confirm destructive deletes and fall back for orphan accounts

Account and transaction deletes were one misclick away from wiping history with no prompt. New ConfirmDialog gates every Trash2 button (account delete, transaction delete on Transactions and AccountDetail, valuation delete). Transaction lists now render "(Deleted account)" as a defensive fallback if an account reference ever fails to resolve. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-22 15:31:17 -07:00
parent e5fe8568b0
commit 7308d6d847
5 changed files with 275 additions and 12 deletions
@@ -0,0 +1,89 @@
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { describe, it, expect, vi } from 'vitest';
+import { ConfirmDialog } from './ConfirmDialog';
+
+describe('ConfirmDialog', () => {
+  function setup(overrides: Partial<React.ComponentProps<typeof ConfirmDialog>> = {}) {
+    const onOpenChange = vi.fn();
+    const onConfirm = vi.fn().mockResolvedValue(undefined);
+    const utils = render(
+      <ConfirmDialog
+        open
+        onOpenChange={onOpenChange}
+        title="Delete thing?"
+        description="This cannot be undone."
+        confirmLabel="Delete"
+        onConfirm={onConfirm}
+        {...overrides}
+      />,
+    );
+    return { onOpenChange, onConfirm, ...utils };
+  }
+
+  it('renders the title, description, and labels', () => {
+    setup();
+    expect(screen.getByText('Delete thing?')).toBeTruthy();
+    expect(screen.getByText('This cannot be undone.')).toBeTruthy();
+    expect(screen.getByRole('button', { name: 'Delete' })).toBeTruthy();
+    expect(screen.getByRole('button', { name: 'Cancel' })).toBeTruthy();
+  });
+
+  it('calls onConfirm and closes on confirm click', async () => {
+    const { onConfirm, onOpenChange } = setup();
+    fireEvent.click(screen.getByRole('button', { name: 'Delete' }));
+    await waitFor(() => expect(onConfirm).toHaveBeenCalled());
+    expect(onOpenChange).toHaveBeenCalledWith(false);
+  });
+
+  it('calls onOpenChange(false) on cancel click without invoking onConfirm', () => {
+    const { onConfirm, onOpenChange } = setup();
+    fireEvent.click(screen.getByRole('button', { name: 'Cancel' }));
+    expect(onConfirm).not.toHaveBeenCalled();
+    expect(onOpenChange).toHaveBeenCalledWith(false);
+  });
+
+  it('keeps the dialog open if onConfirm rejects', async () => {
+    const err = new Error('boom');
+    const onConfirm = vi.fn().mockRejectedValue(err);
+    const onOpenChange = vi.fn();
+    render(
+      <ConfirmDialog
+        open
+        onOpenChange={onOpenChange}
+        title="t"
+        description="d"
+        onConfirm={onConfirm}
+      />,
+    );
+    fireEvent.click(screen.getByRole('button', { name: 'Confirm' }));
+    await waitFor(() => expect(onConfirm).toHaveBeenCalled());
+    expect(onOpenChange).not.toHaveBeenCalledWith(false);
+  });
+
+  it('disables the confirm button while the action is pending', async () => {
+    let resolve: () => void = () => {};
+    const onConfirm = vi.fn(
+      () => new Promise<void>((r) => (resolve = r)),
+    );
+    render(
+      <ConfirmDialog
+        open
+        onOpenChange={vi.fn()}
+        title="t"
+        description="d"
+        onConfirm={onConfirm}
+      />,
+    );
+    const confirmBtn = screen.getByRole('button', { name: 'Confirm' });
+    fireEvent.click(confirmBtn);
+    await waitFor(() => expect(confirmBtn.hasAttribute('disabled')).toBe(true));
+    resolve();
+  });
+
+  it('renders the destructive variant on the confirm button when flagged', () => {
+    setup({ destructive: true });
+    const btn = screen.getByRole('button', { name: 'Delete' });
+    // The destructive variant is indicated by a text-destructive utility class.
+    expect(btn.className).toMatch(/destructive/);
+  });
+});
@@ -0,0 +1,74 @@
+import { useState, type ReactNode } from 'react';
+import {
+  Dialog,
+  DialogContent,
+  DialogDescription,
+  DialogFooter,
+  DialogHeader,
+  DialogTitle,
+} from '@/components/ui/dialog';
+import { Button } from '@/components/ui/button';
+
+export interface ConfirmDialogProps {
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+  title: ReactNode;
+  description: ReactNode;
+  confirmLabel?: string;
+  cancelLabel?: string;
+  destructive?: boolean;
+  onConfirm: () => Promise<void> | void;
+}
+
+export function ConfirmDialog({
+  open,
+  onOpenChange,
+  title,
+  description,
+  confirmLabel = 'Confirm',
+  cancelLabel = 'Cancel',
+  destructive = false,
+  onConfirm,
+}: ConfirmDialogProps) {
+  const [pending, setPending] = useState(false);
+
+  const handleConfirm = async () => {
+    setPending(true);
+    try {
+      await onConfirm();
+      onOpenChange(false);
+    } catch {
+      // Keep the dialog open so the user can retry; the caller is expected to
+      // surface its own error feedback (toast, inline message, etc).
+    } finally {
+      setPending(false);
+    }
+  };
+
+  return (
+    <Dialog open={open} onOpenChange={onOpenChange}>
+      <DialogContent showCloseButton={false}>
+        <DialogHeader>
+          <DialogTitle>{title}</DialogTitle>
+          <DialogDescription>{description}</DialogDescription>
+        </DialogHeader>
+        <DialogFooter>
+          <Button
+            variant="outline"
+            onClick={() => onOpenChange(false)}
+            disabled={pending}
+          >
+            {cancelLabel}
+          </Button>
+          <Button
+            variant={destructive ? 'destructive' : 'default'}
+            onClick={handleConfirm}
+            disabled={pending}
+          >
+            {confirmLabel}
+          </Button>
+        </DialogFooter>
+      </DialogContent>
+    </Dialog>
+  );
+}
@@ -31,6 +31,7 @@ import {
 } from 'lucide-react';
 import { TransactionForm } from '@/components/TransactionForm';
 import { ReceiptViewer } from '@/components/ReceiptViewer';
+import { ConfirmDialog } from '@/components/ConfirmDialog';
 import { Input } from '@/components/ui/input';
 import { api } from '@/lib/api';
 import { formatDate, todayInputValue } from '@/lib/dates';
@@ -69,6 +70,10 @@ export function AccountDetail() {

  const [editing, setEditing] = useState<Transaction | null>(null);
  const [viewingReceipt, setViewingReceipt] = useState<string | null>(null);
+  const [deletingTxn, setDeletingTxn] = useState<Transaction | null>(null);
+  const [deletingValuation, setDeletingValuation] = useState<
+    { id: string; date: string; value: number } | null
+  >(null);
  const [history, setHistory] = useState<
    {
      date: string;
@@ -320,7 +325,7 @@ export function AccountDetail() {
                      <Button
                        variant="ghost"
                        size="sm"
-                        onClick={() => handleDeleteValuation(v.id)}
+                        onClick={() => setDeletingValuation(v)}
                        aria-label="Delete valuation"
                      >
                        <Trash2 className="size-3" />
@@ -377,14 +382,14 @@ export function AccountDetail() {
                      )}
                    </TableCell>
                    <TableCell className="text-sm">
-                      {txn.type === 'TRANSFER' && txn.destinationAccount ? (
+                      {txn.type === 'TRANSFER' && (txn.destinationAccount || txn.destinationAccountId) ? (
                        <span className="inline-flex items-center gap-1">
-                          {txn.account?.name}
+                          {txn.account?.name ?? '(Deleted account)'}
                          <ArrowRight className="size-3 text-muted-foreground" />
-                          {txn.destinationAccount.name}
+                          {txn.destinationAccount?.name ?? '(Deleted account)'}
                        </span>
                      ) : (
-                        txn.account?.name
+                        txn.account?.name ?? '(Deleted account)'
                      )}
                    </TableCell>
                    <TableCell className="text-right text-sm font-medium">
@@ -413,7 +418,7 @@ export function AccountDetail() {
                        <Button variant="ghost" size="sm" onClick={() => setEditing(txn)} aria-label="Edit transaction">
                          <Pencil className="size-3" />
                        </Button>
-                        <Button variant="ghost" size="sm" onClick={() => deleteTransaction(txn.id)} aria-label="Delete transaction">
+                        <Button variant="ghost" size="sm" onClick={() => setDeletingTxn(txn)} aria-label="Delete transaction">
                          <Trash2 className="size-3" />
                        </Button>
                      </div>
@@ -509,6 +514,52 @@ export function AccountDetail() {
        receiptPath={viewingReceipt}
        onClose={() => setViewingReceipt(null)}
      />
+
+      <ConfirmDialog
+        open={!!deletingTxn}
+        onOpenChange={(open) => !open && setDeletingTxn(null)}
+        title="Delete transaction?"
+        description={
+          deletingTxn ? (
+            <>
+              This will permanently delete the{' '}
+              {deletingTxn.type.toLowerCase()} of{' '}
+              <b>
+                ${Number(deletingTxn.amount).toLocaleString('en-US', {
+                  minimumFractionDigits: 2,
+                })}
+              </b>{' '}
+              ("{deletingTxn.description}") and reverse its effect on the
+              account balance. This cannot be undone.
+            </>
+          ) : null
+        }
+        confirmLabel="Delete"
+        destructive
+        onConfirm={async () => {
+          if (deletingTxn) await deleteTransaction(deletingTxn.id);
+        }}
+      />
+
+      <ConfirmDialog
+        open={!!deletingValuation}
+        onOpenChange={(open) => !open && setDeletingValuation(null)}
+        title="Delete valuation?"
+        description={
+          deletingValuation ? (
+            <>
+              This will permanently delete the valuation of{' '}
+              <b>{formatCurrency(Number(deletingValuation.value))}</b> from{' '}
+              <b>{formatDate(deletingValuation.date)}</b>. This cannot be undone.
+            </>
+          ) : null
+        }
+        confirmLabel="Delete"
+        destructive
+        onConfirm={async () => {
+          if (deletingValuation) await handleDeleteValuation(deletingValuation.id);
+        }}
+      />
    </div>
  );
 }
@@ -20,6 +20,7 @@ import {
  SelectValue,
 } from '@/components/ui/select';
 import { Plus, Pencil, Trash2, GripVertical } from 'lucide-react';
+import { ConfirmDialog } from '@/components/ConfirmDialog';
 import {
  DndContext,
  closestCenter,
@@ -179,6 +180,7 @@ export function Accounts() {
  const { accounts, loading, fetchAccounts, createAccount, updateAccount, deleteAccount, reorderAccounts } = useAccountsStore();
  const [dialogOpen, setDialogOpen] = useState(false);
  const [editing, setEditing] = useState<Account | null>(null);
+  const [deleting, setDeleting] = useState<Account | null>(null);
  const navigate = useNavigate();

  const sensors = useSensors(
@@ -240,7 +242,7 @@ export function Accounts() {
                  account={account}
                  onOpen={() => navigate(`/accounts/${account.id}`)}
                  onEdit={() => setEditing(account)}
-                  onDelete={() => deleteAccount(account.id)}
+                  onDelete={() => setDeleting(account)}
                />
              ))}
            </div>
@@ -261,6 +263,26 @@ export function Accounts() {
          )}
        </DialogContent>
      </Dialog>
+
+      <ConfirmDialog
+        open={!!deleting}
+        onOpenChange={(open) => !open && setDeleting(null)}
+        title="Delete account?"
+        description={
+          deleting ? (
+            <>
+              This will permanently delete <b>{deleting.name}</b> and all of its
+              transactions. Any transfers to or from this account will be
+              reversed on the other account's balance. This cannot be undone.
+            </>
+          ) : null
+        }
+        confirmLabel="Delete"
+        destructive
+        onConfirm={async () => {
+          if (deleting) await deleteAccount(deleting.id);
+        }}
+      />
    </div>
  );
 }
@@ -42,6 +42,7 @@ import {
 } from 'lucide-react';
 import { TransactionForm } from '@/components/TransactionForm';
 import { ReceiptViewer } from '@/components/ReceiptViewer';
+import { ConfirmDialog } from '@/components/ConfirmDialog';
 import { formatDate } from '@/lib/dates';

 const TRANSACTION_TYPES = ['INCOME', 'EXPENSE', 'TRANSFER'] as const;
@@ -68,6 +69,7 @@ export function Transactions() {
  const [createOpen, setCreateOpen] = useState(searchParams.get('new') === '1');
  const [editing, setEditing] = useState<Transaction | null>(null);
  const [viewingReceipt, setViewingReceipt] = useState<string | null>(null);
+  const [deleting, setDeleting] = useState<Transaction | null>(null);

  useEffect(() => {
    let changed = false;
@@ -232,14 +234,14 @@ export function Transactions() {
                      )}
                    </TableCell>
                    <TableCell className="text-sm">
-                      {txn.type === 'TRANSFER' && txn.destinationAccount ? (
+                      {txn.type === 'TRANSFER' && (txn.destinationAccount || txn.destinationAccountId) ? (
                        <span className="inline-flex items-center gap-1">
-                          {txn.account?.name}
+                          {txn.account?.name ?? '(Deleted account)'}
                          <ArrowRight className="size-3 text-muted-foreground" />
-                          {txn.destinationAccount.name}
+                          {txn.destinationAccount?.name ?? '(Deleted account)'}
                        </span>
                      ) : (
-                        txn.account?.name
+                        txn.account?.name ?? '(Deleted account)'
                      )}
                    </TableCell>
                    <TableCell className="text-right text-sm font-medium">
@@ -268,7 +270,7 @@ export function Transactions() {
                        <Button variant="ghost" size="sm" onClick={() => setEditing(txn)} aria-label="Edit transaction">
                          <Pencil className="size-3" />
                        </Button>
-                        <Button variant="ghost" size="sm" onClick={() => deleteTransaction(txn.id)} aria-label="Delete transaction">
+                        <Button variant="ghost" size="sm" onClick={() => setDeleting(txn)} aria-label="Delete transaction">
                          <Trash2 className="size-3" />
                        </Button>
                      </div>
@@ -325,6 +327,31 @@ export function Transactions() {
        receiptPath={viewingReceipt}
        onClose={() => setViewingReceipt(null)}
      />
+
+      <ConfirmDialog
+        open={!!deleting}
+        onOpenChange={(open) => !open && setDeleting(null)}
+        title="Delete transaction?"
+        description={
+          deleting ? (
+            <>
+              This will permanently delete the {deleting.type.toLowerCase()} of{' '}
+              <b>
+                ${Number(deleting.amount).toLocaleString('en-US', {
+                  minimumFractionDigits: 2,
+                })}
+              </b>{' '}
+              ("{deleting.description}") and reverse its effect on the account
+              balance. This cannot be undone.
+            </>
+          ) : null
+        }
+        confirmLabel="Delete"
+        destructive
+        onConfirm={async () => {
+          if (deleting) await deleteTransaction(deleting.id);
+        }}
+      />
    </div>
  );
 }