fix(recipe): measurement_interval_minutes e image_path si possono di nuovo azzerare (#37.3)
recipe_editor.html saveRecipe(): i due campi vengono sempre inviati nel payload (valore o null esplicito), non più omessi quando svuotati. recipe_service.py _header_fields(): per i due campi (unica eccezione concordata al confine dei lotti, righe ~37-60) distingue "non inviato" da "inviato null" con model_fields_set, invece di getattr(...) is not None — altrimenti un null esplicito veniva scartato allo stesso modo di un campo assente e il valore restava bloccato per sempre. Gli altri campi header (name, description, requires_*, allow_manual_input) non toccati. Test backend in test_recipes.py: TestUpdateRecipeClearNullableFields. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -34,13 +34,34 @@ HEADER_FIELDS = (
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# image_path and measurement_interval_minutes are nullable columns where the
|
||||||
|
# caller may deliberately want to clear the value (send null), not merely
|
||||||
|
# leave it untouched by not mentioning the field at all. A plain
|
||||||
|
# "getattr(...) is not None" check cannot tell those two cases apart -
|
||||||
|
# once set, neither field could ever be cleared again (#37.3). The other
|
||||||
|
# header fields keep that simpler check: booleans are always sent by the
|
||||||
|
# client (never null) and clearing name/description to null is not a case
|
||||||
|
# this endpoint needs to support.
|
||||||
|
_NULLABLE_HEADER_FIELDS = ("image_path", "measurement_interval_minutes")
|
||||||
|
|
||||||
|
|
||||||
def _header_fields(data: RecipeUpdate) -> dict:
|
def _header_fields(data: RecipeUpdate) -> dict:
|
||||||
"""The header columns the caller actually asked to change."""
|
"""The header columns the caller actually asked to change.
|
||||||
return {
|
|
||||||
field: getattr(data, field)
|
For image_path/measurement_interval_minutes, "asked to change" includes
|
||||||
for field in HEADER_FIELDS
|
explicitly setting it to null - told apart from "field absent from the
|
||||||
if getattr(data, field) is not None
|
request" via model_fields_set, since RecipeUpdate defaults both to None.
|
||||||
}
|
"""
|
||||||
|
result = {}
|
||||||
|
for field in HEADER_FIELDS:
|
||||||
|
if field in _NULLABLE_HEADER_FIELDS:
|
||||||
|
if field in data.model_fields_set:
|
||||||
|
result[field] = getattr(data, field)
|
||||||
|
else:
|
||||||
|
value = getattr(data, field)
|
||||||
|
if value is not None:
|
||||||
|
result[field] = value
|
||||||
|
return result
|
||||||
|
|
||||||
|
|
||||||
async def _get_recipe_or_404(db: AsyncSession, recipe_id: int) -> Recipe:
|
async def _get_recipe_or_404(db: AsyncSession, recipe_id: int) -> Recipe:
|
||||||
|
|||||||
@@ -250,6 +250,79 @@ class TestUpdateRecipe:
|
|||||||
assert version_numbers == [1, 2]
|
assert version_numbers == [1, 2]
|
||||||
|
|
||||||
|
|
||||||
|
class TestUpdateRecipeClearNullableFields:
|
||||||
|
"""gitea#37.3: measurement_interval_minutes/image_path, once set, could
|
||||||
|
never be cleared again - the request set them and the update endpoint
|
||||||
|
could not tell "not sent" apart from "sent as null" (both landed on the
|
||||||
|
same getattr(...) is not None check in recipe_service._header_fields)."""
|
||||||
|
|
||||||
|
async def test_explicit_null_clears_measurement_interval(
|
||||||
|
self, client: AsyncClient, maker_user: User, db_session: AsyncSession,
|
||||||
|
):
|
||||||
|
recipe = await create_test_recipe(db_session, maker_user.id)
|
||||||
|
|
||||||
|
set_resp = await client.put(
|
||||||
|
f"/api/recipes/{recipe.id}",
|
||||||
|
headers=auth_headers(maker_user),
|
||||||
|
json={"measurement_interval_minutes": 30},
|
||||||
|
)
|
||||||
|
assert set_resp.status_code == 200
|
||||||
|
assert set_resp.json()["measurement_interval_minutes"] == 30
|
||||||
|
|
||||||
|
clear_resp = await client.put(
|
||||||
|
f"/api/recipes/{recipe.id}",
|
||||||
|
headers=auth_headers(maker_user),
|
||||||
|
json={"measurement_interval_minutes": None},
|
||||||
|
)
|
||||||
|
assert clear_resp.status_code == 200
|
||||||
|
assert clear_resp.json()["measurement_interval_minutes"] is None
|
||||||
|
|
||||||
|
async def test_explicit_null_clears_image_path(
|
||||||
|
self, client: AsyncClient, maker_user: User, db_session: AsyncSession,
|
||||||
|
):
|
||||||
|
recipe = await create_test_recipe(db_session, maker_user.id)
|
||||||
|
|
||||||
|
set_resp = await client.put(
|
||||||
|
f"/api/recipes/{recipe.id}",
|
||||||
|
headers=auth_headers(maker_user),
|
||||||
|
json={"image_path": "uploads/1/1/preview.png"},
|
||||||
|
)
|
||||||
|
assert set_resp.status_code == 200
|
||||||
|
assert set_resp.json()["image_path"] == "uploads/1/1/preview.png"
|
||||||
|
|
||||||
|
clear_resp = await client.put(
|
||||||
|
f"/api/recipes/{recipe.id}",
|
||||||
|
headers=auth_headers(maker_user),
|
||||||
|
json={"image_path": None},
|
||||||
|
)
|
||||||
|
assert clear_resp.status_code == 200
|
||||||
|
assert clear_resp.json()["image_path"] is None
|
||||||
|
|
||||||
|
async def test_field_not_sent_leaves_existing_value_untouched(
|
||||||
|
self, client: AsyncClient, maker_user: User, db_session: AsyncSession,
|
||||||
|
):
|
||||||
|
"""Regression guard for the other half of the distinction: a request
|
||||||
|
that never mentions the field must still leave it as it was."""
|
||||||
|
recipe = await create_test_recipe(db_session, maker_user.id)
|
||||||
|
|
||||||
|
set_resp = await client.put(
|
||||||
|
f"/api/recipes/{recipe.id}",
|
||||||
|
headers=auth_headers(maker_user),
|
||||||
|
json={"measurement_interval_minutes": 45},
|
||||||
|
)
|
||||||
|
assert set_resp.status_code == 200
|
||||||
|
assert set_resp.json()["measurement_interval_minutes"] == 45
|
||||||
|
|
||||||
|
unrelated_resp = await client.put(
|
||||||
|
f"/api/recipes/{recipe.id}",
|
||||||
|
headers=auth_headers(maker_user),
|
||||||
|
json={"name": "Renamed Recipe"},
|
||||||
|
)
|
||||||
|
assert unrelated_resp.status_code == 200
|
||||||
|
assert unrelated_resp.json()["measurement_interval_minutes"] == 45
|
||||||
|
assert unrelated_resp.json()["name"] == "Renamed Recipe"
|
||||||
|
|
||||||
|
|
||||||
class TestDeleteRecipe:
|
class TestDeleteRecipe:
|
||||||
"""DELETE /api/recipes/{recipe_id} tests."""
|
"""DELETE /api/recipes/{recipe_id} tests."""
|
||||||
|
|
||||||
|
|||||||
@@ -556,10 +556,13 @@ function recipeEditor() {
|
|||||||
description: this.description.trim(),
|
description: this.description.trim(),
|
||||||
};
|
};
|
||||||
|
|
||||||
// Measurement interval
|
// Measurement interval - always sent, explicit null when cleared
|
||||||
if (this.measurementIntervalMinutes) {
|
// (#37.3): omitting the key when the field was emptied used to leave
|
||||||
payload.measurement_interval_minutes = parseInt(this.measurementIntervalMinutes, 10);
|
// the old value in place forever, because the backend could not tell
|
||||||
}
|
// "not sent" from "cleared".
|
||||||
|
payload.measurement_interval_minutes = this.measurementIntervalMinutes
|
||||||
|
? parseInt(this.measurementIntervalMinutes, 10)
|
||||||
|
: null;
|
||||||
|
|
||||||
// Rules of measurement. Sent every time, including when false: these are the
|
// Rules of measurement. Sent every time, including when false: these are the
|
||||||
// settings an operator gets told about, so turning one off has to travel.
|
// settings an operator gets told about, so turning one off has to travel.
|
||||||
@@ -567,10 +570,8 @@ function recipeEditor() {
|
|||||||
payload.requires_serial = !!this.requiresSerial;
|
payload.requires_serial = !!this.requiresSerial;
|
||||||
payload.allow_manual_input = !!this.allowManualInput;
|
payload.allow_manual_input = !!this.allowManualInput;
|
||||||
|
|
||||||
// Include image_path for preview thumbnail
|
// image_path for preview thumbnail - same null-when-cleared rule (#37.3).
|
||||||
if (this.currentFilePath) {
|
payload.image_path = this.currentFilePath || null;
|
||||||
payload.image_path = this.currentFilePath;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Include change notes if editing
|
// Include change notes if editing
|
||||||
if (!this.isNew && this.changeNotes.trim()) {
|
if (!this.isNew && this.changeNotes.trim()) {
|
||||||
|
|||||||
Reference in New Issue
Block a user