#4 - api/routers/tasks.py: update_task, delete_task, create_subtask, update_subtask, delete_subtask controllavano solo is_current, non version_has_measurements: la versione con misure veniva modificata sul posto, cambiando i limiti di quote gia' giudicate pass/fail. Nuovi helper _task_for_write / _subtask_for_write riusano recipe_service.create_new_version (lo stesso copy-on-write di create_task): se la versione ha misure creano una nuova versione, ritrovano il task per order_index e la quota per marker_number (chiavi che _copy_tasks_to_version preserva), e la scrittura va sulla copia. La versione misurata resta intatta. Trovato e corretto nello stesso punto: il task ritrovato nella copia non ha .version precaricato (new_version.tasks viene da selectinload, che non popola il backref figlio->genitore), e TaskResponse.recipe_id legge task.version.recipe_id - senza assegnare esplicitamente candidate.version = new_version, update_task va in MissingGreenlet (lazy load fuori da un await) invece di rispondere 200. #26 - models/api/task.py: nuovo validate_tolerance_order (ltl<=lwl<=nominal<= uwl<=utl sui soli campi valorizzati), usato come model_validator su SubtaskCreate e richiamato dal router in update_subtask sui valori risultanti (esistenti + parziali) prima del flush, perche' SubtaskUpdate non ha un insieme fisso di campi da validare da solo. Test: src/backend/tests/test_tasks.py, TestCopyOnWriteOnEdit (5) e TestToleranceOrder (4); adattato test_update_subtask esistente (nominal 12.0 violava il nuovo ordine contro utl=10.5 invariato). PROVA: uv run --extra server --extra dev pytest src/backend/tests/test_tasks.py -q -> 16 passed Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -17,6 +17,7 @@ from src.backend.models.api.task import (
|
||||
TaskReorderRequest,
|
||||
TaskResponse,
|
||||
TaskUpdate,
|
||||
validate_tolerance_order,
|
||||
)
|
||||
from src.backend.services import recipe_service
|
||||
|
||||
@@ -99,6 +100,64 @@ async def _ensure_version_is_current(db: AsyncSession, version_id: int) -> Recip
|
||||
return version
|
||||
|
||||
|
||||
async def _task_for_write(db: AsyncSession, task: RecipeTask, user: User) -> RecipeTask:
|
||||
"""Return the task to write to: the same one, or its copy in a new version.
|
||||
|
||||
A version that already has measurements cannot be edited on the spot - its
|
||||
recorded pass/fail would silently start meaning something else. Copy-on-write
|
||||
(the same one create_task/update_recipe use) gives the edit a fresh version
|
||||
and leaves the measured one exactly as it was when the readings were taken.
|
||||
The task is matched in the copy by its position (order_index), which
|
||||
_copy_tasks_to_version preserves exactly.
|
||||
"""
|
||||
if not await recipe_service.version_has_measurements(db, task.version_id):
|
||||
return task
|
||||
|
||||
recipe_id = task.recipe_id
|
||||
if recipe_id is None:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail="Cannot determine the recipe for this task",
|
||||
)
|
||||
from src.backend.models.api.recipe import RecipeUpdate
|
||||
new_version = await recipe_service.create_new_version(
|
||||
db, recipe_id, RecipeUpdate(change_notes=f"Edited task: {task.title}"), user,
|
||||
)
|
||||
for candidate in new_version.tasks:
|
||||
if candidate.order_index == task.order_index:
|
||||
# TaskResponse.recipe_id reads task.version.recipe_id; new_version's
|
||||
# own tasks are loaded via selectinload, which does not also
|
||||
# populate the child->parent backref, so left alone this would
|
||||
# lazy-load .version outside of an awaited context and crash.
|
||||
candidate.version = new_version
|
||||
return candidate
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail="Could not locate the task in the new version",
|
||||
)
|
||||
|
||||
|
||||
async def _subtask_for_write(
|
||||
db: AsyncSession, task: RecipeTask, subtask: RecipeSubtask, user: User,
|
||||
) -> tuple[RecipeTask, RecipeSubtask]:
|
||||
"""Return (task, subtask) to write to: the same ones, or their copies.
|
||||
|
||||
Same copy-on-write as _task_for_write, one level down: once the task's copy
|
||||
is found, the quote is matched inside it by marker_number (unique per task).
|
||||
"""
|
||||
new_task = await _task_for_write(db, task, user)
|
||||
if new_task is task:
|
||||
return task, subtask
|
||||
|
||||
for candidate in new_task.subtasks:
|
||||
if candidate.marker_number == subtask.marker_number:
|
||||
return new_task, candidate
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
detail="Could not locate the quote in the new version",
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Task endpoints
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -264,10 +323,16 @@ async def reorder_tasks(
|
||||
async def update_task(
|
||||
task_id: int,
|
||||
data: TaskUpdate,
|
||||
_user: User = Depends(require_maker),
|
||||
user: User = Depends(require_maker),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Update a task. The task must belong to the current version. Requires Maker role."""
|
||||
"""Update a task. The task must belong to the current version. Requires Maker role.
|
||||
|
||||
If that version already has measurements the edit goes through copy-on-write:
|
||||
a new version is created and its copy of the task is the one changed. The
|
||||
version with measurements is left untouched, so its recorded pass/fail keeps
|
||||
meaning what it meant when it was taken.
|
||||
"""
|
||||
task = await _get_task_or_404(db, task_id)
|
||||
await _ensure_version_is_current(db, task.version_id)
|
||||
|
||||
@@ -275,6 +340,8 @@ async def update_task(
|
||||
if not update_data:
|
||||
return TaskResponse.model_validate(task)
|
||||
|
||||
task = await _task_for_write(db, task, user)
|
||||
|
||||
for field, value in update_data.items():
|
||||
setattr(task, field, value)
|
||||
|
||||
@@ -286,16 +353,19 @@ async def update_task(
|
||||
@router.delete("/api/tasks/{task_id}", status_code=204)
|
||||
async def delete_task(
|
||||
task_id: int,
|
||||
_user: User = Depends(require_maker),
|
||||
user: User = Depends(require_maker),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Delete a task and its subtasks. The task must belong to the current version.
|
||||
|
||||
Requires Maker role.
|
||||
Requires Maker role. Copy-on-write applies as in update_task: a version with
|
||||
measurements is never touched, the deletion lands on its copy instead.
|
||||
"""
|
||||
task = await _get_task_or_404(db, task_id)
|
||||
await _ensure_version_is_current(db, task.version_id)
|
||||
|
||||
task = await _task_for_write(db, task, user)
|
||||
|
||||
await db.delete(task)
|
||||
await db.flush()
|
||||
|
||||
@@ -312,16 +382,19 @@ async def delete_task(
|
||||
async def create_subtask(
|
||||
task_id: int,
|
||||
data: SubtaskCreate,
|
||||
_user: User = Depends(require_maker),
|
||||
user: User = Depends(require_maker),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Add a subtask to a task. The task must belong to the current version.
|
||||
|
||||
Requires Maker role.
|
||||
Requires Maker role. Copy-on-write applies as in create_task: a version with
|
||||
measurements gets a new version, and the subtask is added there instead.
|
||||
"""
|
||||
task = await _get_task_or_404(db, task_id)
|
||||
await _ensure_version_is_current(db, task.version_id)
|
||||
|
||||
task = await _task_for_write(db, task, user)
|
||||
|
||||
# Check marker_number uniqueness within the task
|
||||
existing_markers = {s.marker_number for s in task.subtasks}
|
||||
if data.marker_number in existing_markers:
|
||||
@@ -331,7 +404,7 @@ async def create_subtask(
|
||||
)
|
||||
|
||||
subtask = RecipeSubtask(
|
||||
task_id=task_id,
|
||||
task_id=task.id,
|
||||
marker_number=data.marker_number,
|
||||
description=data.description,
|
||||
measurement_type=data.measurement_type,
|
||||
@@ -354,12 +427,13 @@ async def create_subtask(
|
||||
async def update_subtask(
|
||||
subtask_id: int,
|
||||
data: SubtaskUpdate,
|
||||
_user: User = Depends(require_maker),
|
||||
user: User = Depends(require_maker),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Update a subtask. Its parent task must belong to the current version.
|
||||
|
||||
Requires Maker role.
|
||||
Requires Maker role. Copy-on-write applies as in update_task, one level
|
||||
down: a version with measurements is never touched, the quote's copy is.
|
||||
"""
|
||||
subtask = await _get_subtask_or_404(db, subtask_id)
|
||||
|
||||
@@ -371,6 +445,24 @@ async def update_subtask(
|
||||
if not update_data:
|
||||
return SubtaskResponse.model_validate(subtask)
|
||||
|
||||
# SubtaskUpdate is partial, so the order can only be checked against the
|
||||
# values the write would actually leave in place: existing + incoming.
|
||||
def _resulting(field: str) -> float | None:
|
||||
value = update_data.get(field, getattr(subtask, field))
|
||||
return None if value is None else float(value)
|
||||
|
||||
try:
|
||||
validate_tolerance_order(
|
||||
_resulting("ltl"), _resulting("lwl"), _resulting("nominal"),
|
||||
_resulting("uwl"), _resulting("utl"),
|
||||
)
|
||||
except ValueError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_422_UNPROCESSABLE_CONTENT, detail=str(exc),
|
||||
)
|
||||
|
||||
task, subtask = await _subtask_for_write(db, task, subtask, user)
|
||||
|
||||
for field, value in update_data.items():
|
||||
setattr(subtask, field, value)
|
||||
|
||||
@@ -382,12 +474,13 @@ async def update_subtask(
|
||||
@router.delete("/api/subtasks/{subtask_id}", status_code=204)
|
||||
async def delete_subtask(
|
||||
subtask_id: int,
|
||||
_user: User = Depends(require_maker),
|
||||
user: User = Depends(require_maker),
|
||||
db: AsyncSession = Depends(get_db),
|
||||
):
|
||||
"""Delete a subtask. Its parent task must belong to the current version.
|
||||
|
||||
Requires Maker role.
|
||||
Requires Maker role. Copy-on-write applies as in update_task, one level
|
||||
down: a version with measurements is never touched, the quote's copy is.
|
||||
"""
|
||||
subtask = await _get_subtask_or_404(db, subtask_id)
|
||||
|
||||
@@ -395,5 +488,7 @@ async def delete_subtask(
|
||||
task = await _get_task_or_404(db, subtask.task_id)
|
||||
await _ensure_version_is_current(db, task.version_id)
|
||||
|
||||
task, subtask = await _subtask_for_write(db, task, subtask, user)
|
||||
|
||||
await db.delete(subtask)
|
||||
await db.flush()
|
||||
|
||||
Reference in New Issue
Block a user