Move inline API request/response models out of main.py into feature modules, Add enums/constants for participant roles

api/meals.py

@@ -136,43 +136,9 @@ async def delete_meal(
     return meal
 
 
-def get_duplicates(items: List[meals.Person]) -> set[str]:
-    seen: set[int] = set()
-    duplicates: set[str] = set()
-    for item in items:
-        if item.id in seen:
-            duplicates.add(item.name)
-        seen.add(item.id)
-    return duplicates
-
-
 def validate_meal(meal: meals.Meal, request: Optional[Request] = None) -> Optional[Request]:
-    if not meal.chefs:
+    """HTTP-friendly wrapper that maps service validation to ProblemDetails."""
-        return error_response(request, 400, "Meal must have at least one chef")
+    msg = meals.validate_meal(meal)
-
+    if msg:
-    if not meal.cleanup:
+        return error_response(request, 400, msg)
-        return error_response(request, 400, "Meal must have at least one cleanup person")
-
-    if not meal.consumers:
-        return error_response(request, 400, "Meal must have at least one consumer")
-
-    if len(meal.recipes) == 0 and len(meal.extra_ingredients) == 0:
-        return error_response(request, 400, "Meal must have at least one recipe or ingredient")
-
-    duplicates = get_duplicates(meal.chefs)
-    if duplicates:
-        return error_response(request, 400, f'Duplicate chef: {", ".join(duplicates)}')
-
-    duplicates = get_duplicates(meal.cleanup)
-    if duplicates:
-        return error_response(request, 400, f'Duplicate cleanup person: {", ".join(duplicates)}')
-
-    duplicates = get_duplicates(meal.consumers)
-    if duplicates:
-        return error_response(request, 400, f'Duplicate consumer: {", ".join(duplicates)}')
-
-    zero_servings = [r for r in meal.recipes if r.servings == 0]
-    if zero_servings:
-        return error_response(request, 400, "Recipe servings must be greater than 0")
-
     return None

api/products.py

@@ -0,0 +1,34 @@
+from __future__ import annotations
+
+from typing import List, Optional
+
+import aiosqlite
+from fastapi import APIRouter, Depends, Response
+from pydantic import Field
+
+import products
+from common import ApiModel
+from api.deps import get_db
+
+
+router = APIRouter(prefix="/products", tags=["products"])
+
+
+class ProductUrl(ApiModel):
+    url: str
+    tags: List[str] = Field(default_factory=list)
+
+
+@router.post(
+    "",
+    operation_id="createProduct",
+    summary="Create or fetch a product from a URL",
+    response_model=products.Product,
+)
+async def create_product(
+    url: ProductUrl, response: Response, conn: aiosqlite.Connection = Depends(get_db)
+) -> Optional[products.Product]:
+    product = await products.get_or_create(conn, url.url, url.tags)
+    if product:
+        response.headers["Location"] = f"/api/v1/products/{product.id}"
+    return product

main.py

@@ -160,25 +160,7 @@ def _extend_openapi_with_problem_responses(app: FastAPI) -> None:
 _extend_openapi_with_problem_responses(app)
 
 
-class ProductUrl(ApiModel):
+from api import products as products_router  # type: ignore
-    url: str
-    tags: List[str] = Field(default_factory=list)
-
-
-@api_v1.post(
-    "/products",
-    operation_id="createProduct",
-    tags=["products"],
-    summary="Create or fetch a product from a URL",
-    response_model=products.Product,
-)
-async def create_product(
-    url: ProductUrl, response: Response, conn: aiosqlite.Connection = Depends(get_db)
-) -> Optional[products.Product]:
-    product = await products.get_or_create(conn, url.url, url.tags)
-    if product:
-        response.headers["Location"] = f"/api/v1/products/{product.id}"
-    return product
 
 
  
@@ -188,7 +170,6 @@ from api import recipes as recipes_router  # type: ignore
 
 
 from api import meals as meals_router  # type: ignore
-from api.meals import validate_meal as _validate_meal, get_duplicates as _get_duplicates
 from api import shopping as shopping_router  # type: ignore
 from api import persons as persons_router  # type: ignore
 from api import auth as auth_router  # type: ignore
@@ -252,6 +233,7 @@ async def request_validation_exc_handler(request: Request, exc: RequestValidatio
 
 # Mount versioned API router
 app.include_router(api_v1, prefix="/api/v1", tags=["v1"]) 
+app.include_router(products_router.router, prefix="/api/v1", tags=["v1"])  # extracted
 app.include_router(recipes_router.router, prefix="/api/v1", tags=["v1"])  # extracted
 app.include_router(meals_router.router, prefix="/api/v1", tags=["v1"])  # extracted
 app.include_router(shopping_router.router, prefix="/api/v1", tags=["v1"])  # extracted
@@ -263,13 +245,7 @@ app.include_router(auth_router.router, prefix="/api/v1", tags=["v1"])  # extract
 async def healthz():
     return {"status": "ok"}
 
-# Backward-compatibility: expose helper functions expected by tests in main
+# Back-compat shims removed; tests should import helpers from feature modules
-def get_duplicates(items):
-    return _get_duplicates(items)
-
-
-def validate_meal(meal, request: Request | None = None):
-    return _validate_meal(meal, request)
 
 
 if settings.prod:

meals/__init__.py

@@ -20,3 +20,5 @@ from meals.db import (
     update_meal as update_meal,
 )
 from persons import Person as Person
+from meals.service import get_duplicates as get_duplicates, validate_meal as validate_meal
+from meals.roles import ROLE_CHEF as ROLE_CHEF, ROLE_CLEANUP as ROLE_CLEANUP, ROLE_CONSUMER as ROLE_CONSUMER

meals/db.py

@@ -6,6 +6,7 @@ from common import ApiModel
 
 import persons
 from persons import get_by_ids as persons_get_by_ids
+from .roles import ROLE_CHEF, ROLE_CLEANUP, ROLE_CONSUMER
 from ingredients import (
     Ingredient,
     delete_ingredients_by_meal_id,
@@ -132,9 +133,9 @@ async def insert_meal(conn, meal: Meal):
     ) as cursor:
         meal.id = cursor.lastrowid
 
-    await sync_meal_participants(conn, meal.id, meal.chefs, "chef")
+    await sync_meal_participants(conn, meal.id, meal.chefs, ROLE_CHEF)
-    await sync_meal_participants(conn, meal.id, meal.cleanup, "cleanup")
+    await sync_meal_participants(conn, meal.id, meal.cleanup, ROLE_CLEANUP)
-    await sync_meal_participants(conn, meal.id, meal.consumers, "consumer")
+    await sync_meal_participants(conn, meal.id, meal.consumers, ROLE_CONSUMER)
 
     for meal_recipe in meal.recipes:
         meal_recipe.meal_id = meal.id
@@ -199,13 +200,13 @@ async def load_participants(conn, meal: Meal) -> None:
 
     for pid, role in links:
         person = people.get(pid)
-        if role == "chef":
+        if role == ROLE_CHEF:
             if person:
                 meal.chefs.append(person)
-        elif role == "cleanup":
+        elif role == ROLE_CLEANUP:
             if person:
                 meal.cleanup.append(person)
-        elif role == "consumer":
+        elif role == ROLE_CONSUMER:
             if person:
                 meal.consumers.append(person)
         else:
@@ -260,11 +261,11 @@ async def bulk_load_participants(conn, meals: List[Meal]) -> None:
             person = people.get(pid)
             if not person:
                 continue
-            if role == "chef":
+            if role == ROLE_CHEF:
                 meal.chefs.append(person)
-            elif role == "cleanup":
+            elif role == ROLE_CLEANUP:
                 meal.cleanup.append(person)
-            elif role == "consumer":
+            elif role == ROLE_CONSUMER:
                 meal.consumers.append(person)
             else:
                 raise Exception(f"Unknown role: {role}")
@@ -342,9 +343,9 @@ async def update_meal(conn, meal: Meal) -> None:
         (meal.suggested_date.isoformat(), meal.id),
     )
 
-    await sync_meal_participants(conn, meal.id, meal.chefs, "chef")
+    await sync_meal_participants(conn, meal.id, meal.chefs, ROLE_CHEF)
-    await sync_meal_participants(conn, meal.id, meal.cleanup, "cleanup")
+    await sync_meal_participants(conn, meal.id, meal.cleanup, ROLE_CLEANUP)
-    await sync_meal_participants(conn, meal.id, meal.consumers, "consumer")
+    await sync_meal_participants(conn, meal.id, meal.consumers, ROLE_CONSUMER)
 
     await sync_extra_ingredients(conn, meal.id, meal.extra_ingredients)
     await sync_meal_recipes(conn, meal.id, meal.recipes)

meals/roles.py

@@ -0,0 +1,6 @@
+from __future__ import annotations
+
+# Centralized participant role constants to avoid string duplication/typos
+ROLE_CHEF = "chef"
+ROLE_CLEANUP = "cleanup"
+ROLE_CONSUMER = "consumer"

meals/service.py

@@ -0,0 +1,54 @@
+from __future__ import annotations
+
+from typing import List, Set
+
+from persons import Person
+from meals.db import Meal
+
+
+def get_duplicates(items: List[Person]) -> Set[str]:
+    """Return the set of duplicate person names based on repeated ids."""
+    seen: set[int] = set()
+    duplicates: set[str] = set()
+    for item in items:
+        if item.id in seen:
+            duplicates.add(item.name)
+        seen.add(item.id)
+    return duplicates
+
+
+def validate_meal(meal: Meal) -> str | None:
+    """Validate a Meal domain model.
+
+    Returns:
+        None if valid, otherwise a human-readable error message.
+    """
+    if not meal.chefs:
+        return "Meal must have at least one chef"
+
+    if not meal.cleanup:
+        return "Meal must have at least one cleanup person"
+
+    if not meal.consumers:
+        return "Meal must have at least one consumer"
+
+    if len(meal.recipes) == 0 and len(meal.extra_ingredients) == 0:
+        return "Meal must have at least one recipe or ingredient"
+
+    duplicates = get_duplicates(meal.chefs)
+    if duplicates:
+        return f"Duplicate chef: {', '.join(duplicates)}"
+
+    duplicates = get_duplicates(meal.cleanup)
+    if duplicates:
+        return f"Duplicate cleanup person: {', '.join(duplicates)}"
+
+    duplicates = get_duplicates(meal.consumers)
+    if duplicates:
+        return f"Duplicate consumer: {', '.join(duplicates)}"
+
+    zero_servings = [r for r in meal.recipes if r.servings == 0]
+    if zero_servings:
+        return "Recipe servings must be greater than 0"
+
+    return None

refactor-project-strategy.md

@@ -105,13 +105,13 @@ Notes
 ---
 
 ## Phase 4 — Modeling and validation
-- [ ] Move inline API request/response models out of main.py into feature modules
+- [X] Move inline API request/response models out of main.py into feature modules
-  - [ ] ProductUrl
+  - [x] ProductUrl (moved to api/products.py)
-  - [ ] CurrentShoppingList
+  - [x] CurrentShoppingList (in api/shopping.py)
-  - [ ] PurchasedShoppingList
+  - [x] PurchasedShoppingList (in api/shopping.py)
-  - [ ] LoginBody
+  - [x] LoginBody (in api/auth.py)
-- [ ] Extract validation (e.g., validate_meal) into a service layer for reuse
+- [x] Extract validation (e.g., validate_meal) into a service layer for reuse
-- [ ] Add enums/constants for participant roles
+- [x] Add enums/constants for participant roles
 
 Acceptance criteria
 - Cleaner main.py; feature modules own their request/response contracts
@@ -178,6 +178,9 @@ Note: We can adopt this structure gradually without moving DB code immediately;
 - 2025-10-19: Fixed SQLite error during test setup by creating the `MealRecipe` table before indexing it; corrected `update_meal` to call `get_meal` with explicit `(request, conn)` avoiding a Depends object leak. Full test suite now passes (100%). Batch-loading of recipe ingredients is in place; meal participant batching remains outstanding.
 -. 
 - 2025-10-19: Implemented participant batch-loading (`meals.bulk_load_participants`) and updated `api/meals.get_upcoming_meals` to use it; re-ran the test suite (green). Phase 3 marked complete; Phase 4-5 next.
+  - 2025-10-19: Phase 4 (partial) — Extracted ProductUrl and product creation endpoint to `api/products.py`; wired new router; extracted `get_duplicates` to `meals/service.py` and re-exported via `meals.__init__`; tests still green.
+  - 2025-10-19: Phase 4 — Extracted `validate_meal` into `meals/service.py`, re-exported via `meals.__init__`, and updated `api/meals.validate_meal` wrapper to map to ProblemDetails. Full test suite remains green.
+  - 2025-10-19: Phase 4 — Added centralized role constants in `meals/roles.py` and replaced string literals in `meals/db.py`; re-exported constants via `meals.__init__`. Removed back-compat shims from `main.py` and updated tests to import helpers directly from modules. Tests remain green.
 
 ---
 

tests/test_main.py

@@ -15,6 +15,8 @@ def reload_test_data():
 
 from db import connect, create
 import main
+from meals import get_duplicates
+from api.meals import validate_meal
 import meals
 import meals.db as meals_db
 from meals.db import Meal, MealRecipe
@@ -412,7 +414,7 @@ class TestMainHelperFunctions(unittest.TestCase):
             persons.Person(id=2, name="Ryan"),
             persons.Person(id=3, name="Ellie"),
         ]
