feat(recipes): include createdBy (MemberRef) and createdById in v2 responses; map from User; update tests and spec
This commit is contained in:
parent
06b81f0b2a
commit
b324e1101c
4 changed files with 114 additions and 11 deletions
|
|
@ -1,14 +1,15 @@
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from typing import List, Optional
|
from typing import Dict, List, Optional
|
||||||
|
|
||||||
import aiosqlite
|
import aiosqlite
|
||||||
from fastapi import APIRouter, Depends, Query, Response
|
from fastapi import APIRouter, Depends, Query, Response
|
||||||
|
|
||||||
import ingredients as ingredients_mod
|
import ingredients as ingredients_mod
|
||||||
import recipes
|
import recipes
|
||||||
from api.deps import error_response, get_db, get_household_from_slug
|
from api.deps import error_response, get_db, get_household_from_slug, get_current_user
|
||||||
from common import Page, ProblemDetails, ApiModel, Field
|
from common import Page, ProblemDetails, ApiModel, Field
|
||||||
|
from api.dtos import MemberRef
|
||||||
|
|
||||||
router = APIRouter(prefix="/households/{householdSlug}/recipes", tags=["recipes"])
|
router = APIRouter(prefix="/households/{householdSlug}/recipes", tags=["recipes"])
|
||||||
|
|
||||||
|
|
@ -22,6 +23,8 @@ class RecipeOut(ApiModel):
|
||||||
ingredients: List[ingredients_mod.Ingredient] = Field(
|
ingredients: List[ingredients_mod.Ingredient] = Field(
|
||||||
min_length=0, json_schema_extra={"minItems": 0}
|
min_length=0, json_schema_extra={"minItems": 0}
|
||||||
)
|
)
|
||||||
|
created_by_id: int
|
||||||
|
created_by: Optional[MemberRef] = None
|
||||||
|
|
||||||
|
|
||||||
class RecipeCreate(ApiModel):
|
class RecipeCreate(ApiModel):
|
||||||
|
|
@ -67,11 +70,40 @@ async def list_recipes(
|
||||||
by_recipe = await ingredients_mod.find_ingredients_by_recipe_ids(conn, recipe_ids)
|
by_recipe = await ingredients_mod.find_ingredients_by_recipe_ids(conn, recipe_ids)
|
||||||
for r in items:
|
for r in items:
|
||||||
r.ingredients = by_recipe.get(r.id, [])
|
r.ingredients = by_recipe.get(r.id, [])
|
||||||
|
# Lookup creators' display names
|
||||||
|
creator_ids = {r.created_by_id for r in items if getattr(r, "created_by_id", None) is not None}
|
||||||
|
creator_lookup: Dict[int, str] = {}
|
||||||
|
if creator_ids:
|
||||||
|
placeholders = ",".join(["?"] * len(creator_ids))
|
||||||
|
async with conn.execute(
|
||||||
|
f"SELECT id, display_name FROM User WHERE id IN ({placeholders})",
|
||||||
|
list(creator_ids),
|
||||||
|
) as c:
|
||||||
|
async for row in c:
|
||||||
|
creator_lookup[int(row[0])] = row[1]
|
||||||
|
|
||||||
|
def to_recipe_out(r: recipes.Recipe) -> RecipeOut:
|
||||||
|
mref = None
|
||||||
|
name = creator_lookup.get(r.created_by_id)
|
||||||
|
if name is not None:
|
||||||
|
mref = MemberRef(id=r.created_by_id, display_name=name)
|
||||||
|
return RecipeOut(
|
||||||
|
id=r.id,
|
||||||
|
name=r.name,
|
||||||
|
link=r.link,
|
||||||
|
serves=r.serves,
|
||||||
|
image_urls=r.image_urls,
|
||||||
|
ingredients=r.ingredients,
|
||||||
|
created_by_id=r.created_by_id,
|
||||||
|
created_by=mref,
|
||||||
|
)
|
||||||
|
|
||||||
next_cursor = str(items[-1].id) if has_more and items else None
|
next_cursor = str(items[-1].id) if has_more and items else None
|
||||||
total = await (
|
total = await (
|
||||||
recipes.count_by_name_scoped(conn, q, hid) if q else recipes.count_all_scoped(conn, hid)
|
recipes.count_by_name_scoped(conn, q, hid) if q else recipes.count_all_scoped(conn, hid)
|
||||||
)
|
)
|
||||||
return Page(items=items, nextCursor=next_cursor, prevCursor=None, total=total)
|
outward_items = [to_recipe_out(r) for r in items]
|
||||||
|
return Page(items=outward_items, nextCursor=next_cursor, prevCursor=None, total=total)
|
||||||
|
|
||||||
|
|
||||||
@router.get("/{recipe_id}", response_model=RecipeOut, responses={404: {"model": ProblemDetails}})
|
@router.get("/{recipe_id}", response_model=RecipeOut, responses={404: {"model": ProblemDetails}})
|
||||||
|
|
@ -83,7 +115,24 @@ async def get_recipe(
|
||||||
r = await recipes.find_recipe_by_id_scoped(conn, recipe_id, household["id"])
|
r = await recipes.find_recipe_by_id_scoped(conn, recipe_id, household["id"])
|
||||||
if not r:
|
if not r:
|
||||||
return error_response(None, 404, "Recipe not found")
|
return error_response(None, 404, "Recipe not found")
|
||||||
return r
|
# load creator display name
|
||||||
|
mref = None
|
||||||
|
async with conn.execute(
|
||||||
|
"SELECT display_name FROM User WHERE id = ? LIMIT 1", (r.created_by_id,)
|
||||||
|
) as c:
|
||||||
|
row = await c.fetchone()
|
||||||
|
if row:
|
||||||
|
mref = MemberRef(id=r.created_by_id, display_name=row[0])
|
||||||
|
return RecipeOut(
|
||||||
|
id=r.id,
|
||||||
|
name=r.name,
|
||||||
|
link=r.link,
|
||||||
|
serves=r.serves,
|
||||||
|
image_urls=r.image_urls,
|
||||||
|
ingredients=r.ingredients,
|
||||||
|
created_by_id=r.created_by_id,
|
||||||
|
created_by=mref,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@router.delete("/{recipe_id}", response_model=RecipeOut, responses={404: {"model": ProblemDetails}})
|
@router.delete("/{recipe_id}", response_model=RecipeOut, responses={404: {"model": ProblemDetails}})
|
||||||
|
|
@ -102,7 +151,24 @@ async def delete_recipe(
|
||||||
ok = await hide_recipe_scoped(conn, recipe_id, household["id"])
|
ok = await hide_recipe_scoped(conn, recipe_id, household["id"])
|
||||||
if not ok:
|
if not ok:
|
||||||
return error_response(None, 404, "Recipe not found")
|
return error_response(None, 404, "Recipe not found")
|
||||||
return r
|
# Best-effort creator lookup
|
||||||
|
mref = None
|
||||||
|
async with conn.execute(
|
||||||
|
"SELECT display_name FROM User WHERE id = ? LIMIT 1", (r.created_by_id,)
|
||||||
|
) as c:
|
||||||
|
row = await c.fetchone()
|
||||||
|
if row:
|
||||||
|
mref = MemberRef(id=r.created_by_id, display_name=row[0])
|
||||||
|
return RecipeOut(
|
||||||
|
id=r.id,
|
||||||
|
name=r.name,
|
||||||
|
link=r.link,
|
||||||
|
serves=r.serves,
|
||||||
|
image_urls=r.image_urls,
|
||||||
|
ingredients=r.ingredients,
|
||||||
|
created_by_id=r.created_by_id,
|
||||||
|
created_by=mref,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
@router.post("", response_model=RecipeOut, responses={400: {"model": ProblemDetails}})
|
@router.post("", response_model=RecipeOut, responses={400: {"model": ProblemDetails}})
|
||||||
|
|
@ -110,12 +176,13 @@ async def create_recipe(
|
||||||
recipe: RecipeCreate,
|
recipe: RecipeCreate,
|
||||||
response: Response,
|
response: Response,
|
||||||
household=Depends(get_household_from_slug),
|
household=Depends(get_household_from_slug),
|
||||||
|
user=Depends(get_current_user),
|
||||||
conn: aiosqlite.Connection = Depends(get_db),
|
conn: aiosqlite.Connection = Depends(get_db),
|
||||||
):
|
):
|
||||||
if not recipe.ingredients:
|
if not recipe.ingredients:
|
||||||
return error_response(None, 400, "Recipe must have at least one ingredient")
|
return error_response(None, 400, "Recipe must have at least one ingredient")
|
||||||
hid = household["id"]
|
hid = household["id"]
|
||||||
# v1 Recipe model requires created_by_id; use 0 placeholder until users replace persons
|
# Set creator to the current user
|
||||||
r = recipes.Recipe(
|
r = recipes.Recipe(
|
||||||
id=-1,
|
id=-1,
|
||||||
name=recipe.name,
|
name=recipe.name,
|
||||||
|
|
@ -123,7 +190,7 @@ async def create_recipe(
|
||||||
serves=recipe.serves,
|
serves=recipe.serves,
|
||||||
image_urls=recipe.image_urls,
|
image_urls=recipe.image_urls,
|
||||||
ingredients=list(recipe.ingredients),
|
ingredients=list(recipe.ingredients),
|
||||||
created_by_id=0,
|
created_by_id=user.id,
|
||||||
)
|
)
|
||||||
await recipes.insert_recipe_scoped(conn, r, hid)
|
await recipes.insert_recipe_scoped(conn, r, hid)
|
||||||
for ingredient in recipe.ingredients:
|
for ingredient in recipe.ingredients:
|
||||||
|
|
@ -132,4 +199,14 @@ async def create_recipe(
|
||||||
ingredient.product_id = ingredient.product.id
|
ingredient.product_id = ingredient.product.id
|
||||||
await ingredients_mod.insert_ingredient(conn, ingredient)
|
await ingredients_mod.insert_ingredient(conn, ingredient)
|
||||||
response.headers["Location"] = f"/api/v1/households/{household['slug']}/recipes/{r.id}"
|
response.headers["Location"] = f"/api/v1/households/{household['slug']}/recipes/{r.id}"
|
||||||
return r
|
created = MemberRef(id=user.id, display_name=user.display_name)
|
||||||
|
return RecipeOut(
|
||||||
|
id=r.id,
|
||||||
|
name=r.name,
|
||||||
|
link=r.link,
|
||||||
|
serves=r.serves,
|
||||||
|
image_urls=r.image_urls,
|
||||||
|
ingredients=r.ingredients,
|
||||||
|
created_by_id=r.created_by_id,
|
||||||
|
created_by=created,
|
||||||
|
)
|
||||||
|
|
|
||||||
|
|
@ -278,14 +278,14 @@ Status summary:
|
||||||
- New: Household members listing `GET /api/v1/households/{householdSlug}/members` returning [{ id, displayName, role }].
|
- New: Household members listing `GET /api/v1/households/{householdSlug}/members` returning [{ id, displayName, role }].
|
||||||
- Preserved: camelCase responses, `Page<T>` semantics, `Location` headers on create, shopping storeName normalization ("home").
|
- Preserved: camelCase responses, `Page<T>` semantics, `Location` headers on create, shopping storeName normalization ("home").
|
||||||
- Codebase cleanup: v2 routers are inlined as canonical modules; legacy `*_v2.py` files removed. v1 cookie auth and routers are not mounted.
|
- Codebase cleanup: v2 routers are inlined as canonical modules; legacy `*_v2.py` files removed. v1 cookie auth and routers are not mounted.
|
||||||
- Current v2 recipes shape: `createdBy`/`hiddenBy` are currently omitted from outward Recipe responses; they will be reintroduced (as MemberRef) once persons are fully retired.
|
- Current v2 recipes shape: `createdById` and `createdBy` (MemberRef) are included in outward Recipe responses. `hiddenBy` will be added when hide flows are wired to users.
|
||||||
|
|
||||||
Remaining work (prioritized cleanup to final state):
|
Remaining work (prioritized cleanup to final state):
|
||||||
1. Remove the `persons/` package and all code references across domains (recipes, meals, shopping). Replace `Person` with `users`/HouseholdMember everywhere:
|
1. Remove the `persons/` package and all code references across domains (recipes, meals, shopping). Replace `Person` with `users`/HouseholdMember everywhere:
|
||||||
- Code hotspots today: `recipes/models.py` (imports Person), `recipes/repository.py` (FKs, hide_recipe signature), `meals/models.py` (participants as List[Person]), `api/shopping_models.py` and `shopping/models.py` (ShoppingList.purchased_by typed as Person), and `shopping/repository.py` (FKs to Person).
|
- Code hotspots today: `recipes/models.py` (imports Person), `recipes/repository.py` (FKs, hide_recipe signature), `meals/models.py` (participants as List[Person]), `api/shopping_models.py` and `shopping/models.py` (ShoppingList.purchased_by typed as Person), and `shopping/repository.py` (FKs to Person).
|
||||||
- Replace internal usages with `user_id`/`MemberRef` where outward, and update repository DDL to stop referencing `Person`.
|
- Replace internal usages with `user_id`/`MemberRef` where outward, and update repository DDL to stop referencing `Person`.
|
||||||
- Remove `api.deps.cookie_person` once no tests or code depend on it.
|
- Remove `api.deps.cookie_person` once no tests or code depend on it.
|
||||||
2. Recipes outward schema: add `createdBy`/`hiddenBy` back as `MemberRef`-style DTOs (user/member) so the API exposes user info without `Person`. Populate from the authenticated user and (when hiding) the acting member; keep fields optional.
|
2. Recipes outward schema: DONE for `createdById`/`createdBy` (MemberRef). Remaining: add `hiddenBy` (MemberRef) when hide flows are wired to users.
|
||||||
3. Shopping outward DTOs: change `ShoppingListOut.purchasedBy` from `Person` to `MemberRef` and adjust mapping code in `api/shopping_models.py`; ensure OpenAPI reflects the new shape. Consider exposing only `purchasedById` initially if membership lookup isn’t available.
|
3. Shopping outward DTOs: change `ShoppingListOut.purchasedBy` from `Person` to `MemberRef` and adjust mapping code in `api/shopping_models.py`; ensure OpenAPI reflects the new shape. Consider exposing only `purchasedById` initially if membership lookup isn’t available.
|
||||||
4. Delete or port legacy v1 test modules that are currently skipped (`tests/test_main.py`, `tests/test_v1.py`). Either migrate assertions to v2 routes or remove them so the full test run has no skips. Then remove the last vestiges of v1-only helpers.
|
4. Delete or port legacy v1 test modules that are currently skipped (`tests/test_main.py`, `tests/test_v1.py`). Either migrate assertions to v2 routes or remove them so the full test run has no skips. Then remove the last vestiges of v1-only helpers.
|
||||||
5. Database polish:
|
5. Database polish:
|
||||||
|
|
|
||||||
17
openapi.json
17
openapi.json
|
|
@ -2615,6 +2615,20 @@
|
||||||
"type": "array",
|
"type": "array",
|
||||||
"minItems": 0,
|
"minItems": 0,
|
||||||
"title": "Ingredients"
|
"title": "Ingredients"
|
||||||
|
},
|
||||||
|
"createdById": {
|
||||||
|
"type": "integer",
|
||||||
|
"title": "Createdbyid"
|
||||||
|
},
|
||||||
|
"createdBy": {
|
||||||
|
"anyOf": [
|
||||||
|
{
|
||||||
|
"$ref": "#/components/schemas/MemberRef"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"type": "null"
|
||||||
|
}
|
||||||
|
]
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"type": "object",
|
"type": "object",
|
||||||
|
|
@ -2623,7 +2637,8 @@
|
||||||
"link",
|
"link",
|
||||||
"serves",
|
"serves",
|
||||||
"imageUrls",
|
"imageUrls",
|
||||||
"ingredients"
|
"ingredients",
|
||||||
|
"createdById"
|
||||||
],
|
],
|
||||||
"title": "RecipeOut"
|
"title": "RecipeOut"
|
||||||
},
|
},
|
||||||
|
|
|
||||||
|
|
@ -66,12 +66,21 @@ class TestRecipesHouseholdV2(unittest.IsolatedAsyncioTestCase):
|
||||||
)
|
)
|
||||||
assert r.status_code == 200, r.text
|
assert r.status_code == 200, r.text
|
||||||
rid = r.json()["id"]
|
rid = r.json()["id"]
|
||||||
|
# createdBy fields present and match current user
|
||||||
|
created_by = r.json().get("createdBy")
|
||||||
|
assert created_by is not None
|
||||||
|
assert isinstance(created_by.get("id"), int)
|
||||||
|
assert created_by.get("displayName") == "R"
|
||||||
|
assert r.json().get("createdById") == created_by["id"]
|
||||||
|
|
||||||
# List H1 should include
|
# List H1 should include
|
||||||
r = self.client.get(f"/api/v1/households/{self.h1}/recipes", headers=self.headers)
|
r = self.client.get(f"/api/v1/households/{self.h1}/recipes", headers=self.headers)
|
||||||
assert r.status_code == 200
|
assert r.status_code == 200
|
||||||
items = r.json()["items"]
|
items = r.json()["items"]
|
||||||
assert any(it["id"] == rid for it in items)
|
assert any(it["id"] == rid for it in items)
|
||||||
|
# list items include createdBy
|
||||||
|
found = next(it for it in items if it["id"] == rid)
|
||||||
|
assert "createdBy" in found and "createdById" in found
|
||||||
|
|
||||||
# List H2 should not include
|
# List H2 should not include
|
||||||
r = self.client.get(f"/api/v1/households/{self.h2}/recipes", headers=self.headers)
|
r = self.client.get(f"/api/v1/households/{self.h2}/recipes", headers=self.headers)
|
||||||
|
|
@ -108,6 +117,8 @@ class TestRecipesHouseholdV2(unittest.IsolatedAsyncioTestCase):
|
||||||
)
|
)
|
||||||
assert r.status_code == 200, r.text
|
assert r.status_code == 200, r.text
|
||||||
rid = r.json()["id"]
|
rid = r.json()["id"]
|
||||||
|
# createdBy present
|
||||||
|
assert "createdBy" in r.json() and "createdById" in r.json()
|
||||||
|
|
||||||
# Delete it via v2 scoped route
|
# Delete it via v2 scoped route
|
||||||
r2 = self.client.delete(f"/api/v1/households/{self.h1}/recipes/{rid}", headers=self.headers)
|
r2 = self.client.delete(f"/api/v1/households/{self.h1}/recipes/{rid}", headers=self.headers)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue