diff --git a/server/src/__tests__/goals.test.ts b/server/src/__tests__/goals.test.ts new file mode 100644 index 0000000..a89daf7 --- /dev/null +++ b/server/src/__tests__/goals.test.ts @@ -0,0 +1,178 @@ +import { beforeEach, describe, expect, it } from 'vitest'; +import request from 'supertest'; +import { randomUUID } from 'node:crypto'; +import { createTestApp, createTestDb } from '../test-utils.js'; + +describe('Goals API', () => { + let app: ReturnType; + let companyId: string; + let ownerAgentId: string; + + beforeEach(async () => { + const db = await createTestDb(); + app = createTestApp(db); + + const company = await request(app) + .post('/api/companies') + .send({ name: 'Goal Test Corp' }) + .expect(201); + companyId = company.body.data.id; + + const owner = await request(app) + .post(`/api/companies/${companyId}/agents`) + .send({ name: 'Goal Owner', role: 'ceo' }) + .expect(201); + ownerAgentId = owner.body.data.id; + }); + + it('creates nested goals and persists operator updates', async () => { + const root = await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ + title: 'Ship the operator workflow', + description: 'Make goal management durable.', + level: 'company', + status: 'active', + ownerAgentId, + progress: 20, + }) + .expect(201); + + const child = await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ + title: 'Verify progress updates', + level: 'team', + status: 'draft', + parentId: root.body.data.id, + ownerAgentId, + progress: 0, + }) + .expect(201); + + const updated = await request(app) + .patch(`/api/companies/${companyId}/goals/${child.body.data.id}`) + .send({ title: 'Verify durable progress updates', status: 'active', progress: 65 }) + .expect(200); + + expect(updated.body.data).toEqual(expect.objectContaining({ + parentId: root.body.data.id, + ownerAgentId, + status: 'active', + progress: 65, + })); + + const list = await request(app) + .get(`/api/companies/${companyId}/goals`) + .expect(200); + expect(list.body.data).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: root.body.data.id, parentId: null }), + expect.objectContaining({ id: child.body.data.id, progress: 65 }), + ])); + }); + + it('rejects parents outside the company', async () => { + await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Invalid parent', parentId: randomUUID() }) + .expect(400) + .expect(({ body }) => { + expect(body.message).toBe('Choose a parent goal from this company.'); + }); + }); + + it('rejects self-parent and descendant cycles', async () => { + const root = await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Root goal' }) + .expect(201); + const child = await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Child goal', level: 'department', parentId: root.body.data.id }) + .expect(201); + + await request(app) + .patch(`/api/companies/${companyId}/goals/${root.body.data.id}`) + .send({ parentId: root.body.data.id }) + .expect(400); + + await request(app) + .patch(`/api/companies/${companyId}/goals/${root.body.data.id}`) + .send({ parentId: child.body.data.id }) + .expect(400) + .expect(({ body }) => { + expect(body.message).toContain('descendants'); + }); + }); + + it('rejects owners outside the company', async () => { + const otherCompany = await request(app) + .post('/api/companies') + .send({ name: 'Other Goal Corp' }) + .expect(201); + const otherOwner = await request(app) + .post(`/api/companies/${otherCompany.body.data.id}/agents`) + .send({ name: 'Other Owner', role: 'ceo' }) + .expect(201); + + await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Wrong owner', ownerAgentId: otherOwner.body.data.id }) + .expect(400) + .expect(({ body }) => { + expect(body.message).toBe('Choose an owner from this company.'); + }); + }); + + it('rejects blank titles on create and update', async () => { + await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: ' ' }) + .expect(400); + + const goal = await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Valid goal' }) + .expect(201); + + await request(app) + .patch(`/api/companies/${companyId}/goals/${goal.body.data.id}`) + .send({ title: ' ' }) + .expect(400); + }); + + it('enforces levels from parent to child while allowing skipped levels', async () => { + const root = await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Company goal', level: 'company' }) + .expect(201); + + await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Invalid peer', level: 'company', parentId: root.body.data.id }) + .expect(400); + + const team = await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Skipped-level team goal', level: 'team', parentId: root.body.data.id }) + .expect(201); + + const individual = await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Individual goal', level: 'individual', parentId: team.body.data.id }) + .expect(201); + + await request(app) + .post(`/api/companies/${companyId}/goals`) + .send({ title: 'Too deep', level: 'individual', parentId: individual.body.data.id }) + .expect(400); + + await request(app) + .patch(`/api/companies/${companyId}/goals/${team.body.data.id}`) + .send({ level: 'individual' }) + .expect(400) + .expect(({ body }) => { + expect(body.message).toBe('A parent goal must use a level above each child.'); + }); + }); +}); diff --git a/server/src/routes/goals.ts b/server/src/routes/goals.ts index abbef8d..2ede2d9 100644 --- a/server/src/routes/goals.ts +++ b/server/src/routes/goals.ts @@ -8,7 +8,7 @@ import type { DbInstance } from '../types.js'; import { routeParams } from '../utils/route-params.js'; const CreateGoalBody = z.object({ - title: z.string().min(1).max(500), + title: z.string().trim().min(1).max(500), description: z.string().max(5000).optional(), level: z.enum(['company', 'department', 'team', 'individual']).default('company'), status: z.enum(['draft', 'active', 'completed', 'cancelled']).default('draft'), @@ -20,7 +20,7 @@ const CreateGoalBody = z.object({ }); const UpdateGoalBody = z.object({ - title: z.string().min(1).max(500).optional(), + title: z.string().trim().min(1).max(500).optional(), description: z.string().max(5000).nullable().optional(), level: z.enum(['company', 'department', 'team', 'individual']).optional(), status: z.enum(['draft', 'active', 'completed', 'cancelled']).optional(), @@ -31,9 +31,103 @@ const UpdateGoalBody = z.object({ metrics: z.record(z.unknown()).optional(), }); +type GoalLevel = z.infer['level']; + +const GOAL_LEVEL_RANK: Record = { + company: 0, + department: 1, + team: 2, + individual: 3, +}; + export function goalsRouter(db: DbInstance): Router { const router = Router({ mergeParams: true }); - const { goals } = db.schema; + const { agents, goals } = db.schema; + + async function validateGoalReferences({ + companyId, + goalId, + ownerAgentId, + parentId, + level, + }: { + companyId: string; + goalId?: string; + ownerAgentId?: string | null; + parentId?: string | null; + level?: GoalLevel; + }) { + if (parentId !== undefined || (goalId && level !== undefined)) { + const companyGoals = await db.drizzle + .select({ id: goals.id, parentId: goals.parentId, level: goals.level }) + .from(goals) + .where(eq(goals.companyId, companyId)); + const goalsById = new Map(companyGoals.map((goal) => [goal.id, goal])); + + if (parentId !== undefined && parentId !== null) { + const parent = goalsById.get(parentId); + if (!parent) { + throw new AppError( + 400, + 'GOAL_PARENT_INVALID', + 'Choose a parent goal from this company.', + ); + } + + let ancestorId: string | null = parentId; + const visited = new Set(); + while (ancestorId) { + if (ancestorId === goalId || visited.has(ancestorId)) { + throw new AppError( + 400, + 'GOAL_PARENT_CYCLE', + 'A goal cannot be its own parent or a child of one of its descendants.', + ); + } + visited.add(ancestorId); + ancestorId = goalsById.get(ancestorId)?.parentId ?? null; + } + + if (level !== undefined && GOAL_LEVEL_RANK[level] <= GOAL_LEVEL_RANK[parent.level]) { + throw new AppError( + 400, + 'GOAL_LEVEL_INVALID', + 'A child goal must use a level below its parent.', + ); + } + } + + if ( + goalId + && level !== undefined + && companyGoals.some( + (goal) => goal.parentId === goalId && GOAL_LEVEL_RANK[goal.level] <= GOAL_LEVEL_RANK[level], + ) + ) { + throw new AppError( + 400, + 'GOAL_LEVEL_INVALID', + 'A parent goal must use a level above each child.', + ); + } + } + + if (ownerAgentId !== undefined && ownerAgentId !== null) { + const [owner] = await db.drizzle + .select({ id: agents.id }) + .from(agents) + .where(and(eq(agents.id, ownerAgentId), eq(agents.companyId, companyId))) + .limit(1); + + if (!owner) { + throw new AppError( + 400, + 'GOAL_OWNER_INVALID', + 'Choose an owner from this company.', + ); + } + } + } // GET /api/companies/:companyId/goals router.get('/', async (req, res) => { @@ -78,6 +172,13 @@ export function goalsRouter(db: DbInstance): Router { const companyId = routeParams(req).companyId; const now = new Date(); + await validateGoalReferences({ + companyId, + ownerAgentId: body.ownerAgentId, + parentId: body.parentId, + level: body.level, + }); + const [row] = await db.drizzle .insert(goals) .values({ @@ -140,13 +241,24 @@ export function goalsRouter(db: DbInstance): Router { throw new AppError(404, 'GOAL_NOT_FOUND', `Goal ${id} not found`); } + const relationChanged = body.parentId !== undefined || body.level !== undefined; + await validateGoalReferences({ + companyId, + goalId: id, + ownerAgentId: body.ownerAgentId, + parentId: relationChanged + ? body.parentId !== undefined ? body.parentId : existing.parentId + : undefined, + level: relationChanged ? body.level ?? existing.level : undefined, + }); + const progressChanged = body.progress !== undefined && body.progress !== existing.progress; const [updated] = await db.drizzle .update(goals) .set({ ...body, updatedAt: new Date() }) - .where(eq(goals.id, id)) + .where(and(eq(goals.id, id), eq(goals.companyId, companyId))) .returning(); if (progressChanged) { diff --git a/ui/src/components/goals/GoalFormModal.tsx b/ui/src/components/goals/GoalFormModal.tsx new file mode 100644 index 0000000..20681da --- /dev/null +++ b/ui/src/components/goals/GoalFormModal.tsx @@ -0,0 +1,348 @@ +import { useState } from "react"; +import { z } from "zod"; +import { Button } from "@/components/ui/Button"; +import { Input, Select, Textarea } from "@/components/ui/Input"; +import { Modal } from "@/components/ui/Modal"; +import { useCreateGoal, useUpdateGoal } from "@/lib/hooks"; +import type { Agent, Goal, GoalLevel, GoalStatus } from "@/lib/api"; + +interface GoalFormModalProps { + agents: Agent[]; + companyId: string; + defaultParentId?: string; + goal?: Goal; + goals: Goal[]; + onClose: () => void; + ownerDataState?: "ready" | "loading" | "error"; +} + +const goalLevels: { value: GoalLevel; label: string }[] = [ + { value: "company", label: "Company" }, + { value: "department", label: "Department" }, + { value: "team", label: "Team" }, + { value: "individual", label: "Individual" }, +]; + +const goalStatuses: { value: GoalStatus; label: string }[] = [ + { value: "draft", label: "Draft" }, + { value: "active", label: "Active" }, + { value: "completed", label: "Completed" }, + { value: "cancelled", label: "Cancelled" }, +]; + +const goalLevelRank: Record = { + company: 0, + department: 1, + team: 2, + individual: 3, +}; + +const goalSchema = z.object({ + title: z.string().trim().min(1, "Enter a goal title.").max(500, "Use 500 characters or fewer."), + description: z.string().max(5000, "Use 5,000 characters or fewer."), + level: z.enum(["company", "department", "team", "individual"]), + status: z.enum(["draft", "active", "completed", "cancelled"]), + parentId: z.union([z.literal(""), z.uuid("Choose a valid parent goal.")]), + ownerAgentId: z.union([z.literal(""), z.uuid("Choose a valid owner.")]), + progress: z.preprocess( + (value) => value === "" ? undefined : value, + z.coerce.number({ error: "Enter progress from 0 to 100." }) + .int("Use a whole percentage.") + .min(0, "Enter progress from 0 to 100.") + .max(100, "Enter progress from 0 to 100."), + ), +}); + +type GoalFormErrors = Partial, string>>; + +function FieldError({ id, message }: { id: string; message?: string }) { + if (!message) return null; + return ( + + ); +} + +function collectDescendantIds(goals: Goal[], rootId: string) { + const descendants = new Set(); + const queue = [rootId]; + while (queue.length > 0) { + const parentId = queue.shift()!; + for (const goal of goals) { + if (goal.parentId === parentId && !descendants.has(goal.id)) { + descendants.add(goal.id); + queue.push(goal.id); + } + } + } + return descendants; +} + +function childLevel(parent?: Goal): GoalLevel | null { + if (!parent) return "company"; + return { + company: "department", + department: "team", + team: "individual", + individual: null, + }[parent.level] as GoalLevel | null; +} + +export function GoalFormModal({ + agents, + companyId, + defaultParentId, + goal, + goals, + onClose, + ownerDataState = "ready", +}: GoalFormModalProps) { + const defaultParent = goals.find((item) => item.id === defaultParentId); + const [title, setTitle] = useState(goal?.title ?? ""); + const [description, setDescription] = useState(goal?.description ?? ""); + const [level, setLevel] = useState(goal?.level ?? childLevel(defaultParent) ?? "individual"); + const [status, setStatus] = useState(goal?.status ?? "draft"); + const [parentId, setParentId] = useState(goal?.parentId ?? defaultParentId ?? ""); + const [ownerAgentId, setOwnerAgentId] = useState(goal?.ownerAgentId ?? ""); + const [progress, setProgress] = useState(String(goal?.progress ?? 0)); + const [errors, setErrors] = useState({}); + const [submitError, setSubmitError] = useState(null); + const createMutation = useCreateGoal(companyId); + const updateMutation = useUpdateGoal(companyId); + const isPending = createMutation.isPending || updateMutation.isPending; + + const unavailableParents = goal ? collectDescendantIds(goals, goal.id) : new Set(); + if (goal) unavailableParents.add(goal.id); + const parentOptions = [ + { value: "", label: "No parent (root goal)" }, + ...goals + .filter( + (item) => !unavailableParents.has(item.id) + && goalLevelRank[item.level] < goalLevelRank[level], + ) + .map((item) => ({ value: item.id, label: item.title })), + ]; + const currentOwnerIsUnavailable = Boolean( + goal?.ownerAgentId && !agents.some((agent) => agent.id === goal.ownerAgentId), + ); + const ownerOptions = [ + { value: "", label: "Unassigned" }, + ...(currentOwnerIsUnavailable && goal?.ownerAgentId + ? [{ + value: goal.ownerAgentId, + label: ownerDataState === "loading" ? "Current owner (loading)" : "Current owner (unavailable)", + }] + : []), + ...agents.map((agent) => ({ value: agent.id, label: agent.name })), + ]; + + function handleSubmit(event: React.FormEvent) { + event.preventDefault(); + setSubmitError(null); + + const parsed = goalSchema.safeParse({ + title, + description, + level, + status, + parentId, + ownerAgentId, + progress, + }); + if (!parsed.success) { + const fieldErrors = parsed.error.flatten().fieldErrors; + setErrors({ + title: fieldErrors.title?.[0], + description: fieldErrors.description?.[0], + level: fieldErrors.level?.[0], + status: fieldErrors.status?.[0], + parentId: fieldErrors.parentId?.[0], + ownerAgentId: fieldErrors.ownerAgentId?.[0], + progress: fieldErrors.progress?.[0], + }); + return; + } + + if (parsed.data.parentId) { + const selectedParent = goals.find((item) => item.id === parsed.data.parentId); + if ( + !selectedParent + || goalLevelRank[parsed.data.level] <= goalLevelRank[selectedParent.level] + ) { + setErrors({ parentId: "Choose a parent above this goal's level." }); + return; + } + } + + setErrors({}); + const sharedData = { + title: parsed.data.title, + description: parsed.data.description.trim(), + level: parsed.data.level, + status: parsed.data.status, + parentId: parsed.data.parentId || null, + ownerAgentId: parsed.data.ownerAgentId || null, + progress: parsed.data.progress, + }; + const callbacks = { + onSuccess: onClose, + onError: (error: Error) => { + setSubmitError(error.message || "Goal changes could not be saved. Try again."); + }, + }; + + if (goal) { + updateMutation.mutate( + { + goalId: goal.id, + data: { ...sharedData, description: sharedData.description || null }, + }, + callbacks, + ); + } else { + createMutation.mutate( + { + ...sharedData, + description: sharedData.description || undefined, + }, + callbacks, + ); + } + } + + return ( + +
+
+ setTitle(event.target.value)} + aria-invalid={!!errors.title} + aria-describedby={errors.title ? "goal-title-error" : undefined} + autoFocus + disabled={isPending} + maxLength={500} + required + /> + +
+ +
+