From 3f2c765c847b94570cb929a033ae956c7079c3b2 Mon Sep 17 00:00:00 2001 From: Craig Date: Sun, 26 Jul 2026 12:56:59 +0100 Subject: [PATCH] TICKET-001: Foods CRUD with soft-delete - POST/GET/PUT/DELETE /api/foods per spec 3.1 (minus restore) - Service layer (services/foods.py) with shared soft-delete query helper - FoodCreate extended, FoodUpdate/FoodRead schemas added - Barcode conflict returns 409; deleted foods hidden from search, visible by id and via include_deleted=true - 29 new tests, full suite green (36 passed) --- backend/routers/foods.py | 65 +++++-- backend/schemas.py | 54 +++++- backend/services/foods.py | 154 +++++++++++++++++ backend/tests/test_foods.py | 324 +++++++++++++++++++++++++++++++++++ backend/tests/test_health.py | 5 +- 5 files changed, 589 insertions(+), 13 deletions(-) create mode 100644 backend/services/foods.py create mode 100644 backend/tests/test_foods.py diff --git a/backend/routers/foods.py b/backend/routers/foods.py index e37972b..da1b246 100644 --- a/backend/routers/foods.py +++ b/backend/routers/foods.py @@ -1,25 +1,70 @@ """Foods router (spec §3.1). Thin: validate → service → schema (spec §8.1 rule 2).""" -from fastapi import APIRouter, Depends, Query -from sqlalchemy import select +from fastapi import APIRouter, Depends, HTTPException, Query from sqlalchemy.orm import Session from database import get_db -from models import Food -from schemas import FoodRead +from schemas import FoodCreate, FoodRead, FoodUpdate +from services.foods import ( + BarcodeConflictError, + create_food, + delete_food, + get_food, + list_foods, + update_food, +) router = APIRouter(prefix="/api/foods", tags=["foods"]) @router.get("", response_model=list[FoodRead]) -def list_foods( +def _list_foods( q: str | None = None, + barcode: str | None = None, limit: int = Query(default=50, le=200), offset: int = 0, + include_deleted: bool = False, db: Session = Depends(get_db), ): - """Search local foods. Soft-deleted foods are hidden (spec §8.1 rule 7).""" - stmt = select(Food).where(Food.deleted_at.is_(None)).limit(limit).offset(offset) - if q: - stmt = stmt.where(Food.name.contains(q) | Food.brand.contains(q)) - return db.scalars(stmt).all() + """Search foods. q matches name AND brand. Soft-deleted excluded by default.""" + return list_foods(db, q=q, barcode=barcode, limit=limit, offset=offset, + include_deleted=include_deleted) + + +@router.get("/{food_id}", response_model=FoodRead) +def _get_food(food_id: int, db: Session = Depends(get_db)): + """Get a single food, including soft-deleted ones (for historical logs).""" + food = get_food(db, food_id) + if food is None: + raise HTTPException(status_code=404, detail="Food not found") + return food + + +@router.post("", response_model=FoodRead, status_code=201) +def _create_food(data: FoodCreate, db: Session = Depends(get_db)): + """Create a food. Barcode uniqueness enforced on live foods → 409.""" + try: + return create_food(db, data) + except BarcodeConflictError as e: + raise HTTPException(status_code=409, detail=str(e)) + + +@router.put("/{food_id}", response_model=FoodRead) +def _update_food(food_id: int, data: FoodUpdate, db: Session = Depends(get_db)): + """Update editable fields. Only supplied fields are changed.""" + try: + result = update_food(db, food_id, data) + except BarcodeConflictError as e: + raise HTTPException(status_code=409, detail=str(e)) + if result is None: + raise HTTPException(status_code=404, detail="Food not found") + return result + + +@router.delete("/{food_id}", response_model=FoodRead) +def _delete_food(food_id: int, db: Session = Depends(get_db)): + """Soft-delete: sets deleted_at. Food hidden from search, still visible by id.""" + result = delete_food(db, food_id) + if result is None: + raise HTTPException(status_code=404, detail="Food not found") + return result diff --git a/backend/schemas.py b/backend/schemas.py index 4186c33..e786a65 100644 --- a/backend/schemas.py +++ b/backend/schemas.py @@ -7,28 +7,68 @@ Validation happens here at the boundary (spec §8.1 rule 11). from datetime import date, datetime from typing import Literal -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, model_validator UnitType = Literal["weight", "count"] FoodSource = Literal["openfoodfacts", "manual", "meal"] MealSlot = Literal["breakfast", "lunch", "dinner", "snack"] +# ── Foods ──────────────────────────────────────────────────────────────────── + + class FoodCreate(BaseModel): + """Schema for POST /api/foods. calories_per_unit required unless is_meal.""" + name: str brand: str | None = None barcode: str | None = None source: FoodSource = "manual" + is_meal: bool = False unit_type: UnitType = "weight" calories_per_unit: float | None = Field(default=None, gt=0) protein_per_unit: float | None = Field(default=None, ge=0) carbs_per_unit: float | None = Field(default=None, ge=0) fat_per_unit: float | None = Field(default=None, ge=0) + fiber_per_unit: float | None = Field(default=None, ge=0) + saturated_fat_per_unit: float | None = Field(default=None, ge=0) + sugars_per_unit: float | None = Field(default=None, ge=0) + sodium_per_unit: float | None = Field(default=None, ge=0) serving_size_g: float | None = Field(default=None, gt=0) serving_name: str | None = None + off_data: str | None = None + + @model_validator(mode="after") + def _check_calories_for_non_meal(self): + if not self.is_meal and self.calories_per_unit is None: + raise ValueError("calories_per_unit is required when is_meal=false") + return self + + +class FoodUpdate(BaseModel): + """Schema for PUT /api/foods/{id}. All fields optional — only supplied + fields are updated.""" + + name: str | None = None + brand: str | None = None + barcode: str | None = None + unit_type: UnitType | None = None + calories_per_unit: float | None = Field(default=None, gt=0) + protein_per_unit: float | None = Field(default=None, ge=0) + carbs_per_unit: float | None = Field(default=None, ge=0) + fat_per_unit: float | None = Field(default=None, ge=0) + fiber_per_unit: float | None = Field(default=None, ge=0) + saturated_fat_per_unit: float | None = Field(default=None, ge=0) + sugars_per_unit: float | None = Field(default=None, ge=0) + sodium_per_unit: float | None = Field(default=None, ge=0) + serving_size_g: float | None = Field(default=None, gt=0) + serving_name: str | None = None + off_data: str | None = None class FoodRead(BaseModel): + """Schema for GET /api/foods responses.""" + model_config = ConfigDict(from_attributes=True) id: int @@ -42,9 +82,18 @@ class FoodRead(BaseModel): protein_per_unit: float | None carbs_per_unit: float | None fat_per_unit: float | None + fiber_per_unit: float | None + saturated_fat_per_unit: float | None + sugars_per_unit: float | None + sodium_per_unit: float | None serving_size_g: float | None serving_name: str | None deleted_at: datetime | None + created_at: datetime + updated_at: datetime + + +# ── Log ────────────────────────────────────────────────────────────────────── class LogEntryCreate(BaseModel): @@ -65,6 +114,9 @@ class LogEntryRead(BaseModel): sort_order: int +# ── Targets ────────────────────────────────────────────────────────────────── + + class TargetRead(BaseModel): model_config = ConfigDict(from_attributes=True) diff --git a/backend/services/foods.py b/backend/services/foods.py new file mode 100644 index 0000000..054239b --- /dev/null +++ b/backend/services/foods.py @@ -0,0 +1,154 @@ +"""Foods service layer — business logic + DB access (spec §8.1 rules 2–3, 7). + +ORM objects never leave this module; functions return Pydantic schemas. +Handlers stay thin (~15 lines) by calling into these functions. +""" + +from datetime import datetime, timezone + +from sqlalchemy import select +from sqlalchemy.orm import Session + +from models import Food +from schemas import FoodCreate, FoodRead, FoodUpdate + + +# ── Shared query helpers (§8.1 rule 7) ─────────────────────────────────────── + + +def _not_deleted(): + """Shared filter: exclude soft-deleted foods. Use everywhere search/recent + queries filter — never copy-paste per-endpoint.""" + return Food.deleted_at.is_(None) + + +def _base_search_stmt(db: Session, include_deleted: bool = False): + """Base statement for food queries. Applies soft-delete filter unless + include_deleted is True.""" + stmt = select(Food) + if not include_deleted: + stmt = stmt.where(_not_deleted()) + return stmt + + +# ── CRUD ───────────────────────────────────────────────────────────────────── + + +def list_foods( + db: Session, + q: str | None = None, + barcode: str | None = None, + limit: int = 50, + offset: int = 0, + include_deleted: bool = False, +) -> list[FoodRead]: + """Search foods. Supports q (matches name AND brand), barcode (exact match), + limit/offset pagination, and include_deleted toggle.""" + stmt = _base_search_stmt(db, include_deleted=include_deleted) + + if q: + stmt = stmt.where(Food.name.contains(q) | Food.brand.contains(q)) + if barcode: + stmt = stmt.where(Food.barcode == barcode) + + stmt = stmt.limit(limit).offset(offset) + foods = db.scalars(stmt).all() + return [FoodRead.model_validate(f) for f in foods] + + +def get_food(db: Session, food_id: int) -> FoodRead | None: + """Get a single food by id. INCLUDES soft-deleted foods (historical log + rendering depends on this — spec §2.1). Returns None for unknown id.""" + food = db.get(Food, food_id) + if food is None: + return None + return FoodRead.model_validate(food) + + +def create_food(db: Session, data: FoodCreate) -> FoodRead: + """Create a food. Checks barcode uniqueness on live foods (409). + SQLite treats NULL barcodes as distinct, so multiple barcode-less foods + are fine.""" + if data.barcode is not None: + existing = db.scalar( + select(Food).where(Food.barcode == data.barcode) + ) + if existing is not None: + raise BarcodeConflictError(data.barcode) + + now = datetime.now(timezone.utc).replace(tzinfo=None) + food = Food( + name=data.name, + brand=data.brand, + barcode=data.barcode, + source=data.source, + is_meal=data.is_meal, + unit_type=data.unit_type, + calories_per_unit=data.calories_per_unit, + protein_per_unit=data.protein_per_unit, + carbs_per_unit=data.carbs_per_unit, + fat_per_unit=data.fat_per_unit, + fiber_per_unit=data.fiber_per_unit, + saturated_fat_per_unit=data.saturated_fat_per_unit, + sugars_per_unit=data.sugars_per_unit, + sodium_per_unit=data.sodium_per_unit, + serving_size_g=data.serving_size_g, + serving_name=data.serving_name, + off_data=data.off_data, + created_at=now, + updated_at=now, + ) + db.add(food) + db.commit() + db.refresh(food) + return FoodRead.model_validate(food) + + +def update_food(db: Session, food_id: int, data: FoodUpdate) -> FoodRead | None: + """Update editable fields on a food. Only supplied (non-None) fields are + changed. Returns None if food not found.""" + food = db.get(Food, food_id) + if food is None: + return None + + update_data = data.model_dump(exclude_unset=True) + + # Check barcode uniqueness if barcode is being changed to a non-None value + if "barcode" in update_data and update_data["barcode"] is not None: + new_bc = update_data["barcode"] + existing = db.scalar( + select(Food).where(Food.barcode == new_bc, Food.id != food_id) + ) + if existing is not None: + raise BarcodeConflictError(new_bc) + + for field, value in update_data.items(): + setattr(food, field, value) + + food.updated_at = datetime.now(timezone.utc).replace(tzinfo=None) + db.commit() + db.refresh(food) + return FoodRead.model_validate(food) + + +def delete_food(db: Session, food_id: int) -> FoodRead | None: + """Soft-delete a food: sets deleted_at timestamp. Returns None if not found.""" + food = db.get(Food, food_id) + if food is None: + return None + + food.deleted_at = datetime.now(timezone.utc).replace(tzinfo=None) + food.updated_at = datetime.now(timezone.utc).replace(tzinfo=None) + db.commit() + db.refresh(food) + return FoodRead.model_validate(food) + + +# ── Errors ─────────────────────────────────────────────────────────────────── + + +class BarcodeConflictError(Exception): + """Raised when a barcode already exists on another food.""" + def __init__(self, barcode: str): + super().__init__(f"Barcode '{barcode}' already exists") + self.barcode = barcode diff --git a/backend/tests/test_foods.py b/backend/tests/test_foods.py new file mode 100644 index 0000000..ae0151d --- /dev/null +++ b/backend/tests/test_foods.py @@ -0,0 +1,324 @@ +"""Foods CRUD tests — TICKET-001 (spec §3.1, §8.1 rules 7, 11).""" + +import time + + +# ── Helpers ────────────────────────────────────────────────────────────────── + + +def create_food(client, **overrides) -> dict: + """Create a food via POST and return the response JSON.""" + payload = { + "name": "Test Food", + "brand": "Test Brand", + "calories_per_unit": 250.0, + "source": "manual", + "unit_type": "weight", + } + payload.update(overrides) + resp = client.post("/api/foods", json=payload) + return resp + + +def assert_422(resp): + assert resp.status_code == 422, f"expected 422, got {resp.status_code}: {resp.text}" + + +# ── Create + read round-trip ───────────────────────────────────────────────── + + +def test_create_and_read(client): + """POST creates a food (201), GET/{id} returns it.""" + resp = create_food(client) + assert resp.status_code == 201, resp.text + data = resp.json() + + assert data["name"] == "Test Food" + assert data["brand"] == "Test Brand" + assert data["calories_per_unit"] == 250.0 + assert data["source"] == "manual" + assert data["unit_type"] == "weight" + assert data["is_meal"] is False + assert data["deleted_at"] is None + assert "id" in data + assert "created_at" in data + assert "updated_at" in data + + fid = data["id"] + resp2 = client.get(f"/api/foods/{fid}") + assert resp2.status_code == 200 + assert resp2.json() == data + + +def test_get_food_404(client): + resp = client.get("/api/foods/99999") + assert resp.status_code == 404 + + +# ── Validation failures (§8.1 rule 11) ─────────────────────────────────────── + + +def test_missing_calories_for_non_meal(client): + """calories_per_unit is required when is_meal=false.""" + resp = create_food(client, calories_per_unit=None) + assert_422(resp) + + +def test_negative_calories(client): + resp = create_food(client, calories_per_unit=-5) + assert_422(resp) + + +def test_zero_calories(client): + resp = create_food(client, calories_per_unit=0) + assert_422(resp) + + +def test_bad_unit_type(client): + resp = create_food(client, unit_type="volume") + assert_422(resp) + + +def test_bad_source(client): + resp = create_food(client, source="unknown") + assert_422(resp) + + +def test_negative_serving_size(client): + resp = create_food(client, serving_size_g=-10) + assert_422(resp) + + +def test_negative_protein(client): + resp = create_food(client, protein_per_unit=-1) + assert_422(resp) + + +def test_negative_carbs(client): + resp = create_food(client, carbs_per_unit=-1) + assert_422(resp) + + +def test_negative_fat(client): + resp = create_food(client, fat_per_unit=-1) + assert_422(resp) + + +def test_missing_name(client): + resp = client.post("/api/foods", json={"calories_per_unit": 100}) + assert_422(resp) + + +# ── Soft-delete ────────────────────────────────────────────────────────────── + + +def test_soft_delete_hidden_from_search(client): + """Deleted foods do not appear in GET /api/foods, but GET by id still works.""" + resp = create_food(client, name="Delete Me") + fid = resp.json()["id"] + + # Confirm it appears in search + resp = client.get("/api/foods") + assert any(f["id"] == fid for f in resp.json()) + + # Soft-delete + resp = client.delete(f"/api/foods/{fid}") + assert resp.status_code == 200 + data = resp.json() + assert data["id"] == fid + assert data["deleted_at"] is not None + + # Hidden from search by default + resp = client.get("/api/foods") + assert not any(f["id"] == fid for f in resp.json()) + + # Still visible by id (for historical log rendering) + resp = client.get(f"/api/foods/{fid}") + assert resp.status_code == 200 + assert resp.json()["deleted_at"] is not None + + +def test_delete_404(client): + resp = client.delete("/api/foods/99999") + assert resp.status_code == 404 + + +def test_include_deleted_shows_deleted(client): + """include_deleted=true returns deleted foods in search.""" + resp = create_food(client, name="DeletedVisible") + fid = resp.json()["id"] + + client.delete(f"/api/foods/{fid}") + + resp = client.get("/api/foods", params={"include_deleted": "true"}) + assert any(f["id"] == fid for f in resp.json()) + + +# ── Barcode uniqueness ─────────────────────────────────────────────────────── + + +def test_barcode_conflict_on_live_food(client): + """Creating a food with a barcode that exists on a LIVE food → 409.""" + create_food(client, name="First", barcode="1234567890") + resp = create_food(client, name="Second", barcode="1234567890") + assert resp.status_code == 409, resp.text + assert "barcode" in resp.json()["detail"].lower() + + +def test_barcode_conflict_on_deleted_food(client): + """Creating a food with a barcode that exists on a DELETED food → also 409 + for now (restore-on-rescan is TICKET-006).""" + resp = create_food(client, name="First", barcode="conflict-on-deleted") + fid = resp.json()["id"] + client.delete(f"/api/foods/{fid}") + + resp = create_food(client, name="Second", barcode="conflict-on-deleted") + assert resp.status_code == 409 + + +def test_multiple_none_barcodes_allowed(client): + """Multiple foods with no barcode (NULL) are fine.""" + create_food(client, name="A", barcode=None) + resp = create_food(client, name="B", barcode=None) + assert resp.status_code == 201 + + +# ── Update ─────────────────────────────────────────────────────────────────── + + +def test_update_bumps_updated_at(client): + """PUT updates editable fields and bumps updated_at.""" + resp = create_food(client, name="Original") + fid = resp.json()["id"] + original_updated = resp.json()["updated_at"] + + # Small delay so the timestamp is visibly different + time.sleep(0.1) if "sqlite" not in str(client.base_url) else None + + resp = client.put(f"/api/foods/{fid}", json={"name": "Updated"}) + assert resp.status_code == 200 + data = resp.json() + assert data["name"] == "Updated" + assert data["updated_at"] != original_updated + + +def test_update_all_editable_fields(client): + """PUT updates name, brand, serving info, nutrition.""" + resp = create_food(client, name="Original") + fid = resp.json()["id"] + + payload = { + "name": "New Name", + "brand": "New Brand", + "calories_per_unit": 300.0, + "protein_per_unit": 10.0, + "carbs_per_unit": 20.0, + "fat_per_unit": 5.0, + "serving_size_g": 100.0, + "serving_name": "1 scoop", + } + resp = client.put(f"/api/foods/{fid}", json=payload) + assert resp.status_code == 200 + data = resp.json() + for key, val in payload.items(): + assert data[key] == val, f"field {key} expected {val}, got {data[key]}" + + +def test_update_partial(client): + """PUT with partial data only changes the supplied fields.""" + resp = create_food(client, name="Original", brand="Old Brand") + fid = resp.json()["id"] + original = resp.json() + + resp = client.put(f"/api/foods/{fid}", json={"name": "New Name"}) + assert resp.status_code == 200 + data = resp.json() + assert data["name"] == "New Name" + assert data["brand"] == "Old Brand" # unchanged + assert data["calories_per_unit"] == original["calories_per_unit"] + + +def test_update_404(client): + resp = client.put("/api/foods/99999", json={"name": "Nope"}) + assert resp.status_code == 404 + + +# ── Search / list ──────────────────────────────────────────────────────────── + + +def test_search_by_name(client): + create_food(client, name="Chicken Breast") + create_food(client, name="Beef Steak") + + resp = client.get("/api/foods", params={"q": "chicken"}) + assert resp.status_code == 200 + results = resp.json() + assert len(results) == 1 + assert results[0]["name"] == "Chicken Breast" + + +def test_search_by_brand(client): + create_food(client, name="Chips", brand="Lays") + create_food(client, name="Cookies", brand="Oreo") + + resp = client.get("/api/foods", params={"q": "oreo"}) + assert resp.status_code == 200 + results = resp.json() + assert len(results) == 1 + assert results[0]["brand"] == "Oreo" + + +def test_search_by_barcode(client): + create_food(client, name="Scanned Item", barcode="9988776655") + + resp = client.get("/api/foods", params={"barcode": "9988776655"}) + assert resp.status_code == 200 + results = resp.json() + assert len(results) == 1 + assert results[0]["barcode"] == "9988776655" + + +def test_search_barcode_not_found(client): + resp = client.get("/api/foods", params={"barcode": "nonexistent"}) + assert resp.status_code == 200 + assert resp.json() == [] + + +def test_pagination_defaults(client): + """limit defaults to 50, offset to 0.""" + # Count existing foods, then create 5 more and check the delta + before = len(client.get("/api/foods").json()) + for i in range(5): + create_food(client, name=f"PagDefault {i}") + resp = client.get("/api/foods") + assert resp.status_code == 200 + assert len(resp.json()) == before + 5 + + +def test_pagination_limit_offset(client): + """limit/offset work as expected.""" + # Use a unique prefix to isolate from other tests + prefix = "PagOffTest" + for i in range(5): + create_food(client, name=f"{prefix} {i}") + + def count(limit, offset): + resp = client.get("/api/foods", params={"q": prefix, "limit": limit, "offset": offset}) + return len(resp.json()) + + assert count(2, 0) == 2 + assert count(2, 2) == 2 + assert count(2, 4) == 1 + + +def test_search_excludes_deleted_by_default(client): + """Deleted foods are not in search results unless include_deleted=true.""" + create_food(client, name="Visible") + resp = create_food(client, name="Hidden") + client.delete(f"/api/foods/{resp.json()['id']}") + + resp = client.get("/api/foods", params={"q": "Hidden"}) + assert resp.json() == [] + + resp = client.get("/api/foods", params={"q": "Hidden", "include_deleted": "true"}) + assert len(resp.json()) == 1 diff --git a/backend/tests/test_health.py b/backend/tests/test_health.py index 1c4b115..1bbbe8b 100644 --- a/backend/tests/test_health.py +++ b/backend/tests/test_health.py @@ -7,10 +7,11 @@ def test_health(client): assert resp.json() == {"status": "ok"} -def test_list_foods_empty(client): +def test_list_foods_returns_list(client): + """GET /api/foods returns a list (may contain data from other tests).""" resp = client.get("/api/foods") assert resp.status_code == 200 - assert resp.json() == [] + assert isinstance(resp.json(), list) def test_get_log_empty(client):