-
-
Notifications
You must be signed in to change notification settings - Fork 910
fix: Add missing group_id to RecipeTag and TagBase schemas #5342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Add missing group_id to RecipeTag and TagBase schemas #5342
Conversation
Full disclosure: I diagnosed and fixed this using AI. The fix works in my environment, but it's been years since I've slung code and I don't know this project well enough to confirm whether or not there are other things that might need to be changed along with this fix. |
looking into the test failures |
6486e46
to
1f4bd7e
Compare
My first try at this broke tests. I made the fix for that, then squashed that fix into the original fix. This is now passing the python tests locally. |
1f4bd7e
to
b258447
Compare
Adds the optional group_id field to RecipeTag and CategoryBase Pydantic schemas to resolve API validation errors (TypeError/IntegrityError) when updating recipe tags/categories via PATCH or bulk actions. This ensures group_id provided in payloads is not stripped during validation. Also adds the required group_id field to the RecipeToolOut schema. This ensures group_id is present when validating Recipe objects during creation in tests, resolving Pydantic validation errors and downstream IntegrityErrors that occurred during test setup fixtures.
b258447
to
7f76bcd
Compare
fixed the formatting issue from the last test run. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
…cipes#5342) Co-authored-by: Robert Dana <[email protected]> Co-authored-by: Michael Genson <[email protected]>
Problem:
When updating recipe tags via the API (
PATCH /api/recipes/{recipe_slug}
or using bulk actions like/api/recipes/bulk-actions/tag
), requests can fail with either aTypeError: __init__() missing 1 required positional argument: 'group_id'
or aSQL Integrity Error
(violating the unique constraint ontags.slug
+tags.group_id
).Root Cause:
There is an inconsistency between the Pydantic schemas used for API validation and the underlying SQLAlchemy database model for Tags:
Tag
database model (mealie/db/models/recipe/tag.py
) correctly requiresgroup_id
during initialization (__init__
).RecipeTag
(mealie/schema/recipe/recipe.py
) andTagBase
(mealie/schema/recipe/recipe_category.py
), which are used to validate thetags
array in API request payloads, do not define agroup_id
field.group_id
within the tag objects is sent, Pydantic strips this field during validation.group_id
, leading to errors when attempting to interact with theTag
database model.Solution:
This PR adds the missing
group_id: UUID4 | None = None
field to theRecipeTag
andCategoryBase
(whichTagBase
inherits from) Pydantic schemas.mealie/schema/recipe/recipe.py
(modifiedRecipeTag
)mealie/schema/recipe/recipe_category.py
(modifiedCategoryBase
)This ensures that
group_id
, if provided in an API request for associating tags, is correctly validated and passed through to the backend database logic, resolving theTypeError
andSQL Integrity Error
. The field is made optional (| None = None
) to minimize potential side effects in other parts of the code that might use these schemas without providing a group ID.However, this change broke tests. Fixing it required adding the required group_id field to the RecipeToolOut schema. This ensures group_id is present when validating Recipe objects during creation in tests, resolving Pydantic validation errors and downstream IntegrityErrors that occurred during test setup fixtures.
Testing:
Tested locally by applying these schema changes and using an external script to PATCH recipes with tags including
id
,name
,slug
, andgroup_id
. The previousTypeError
andSQL Integrity Error
were resolved, and tags were associated successfully.Ran the python test suite locally, all tests are now passing.
Fixes #5286