From 589b1cc8de978dd8a63ffaf7e68833b5ace2e844 Mon Sep 17 00:00:00 2001 From: zhi Date: Tue, 17 Mar 2026 08:02:37 +0000 Subject: [PATCH] =?UTF-8?q?feat(P5.1-P5.6):=20task=20state-machine=20valid?= =?UTF-8?q?ation=20=E2=80=94=20enforce=20legal=20transitions=20in=20transi?= =?UTF-8?q?tion/batch/update=20endpoints?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/api/routers/tasks.py | 66 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/app/api/routers/tasks.py b/app/api/routers/tasks.py index cc7ea46..f4e7c47 100644 --- a/app/api/routers/tasks.py +++ b/app/api/routers/tasks.py @@ -19,6 +19,25 @@ from app.services.activity import log_activity router = APIRouter(tags=["Tasks"]) +# ---- State-machine: valid transitions (P5.1-P5.6) ---- +VALID_TRANSITIONS: dict[str, set[str]] = { + "pending": {"open", "closed"}, + "open": {"undergoing", "closed"}, + "undergoing": {"completed", "closed"}, + "completed": {"open"}, # reopen + "closed": {"open"}, # reopen +} + +def _check_transition(old_status: str, new_status: str) -> None: + """Raise 400 if the transition is not allowed by the state machine.""" + allowed = VALID_TRANSITIONS.get(old_status, set()) + if new_status not in allowed: + raise HTTPException( + status_code=400, + detail=f"Cannot transition from '{old_status}' to '{new_status}'. " + f"Allowed targets from '{old_status}': {sorted(allowed) if allowed else 'none'}", + ) + # ---- Type / Subtype validation ---- TASK_SUBTYPE_MAP = { 'issue': {'infrastructure', 'performance', 'regression', 'security', 'user_experience', 'defect'}, @@ -167,6 +186,11 @@ def update_task(task_id: int, task_update: schemas.TaskUpdate, db: Session = Dep update_data = task_update.model_dump(exclude_unset=True) if "status" in update_data: new_status = update_data["status"] + old_status = task.status.value if hasattr(task.status, 'value') else task.status + # P5.1: enforce state-machine even through PATCH + _check_transition(old_status, new_status) + if new_status == "open" and old_status in ("completed", "closed"): + task.finished_on = None if new_status == "undergoing" and not task.started_on: task.started_on = datetime.utcnow() if new_status in ("closed", "completed") and not task.finished_on: @@ -208,6 +232,31 @@ def transition_task(task_id: int, new_status: str, bg: BackgroundTasks, db: Sess if not task: raise HTTPException(status_code=404, detail="Task not found") old_status = task.status.value if hasattr(task.status, 'value') else task.status + + # P5.1: enforce state-machine + _check_transition(old_status, new_status) + + # P5.2: pending -> open requires milestone to be undergoing (dependencies checked later) + if old_status == "pending" and new_status == "open": + milestone = db.query(Milestone).filter(Milestone.id == task.milestone_id).first() + if milestone: + ms_status = milestone.status.value if hasattr(milestone.status, 'value') else milestone.status + if ms_status != "undergoing": + raise HTTPException( + status_code=400, + detail=f"Cannot open task: milestone is '{ms_status}', must be 'undergoing'", + ) + + # P5.3: open -> undergoing requires assignee + if old_status == "open" and new_status == "undergoing": + if not task.assignee_id: + raise HTTPException(status_code=400, detail="Cannot start task: assignee must be set first") + + # P5.6: reopen from completed/closed -> open + if new_status == "open" and old_status in ("completed", "closed"): + # Clear finished_on on reopen so lifecycle timestamps are accurate + task.finished_on = None + if new_status == "undergoing" and not task.started_on: task.started_on = datetime.utcnow() if new_status in ("closed", "completed") and not task.finished_on: @@ -306,17 +355,32 @@ def batch_transition(data: BatchTransition, bg: BackgroundTasks, db: Session = D if data.new_status not in valid_statuses: raise HTTPException(status_code=400, detail="Invalid status") updated = [] + skipped = [] for task_id in data.task_ids: task = db.query(Task).filter(Task.id == task_id).first() if task: old_status = task.status.value if hasattr(task.status, 'value') else task.status + allowed = VALID_TRANSITIONS.get(old_status, set()) + if data.new_status not in allowed: + skipped.append({"id": task.id, "title": task.title, "old": old_status, + "reason": f"Cannot transition from '{old_status}' to '{data.new_status}'"}) + continue + if data.new_status == "undergoing" and not task.started_on: + task.started_on = datetime.utcnow() + if data.new_status in ("closed", "completed") and not task.finished_on: + task.finished_on = datetime.utcnow() + if data.new_status == "open" and old_status in ("completed", "closed"): + task.finished_on = None task.status = data.new_status updated.append({"id": task.id, "title": task.title, "old": old_status, "new": data.new_status}) db.commit() for u in updated: event = "task.closed" if data.new_status == "closed" else "task.updated" bg.add_task(fire_webhooks_sync, event, u, None, db) - return {"updated": len(updated), "tasks": updated} + result = {"updated": len(updated), "tasks": updated} + if skipped: + result["skipped"] = skipped + return result @router.post("/tasks/batch/assign")