diff --git a/app/api/routers/milestone_actions.py b/app/api/routers/milestone_actions.py index 86387e8..cbe0f49 100644 --- a/app/api/routers/milestone_actions.py +++ b/app/api/routers/milestone_actions.py @@ -3,7 +3,6 @@ Provides freeze / start / close actions on milestones. completed is triggered automatically when the sole release maintenance task finishes. """ -import json from datetime import datetime, timezone from typing import Optional @@ -18,6 +17,7 @@ from app.models import models from app.models.milestone import Milestone, MilestoneStatus from app.models.task import Task, TaskStatus from app.services.activity import log_activity +from app.services.dependency_check import check_milestone_deps router = APIRouter( prefix="/projects/{project_id}/milestones/{milestone_id}/actions", @@ -101,38 +101,13 @@ def preflight_milestone_actions( # --- start pre-check (only meaningful when status == freeze) --- if ms_status == "freeze": - blockers: list[str] = [] - - # milestone dependencies - dep_ms_ids = [] - if ms.depend_on_milestones: - try: - dep_ms_ids = json.loads(ms.depend_on_milestones) - except (json.JSONDecodeError, TypeError): - dep_ms_ids = [] - if dep_ms_ids: - dep_milestones = db.query(Milestone).filter(Milestone.id.in_(dep_ms_ids)).all() - incomplete = [m.id for m in dep_milestones if _ms_status_value(m) != "completed"] - if incomplete: - blockers.append(f"Dependent milestones not completed: {incomplete}") - - # task dependencies - dep_task_ids = [] - if ms.depend_on_tasks: - try: - dep_task_ids = json.loads(ms.depend_on_tasks) - except (json.JSONDecodeError, TypeError): - dep_task_ids = [] - if dep_task_ids: - dep_tasks = db.query(Task).filter(Task.id.in_(dep_task_ids)).all() - incomplete_tasks = [t.id for t in dep_tasks if (t.status.value if hasattr(t.status, "value") else t.status) != "completed"] - if incomplete_tasks: - blockers.append(f"Dependent tasks not completed: {incomplete_tasks}") - - if blockers: - result["start"] = {"allowed": False, "reason": "; ".join(blockers)} - else: + dep_result = check_milestone_deps( + db, ms.depend_on_milestones, ms.depend_on_tasks, + ) + if dep_result.ok: result["start"] = {"allowed": True, "reason": None} + else: + result["start"] = {"allowed": False, "reason": dep_result.reason} return result @@ -232,51 +207,15 @@ def start_milestone( detail=f"Cannot start: milestone is '{_ms_status_value(ms)}', expected 'freeze'", ) - # Dependency check — milestone dependencies - dep_ms_ids = [] - if ms.depend_on_milestones: - try: - dep_ms_ids = json.loads(ms.depend_on_milestones) - except (json.JSONDecodeError, TypeError): - dep_ms_ids = [] - - if dep_ms_ids: - dep_milestones = ( - db.query(Milestone) - .filter(Milestone.id.in_(dep_ms_ids)) - .all() + # Dependency check (P4.1 — shared helper) + dep_result = check_milestone_deps( + db, ms.depend_on_milestones, ms.depend_on_tasks, + ) + if not dep_result.ok: + raise HTTPException( + status_code=400, + detail=f"Cannot start: {dep_result.reason}", ) - incomplete = [ - m.id - for m in dep_milestones - if _ms_status_value(m) != "completed" - ] - if incomplete: - raise HTTPException( - status_code=400, - detail=f"Cannot start: dependent milestones not completed: {incomplete}", - ) - - # Dependency check — task dependencies - dep_task_ids = [] - if ms.depend_on_tasks: - try: - dep_task_ids = json.loads(ms.depend_on_tasks) - except (json.JSONDecodeError, TypeError): - dep_task_ids = [] - - if dep_task_ids: - dep_tasks = db.query(Task).filter(Task.id.in_(dep_task_ids)).all() - incomplete_tasks = [ - t.id - for t in dep_tasks - if (t.status.value if hasattr(t.status, "value") else t.status) != "completed" - ] - if incomplete_tasks: - raise HTTPException( - status_code=400, - detail=f"Cannot start: dependent tasks not completed: {incomplete_tasks}", - ) ms.status = MilestoneStatus.UNDERGOING ms.started_at = datetime.now(timezone.utc) diff --git a/app/services/dependency_check.py b/app/services/dependency_check.py new file mode 100644 index 0000000..690065c --- /dev/null +++ b/app/services/dependency_check.py @@ -0,0 +1,113 @@ +"""P4.1 — Reusable dependency-check helpers. + +Used by milestone start, milestone preflight, and (future) task pending→open +to verify that all declared dependencies are completed before allowing the +entity to proceed. +""" +from __future__ import annotations + +import json +from dataclasses import dataclass, field +from typing import Sequence + +from sqlalchemy.orm import Session + +from app.models.milestone import Milestone +from app.models.task import Task + + +# --------------------------------------------------------------------------- +# Result type +# --------------------------------------------------------------------------- + +@dataclass +class DepCheckResult: + """Outcome of a dependency check.""" + + ok: bool = True + blockers: list[str] = field(default_factory=list) + + @property + def reason(self) -> str | None: + return "; ".join(self.blockers) if self.blockers else None + + +# --------------------------------------------------------------------------- +# Internal helpers +# --------------------------------------------------------------------------- + +def _parse_json_ids(raw: str | None) -> list[int]: + """Safely parse a JSON-encoded list of integer IDs.""" + if not raw: + return [] + try: + ids = json.loads(raw) + if isinstance(ids, list): + return [int(i) for i in ids] + except (json.JSONDecodeError, TypeError, ValueError): + pass + return [] + + +def _ms_status(ms: Milestone) -> str: + return ms.status.value if hasattr(ms.status, "value") else ms.status + + +def _task_status(t: Task) -> str: + return t.status.value if hasattr(t.status, "value") else t.status + + +# --------------------------------------------------------------------------- +# Public API +# --------------------------------------------------------------------------- + +def check_milestone_deps( + db: Session, + depend_on_milestones: str | None, + depend_on_tasks: str | None, + *, + required_status: str = "completed", +) -> DepCheckResult: + """Check whether all milestone + task dependencies are satisfied. + + Parameters + ---------- + db: + Active DB session. + depend_on_milestones: + JSON-encoded list of milestone IDs (from the entity's field). + depend_on_tasks: + JSON-encoded list of task IDs (from the entity's field). + required_status: + The status that dependees must have reached (default ``"completed"``). + + Returns + ------- + DepCheckResult with ``ok=True`` if all deps satisfied, else ``ok=False`` + with human-readable ``blockers``. + """ + result = DepCheckResult() + + # --- milestone dependencies --- + ms_ids = _parse_json_ids(depend_on_milestones) + if ms_ids: + dep_milestones: Sequence[Milestone] = ( + db.query(Milestone).filter(Milestone.id.in_(ms_ids)).all() + ) + incomplete = [m.id for m in dep_milestones if _ms_status(m) != required_status] + if incomplete: + result.ok = False + result.blockers.append(f"Dependent milestones not {required_status}: {incomplete}") + + # --- task dependencies --- + task_ids = _parse_json_ids(depend_on_tasks) + if task_ids: + dep_tasks: Sequence[Task] = ( + db.query(Task).filter(Task.id.in_(task_ids)).all() + ) + incomplete_tasks = [t.id for t in dep_tasks if _task_status(t) != required_status] + if incomplete_tasks: + result.ok = False + result.blockers.append(f"Dependent tasks not {required_status}: {incomplete_tasks}") + + return result