-        duplicates = main.get_duplicates(persons_list)
+        duplicates = get_duplicates(persons_list)
         self.assertEqual(len(duplicates), 0)
 
     def test_get_duplicates_with_duplicates(self):
@@ -423,7 +425,7 @@ class TestMainHelperFunctions(unittest.TestCase):
             persons.Person(id=1, name="Jacob"),  # Duplicate
             persons.Person(id=3, name="Ellie"),
         ]
-        duplicates = main.get_duplicates(persons_list)
+        duplicates = get_duplicates(persons_list)
         self.assertEqual(len(duplicates), 1)
         self.assertIn("Jacob", duplicates)
 
@@ -438,7 +440,7 @@ class TestMainHelperFunctions(unittest.TestCase):
             recipes=[MealRecipe(meal_id=1, recipe_id=1, servings=2.0)],
             extra_ingredients=[],
         )
-        result = main.validate_meal(meal)
+        result = validate_meal(meal)
         self.assertIsNone(result)
 
     def test_validate_meal_no_chefs(self):
@@ -452,7 +454,7 @@ class TestMainHelperFunctions(unittest.TestCase):
             recipes=[MealRecipe(meal_id=1, recipe_id=1, servings=2.0)],
             extra_ingredients=[],
         )
-        result = main.validate_meal(meal)
+        result = validate_meal(meal)
         self.assertIsNotNone(result)
         self.assertEqual(result.status_code, 400)
 
