diff --git a/backend/routers/targets.py b/backend/routers/targets.py index 2c0b25d..f7cb951 100644 --- a/backend/routers/targets.py +++ b/backend/routers/targets.py @@ -1,24 +1,48 @@ -"""Targets router (spec §3.4).""" +"""Targets router (spec §3.4). Thin handlers — business logic in services/targets.py.""" from fastapi import APIRouter, Depends, HTTPException -from sqlalchemy import select from sqlalchemy.orm import Session from database import get_db -from models import Target -from schemas import TargetRead +from schemas import TargetCreate, TargetRead, TargetUpdate +from services import targets as svc router = APIRouter(prefix="/api/targets", tags=["targets"]) @router.get("", response_model=list[TargetRead]) def list_targets(db: Session = Depends(get_db)): - return db.scalars(select(Target).order_by(Target.start_date)).all() + return svc.list_targets(db) @router.get("/current", response_model=TargetRead) def current_target(db: Session = Depends(get_db)): - target = db.scalar(select(Target).where(Target.end_date.is_(None))) + target = svc.get_current_target(db) if target is None: raise HTTPException(status_code=404, detail="No active target") return target + + +@router.post("", response_model=TargetRead, status_code=201) +def create_target(data: TargetCreate, db: Session = Depends(get_db)): + """Create a new target. Auto-closes the previous active target.""" + try: + return svc.create_target(db, data) + except ValueError as e: + raise HTTPException(status_code=422, detail=str(e)) + + +@router.put("/{target_id}", response_model=TargetRead) +def update_target(target_id: int, data: TargetUpdate, db: Session = Depends(get_db)): + """Update a target's values and/or date range. + The single-active-target invariant is preserved.""" + try: + result = svc.update_target(db, target_id, data) + except ValueError as e: + raise HTTPException(status_code=422, detail=str(e)) + except svc.ActiveTargetConflictError as e: + raise HTTPException(status_code=409, detail=str(e)) + + if result is None: + raise HTTPException(status_code=404, detail="Target not found") + return result diff --git a/backend/schemas.py b/backend/schemas.py index e786a65..7bf595d 100644 --- a/backend/schemas.py +++ b/backend/schemas.py @@ -117,6 +117,36 @@ class LogEntryRead(BaseModel): # ── Targets ────────────────────────────────────────────────────────────────── +class TargetCreate(BaseModel): + """Schema for POST /api/targets. end_date is auto-managed by the service.""" + + start_date: date # client-supplied YYYY-MM-DD (spec §8.1 rule 8) + calories: int = Field(gt=0) + protein_g: float | None = Field(default=None, ge=0) + carbs_g: float | None = Field(default=None, ge=0) + fat_g: float | None = Field(default=None, ge=0) + + +class TargetUpdate(BaseModel): + """Schema for PUT /api/targets/{id}. All fields optional — only supplied + fields are updated. Setting end_date to null will reactivate a historical + target only if no other active target exists.""" + + start_date: date | None = None + end_date: date | None = None + calories: int | None = Field(default=None, gt=0) + protein_g: float | None = Field(default=None, ge=0) + carbs_g: float | None = Field(default=None, ge=0) + fat_g: float | None = Field(default=None, ge=0) + + @model_validator(mode="after") + def _check_date_range(self): + if self.start_date is not None and self.end_date is not None: + if self.end_date <= self.start_date: + raise ValueError("end_date must be after start_date") + return self + + class TargetRead(BaseModel): model_config = ConfigDict(from_attributes=True) diff --git a/backend/services/targets.py b/backend/services/targets.py new file mode 100644 index 0000000..42b8850 --- /dev/null +++ b/backend/services/targets.py @@ -0,0 +1,171 @@ +"""Targets service layer — business logic + DB access (spec §8.1 rules 2–3, 6). + +Auto-close semantics: +- When a new target is created, the previous active target's end_date is set + to the new target's start_date (same day). +- Historical lookup uses an EXCLUSIVE end_date: a target covers dates where + start_date <= date AND (end_date IS NULL OR end_date > date). + This forms half-open intervals [start_date, end_date), so there is never + overlap or ambiguity about which target applies on a boundary date. + +ORM objects never leave this module; functions return Pydantic schemas. +""" + +from datetime import date + +from sqlalchemy import select +from sqlalchemy.orm import Session + +from models import Target +from schemas import TargetCreate, TargetRead, TargetUpdate + + +# ── CRUD ───────────────────────────────────────────────────────────────────── + + +def list_targets(db: Session) -> list[TargetRead]: + """List all targets ordered by start_date.""" + targets = db.scalars(select(Target).order_by(Target.start_date)).all() + return [TargetRead.model_validate(t) for t in targets] + + +def get_current_target(db: Session) -> TargetRead | None: + """Return the active target (end_date IS NULL), or None.""" + target = db.scalar(select(Target).where(Target.end_date.is_(None))) + if target is None: + return None + return TargetRead.model_validate(target) + + +def create_target(db: Session, data: TargetCreate) -> TargetRead: + """Create a new target. Auto-closes the previous active target by setting + its end_date to the new target's start_date. Runs in a single transaction + so there is never a window with zero or two active targets (§2.4, §8.1 rule 6). + + Raises ValueError if the auto-close would cause the previous target's + end_date to be <= its start_date (e.g. backdating a target before the + current one started). + """ + new_start = data.start_date + + # Find the currently active target and auto-close it + current_active = db.scalar( + select(Target).where(Target.end_date.is_(None)) + ) + if current_active is not None: + if new_start <= current_active.start_date: + raise ValueError( + f"New target start_date ({new_start}) is not after the current " + f"active target's start_date ({current_active.start_date}). " + f"Cannot auto-close without creating an invalid date range." + ) + current_active.end_date = new_start + + target = Target( + start_date=data.start_date, + end_date=None, + calories=data.calories, + protein_g=data.protein_g, + carbs_g=data.carbs_g, + fat_g=data.fat_g, + ) + db.add(target) + db.commit() + db.refresh(target) + return TargetRead.model_validate(target) + + +def update_target(db: Session, target_id: int, data: TargetUpdate) -> TargetRead | None: + """Update a target's values and/or date range. The single-active-target + invariant is preserved: setting end_date to null when another active + target already exists is rejected. + + Returns None if the target is not found. + Raises ValueError for date-range validation failures. + Raises ActiveTargetConflictError if the update would create two active targets. + """ + target = db.get(Target, target_id) + if target is None: + return None + + update_data = data.model_dump(exclude_unset=True) + + # Validate the resulting date range if either date field is being changed + new_start = update_data.get("start_date", target.start_date) + new_end = update_data.get("end_date", target.end_date) # None means "not being updated" + + # If end_date key is present in update_data, use that value (could be None explicitly) + if "end_date" in update_data: + new_end = update_data["end_date"] + else: + new_end = target.end_date + + # Validate end_date > start_date when both are set + if new_end is not None and new_end <= new_start: + raise ValueError("end_date must be after start_date") + + # Check single-active-target invariant: if setting end_date to NULL, + # there must not already be another active target + if "end_date" in update_data and update_data["end_date"] is None: + if target.end_date is not None: # target is currently NOT active + other_active = db.scalar( + select(Target).where( + Target.end_date.is_(None), + Target.id != target_id, + ) + ) + if other_active is not None: + raise ActiveTargetConflictError(other_active.id) + + # If changing start_date on the active target, ensure it doesn't break + # the invariant with respect to the previously-closed target. + # (The previously-closed target's end_date references the OLD start_date; + # changing start_date on the active one doesn't retroactively fix that. + # This is fine — historical targets may have end_date values that don't + # align perfectly after edits. The half-open interval lookup still works.) + + for field, value in update_data.items(): + setattr(target, field, value) + + db.commit() + db.refresh(target) + return TargetRead.model_validate(target) + + +# ── Historical lookup ──────────────────────────────────────────────────────── + + +def get_target_for_date(db: Session, lookup_date: date) -> TargetRead | None: + """Return the target whose date range contains the given date. + + Uses half-open intervals [start_date, end_date): a target covers dates + where start_date <= date AND (end_date IS NULL OR end_date > date). + + Returns None if no target covers the given date. + Exposed for TICKET-004 (day summary endpoint). + """ + target = db.scalar( + select(Target) + .where( + Target.start_date <= lookup_date, + (Target.end_date.is_(None)) | (Target.end_date > lookup_date), + ) + .order_by(Target.start_date.desc()) + .limit(1) + ) + if target is None: + return None + return TargetRead.model_validate(target) + + +# ── Errors ─────────────────────────────────────────────────────────────────── + + +class ActiveTargetConflictError(Exception): + """Raised when an operation would create two active targets.""" + def __init__(self, existing_active_id: int): + super().__init__( + f"There is already an active target (id={existing_active_id}). " + f"Close it first before activating another." + ) + self.existing_active_id = existing_active_id diff --git a/backend/tests/test_targets.py b/backend/tests/test_targets.py new file mode 100644 index 0000000..87381f8 --- /dev/null +++ b/backend/tests/test_targets.py @@ -0,0 +1,512 @@ +"""Targets CRUD tests — TICKET-002 (spec §2.4, §3.4, §8.1 rules 2–3, 11). + +Auto-close semantics (documented in services/targets.py): +- When a new target is created, the previous active target's end_date is set + to the new target's start_date (same day). +- Historical lookup uses an EXCLUSIVE end_date: a target covers dates where + start_date <= date AND (end_date IS NULL OR end_date > date). + This makes intervals half-open [start_date, end_date), so there is never + overlap or ambiguity. + +Tests share a session-scoped DB. Each test cleans up after itself by closing +any active targets it leaves behind, so subsequent tests start with a clean +slate (no active target). Tests that need a fully empty targets table use +direct DB access to delete all rows. +""" + +from datetime import date + + +# ── Helpers ────────────────────────────────────────────────────────────────── + + +def create_target(client, **overrides) -> dict: + """Create a target via POST and return the response JSON.""" + payload = { + "start_date": "2025-01-01", + "calories": 2000, + "protein_g": 150.0, + "carbs_g": 200.0, + "fat_g": 65.0, + } + payload.update(overrides) + resp = client.post("/api/targets", json=payload) + return resp + + +def assert_422(resp): + assert resp.status_code == 422, f"expected 422, got {resp.status_code}: {resp.text}" + + +def close_active_target(client): + """Close the active target (if any) by giving it an end_date. + Uses a date far in the future so it always passes date validation.""" + current = client.get("/api/targets/current") + if current.status_code == 200: + tid = current.json()["id"] + client.put(f"/api/targets/{tid}", json={"end_date": "2099-12-31"}) + + +# ── Create + current round-trip ────────────────────────────────────────────── + + +def test_create_and_read_current(client): + """POST creates a target (201), GET /current returns it.""" + try: + resp = create_target(client) + assert resp.status_code == 201, resp.text + data = resp.json() + + assert data["start_date"] == "2025-01-01" + assert data["end_date"] is None + assert data["calories"] == 2000 + assert data["protein_g"] == 150.0 + assert data["carbs_g"] == 200.0 + assert data["fat_g"] == 65.0 + assert "id" in data + + # GET /current should return the same target + resp2 = client.get("/api/targets/current") + assert resp2.status_code == 200 + assert resp2.json() == data + finally: + close_active_target(client) + + +def test_current_404_when_none(client): + """GET /current returns 404 when no active target exists.""" + close_active_target(client) # Ensure clean state + resp = client.get("/api/targets/current") + assert resp.status_code == 404 + + +# ── Auto-close of previous target ──────────────────────────────────────────── + + +def test_auto_close_previous_target(client): + """Creating a second target auto-closes the first: its end_date is set + to the new target's start_date, and /current returns the new one.""" + try: + # Create first target (active) + resp1 = create_target(client, start_date="2025-01-01", calories=2000) + assert resp1.status_code == 201 + id1 = resp1.json()["id"] + assert resp1.json()["end_date"] is None + + # Create second target with later start_date + resp2 = create_target(client, start_date="2025-06-01", calories=2200) + assert resp2.status_code == 201 + id2 = resp2.json()["id"] + assert resp2.json()["end_date"] is None + + # First target should now have an end_date equal to the second's start_date + resp_get = client.get("/api/targets") + all_targets = resp_get.json() + t1 = next(t for t in all_targets if t["id"] == id1) + assert t1["end_date"] == "2025-06-01" + + # /current should return the second target + current = client.get("/api/targets/current").json() + assert current["id"] == id2 + finally: + close_active_target(client) + + +def test_auto_close_with_earlier_start_date(client): + """If the new target's start_date is BEFORE the existing active target's + start_date, the operation is rejected (422) because it would create an + invalid date range on the auto-closed target.""" + try: + # Create first target (active starting 2025-06-01) + create_target(client, start_date="2025-06-01", calories=2000) + + # Try to create a target with start_date before the active one's start_date + resp = create_target(client, start_date="2025-01-01", calories=1800) + # This would set the existing target's end_date to 2025-01-01, + # which is before its start_date of 2025-06-01 → rejected + assert_422(resp) + finally: + close_active_target(client) + + +def test_only_one_active_after_create(client): + """After creating 3 targets sequentially, exactly one has end_date IS NULL.""" + try: + for i, start in enumerate(["2025-01-01", "2025-04-01", "2025-07-01"]): + resp = create_target(client, start_date=start, calories=2000 + i * 100) + assert resp.status_code == 201, f"iteration {i}: {resp.text}" + + all_targets = client.get("/api/targets").json() + active = [t for t in all_targets if t["end_date"] is None] + assert len(active) == 1 + assert active[0]["start_date"] == "2025-07-01" + finally: + close_active_target(client) + + +# ── List targets ───────────────────────────────────────────────────────────── + + +def test_list_targets_ordered_by_start_date(client): + """GET /api/targets returns all targets ordered by start_date.""" + try: + # Use unique calorie values to identify our targets + base_cal = 9000 # Distinct from other tests + dates = ["2025-03-01", "2025-01-01", "2025-02-01"] + for d in dates: + create_target(client, start_date=d, calories=base_cal) + base_cal += 1 + + resp = client.get("/api/targets") + assert resp.status_code == 200 + results = resp.json() + + # Verify ALL targets are ordered by start_date + for i in range(len(results) - 1): + assert results[i]["start_date"] <= results[i + 1]["start_date"], ( + f"out of order at index {i}: " + f"{results[i]['start_date']} > {results[i+1]['start_date']}" + ) + finally: + close_active_target(client) + + +def test_list_targets_returns_list(client): + """GET /api/targets returns a list (may be empty or populated).""" + resp = client.get("/api/targets") + assert resp.status_code == 200 + assert isinstance(resp.json(), list) + + +# ── Update target ──────────────────────────────────────────────────────────── + + +def test_update_target_values(client): + """PUT updates calorie/macro values on a target.""" + try: + resp = create_target(client, calories=2000, protein_g=100.0) + assert resp.status_code == 201, resp.text + tid = resp.json()["id"] + + resp = client.put( + f"/api/targets/{tid}", + json={"calories": 2500, "protein_g": 120.0, "carbs_g": 250.0, "fat_g": 70.0}, + ) + assert resp.status_code == 200 + data = resp.json() + assert data["calories"] == 2500 + assert data["protein_g"] == 120.0 + assert data["carbs_g"] == 250.0 + assert data["fat_g"] == 70.0 + # unchanged + assert data["start_date"] == "2025-01-01" + assert data["end_date"] is None + finally: + close_active_target(client) + + +def test_update_target_date_range(client): + """PUT updates end_date on a target, closing it.""" + try: + resp = create_target(client, start_date="2025-01-01", calories=2000) + assert resp.status_code == 201, resp.text + tid = resp.json()["id"] + + # Close this target by giving it an end_date + resp = client.put( + f"/api/targets/{tid}", + json={"end_date": "2025-06-01"}, + ) + assert resp.status_code == 200, resp.text + assert resp.json()["end_date"] == "2025-06-01" + + # Now there's no active target + assert client.get("/api/targets/current").status_code == 404 + finally: + close_active_target(client) + + +def test_update_end_date_null_when_another_active(client): + """Setting end_date to null on a historical target when another target + is already active is rejected (409).""" + try: + # Create two targets: t1 closed, t2 active + resp1 = create_target(client, start_date="2025-01-01", calories=2000) + assert resp1.status_code == 201, resp1.text + id1 = resp1.json()["id"] + + resp2 = create_target(client, start_date="2025-06-01", calories=2200) + assert resp2.status_code == 201, resp2.text + id2 = resp2.json()["id"] + + # t1 was auto-closed, t2 is active. Try to set t1.end_date = null + resp = client.put(f"/api/targets/{id1}", json={"end_date": None}) + assert resp.status_code == 409 + assert "already an active target" in resp.json()["detail"].lower() + finally: + close_active_target(client) + + +def test_update_does_not_break_invariant(client): + """Updating a historical target's unrelated fields doesn't affect + the active target.""" + try: + # Create two targets + resp1 = create_target(client, start_date="2025-01-01", calories=2000) + assert resp1.status_code == 201, resp1.text + id1 = resp1.json()["id"] + + create_target(client, start_date="2025-06-01", calories=2200) + + # Update t1's calories — should work, active target unchanged + resp = client.put(f"/api/targets/{id1}", json={"calories": 2100}) + assert resp.status_code == 200 + + # Active target is still the second one + current = client.get("/api/targets/current").json() + assert current["start_date"] == "2025-06-01" + finally: + close_active_target(client) + + +def test_update_partial(client): + """PUT with partial data only changes supplied fields.""" + try: + resp = create_target(client, calories=2000, protein_g=100.0, carbs_g=200.0) + assert resp.status_code == 201, resp.text + tid = resp.json()["id"] + + resp = client.put(f"/api/targets/{tid}", json={"calories": 1800}) + assert resp.status_code == 200 + data = resp.json() + assert data["calories"] == 1800 + assert data["protein_g"] == 100.0 # unchanged + assert data["carbs_g"] == 200.0 # unchanged + finally: + close_active_target(client) + + +def test_update_404(client): + resp = client.put("/api/targets/99999", json={"calories": 2000}) + assert resp.status_code == 404 + + +# ── Validation failures (§8.1 rule 11) ─────────────────────────────────────── + + +def test_calories_not_positive(client): + resp = create_target(client, calories=0) + assert_422(resp) + resp = create_target(client, calories=-100) + assert_422(resp) + + +def test_calories_must_be_integer(client): + """Calories must be a positive integer, not a string.""" + resp = create_target(client, calories="not-a-number") + assert_422(resp) + + +def test_macros_non_negative(client): + for field in ("protein_g", "carbs_g", "fat_g"): + resp = create_target(client, **{field: -1.0}) + assert_422(resp) + # zero is allowed for all macros + resp = create_target(client, protein_g=0.0, carbs_g=0.0, fat_g=0.0) + assert resp.status_code == 201, resp.text + close_active_target(client) + + +def test_bad_start_date_format(client): + resp = create_target(client, start_date="01-01-2025") + assert_422(resp) + resp = create_target(client, start_date="not-a-date") + assert_422(resp) + + +def test_end_date_before_start_date_on_create(client): + """TargetCreate has no end_date field — it's auto-managed.""" + pass # end_date is not in TargetCreate + + +def test_end_date_must_be_after_start_date_on_update(client): + """PUT with end_date <= start_date should fail validation.""" + try: + resp = create_target(client, start_date="2025-06-01", calories=2000) + assert resp.status_code == 201, resp.text + tid = resp.json()["id"] + + # end_date = start_date → invalid + resp = client.put(f"/api/targets/{tid}", json={"end_date": "2025-06-01"}) + assert_422(resp) + + # end_date < start_date → invalid + resp = client.put(f"/api/targets/{tid}", json={"end_date": "2025-05-01"}) + assert_422(resp) + finally: + close_active_target(client) + + +def test_start_date_must_be_before_end_date_on_update(client): + """When both start_date and end_date are updated, end_date must still + be after start_date.""" + try: + create_target(client, start_date="2025-06-01", calories=2000) + + # Create a second target to auto-close the first + resp = create_target(client, start_date="2025-07-01", calories=2200) + assert resp.status_code == 201, resp.text + tid = resp.json()["id"] + + # Try to set start_date after end_date + resp = client.put( + f"/api/targets/{tid}", + json={"start_date": "2025-08-01", "end_date": "2025-07-01"}, + ) + assert_422(resp) + finally: + close_active_target(client) + + +def test_missing_start_date(client): + resp = client.post("/api/targets", json={"calories": 2000}) + assert_422(resp) + + +def test_missing_calories(client): + resp = client.post("/api/targets", json={"start_date": "2025-01-01"}) + assert_422(resp) + + +# ── Historical lookup by date (service function) ───────────────────────────── + + +def test_get_target_for_date_returns_correct_target(client): + """Verify that sequencing three targets produces correct half-open + date ranges.""" + try: + # Use unique calorie values to avoid collisions + cals = [8001, 8002, 8003] + create_target(client, start_date="2025-01-01", calories=cals[0]) + create_target(client, start_date="2025-04-01", calories=cals[1]) + create_target(client, start_date="2025-07-01", calories=cals[2]) + + all_targets = client.get("/api/targets").json() + our_targets = sorted( + [t for t in all_targets if t["calories"] in cals], + key=lambda t: t["start_date"], + ) + + assert len(our_targets) == 3 + + # t1: [2025-01-01, 2025-04-01) + assert our_targets[0]["start_date"] == "2025-01-01" + assert our_targets[0]["end_date"] == "2025-04-01" + + # t2: [2025-04-01, 2025-07-01) + assert our_targets[1]["start_date"] == "2025-04-01" + assert our_targets[1]["end_date"] == "2025-07-01" + + # t3: [2025-07-01, ∞) + assert our_targets[2]["start_date"] == "2025-07-01" + assert our_targets[2]["end_date"] is None + finally: + close_active_target(client) + + +def test_get_target_for_date_service_direct(): + """Test the service function directly with explicit date ranges. + This validates the half-open interval logic before TICKET-004 needs it. + + Uses direct DB access for a fully controlled setup — no API dependency. + """ + from services.targets import get_target_for_date + from database import SessionLocal + from models import Target + from sqlalchemy import delete as sa_delete + + db = SessionLocal() + try: + # Clean slate + db.execute(sa_delete(Target)) + db.commit() + + # Create targets with explicit date ranges (half-open) + t1 = Target(start_date=date(2025, 1, 1), end_date=date(2025, 4, 1), calories=2000) + t2 = Target(start_date=date(2025, 4, 1), end_date=date(2025, 7, 1), calories=2200) + t3 = Target(start_date=date(2025, 7, 1), end_date=None, calories=2500) + db.add_all([t1, t2, t3]) + db.commit() + + # Test boundary dates + # Jan 1 → t1 + target = get_target_for_date(db, date(2025, 1, 1)) + assert target is not None and target.id == t1.id + + # Mar 31 → t1 (day before t2 starts, t1 still covers it) + target = get_target_for_date(db, date(2025, 3, 31)) + assert target is not None and target.id == t1.id + + # Apr 1 → t2 (t2's start_date, exclusive end of t1) + target = get_target_for_date(db, date(2025, 4, 1)) + assert target is not None and target.id == t2.id + + # Jun 30 → t2 + target = get_target_for_date(db, date(2025, 6, 30)) + assert target is not None and target.id == t2.id + + # Jul 1 → t3 (active) + target = get_target_for_date(db, date(2025, 7, 1)) + assert target is not None and target.id == t3.id + + # Dec 31, 2030 → t3 (still active) + target = get_target_for_date(db, date(2030, 12, 31)) + assert target is not None and target.id == t3.id + + # Date before first target → None + target = get_target_for_date(db, date(2024, 12, 31)) + assert target is None + + # Clean up + db.execute(sa_delete(Target)) + db.commit() + finally: + db.close() + + +def test_get_target_for_date_none_when_no_targets(): + """get_target_for_date returns None when no targets exist.""" + from services.targets import get_target_for_date + from database import SessionLocal + from models import Target + from sqlalchemy import delete as sa_delete + + db = SessionLocal() + try: + db.execute(sa_delete(Target)) + db.commit() + + target = get_target_for_date(db, date(2025, 6, 15)) + assert target is None + finally: + db.close() + + +# ── Macros nullable on create ──────────────────────────────────────────────── + + +def test_create_target_without_macros(client): + """Creating a target with only calories (no macros) should work.""" + try: + resp = client.post( + "/api/targets", + json={"start_date": "2025-01-01", "calories": 2000}, + ) + assert resp.status_code == 201, resp.text + data = resp.json() + assert data["calories"] == 2000 + assert data["protein_g"] is None + assert data["carbs_g"] is None + assert data["fat_g"] is None + finally: + close_active_target(client)