fix(projects): audit pass — 8 correctness and consistency fixes
- Project.to_dict() now includes user_id and auto_summary - Status validation unified to (active/completed/archived) on both create and update project routes; update route previously had none - Milestone routes: replace get_project (ownership-only) with get_project_for_user so shared viewers/editors can access milestones - Add get_milestone_in_project() to milestones service for project- scoped lookup without user_id filter; all milestone routes use it - Milestone PATCH now validates status as 'active'|'done'; fix tool enum which was wrongly ['active','completed','cancelled'] - Write mutation routes (POST/PATCH/DELETE milestones) now check can_write_project() and return 403 for read-only shared users - update_project tool now exposes title and color fields so projects can be renamed or recolored via chat - create_project tool now exposes color field - GET /api/projects?include_summary=true embeds summaries in one backend pass; ProjectListView switches to this, eliminating N+1 per-project fetches Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -16,11 +16,15 @@ interface MilestoneSummary {
|
||||
|
||||
interface Project {
|
||||
id: number;
|
||||
user_id: number;
|
||||
title: string;
|
||||
description: string | null;
|
||||
goal: string | null;
|
||||
status: "active" | "completed" | "archived";
|
||||
color: string | null;
|
||||
auto_summary: string | null;
|
||||
permission?: string;
|
||||
is_shared?: boolean;
|
||||
created_at: string;
|
||||
updated_at: string;
|
||||
summary?: {
|
||||
@@ -54,19 +58,8 @@ async function loadProjects() {
|
||||
loading.value = true;
|
||||
error.value = null;
|
||||
try {
|
||||
const data = await apiGet<{ projects: Project[] }>("/api/projects");
|
||||
const data = await apiGet<{ projects: Project[] }>("/api/projects?include_summary=true");
|
||||
projects.value = data.projects;
|
||||
// Fetch summaries (including milestone_summary) in parallel
|
||||
await Promise.allSettled(
|
||||
projects.value.map(async (p) => {
|
||||
try {
|
||||
const full = await apiGet<Project>(`/api/projects/${p.id}`);
|
||||
p.summary = full.summary;
|
||||
} catch {
|
||||
// non-fatal
|
||||
}
|
||||
})
|
||||
);
|
||||
} catch {
|
||||
error.value = "Failed to load projects.";
|
||||
} finally {
|
||||
|
||||
@@ -19,11 +19,14 @@ interface Milestone {
|
||||
|
||||
interface Project {
|
||||
id: number;
|
||||
user_id: number;
|
||||
title: string;
|
||||
description: string | null;
|
||||
goal: string | null;
|
||||
status: "active" | "completed" | "archived";
|
||||
color: string | null;
|
||||
auto_summary: string | null;
|
||||
permission?: string;
|
||||
created_at: string;
|
||||
updated_at: string;
|
||||
summary?: {
|
||||
|
||||
@@ -29,11 +29,13 @@ class Project(Base, TimestampMixin):
|
||||
def to_dict(self) -> dict:
|
||||
return {
|
||||
"id": self.id,
|
||||
"user_id": self.user_id,
|
||||
"title": self.title,
|
||||
"description": self.description,
|
||||
"goal": self.goal,
|
||||
"status": self.status,
|
||||
"color": self.color,
|
||||
"auto_summary": self.auto_summary,
|
||||
"created_at": self.created_at.isoformat(),
|
||||
"updated_at": self.updated_at.isoformat(),
|
||||
}
|
||||
|
||||
@@ -5,16 +5,17 @@ from quart import Blueprint, jsonify, request
|
||||
|
||||
from fabledassistant.auth import login_required, get_current_user_id
|
||||
from fabledassistant.routes.utils import not_found, parse_pagination
|
||||
from fabledassistant.services.access import can_write_project
|
||||
from fabledassistant.services.milestones import (
|
||||
create_milestone,
|
||||
delete_milestone,
|
||||
get_milestone,
|
||||
get_milestone_in_project,
|
||||
get_milestone_progress,
|
||||
list_milestones,
|
||||
update_milestone,
|
||||
)
|
||||
from fabledassistant.services.notes import list_notes
|
||||
from fabledassistant.services.projects import get_project
|
||||
from fabledassistant.services.projects import get_project_for_user
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -31,22 +32,25 @@ async def _milestone_dict(m) -> dict:
|
||||
@login_required
|
||||
async def list_milestones_route(project_id: int):
|
||||
uid = get_current_user_id()
|
||||
project = await get_project(uid, project_id)
|
||||
if project is None:
|
||||
result = await get_project_for_user(uid, project_id)
|
||||
if result is None:
|
||||
return not_found("Project")
|
||||
project, _ = result
|
||||
# List milestones using the project owner's uid so ownership filter matches
|
||||
owner_uid = project.user_id or uid
|
||||
status = request.args.get("status")
|
||||
milestones = await list_milestones(uid, project_id, status=status)
|
||||
result = [await _milestone_dict(m) for m in milestones]
|
||||
return jsonify({"milestones": result})
|
||||
milestones = await list_milestones(owner_uid, project_id, status=status)
|
||||
return jsonify({"milestones": [await _milestone_dict(m) for m in milestones]})
|
||||
|
||||
|
||||
@milestones_bp.route("/<int:project_id>/milestones", methods=["POST"])
|
||||
@login_required
|
||||
async def create_milestone_route(project_id: int):
|
||||
uid = get_current_user_id()
|
||||
project = await get_project(uid, project_id)
|
||||
if project is None:
|
||||
if await get_project_for_user(uid, project_id) is None:
|
||||
return not_found("Project")
|
||||
if not await can_write_project(uid, project_id):
|
||||
return jsonify({"error": "Permission denied"}), 403
|
||||
data = await request.get_json()
|
||||
if not data.get("title"):
|
||||
return jsonify({"error": "title is required"}), 400
|
||||
@@ -68,8 +72,10 @@ async def create_milestone_route(project_id: int):
|
||||
@login_required
|
||||
async def get_milestone_route(project_id: int, milestone_id: int):
|
||||
uid = get_current_user_id()
|
||||
milestone = await get_milestone(uid, milestone_id)
|
||||
if milestone is None or milestone.project_id != project_id:
|
||||
if await get_project_for_user(uid, project_id) is None:
|
||||
return not_found("Project")
|
||||
milestone = await get_milestone_in_project(project_id, milestone_id)
|
||||
if milestone is None:
|
||||
return not_found("Milestone")
|
||||
return jsonify(await _milestone_dict(milestone))
|
||||
|
||||
@@ -78,13 +84,19 @@ async def get_milestone_route(project_id: int, milestone_id: int):
|
||||
@login_required
|
||||
async def update_milestone_route(project_id: int, milestone_id: int):
|
||||
uid = get_current_user_id()
|
||||
milestone = await get_milestone(uid, milestone_id)
|
||||
if milestone is None or milestone.project_id != project_id:
|
||||
if await get_project_for_user(uid, project_id) is None:
|
||||
return not_found("Project")
|
||||
if not await can_write_project(uid, project_id):
|
||||
return jsonify({"error": "Permission denied"}), 403
|
||||
milestone = await get_milestone_in_project(project_id, milestone_id)
|
||||
if milestone is None:
|
||||
return not_found("Milestone")
|
||||
data = await request.get_json()
|
||||
allowed = {"title", "description", "status", "order_index"}
|
||||
fields = {k: v for k, v in data.items() if k in allowed}
|
||||
updated = await update_milestone(uid, milestone_id, **fields)
|
||||
if "status" in fields and fields["status"] not in ("active", "done"):
|
||||
return jsonify({"error": "status must be 'active' or 'done'"}), 400
|
||||
updated = await update_milestone(milestone.user_id, milestone_id, **fields)
|
||||
if updated is None:
|
||||
return not_found("Milestone")
|
||||
return jsonify(await _milestone_dict(updated))
|
||||
@@ -94,10 +106,14 @@ async def update_milestone_route(project_id: int, milestone_id: int):
|
||||
@login_required
|
||||
async def delete_milestone_route(project_id: int, milestone_id: int):
|
||||
uid = get_current_user_id()
|
||||
milestone = await get_milestone(uid, milestone_id)
|
||||
if milestone is None or milestone.project_id != project_id:
|
||||
if await get_project_for_user(uid, project_id) is None:
|
||||
return not_found("Project")
|
||||
if not await can_write_project(uid, project_id):
|
||||
return jsonify({"error": "Permission denied"}), 403
|
||||
milestone = await get_milestone_in_project(project_id, milestone_id)
|
||||
if milestone is None:
|
||||
return not_found("Milestone")
|
||||
deleted = await delete_milestone(uid, milestone_id)
|
||||
deleted = await delete_milestone(milestone.user_id, milestone_id)
|
||||
if not deleted:
|
||||
return not_found("Milestone")
|
||||
return "", 204
|
||||
@@ -107,13 +123,15 @@ async def delete_milestone_route(project_id: int, milestone_id: int):
|
||||
@login_required
|
||||
async def get_milestone_tasks_route(project_id: int, milestone_id: int):
|
||||
uid = get_current_user_id()
|
||||
milestone = await get_milestone(uid, milestone_id)
|
||||
if milestone is None or milestone.project_id != project_id:
|
||||
if await get_project_for_user(uid, project_id) is None:
|
||||
return not_found("Project")
|
||||
milestone = await get_milestone_in_project(project_id, milestone_id)
|
||||
if milestone is None:
|
||||
return not_found("Milestone")
|
||||
status_filter = request.args.get("status")
|
||||
limit, offset = parse_pagination(default_limit=100)
|
||||
notes, total = await list_notes(
|
||||
uid,
|
||||
milestone.user_id,
|
||||
is_task=True,
|
||||
status=status_filter,
|
||||
milestone_id=milestone_id,
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
"""Project management routes."""
|
||||
import asyncio
|
||||
import logging
|
||||
|
||||
from quart import Blueprint, jsonify, request
|
||||
@@ -27,7 +28,19 @@ projects_bp = Blueprint("projects", __name__, url_prefix="/api/projects")
|
||||
async def list_projects_route():
|
||||
uid = get_current_user_id()
|
||||
status = request.args.get("status")
|
||||
include_summary = request.args.get("include_summary", "").lower() in ("1", "true")
|
||||
projects = await list_projects_for_user(uid, status=status)
|
||||
if include_summary:
|
||||
# Fetch all summaries in parallel — one backend pass instead of N+1 frontend calls
|
||||
async def _attach(project_dict: dict) -> dict:
|
||||
try:
|
||||
owner_uid = project_dict.get("user_id") or uid # user_id now in to_dict()
|
||||
summary = await get_project_summary(owner_uid, project_dict["id"])
|
||||
project_dict["summary"] = summary
|
||||
except Exception:
|
||||
pass
|
||||
return project_dict
|
||||
projects = list(await asyncio.gather(*[_attach(p) for p in projects]))
|
||||
return jsonify({"projects": projects})
|
||||
|
||||
|
||||
@@ -39,8 +52,8 @@ async def create_project_route():
|
||||
if not data.get("title"):
|
||||
return jsonify({"error": "title is required"}), 400
|
||||
status = data.get("status", "active")
|
||||
if status not in ("active", "archived"):
|
||||
return jsonify({"error": "status must be 'active' or 'archived'"}), 400
|
||||
if status not in ("active", "completed", "archived"):
|
||||
return jsonify({"error": "status must be 'active', 'completed', or 'archived'"}), 400
|
||||
project = await create_project(
|
||||
uid,
|
||||
title=data["title"],
|
||||
@@ -76,6 +89,8 @@ async def update_project_route(project_id: int):
|
||||
data = await request.get_json()
|
||||
allowed = {"title", "description", "goal", "status", "color"}
|
||||
fields = {k: v for k, v in data.items() if k in allowed}
|
||||
if "status" in fields and fields["status"] not in ("active", "completed", "archived"):
|
||||
return jsonify({"error": "status must be 'active', 'completed', or 'archived'"}), 400
|
||||
project = await update_project(uid, project_id, **fields)
|
||||
if project is None:
|
||||
return not_found("Project")
|
||||
|
||||
@@ -42,6 +42,19 @@ async def get_milestone(user_id: int, milestone_id: int) -> Milestone | None:
|
||||
return result.scalars().first()
|
||||
|
||||
|
||||
async def get_milestone_in_project(project_id: int, milestone_id: int) -> Milestone | None:
|
||||
"""Fetch a milestone by id within a project, without a user_id ownership check.
|
||||
Callers must verify project access separately before using this."""
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
select(Milestone).where(
|
||||
Milestone.id == milestone_id,
|
||||
Milestone.project_id == project_id,
|
||||
)
|
||||
)
|
||||
return result.scalars().first()
|
||||
|
||||
|
||||
async def get_milestone_by_title(user_id: int, project_id: int, title: str) -> Milestone | None:
|
||||
async with async_session() as session:
|
||||
result = await session.execute(
|
||||
|
||||
@@ -392,6 +392,7 @@ _CORE_TOOLS = [
|
||||
"title": {"type": "string", "description": "Project title"},
|
||||
"description": {"type": "string", "description": "Brief description"},
|
||||
"goal": {"type": "string", "description": "The goal or desired outcome"},
|
||||
"color": {"type": "string", "description": "Optional hex color for the project (e.g. '#6366f1')"},
|
||||
"confirmed": {"type": "boolean", "description": "Must be true — only set after the user has explicitly confirmed they want this project created."},
|
||||
},
|
||||
"required": ["title", "confirmed"],
|
||||
@@ -433,14 +434,16 @@ _CORE_TOOLS = [
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "update_project",
|
||||
"description": "Update a project's status, description, or goal.",
|
||||
"description": "Update a project's title, status, description, goal, or color.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"query": {"type": "string", "description": "Project name to find"},
|
||||
"title": {"type": "string", "description": "New project title"},
|
||||
"status": {"type": "string", "enum": ["active", "completed", "archived"]},
|
||||
"description": {"type": "string"},
|
||||
"goal": {"type": "string"},
|
||||
"color": {"type": "string", "description": "Hex color (e.g. '#6366f1'), or empty string to clear"},
|
||||
},
|
||||
"required": ["query"],
|
||||
},
|
||||
@@ -475,7 +478,7 @@ _CORE_TOOLS = [
|
||||
"milestone": {"type": "string", "description": "Current milestone title to look up"},
|
||||
"title": {"type": "string", "description": "New title (omit to keep current)"},
|
||||
"description": {"type": "string", "description": "New description (omit to keep current)"},
|
||||
"status": {"type": "string", "enum": ["active", "completed", "cancelled"], "description": "New status (omit to keep current)"},
|
||||
"status": {"type": "string", "enum": ["active", "done"], "description": "New status: 'active' (in progress) or 'done' (completed)"},
|
||||
},
|
||||
"required": ["project", "milestone"],
|
||||
},
|
||||
@@ -1768,6 +1771,7 @@ async def execute_tool(
|
||||
title=proj_title,
|
||||
description=arguments.get("description", ""),
|
||||
goal=arguments.get("goal", ""),
|
||||
color=arguments.get("color") or None,
|
||||
)
|
||||
return {"success": True, "type": "project", "data": project.to_dict()}
|
||||
|
||||
@@ -1816,9 +1820,11 @@ async def execute_tool(
|
||||
if project is None:
|
||||
return {"success": False, "error": f"No project found matching '{query}'"}
|
||||
fields = {}
|
||||
for k in ("status", "description", "goal"):
|
||||
for k in ("title", "status", "description", "goal"):
|
||||
if k in arguments:
|
||||
fields[k] = arguments[k]
|
||||
if "color" in arguments:
|
||||
fields["color"] = arguments["color"] or None # empty string → None (clear)
|
||||
updated = await _up(user_id, project.id, **fields)
|
||||
return {"success": True, "type": "project_updated", "data": updated.to_dict()}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user