mirror of
https://github.com/escalante29/WealthySmart.git
synced 2026-07-17 11:28:47 +02:00
Import dedup transparency + override; bulk transaction actions
Paste-import now returns each skipped duplicate WITH the matched existing row (id, merchant, date, amount, source) instead of a bare count, and accepts allow_duplicates to force-import false positives — the practical fix for same-day identical purchases hashing together (UX-20; supersedes the BE-08 hash change, which would have orphaned every existing hash). POST /transactions/bulk applies delete/set_category/set_deferred to up to 500 ids atomically. 82 tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -26,12 +26,29 @@ class PasteImportRequest(BaseModel):
|
|||||||
text: str = Field(max_length=1_000_000)
|
text: str = Field(max_length=1_000_000)
|
||||||
bank: Bank = Bank.BAC
|
bank: Bank = Bank.BAC
|
||||||
source: TransactionSource = TransactionSource.CREDIT_CARD
|
source: TransactionSource = TransactionSource.CREDIT_CARD
|
||||||
|
# Override for false-positive dedup (legitimate same-day, same-amount
|
||||||
|
# repeat purchases hash identically) — review UX-20/BE-08.
|
||||||
|
allow_duplicates: bool = False
|
||||||
|
|
||||||
|
|
||||||
|
class SkippedDuplicate(BaseModel):
|
||||||
|
line: int
|
||||||
|
merchant: str
|
||||||
|
date: str
|
||||||
|
amount: float
|
||||||
|
currency: str
|
||||||
|
existing_id: int
|
||||||
|
existing_merchant: str
|
||||||
|
existing_date: str
|
||||||
|
existing_amount: float
|
||||||
|
existing_source: str
|
||||||
|
|
||||||
|
|
||||||
class PasteImportResult(BaseModel):
|
class PasteImportResult(BaseModel):
|
||||||
imported: int
|
imported: int
|
||||||
duplicates: int
|
duplicates: int
|
||||||
errors: list[str]
|
errors: list[str]
|
||||||
|
skipped: list[SkippedDuplicate] = []
|
||||||
|
|
||||||
|
|
||||||
def make_reference_hash(date: datetime, merchant: str, amount: float, currency: str) -> str:
|
def make_reference_hash(date: datetime, merchant: str, amount: float, currency: str) -> str:
|
||||||
@@ -103,6 +120,7 @@ def paste_import(
|
|||||||
imported = 0
|
imported = 0
|
||||||
duplicates = 0
|
duplicates = 0
|
||||||
errors: list[str] = []
|
errors: list[str] = []
|
||||||
|
skipped: list[SkippedDuplicate] = []
|
||||||
|
|
||||||
lines = req.text.strip().splitlines()
|
lines = req.text.strip().splitlines()
|
||||||
|
|
||||||
@@ -120,13 +138,29 @@ def paste_import(
|
|||||||
parsed["date"], parsed["merchant"], parsed["amount"], parsed["currency"]
|
parsed["date"], parsed["merchant"], parsed["amount"], parsed["currency"]
|
||||||
)
|
)
|
||||||
|
|
||||||
# Check for duplicates
|
# Check for duplicates — and SHOW the user what matched instead of
|
||||||
existing = session.exec(
|
# silently dropping the line (UX-20).
|
||||||
select(Transaction).where(Transaction.reference == ref_hash)
|
if not req.allow_duplicates:
|
||||||
).first()
|
existing = session.exec(
|
||||||
if existing:
|
select(Transaction).where(Transaction.reference == ref_hash)
|
||||||
duplicates += 1
|
).first()
|
||||||
continue
|
if existing:
|
||||||
|
duplicates += 1
|
||||||
|
skipped.append(
|
||||||
|
SkippedDuplicate(
|
||||||
|
line=i,
|
||||||
|
merchant=parsed["merchant"],
|
||||||
|
date=parsed["date"].isoformat(),
|
||||||
|
amount=parsed["amount"],
|
||||||
|
currency=parsed["currency"],
|
||||||
|
existing_id=existing.id or 0,
|
||||||
|
existing_merchant=existing.merchant,
|
||||||
|
existing_date=existing.date.isoformat(),
|
||||||
|
existing_amount=float(existing.amount),
|
||||||
|
existing_source=existing.source.value,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
|
||||||
tx = Transaction(
|
tx = Transaction(
|
||||||
amount=parsed["amount"],
|
amount=parsed["amount"],
|
||||||
@@ -147,4 +181,6 @@ def paste_import(
|
|||||||
if imported > 0:
|
if imported > 0:
|
||||||
session.commit()
|
session.commit()
|
||||||
|
|
||||||
return PasteImportResult(imported=imported, duplicates=duplicates, errors=errors)
|
return PasteImportResult(
|
||||||
|
imported=imported, duplicates=duplicates, errors=errors, skipped=skipped
|
||||||
|
)
|
||||||
|
|||||||
@@ -99,6 +99,46 @@ def list_transactions(
|
|||||||
return session.exec(query).all()
|
return session.exec(query).all()
|
||||||
|
|
||||||
|
|
||||||
|
class BulkActionRequest(BaseModel):
|
||||||
|
ids: list[int]
|
||||||
|
action: str # 'delete' | 'set_category' | 'set_deferred'
|
||||||
|
category_id: Optional[int] = None # for set_category (None clears)
|
||||||
|
deferred: Optional[bool] = None # for set_deferred
|
||||||
|
|
||||||
|
|
||||||
|
@router.post("/bulk")
|
||||||
|
def bulk_action(
|
||||||
|
req: BulkActionRequest,
|
||||||
|
session: Session = Depends(get_session),
|
||||||
|
_user: str = Depends(get_current_user),
|
||||||
|
):
|
||||||
|
"""Apply one action to many transactions atomically (review 4.4)."""
|
||||||
|
if not req.ids or len(req.ids) > 500:
|
||||||
|
raise HTTPException(status_code=400, detail="ids must contain 1-500 entries")
|
||||||
|
if req.action not in ("delete", "set_category", "set_deferred"):
|
||||||
|
raise HTTPException(status_code=400, detail=f"Unknown action: {req.action}")
|
||||||
|
if req.action == "set_deferred" and req.deferred is None:
|
||||||
|
raise HTTPException(status_code=400, detail="deferred is required for set_deferred")
|
||||||
|
if req.action == "set_category" and req.category_id is not None:
|
||||||
|
if session.get(Category, req.category_id) is None:
|
||||||
|
raise HTTPException(status_code=404, detail="Category not found")
|
||||||
|
|
||||||
|
txs = session.exec(
|
||||||
|
select(Transaction).where(col(Transaction.id).in_(req.ids))
|
||||||
|
).all()
|
||||||
|
for tx in txs:
|
||||||
|
if req.action == "delete":
|
||||||
|
session.delete(tx)
|
||||||
|
elif req.action == "set_category":
|
||||||
|
tx.category_id = req.category_id
|
||||||
|
session.add(tx)
|
||||||
|
elif req.action == "set_deferred":
|
||||||
|
tx.deferred_to_next_cycle = bool(req.deferred)
|
||||||
|
session.add(tx)
|
||||||
|
session.commit()
|
||||||
|
return {"affected": len(txs)}
|
||||||
|
|
||||||
|
|
||||||
@router.get("/export")
|
@router.get("/export")
|
||||||
def export_transactions_csv(
|
def export_transactions_csv(
|
||||||
source: Optional[TransactionSource] = None,
|
source: Optional[TransactionSource] = None,
|
||||||
|
|||||||
131
backend/tests/test_import_review_and_bulk.py
Normal file
131
backend/tests/test_import_review_and_bulk.py
Normal file
@@ -0,0 +1,131 @@
|
|||||||
|
"""Paste-import dedup transparency + override (UX-20) and bulk actions (4.4).
|
||||||
|
|
||||||
|
These exercise the endpoint functions directly with a session — no HTTP
|
||||||
|
stack needed.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from datetime import datetime
|
||||||
|
from decimal import Decimal
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from fastapi import HTTPException
|
||||||
|
|
||||||
|
from app.api.v1.endpoints.import_transactions import (
|
||||||
|
PasteImportRequest,
|
||||||
|
paste_import,
|
||||||
|
)
|
||||||
|
from app.api.v1.endpoints.transactions import BulkActionRequest, bulk_action
|
||||||
|
from app.models.models import Category, Transaction, TransactionSource
|
||||||
|
|
||||||
|
LINE = "15/04/2026\tWALMART\\SAN JOSE\\CR\t25,000.50 CRC"
|
||||||
|
|
||||||
|
|
||||||
|
def do_import(session, text=LINE, **kw):
|
||||||
|
return paste_import(
|
||||||
|
PasteImportRequest(text=text, **kw), session=session, _user="t"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestImportReview:
|
||||||
|
def test_duplicate_is_reported_with_matched_row(self, session):
|
||||||
|
first = do_import(session)
|
||||||
|
assert first.imported == 1 and first.duplicates == 0
|
||||||
|
|
||||||
|
second = do_import(session)
|
||||||
|
assert second.imported == 0
|
||||||
|
assert second.duplicates == 1
|
||||||
|
assert len(second.skipped) == 1
|
||||||
|
skip = second.skipped[0]
|
||||||
|
assert skip.merchant == "WALMART"
|
||||||
|
assert skip.amount == 25000.50
|
||||||
|
assert skip.existing_merchant == "WALMART"
|
||||||
|
assert skip.existing_amount == 25000.50
|
||||||
|
assert skip.existing_id > 0
|
||||||
|
|
||||||
|
def test_allow_duplicates_overrides_dedup(self, session):
|
||||||
|
do_import(session)
|
||||||
|
forced = do_import(session, allow_duplicates=True)
|
||||||
|
assert forced.imported == 1
|
||||||
|
assert forced.duplicates == 0
|
||||||
|
assert forced.skipped == []
|
||||||
|
|
||||||
|
|
||||||
|
def make_tx(session, merchant="M", category_id=None):
|
||||||
|
tx = Transaction(
|
||||||
|
amount=Decimal("10"),
|
||||||
|
merchant=merchant,
|
||||||
|
date=datetime(2026, 4, 1),
|
||||||
|
category_id=category_id,
|
||||||
|
source=TransactionSource.CREDIT_CARD,
|
||||||
|
)
|
||||||
|
session.add(tx)
|
||||||
|
session.commit()
|
||||||
|
session.refresh(tx)
|
||||||
|
return tx
|
||||||
|
|
||||||
|
|
||||||
|
class TestBulkActions:
|
||||||
|
def test_bulk_set_category(self, session):
|
||||||
|
cat = Category(name="Bulk")
|
||||||
|
session.add(cat)
|
||||||
|
session.commit()
|
||||||
|
session.refresh(cat)
|
||||||
|
txs = [make_tx(session, f"m{i}") for i in range(3)]
|
||||||
|
|
||||||
|
result = bulk_action(
|
||||||
|
BulkActionRequest(
|
||||||
|
ids=[t.id for t in txs], action="set_category", category_id=cat.id
|
||||||
|
),
|
||||||
|
session=session,
|
||||||
|
_user="t",
|
||||||
|
)
|
||||||
|
assert result["affected"] == 3
|
||||||
|
for t in txs:
|
||||||
|
session.refresh(t)
|
||||||
|
assert t.category_id == cat.id
|
||||||
|
|
||||||
|
def test_bulk_set_deferred_and_clear_category(self, session):
|
||||||
|
cat = Category(name="X")
|
||||||
|
session.add(cat)
|
||||||
|
session.commit()
|
||||||
|
session.refresh(cat)
|
||||||
|
tx = make_tx(session, category_id=cat.id)
|
||||||
|
|
||||||
|
bulk_action(
|
||||||
|
BulkActionRequest(ids=[tx.id], action="set_deferred", deferred=True),
|
||||||
|
session=session,
|
||||||
|
_user="t",
|
||||||
|
)
|
||||||
|
session.refresh(tx)
|
||||||
|
assert tx.deferred_to_next_cycle is True
|
||||||
|
|
||||||
|
bulk_action(
|
||||||
|
BulkActionRequest(ids=[tx.id], action="set_category", category_id=None),
|
||||||
|
session=session,
|
||||||
|
_user="t",
|
||||||
|
)
|
||||||
|
session.refresh(tx)
|
||||||
|
assert tx.category_id is None
|
||||||
|
|
||||||
|
def test_bulk_delete(self, session):
|
||||||
|
txs = [make_tx(session, f"d{i}") for i in range(2)]
|
||||||
|
result = bulk_action(
|
||||||
|
BulkActionRequest(ids=[t.id for t in txs], action="delete"),
|
||||||
|
session=session,
|
||||||
|
_user="t",
|
||||||
|
)
|
||||||
|
assert result["affected"] == 2
|
||||||
|
assert session.get(Transaction, txs[0].id) is None
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"req",
|
||||||
|
[
|
||||||
|
BulkActionRequest(ids=[], action="delete"),
|
||||||
|
BulkActionRequest(ids=[1], action="explode"),
|
||||||
|
BulkActionRequest(ids=[1], action="set_deferred"), # missing flag
|
||||||
|
BulkActionRequest(ids=[1], action="set_category", category_id=99999),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_invalid_requests_rejected(self, session, req):
|
||||||
|
with pytest.raises(HTTPException):
|
||||||
|
bulk_action(req, session=session, _user="t")
|
||||||
Reference in New Issue
Block a user