@@ -467,7 +469,7 @@ class TestMainHelperFunctions(unittest.TestCase):
             recipes=[MealRecipe(meal_id=1, recipe_id=1, servings=2.0)],
             extra_ingredients=[],
         )
-        result = main.validate_meal(meal)
+        result = validate_meal(meal)
         self.assertIsNotNone(result)
         self.assertEqual(result.status_code, 400)
 
@@ -482,7 +484,7 @@ class TestMainHelperFunctions(unittest.TestCase):
             recipes=[MealRecipe(meal_id=1, recipe_id=1, servings=2.0)],
             extra_ingredients=[],
         )
-        result = main.validate_meal(meal)
+        result = validate_meal(meal)
         self.assertIsNotNone(result)
         self.assertEqual(result.status_code, 400)
 
@@ -497,7 +499,7 @@ class TestMainHelperFunctions(unittest.TestCase):
             recipes=[],
             extra_ingredients=[],
         )
-        result = main.validate_meal(meal)
+    result = validate_meal(meal)
         self.assertIsNotNone(result)
         self.assertEqual(result.status_code, 400)
 
@@ -515,7 +517,7 @@ class TestMainHelperFunctions(unittest.TestCase):
             recipes=[MealRecipe(meal_id=1, recipe_id=1, servings=2.0)],
             extra_ingredients=[],
         )
-        result = main.validate_meal(meal)
+        result = validate_meal(meal)
         self.assertIsNotNone(result)
         self.assertEqual(result.status_code, 400)
 
@@ -530,7 +532,7 @@ class TestMainHelperFunctions(unittest.TestCase):
             recipes=[MealRecipe(meal_id=1, recipe_id=1, servings=0)],  # Zero servings
             extra_ingredients=[],
         )
-        result = main.validate_meal(meal)
+        result = validate_meal(meal)
         self.assertIsNotNone(result)
         self.assertEqual(result.status_code, 400)