From 9709283d7a20dc95c4f22e1c26ba735f058cb6df Mon Sep 17 00:00:00 2001 From: derekc Date: Wed, 18 Mar 2026 00:18:58 -0700 Subject: [PATCH] Fix remaining code quality and infrastructure items - admin.py: remove unused get_current_user import - feed.py, flock.py, other.py: add IntegrityError handling on POST/PUT endpoints; duplicate submissions now return 409 instead of crashing with a 500 error - stats.py: extract magic numbers into named module-level constants (DAYS_ROLLING, DAYS_SHORT, PRECISION_AVG, PRECISION_HEN, PRECISION_COST); add return type annotations to _total_feed_cost and _total_other_cost; normalize both helpers to always return Decimal so budget_stats no longer needs Decimal(str(...)) workarounds; simplify _cpe/_cpd helpers - dashboard.js: read --green CSS variable at runtime instead of hardcoding the hex value so chart color stays in sync with the stylesheet - docker-compose.yml: add healthcheck to api service (polls /api/health every 30s) so Docker knows when the API is unhealthy; add password strength guidance comment above the db service Co-Authored-By: Claude Sonnet 4.6 --- backend/routers/admin.py | 2 +- backend/routers/feed.py | 13 +++++++-- backend/routers/flock.py | 13 +++++++-- backend/routers/other.py | 13 +++++++-- backend/routers/stats.py | 58 +++++++++++++++++++++----------------- docker-compose.yml | 8 ++++++ nginx/html/js/dashboard.js | 4 ++- 7 files changed, 77 insertions(+), 34 deletions(-) diff --git a/backend/routers/admin.py b/backend/routers/admin.py index f7d336e..3507d52 100644 --- a/backend/routers/admin.py +++ b/backend/routers/admin.py @@ -7,7 +7,7 @@ from sqlalchemy.orm import Session from database import get_db from models import User from schemas import UserCreate, UserOut, ResetPasswordRequest, TokenResponse -from auth import hash_password, create_access_token, get_current_admin, get_current_user +from auth import hash_password, create_access_token, get_current_admin router = APIRouter(prefix="/api/admin", tags=["admin"]) logger = logging.getLogger("yolkbook") diff --git a/backend/routers/feed.py b/backend/routers/feed.py index 870485e..02bf292 100644 --- a/backend/routers/feed.py +++ b/backend/routers/feed.py @@ -2,6 +2,7 @@ from datetime import date from typing import Optional from fastapi import APIRouter, Depends, HTTPException from sqlalchemy import select +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from database import get_db @@ -39,7 +40,11 @@ def create_feed_purchase( ): record = FeedPurchase(**body.model_dump(), user_id=current_user.id) db.add(record) - db.commit() + try: + db.commit() + except IntegrityError: + db.rollback() + raise HTTPException(status_code=409, detail=f"A feed entry for {body.date} already exists.") db.refresh(record) return record @@ -59,7 +64,11 @@ def update_feed_purchase( raise HTTPException(status_code=404, detail="Record not found") for field, value in body.model_dump(exclude_none=True).items(): setattr(record, field, value) - db.commit() + try: + db.commit() + except IntegrityError: + db.rollback() + raise HTTPException(status_code=409, detail="A feed entry for that date already exists.") db.refresh(record) return record diff --git a/backend/routers/flock.py b/backend/routers/flock.py index 54e5f9c..c7ac3d5 100644 --- a/backend/routers/flock.py +++ b/backend/routers/flock.py @@ -2,6 +2,7 @@ from datetime import date from typing import Optional from fastapi import APIRouter, Depends, HTTPException from sqlalchemy import select +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from database import get_db @@ -63,7 +64,11 @@ def create_flock_entry( ): record = FlockHistory(**body.model_dump(), user_id=current_user.id) db.add(record) - db.commit() + try: + db.commit() + except IntegrityError: + db.rollback() + raise HTTPException(status_code=409, detail=f"A flock entry for {body.date} already exists.") db.refresh(record) return record @@ -83,7 +88,11 @@ def update_flock_entry( raise HTTPException(status_code=404, detail="Record not found") for field, value in body.model_dump(exclude_none=True).items(): setattr(record, field, value) - db.commit() + try: + db.commit() + except IntegrityError: + db.rollback() + raise HTTPException(status_code=409, detail="A flock entry for that date already exists.") db.refresh(record) return record diff --git a/backend/routers/other.py b/backend/routers/other.py index 5721f38..34fcc3e 100644 --- a/backend/routers/other.py +++ b/backend/routers/other.py @@ -2,6 +2,7 @@ from datetime import date from typing import Optional from fastapi import APIRouter, Depends, HTTPException from sqlalchemy import select +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from database import get_db @@ -39,7 +40,11 @@ def create_other_purchase( ): record = OtherPurchase(**body.model_dump(), user_id=current_user.id) db.add(record) - db.commit() + try: + db.commit() + except IntegrityError: + db.rollback() + raise HTTPException(status_code=409, detail=f"An other purchase for {body.date} already exists.") db.refresh(record) return record @@ -59,7 +64,11 @@ def update_other_purchase( raise HTTPException(status_code=404, detail="Record not found") for field, value in body.model_dump(exclude_none=True).items(): setattr(record, field, value) - db.commit() + try: + db.commit() + except IntegrityError: + db.rollback() + raise HTTPException(status_code=409, detail="An other purchase for that date already exists.") db.refresh(record) return record diff --git a/backend/routers/stats.py b/backend/routers/stats.py index cb23d49..a87e3b5 100644 --- a/backend/routers/stats.py +++ b/backend/routers/stats.py @@ -14,6 +14,12 @@ from auth import get_current_user router = APIRouter(prefix="/api/stats", tags=["stats"]) +DAYS_ROLLING = 30 # window for "last 30 days" stats +DAYS_SHORT = 7 # window for "last 7 days" stats +PRECISION_AVG = 2 # decimal places for egg averages +PRECISION_HEN = 3 # decimal places for per-hen averages +PRECISION_COST = 4 # decimal places for cost-per-egg/dozen + def _today(user_timezone: str) -> date: try: @@ -64,7 +70,7 @@ def _total_eggs(db: Session, user_id: int, start: date | None = None, end: date return db.scalar(q) -def _total_feed_cost(db: Session, user_id: int, start: date | None = None, end: date | None = None): +def _total_feed_cost(db: Session, user_id: int, start: date | None = None, end: date | None = None) -> Decimal: q = select( func.coalesce(func.sum(FeedPurchase.bags * FeedPurchase.price_per_bag), 0) ).where(FeedPurchase.user_id == user_id) @@ -72,16 +78,16 @@ def _total_feed_cost(db: Session, user_id: int, start: date | None = None, end: q = q.where(FeedPurchase.date >= start) if end: q = q.where(FeedPurchase.date <= end) - return db.scalar(q) + return Decimal(str(db.scalar(q))) -def _total_other_cost(db: Session, user_id: int, start: date | None = None, end: date | None = None): +def _total_other_cost(db: Session, user_id: int, start: date | None = None, end: date | None = None) -> Decimal: q = select(func.coalesce(func.sum(OtherPurchase.total), 0)).where(OtherPurchase.user_id == user_id) if start: q = q.where(OtherPurchase.date >= start) if end: q = q.where(OtherPurchase.date <= end) - return db.scalar(q) + return Decimal(str(db.scalar(q))) @router.get("/dashboard", response_model=DashboardStats) @@ -91,8 +97,8 @@ def dashboard_stats( ): uid = current_user.id today = _today(current_user.timezone) - start_30d = today - timedelta(days=30) - start_7d = today - timedelta(days=7) + start_30d = today - timedelta(days=DAYS_ROLLING) + start_7d = today - timedelta(days=DAYS_SHORT) total_alltime = _total_eggs(db, uid) total_30d = _total_eggs(db, uid, start=start_30d) @@ -110,7 +116,7 @@ def dashboard_stats( .where(EggCollection.date >= start_30d) ) - avg_per_day = round(total_30d / days_with_data_30d, 2) if days_with_data_30d else None + avg_per_day = round(total_30d / days_with_data_30d, PRECISION_AVG) if days_with_data_30d else None avg_per_hen = _avg_per_hen_30d(db, uid, start_30d) return DashboardStats( @@ -192,17 +198,17 @@ def monthly_stats( total_eggs = int(row.total_eggs) days_logged = int(row.days_logged) - avg_per_day = round(total_eggs / days_logged, 2) if days_logged else None - avg_per_hen = round(avg_per_day / flock, 3) if (avg_per_day and flock) else None + avg_per_day = round(total_eggs / days_logged, PRECISION_AVG) if days_logged else None + avg_per_hen = round(avg_per_day / flock, PRECISION_HEN) if (avg_per_day and flock) else None raw_feed_cost = feed_map.get((y, m)) raw_other_cost = other_map.get((y, m)) - feed_cost = round(raw_feed_cost, 2) if raw_feed_cost else None - other_cost = round(raw_other_cost, 2) if raw_other_cost else None + feed_cost = round(raw_feed_cost, PRECISION_AVG) if raw_feed_cost else None + other_cost = round(raw_other_cost, PRECISION_AVG) if raw_other_cost else None total_cost = (raw_feed_cost or Decimal(0)) + (raw_other_cost or Decimal(0)) - cpe = round(total_cost / total_eggs, 4) if (total_cost and total_eggs) else None - cpd = round(cpe * 12, 4) if cpe else None + cpe = round(total_cost / total_eggs, PRECISION_COST) if (total_cost and total_eggs) else None + cpd = round(cpe * 12, PRECISION_COST) if cpe else None results.append(MonthlySummary( year=y, @@ -229,7 +235,7 @@ def budget_stats( ): uid = current_user.id today = _today(current_user.timezone) - start_30d = today - timedelta(days=30) + start_30d = today - timedelta(days=DAYS_ROLLING) total_feed_cost = _total_feed_cost(db, uid) total_feed_cost_30d = _total_feed_cost(db, uid, start=start_30d) @@ -238,29 +244,29 @@ def budget_stats( total_eggs = _total_eggs(db, uid) total_eggs_30d = _total_eggs(db, uid, start=start_30d) - def cost_per_egg(cost, eggs): + def _cpe(cost: Decimal, eggs: int) -> Decimal | None: if not eggs or not cost: return None - return round(Decimal(str(cost)) / Decimal(eggs), 4) + return round(cost / eggs, PRECISION_COST) - def cost_per_dozen(cpe): - return round(cpe * 12, 4) if cpe else None + def _cpd(cpe: Decimal | None) -> Decimal | None: + return round(cpe * 12, PRECISION_COST) if cpe else None combined_cost = total_feed_cost + total_other_cost combined_cost_30d = total_feed_cost_30d + total_other_cost_30d - cpe = cost_per_egg(combined_cost, total_eggs) - cpe_30d = cost_per_egg(combined_cost_30d, total_eggs_30d) + cpe = _cpe(combined_cost, total_eggs) + cpe_30d = _cpe(combined_cost_30d, total_eggs_30d) return BudgetStats( - total_feed_cost=round(Decimal(str(total_feed_cost)), 2) if total_feed_cost else None, - total_feed_cost_30d=round(Decimal(str(total_feed_cost_30d)), 2) if total_feed_cost_30d else None, - total_other_cost=round(Decimal(str(total_other_cost)), 2) if total_other_cost else None, - total_other_cost_30d=round(Decimal(str(total_other_cost_30d)), 2) if total_other_cost_30d else None, + total_feed_cost=round(total_feed_cost, PRECISION_AVG) if total_feed_cost else None, + total_feed_cost_30d=round(total_feed_cost_30d, PRECISION_AVG) if total_feed_cost_30d else None, + total_other_cost=round(total_other_cost, PRECISION_AVG) if total_other_cost else None, + total_other_cost_30d=round(total_other_cost_30d, PRECISION_AVG) if total_other_cost_30d else None, total_eggs_alltime=total_eggs, total_eggs_30d=total_eggs_30d, cost_per_egg=cpe, - cost_per_dozen=cost_per_dozen(cpe), + cost_per_dozen=_cpd(cpe), cost_per_egg_30d=cpe_30d, - cost_per_dozen_30d=cost_per_dozen(cpe_30d), + cost_per_dozen_30d=_cpd(cpe_30d), ) diff --git a/docker-compose.yml b/docker-compose.yml index bd5e5b9..a1b8893 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,6 +1,8 @@ services: # ── MySQL ──────────────────────────────────────────────────────────────────── + # MYSQL_ROOT_PASSWORD and MYSQL_PASSWORD should each be 20+ random characters. + # Generate with: openssl rand -hex 16 db: image: mysql:8.0 restart: unless-stopped @@ -42,6 +44,12 @@ services: ADMIN_USERNAME: ${ADMIN_USERNAME} ADMIN_PASSWORD: ${ADMIN_PASSWORD} JWT_SECRET: ${JWT_SECRET} + healthcheck: + test: ["CMD", "python", "-c", "import urllib.request; urllib.request.urlopen('http://localhost:8000/api/health')"] + interval: 30s + timeout: 5s + retries: 3 + start_period: 15s depends_on: db: condition: service_healthy # wait for MySQL to be ready before starting diff --git a/nginx/html/js/dashboard.js b/nginx/html/js/dashboard.js index a4929e4..c89b311 100644 --- a/nginx/html/js/dashboard.js +++ b/nginx/html/js/dashboard.js @@ -35,13 +35,15 @@ function buildChart(eggs) { if (eggChart) eggChart.destroy(); // prevent duplicate charts on re-render + const green = getComputedStyle(document.documentElement).getPropertyValue('--green').trim(); + eggChart = new Chart(ctx, { type: 'line', data: { labels, datasets: [{ data, - borderColor: '#3d6b4f', + borderColor: green, backgroundColor: 'rgba(61,107,79,0.08)', borderWidth: 2, pointRadius: 3,