From c3309b9d60cef55250f203cd3527f25f2a8a670b Mon Sep 17 00:00:00 2001 From: ZProger Date: Sun, 30 Aug 2026 05:06:20 +0300 Subject: [PATCH] feat: click a coin row to view its trade history; edit transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches CoinMarketCap's flow: clicking anywhere on a coin's row in the holdings table opens its transaction history (previously only a dedicated icon button did). The table's own History icon is gone since the row now does that job; the remaining "+"/delete icons get stopPropagation so they don't also trigger the row click. Adds the missing piece: transactions could be viewed and deleted but not edited. New PATCH /crypto/transactions/{id} (partial update, never calls CoinGecko, same oversell guard as creating a sell) backs a pencil button on each history row that opens CryptoTransactionModal pre-filled — the same dialog used for "+", now doing double duty as create/edit. 86/86 backend tests passing (4 new). Verified live against the real API: create, edit quantity/note, 404 on unknown id, cleanup — real user data untouched throughout. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01855PiW5Wd8n4aRW4n9kByY --- DOCS.md | 1 + backend/app/api/routes/crypto.py | 9 +++ backend/app/schemas/crypto.py | 8 +++ backend/app/services/crypto_service.py | 39 ++++++++++++ backend/tests/test_crypto.py | 58 ++++++++++++++++++ frontend/src/api/crypto.ts | 4 ++ .../components/crypto/CryptoHoldingsTable.tsx | 26 ++++---- .../crypto/CryptoTransactionHistoryModal.tsx | 33 ++++++---- .../crypto/CryptoTransactionModal.tsx | 60 +++++++++++++------ frontend/src/hooks/useCrypto.ts | 10 ++++ frontend/src/lib/i18n.ts | 2 + frontend/src/pages/CryptoPage.tsx | 33 +++++++++- 12 files changed, 238 insertions(+), 45 deletions(-) diff --git a/DOCS.md b/DOCS.md index 4148f25..2011d24 100644 --- a/DOCS.md +++ b/DOCS.md @@ -534,6 +534,7 @@ of the whole request failing. | `POST` | `/crypto/holdings` | Add a new holding — its first buy transaction, inline. Fetches today's price immediately so it isn't `null` until the next sync. | | `POST` | `/crypto/holdings/{asset_id}/transactions` | Buy more of, or sell some of, a coin already tracked. Never calls CoinGecko — value is recomputed from the last cached price. `400` if a sell would exceed what's currently held. | | `GET` | `/crypto/holdings/{asset_id}/transactions` | Full buy/sell history for one holding, newest first. | +| `PATCH` | `/crypto/transactions/{transaction_id}` | Edit an existing transaction (partial — send only the fields you're changing). Never calls CoinGecko. `400` if changing a sell's quantity would exceed what the rest of the log leaves held. | | `DELETE` | `/crypto/transactions/{transaction_id}` | Remove one transaction; quantity/avg buy price/value are recomputed from what's left. | | `GET` | `/crypto/search` | `?q=` — search CoinGecko for a coin to add (name/ticker, returns its `coingecko_id` + logo). | | `GET` | `/crypto/history` | `?range=7d\|30d\|90d\|all` (default `30d`) — total crypto holdings value over time, for the portfolio chart. No `24h` — resolution is only as dense as the sync cadence above. | diff --git a/backend/app/api/routes/crypto.py b/backend/app/api/routes/crypto.py index 456290b..63bb765 100644 --- a/backend/app/api/routes/crypto.py +++ b/backend/app/api/routes/crypto.py @@ -10,6 +10,7 @@ from app.schemas.crypto import ( CryptoSyncResult, CryptoTransactionCreate, CryptoTransactionRead, + CryptoTransactionUpdate, ) from app.services.crypto_service import ( CRYPTO_RANGE_DAYS, @@ -20,6 +21,7 @@ from app.services.crypto_service import ( list_transactions, refresh_prices, search_coins, + update_transaction, ) router = APIRouter(prefix="/crypto", tags=["crypto"]) @@ -63,6 +65,13 @@ async def list_transactions_route( return await list_transactions(session, asset_id) +@router.patch("/transactions/{transaction_id}", response_model=CryptoHoldingRead) +async def update_transaction_route( + transaction_id: int, payload: CryptoTransactionUpdate, session: AsyncSession = Depends(get_session) +) -> CryptoHoldingRead: + return await update_transaction(session, transaction_id, payload) + + @router.delete("/transactions/{transaction_id}", status_code=204) async def delete_transaction_route(transaction_id: int, session: AsyncSession = Depends(get_session)) -> None: await delete_transaction(session, transaction_id) diff --git a/backend/app/schemas/crypto.py b/backend/app/schemas/crypto.py index d06c9fb..06f6b43 100644 --- a/backend/app/schemas/crypto.py +++ b/backend/app/schemas/crypto.py @@ -19,6 +19,14 @@ class CryptoTransactionCreate(BaseModel): note: str | None = Field(default=None, max_length=500) +class CryptoTransactionUpdate(BaseModel): + type: CryptoTransactionType | None = None + quantity: Decimal | None = Field(default=None, gt=0, max_digits=38, decimal_places=18) + price_per_unit: Decimal | None = Field(default=None, gt=0, max_digits=38, decimal_places=18) + date: date_ | None = None + note: str | None = Field(default=None, max_length=500) + + class CryptoTransactionRead(BaseModel): model_config = ConfigDict(from_attributes=True) diff --git a/backend/app/services/crypto_service.py b/backend/app/services/crypto_service.py index f4dedb6..84caa37 100644 --- a/backend/app/services/crypto_service.py +++ b/backend/app/services/crypto_service.py @@ -44,6 +44,7 @@ from app.schemas.crypto import ( CryptoSearchResult, CryptoSyncResult, CryptoTransactionCreate, + CryptoTransactionUpdate, ) from app.services.settings_service import get_or_create_app_settings @@ -375,6 +376,44 @@ async def add_transaction(session: AsyncSession, asset_id: int, payload: CryptoT return _to_read(holding) +async def update_transaction( + session: AsyncSession, transaction_id: int, payload: CryptoTransactionUpdate +) -> CryptoHoldingRead: + """Edit an existing buy/sell entry — the "view/edit a transaction" flow + a plain delete-and-recreate can't offer (you'd lose the original id and + any history a future feature might key off it). Never calls CoinGecko, + same as add_transaction: value is recomputed from the last cached price.""" + transaction = await session.get(CryptoTransaction, transaction_id) + if transaction is None: + raise HTTPException(status_code=404, detail="Crypto transaction not found") + asset_id = transaction.asset_id + holding = await _get_holding_or_404(session, asset_id) + + updates = payload.model_dump(exclude_unset=True) + effective_type = updates.get("type", transaction.type) + effective_quantity = updates.get("quantity", transaction.quantity) + + if effective_type == CryptoTransactionType.SELL: + # Same guard as add_transaction, checked against every *other* + # transaction on this holding — editing this one in place must not + # let it sell more than what the rest of the log would leave held. + others = [t for t in holding.transactions if t.id != transaction_id] + quantity_without_this, _ = _compute_position(others) + if effective_quantity > quantity_without_this: + raise HTTPException(status_code=400, detail="Cannot sell more than you currently hold") + + for field, value in updates.items(): + setattr(transaction, field, value) + await session.flush() + + quantity, _ = _compute_position(holding.transactions) + if holding.last_price is not None: + await _upsert_valuation(session, asset_id, quantity * holding.last_price, date_.today()) + + await session.commit() + return _to_read(holding) + + async def list_transactions(session: AsyncSession, asset_id: int) -> list[CryptoTransaction]: await _get_holding_or_404(session, asset_id) # 404s if the holding itself doesn't exist result = await session.execute( diff --git a/backend/tests/test_crypto.py b/backend/tests/test_crypto.py index b6c688b..a69eec2 100644 --- a/backend/tests/test_crypto.py +++ b/backend/tests/test_crypto.py @@ -300,3 +300,61 @@ async def test_history_rejects_unknown_range(client: AsyncClient): resp = await client.get("/crypto/history?range=24h") assert resp.status_code == 422 + + +async def test_update_transaction_recomputes_position_without_calling_coingecko(client: AsyncClient, monkeypatch): + monkeypatch.setattr(crypto_service, "_fetch_market_data", _fake_fetch({"bitcoin": _point("50000")})) + holding = await _add_bitcoin(client, "1", "40000") + tx_id = (await client.get(f"/crypto/holdings/{holding['asset_id']}/transactions")).json()[0]["id"] + + async def fail_if_called(coingecko_ids, vs_currency): + raise AssertionError("editing a transaction must not call CoinGecko") + + monkeypatch.setattr(crypto_service, "_fetch_market_data", fail_if_called) + + resp = await client.patch(f"/crypto/transactions/{tx_id}", json={"quantity": "2"}) + + assert resp.status_code == 200, resp.text + body = resp.json() + assert money(body["quantity"]) == money("2") + assert money(body["value"]) == money("100000") # 2 * 50000 (last known price) + + +async def test_update_transaction_edits_are_reflected_in_history(client: AsyncClient, monkeypatch): + monkeypatch.setattr(crypto_service, "_fetch_market_data", _fake_fetch({"bitcoin": _point("50000")})) + holding = await _add_bitcoin(client, "1", "40000") + tx_id = (await client.get(f"/crypto/holdings/{holding['asset_id']}/transactions")).json()[0]["id"] + + resp = await client.patch( + f"/crypto/transactions/{tx_id}", json={"price_per_unit": "45000", "note": "typo fix"} + ) + assert resp.status_code == 200, resp.text + + transactions = (await client.get(f"/crypto/holdings/{holding['asset_id']}/transactions")).json() + assert len(transactions) == 1 + assert money(transactions[0]["price_per_unit"]) == money("45000") + assert transactions[0]["note"] == "typo fix" + + +async def test_update_transaction_rejects_editing_a_sell_to_exceed_holdings(client: AsyncClient, monkeypatch): + monkeypatch.setattr(crypto_service, "_fetch_market_data", _fake_fetch({"bitcoin": _point("50000")})) + holding = await _add_bitcoin(client, "2", "40000") + await client.post( + f"/crypto/holdings/{holding['asset_id']}/transactions", + json={"type": "sell", "quantity": "1", "price_per_unit": "45000", "date": "2026-02-01"}, + ) + sell_tx_id = next( + t["id"] + for t in (await client.get(f"/crypto/holdings/{holding['asset_id']}/transactions")).json() + if t["type"] == "sell" + ) + + resp = await client.patch(f"/crypto/transactions/{sell_tx_id}", json={"quantity": "3"}) + + assert resp.status_code == 400 + + +async def test_update_transaction_404_for_unknown_id(client: AsyncClient): + resp = await client.patch("/crypto/transactions/999999", json={"quantity": "1"}) + + assert resp.status_code == 404 diff --git a/frontend/src/api/crypto.ts b/frontend/src/api/crypto.ts index 5db3603..082989e 100644 --- a/frontend/src/api/crypto.ts +++ b/frontend/src/api/crypto.ts @@ -33,6 +33,10 @@ export function fetchCryptoTransactions(assetId: number) { return api.get(`/crypto/holdings/${assetId}/transactions`); } +export function updateCryptoTransaction(transactionId: number, input: CryptoTransactionInput) { + return api.patch(`/crypto/transactions/${transactionId}`, input); +} + export function deleteCryptoTransaction(transactionId: number) { return api.delete(`/crypto/transactions/${transactionId}`); } diff --git a/frontend/src/components/crypto/CryptoHoldingsTable.tsx b/frontend/src/components/crypto/CryptoHoldingsTable.tsx index 65d932f..0e55d13 100644 --- a/frontend/src/components/crypto/CryptoHoldingsTable.tsx +++ b/frontend/src/components/crypto/CryptoHoldingsTable.tsx @@ -1,4 +1,4 @@ -import { Bitcoin, History, Plus, Trash2 } from "lucide-react"; +import { Bitcoin, Plus, Trash2 } from "lucide-react"; import { formatCurrency, maskAmount } from "@/lib/format"; import { useTranslation } from "@/lib/i18n"; import type { CryptoHolding } from "@/types"; @@ -59,7 +59,11 @@ export function CryptoHoldingsTable({ items, hidden, onTrade, onViewHistory, onD const sign = holding.profit_loss !== null && Number(holding.profit_loss) > 0 ? "+" : ""; return ( - + onViewHistory(holding)} + className="cursor-pointer hover:bg-surface-2" + >
{holding.thumb_url ? ( @@ -120,23 +124,21 @@ export function CryptoHoldingsTable({ items, hidden, onTrade, onViewHistory, onD - + + + + ))} diff --git a/frontend/src/components/crypto/CryptoTransactionModal.tsx b/frontend/src/components/crypto/CryptoTransactionModal.tsx index cb73200..74e7ef2 100644 --- a/frontend/src/components/crypto/CryptoTransactionModal.tsx +++ b/frontend/src/components/crypto/CryptoTransactionModal.tsx @@ -3,24 +3,30 @@ import { ApiError } from "@/api/client"; import { Button } from "@/components/ui/Button"; import { Dialog } from "@/components/ui/Dialog"; import { Input, Label } from "@/components/ui/Input"; -import { useAddCryptoTransaction } from "@/hooks/useCrypto"; +import { useAddCryptoTransaction, useUpdateCryptoTransaction } from "@/hooks/useCrypto"; import { useTranslation } from "@/lib/i18n"; import { cn } from "@/lib/utils"; -import type { CryptoHolding, CryptoTransactionType } from "@/types"; +import type { CryptoHolding, CryptoTransaction, CryptoTransactionType } from "@/types"; interface CryptoTransactionModalProps { open: boolean; onClose: () => void; holding: CryptoHolding | null; + // Present -> editing that transaction (pre-filled, PATCHes it). Absent -> + // recording a new buy/sell against `holding` (the "+" action). + transaction?: CryptoTransaction | null; } /** Buy more of, or sell some of, a coin already being tracked — the "+" - * action on the holdings table. Never calls CoinGecko (see - * services/crypto_service.py's add_transaction): value is recomputed from - * the last cached price. */ -export function CryptoTransactionModal({ open, onClose, holding }: CryptoTransactionModalProps) { + * action on the holdings table — or edit/view an existing entry from its + * trade history. Never calls CoinGecko either way (see + * services/crypto_service.py's add_transaction/update_transaction): value + * is recomputed from the last cached price. */ +export function CryptoTransactionModal({ open, onClose, holding, transaction = null }: CryptoTransactionModalProps) { const { t } = useTranslation(); const addTransaction = useAddCryptoTransaction(); + const updateTransaction = useUpdateCryptoTransaction(); + const isEditing = transaction !== null; const [type, setType] = useState("buy"); const [quantity, setQuantity] = useState(""); @@ -31,23 +37,33 @@ export function CryptoTransactionModal({ open, onClose, holding }: CryptoTransac useEffect(() => { if (!open) return; - setType("buy"); - setQuantity(""); - setPricePerUnit(""); - setDate(new Date().toISOString().slice(0, 10)); - setNote(""); + if (transaction) { + setType(transaction.type); + setQuantity(transaction.quantity); + setPricePerUnit(transaction.price_per_unit); + setDate(transaction.date); + setNote(transaction.note ?? ""); + } else { + setType("buy"); + setQuantity(""); + setPricePerUnit(""); + setDate(new Date().toISOString().slice(0, 10)); + setNote(""); + } setError(null); - }, [open, holding]); + }, [open, holding, transaction]); async function handleSubmit(event: FormEvent) { event.preventDefault(); if (!holding) return; setError(null); + const input = { type, quantity, price_per_unit: pricePerUnit, date, note: note || null }; try { - await addTransaction.mutateAsync({ - assetId: holding.asset_id, - input: { type, quantity, price_per_unit: pricePerUnit, date, note: note || null }, - }); + if (transaction) { + await updateTransaction.mutateAsync({ transactionId: transaction.id, input }); + } else { + await addTransaction.mutateAsync({ assetId: holding.asset_id, input }); + } onClose(); } catch (err) { setError( @@ -58,8 +74,14 @@ export function CryptoTransactionModal({ open, onClose, holding }: CryptoTransac if (!holding) return null; + const isPending = addTransaction.isPending || updateTransaction.isPending; + return ( - +
-
diff --git a/frontend/src/hooks/useCrypto.ts b/frontend/src/hooks/useCrypto.ts index 57bcfcf..9b18902 100644 --- a/frontend/src/hooks/useCrypto.ts +++ b/frontend/src/hooks/useCrypto.ts @@ -8,6 +8,7 @@ import { fetchCryptoHoldings, fetchCryptoTransactions, refreshCryptoPrices, + updateCryptoTransaction, } from "@/api/crypto"; import type { CryptoHoldingCreateInput, CryptoRange, CryptoTransactionInput } from "@/types"; @@ -61,6 +62,15 @@ export function useAddCryptoTransaction() { }); } +export function useUpdateCryptoTransaction() { + const invalidate = useInvalidateCrypto(); + return useMutation({ + mutationFn: ({ transactionId, input }: { transactionId: number; input: CryptoTransactionInput }) => + updateCryptoTransaction(transactionId, input), + onSuccess: invalidate, + }); +} + export function useDeleteCryptoTransaction() { const invalidate = useInvalidateCrypto(); return useMutation({ diff --git a/frontend/src/lib/i18n.ts b/frontend/src/lib/i18n.ts index 369759e..ec67997 100644 --- a/frontend/src/lib/i18n.ts +++ b/frontend/src/lib/i18n.ts @@ -128,6 +128,7 @@ const ru = { "crypto.form.noteLabel": "Заметка (необязательно)", "crypto.form.addTitle": "Добавить {{name}}", "crypto.form.tradeTitle": "Сделка: {{name}}", + "crypto.form.editTradeTitle": "Изменить сделку: {{name}}", "crypto.form.tradeButton": "Купить / продать", "crypto.form.buy": "Покупка", "crypto.form.sell": "Продажа", @@ -589,6 +590,7 @@ const en: Record = { "crypto.form.noteLabel": "Note (optional)", "crypto.form.addTitle": "Add {{name}}", "crypto.form.tradeTitle": "Trade: {{name}}", + "crypto.form.editTradeTitle": "Edit trade: {{name}}", "crypto.form.tradeButton": "Buy / sell", "crypto.form.buy": "Buy", "crypto.form.sell": "Sell", diff --git a/frontend/src/pages/CryptoPage.tsx b/frontend/src/pages/CryptoPage.tsx index 403fa5f..75c1d1d 100644 --- a/frontend/src/pages/CryptoPage.tsx +++ b/frontend/src/pages/CryptoPage.tsx @@ -11,7 +11,7 @@ import { CryptoTransactionModal } from "@/components/crypto/CryptoTransactionMod import { useCryptoHistory, useCryptoHoldings, useDeleteCryptoHolding, useRefreshCryptoPrices } from "@/hooks/useCrypto"; import { getIntlLocale } from "@/lib/format"; import { useTranslation } from "@/lib/i18n"; -import type { CryptoHolding, CryptoRange } from "@/types"; +import type { CryptoHolding, CryptoRange, CryptoTransaction } from "@/types"; function formatSyncedAt(iso: string): string { return new Intl.DateTimeFormat(getIntlLocale(), { dateStyle: "medium", timeStyle: "short" }).format(new Date(iso)); @@ -33,6 +33,11 @@ export function CryptoPage() { const [addOpen, setAddOpen] = useState(false); const [tradingHolding, setTradingHolding] = useState(null); const [historyHolding, setHistoryHolding] = useState(null); + // Set together with tradingHolding when editing an entry from the history + // list — CryptoTransactionModal is the same dialog for both "+" (new + // trade, transaction stays null) and this (pre-filled, PATCHes instead of + // POSTing). Cleared whenever that dialog closes, same as tradingHolding. + const [editingTransaction, setEditingTransaction] = useState(null); const holdings = data?.holdings ?? []; const totalValue = holdings.reduce((sum, holding) => sum + (holding.value !== null ? Number(holding.value) : 0), 0); @@ -43,6 +48,22 @@ export function CryptoPage() { } } + function handleEditTransaction(transaction: CryptoTransaction) { + setEditingTransaction(transaction); + setTradingHolding(historyHolding); + setHistoryHolding(null); + } + + function closeTradeModal() { + setTradingHolding(null); + setEditingTransaction(null); + } + + function openTradeModal(holding: CryptoHolding) { + setEditingTransaction(null); + setTradingHolding(holding); + } + return (
);