diff --git a/doc/PRODUCT.md b/doc/PRODUCT.md index 70aa5810afc..99d897a9aa1 100644 --- a/doc/PRODUCT.md +++ b/doc/PRODUCT.md @@ -141,10 +141,14 @@ the latest agent that assigned it to them. If no assignment is available, it use the latest other commenter, or leaves the recipient empty. A manual recipient choice stays selected while task data refreshes. -Task reply and new-task composers submit with Enter by default; Shift+Enter -adds a new line. Settings → Experimental → **Command/Ctrl+Enter to Submit** -restores the previous behavior, where Enter adds a new line and Command/Ctrl+Enter -submits. +### Routine responsibility + +The routine Overview shows its responsible user beside Default agent. People with +`tasks:assign` can select an active company member from the name's picker. +The change creates a routine revision and applies to future automatic tasks. +Existing tasks keep their responsibility. Manual runs use the person who starts +them. Agents cannot transfer routine responsibility. + ### Task collection creation filters diff --git a/packages/shared/src/types/routine.ts b/packages/shared/src/types/routine.ts index 8190859d4f6..e5e5a93d632 100644 --- a/packages/shared/src/types/routine.ts +++ b/packages/shared/src/types/routine.ts @@ -236,6 +236,8 @@ export interface RoutineTriggerSecretMaterial { } export interface RoutineDetail extends Routine { + /** Actor-specific affordance. The update route remains authoritative. */ + canChangeResponsibleUser?: boolean; project: RoutineProjectSummary | null; assignee: RoutineAgentSummary | null; parentIssue: RoutineIssueSummary | null; diff --git a/packages/shared/src/validators/routine.test.ts b/packages/shared/src/validators/routine.test.ts index 64ea2f49f36..1501907ade5 100644 --- a/packages/shared/src/validators/routine.test.ts +++ b/packages/shared/src/validators/routine.test.ts @@ -11,6 +11,12 @@ const triggerId = "33333333-3333-4333-8333-333333333333"; const baseRevisionId = "44444444-4444-4444-8444-444444444444"; describe("routine validators", () => { + it("accepts non-UUID person IDs for responsibility, but rejects clearing it", () => { + expect(updateRoutineSchema.parse({ responsibleUserId: "person-123" })).toEqual({ responsibleUserId: "person-123" }); + expect(updateRoutineSchema.safeParse({ responsibleUserId: null }).success).toBe(false); + expect(updateRoutineSchema.safeParse({ responsibleUserId: " " }).success).toBe(false); + expect(updateRoutineSchema.parse({ title: "Title" })).not.toHaveProperty("responsibleUserId"); + }); it.each(["bearer", "app_webhook", "fireflies_hmac"])("accepts versioned routine revision snapshots with %s trigger metadata", (signingMode) => { const parsed = routineRevisionSnapshotV1Schema.parse({ version: 1, diff --git a/packages/shared/src/validators/routine.ts b/packages/shared/src/validators/routine.ts index 4e4e261f2d2..18a4125b17d 100644 --- a/packages/shared/src/validators/routine.ts +++ b/packages/shared/src/validators/routine.ts @@ -84,6 +84,7 @@ export type CreateRoutine = z.infer; export const updateRoutineSchema = objectWithoutDefaults(createRoutineSchema).partial().extend({ baseRevisionId: z.string().guid().optional().nullable(), + responsibleUserId: z.string().trim().min(1).optional(), }); export type UpdateRoutine = z.infer; diff --git a/server/src/__tests__/routines-routes.test.ts b/server/src/__tests__/routines-routes.test.ts index 5772938335a..bef72fb8046 100644 --- a/server/src/__tests__/routines-routes.test.ts +++ b/server/src/__tests__/routines-routes.test.ts @@ -529,6 +529,54 @@ describe("routine routes", () => { expect(mockRoutineService.create).not.toHaveBeenCalled(); }); + it("hides responsibility editing from people without tasks:assign and rejects their writes", async () => { + const app = await createApp({ type: "board", userId: "viewer", source: "session", isInstanceAdmin: false, companyIds: [companyId] }); + mockRoutineService.getDetail.mockResolvedValue(routine); + expect((await request(app).get(`/api/routines/${routineId}`)).body.canChangeResponsibleUser).toBe(false); + const res = await request(app).patch(`/api/routines/${routineId}`).send({ responsibleUserId: "viewer" }); + expect(res.status).toBe(403); + expect(mockRoutineService.update).not.toHaveBeenCalled(); + }); + + it.each([ + { name: "viewer", memberships: [{ companyId, membershipRole: "viewer", status: "active" }] }, + { name: "inactive member", memberships: [{ companyId, membershipRole: "operator", status: "suspended" }] }, + { name: "missing member", memberships: [] }, + ])("hides responsibility editing from a $name even with tasks:assign", async ({ memberships }) => { + const app = await createApp({ type: "board", userId: "read-only-user", source: "session", isInstanceAdmin: false, companyIds: [companyId], memberships }); + mockAccessService.canUser.mockResolvedValue(true); + mockRoutineService.getDetail.mockResolvedValue(routine); + + const detail = await request(app).get(`/api/routines/${routineId}`); + expect(detail.status).toBe(200); + expect(detail.body.canChangeResponsibleUser).toBe(false); + const res = await request(app).patch(`/api/routines/${routineId}`).send({ responsibleUserId: "operator" }); + expect(res.status).toBe(403); + expect(mockRoutineService.update).not.toHaveBeenCalled(); + }); + + it.each(["granted", "instance-admin", "local"])("allows responsibility changes for a %s operator", async (kind) => { + const app = await createApp({ type: "board", userId: "operator", source: kind === "local" ? "local_implicit" : "session", isInstanceAdmin: kind === "instance-admin", companyIds: [companyId], memberships: [{ companyId, membershipRole: kind === "granted" ? "operator" : "viewer", status: "active" }] }); + mockAccessService.canUser.mockResolvedValue(kind === "granted"); + mockRoutineService.getDetail.mockResolvedValue(routine); + mockRoutineService.update.mockResolvedValue({ ...routine, responsibleUserId: "operator", previousResponsibleUserId: "actual-predecessor" }); + expect((await request(app).get(`/api/routines/${routineId}`)).body.canChangeResponsibleUser).toBe(true); + const res = await request(app).patch(`/api/routines/${routineId}`).send({ responsibleUserId: "operator", baseRevisionId: revisionId }); + expect(res.status).toBe(200); + expect(mockRoutineService.update).toHaveBeenCalledWith(routineId, expect.objectContaining({ responsibleUserId: "operator", baseRevisionId: revisionId }), expect.objectContaining({ userId: "operator" })); + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ action: "routine.updated", details: expect.objectContaining({ responsibleUserId: "operator", previousResponsibleUserId: "actual-predecessor" }) })); + expect(res.body).not.toHaveProperty("previousResponsibleUserId"); + }); + + it("prevents even the assigned agent from transferring routine responsibility", async () => { + const app = await createApp({ type: "agent", agentId, companyId }); + mockRoutineService.getDetail.mockResolvedValue(routine); + expect((await request(app).get(`/api/routines/${routineId}`)).body.canChangeResponsibleUser).toBe(false); + const res = await request(app).patch(`/api/routines/${routineId}`).send({ responsibleUserId: "operator" }); + expect(res.status).toBe(403); + expect(mockRoutineService.update).not.toHaveBeenCalled(); + }); + it("requires tasks:assign permission to retarget a routine assignee", async () => { const app = await createApp({ type: "board", diff --git a/server/src/__tests__/routines-service.test.ts b/server/src/__tests__/routines-service.test.ts index e4f5b923cfe..8df2e140123 100644 --- a/server/src/__tests__/routines-service.test.ts +++ b/server/src/__tests__/routines-service.test.ts @@ -8,6 +8,7 @@ import { activityLog, agents, companies, + companyMemberships, companySecretBindings, companySecrets, companySecretVersions, @@ -104,6 +105,7 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { await db.delete(projectWorkspaces); await db.delete(projects); await db.delete(agents); + await db.delete(companyMemberships); await db.delete(companies); await db.delete(instanceSettings); }); @@ -1164,6 +1166,87 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { expect(serialized).not.toContain("runtimeConfig"); }); + it("versions responsibility transfers and uses the new person only for future automatic tasks", async () => { + const { companyId, routine, svc } = await seedFixture(); + const userId = "new-responsible-person"; + await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: userId, status: "active" }); + const before = await svc.runRoutine(routine.id, { source: "api" }, {}); + await db.update(issues).set({ status: "done" }).where(eq(issues.id, before.linkedIssueId!)); + const updated = await svc.update(routine.id, { + responsibleUserId: userId, + baseRevisionId: routine.latestRevisionId, + }, { userId: "operator" }); + expect(updated?.responsibleUserId).toBe(userId); + expect(updated?.latestRevisionNumber).toBe(2); + const revisions = await svc.listRevisions(routine.id); + expect(revisions[0]?.snapshot.routine.responsibleUserId).toBe(userId); + expect(revisions[1]?.snapshot.routine.responsibleUserId).toBe(routine.responsibleUserId); + const after = await svc.runRoutine(routine.id, { source: "api" }, {}); + const [oldTask] = await db.select().from(issues).where(eq(issues.id, before.linkedIssueId!)); + const [newTask] = await db.select().from(issues).where(eq(issues.id, after.linkedIssueId!)); + expect(oldTask?.responsibleUserId).toBe(routine.responsibleUserId); + expect(newTask?.responsibleUserId).toBe(userId); + const noOp = await svc.update(routine.id, { responsibleUserId: userId }, {}); + expect(noOp?.latestRevisionId).toBe(updated?.latestRevisionId); + await expect(svc.update(routine.id, { + responsibleUserId: userId, baseRevisionId: routine.latestRevisionId, + }, {})).rejects.toThrow("Routine was updated by someone else"); + const restored = await svc.restoreRevision(routine.id, routine.latestRevisionId!, {}); + expect(restored.routine.responsibleUserId).toBe(userId); + expect(restored.revision.snapshot.routine.responsibleUserId).toBe(userId); + }); + + it("returns the actual predecessor for concurrent transfers without a base revision", async () => { + const { companyId, routine, svc } = await seedFixture(); + const people = ["next-person", "last-person"]; + await db.insert(companyMemberships).values(people.map((principalId) => ({ + companyId, principalType: "user" as const, principalId, status: "active" as const, + }))); + const acquired = Promise.withResolvers(); + const release = Promise.withResolvers(); + const blocker = db.transaction(async (tx) => { + await tx.execute(sql`select id from ${routines} where ${routines.id} = ${routine.id} for update`); + acquired.resolve(); + await release.promise; + }); + await acquired.promise; + const transfers = Promise.all(people.map((responsibleUserId) => svc.update(routine.id, { responsibleUserId }, {}))); + let results; + try { + await vi.waitFor(async () => { + const waiters = await db.execute(sql`select pid from pg_stat_activity + where datname = current_database() and wait_event_type = 'Lock' + and query like 'select id from "routines"%for update'`); + expect(waiters.length).toBe(2); + }, { timeout: 5_000, interval: 20 }); + } finally { + release.resolve(); + await blocker; + results = await transfers; + } + const [first, second] = results.sort((a, b) => a!.latestRevisionNumber! - b!.latestRevisionNumber!); + expect(first?.previousResponsibleUserId).toBe(routine.responsibleUserId); + expect(second?.previousResponsibleUserId).toBe(first?.responsibleUserId); + expect((await svc.get(routine.id))?.responsibleUserId).toBe(second?.responsibleUserId); + }); + + it.each(["missing", "suspended", "archived", "other-company"])("rejects a %s responsible person", async (state) => { + const { companyId, routine, svc } = await seedFixture(); + const userId = randomUUID(); + if (state !== "missing") { + const memberCompanyId = state === "other-company" ? randomUUID() : companyId; + if (state === "other-company") await db.insert(companies).values({ id: memberCompanyId, name: "Other company", issuePrefix: "OTHER" }); + await db.insert(companyMemberships).values({ + companyId: memberCompanyId, principalType: "user", principalId: userId, + status: state === "other-company" ? "active" : state, + }); + } + await expect(svc.update(routine.id, { responsibleUserId: userId }, {})) + .rejects.toThrow("Responsible user must be an active company member"); + expect((await svc.get(routine.id))?.responsibleUserId).toBe(routine.responsibleUserId); + expect(await svc.listRevisions(routine.id)).toHaveLength(1); + }); + it("creates revision 1 on routine create and appends revisions for real updates only", async () => { const { routine, svc } = await seedFixture(); diff --git a/server/src/routes/routines.ts b/server/src/routes/routines.ts index 84587bba5e7..c798c2f4ed4 100644 --- a/server/src/routes/routines.ts +++ b/server/src/routes/routines.ts @@ -96,6 +96,17 @@ export function routineRoutes( } } + async function canChangeResponsibleUser(req: Request, companyId: string) { + if (req.actor.type !== "board") return false; + if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) return true; + // Match assertCompanyAccess's write restrictions even on this GET route. + if (Array.isArray(req.actor.memberships)) { + const membership = req.actor.memberships.find((item) => item.companyId === companyId); + if (!membership || membership.status !== "active" || membership.membershipRole === "viewer") return false; + } + return access.canUser(companyId, req.actor.userId, "tasks:assign"); + } + function assertCanManageCompanyRoutine(req: Request, companyId: string, assigneeAgentId?: string | null) { assertCompanyAccess(req, companyId); if (req.actor.type === "board") return; @@ -195,7 +206,7 @@ export function routineRoutes( router.get("/routines/:id", async (req, res) => { const detail = await getAccessibleResource(req, res, svc.getDetail(req.params.id as string), "Routine not found"); if (!detail) return; - res.json(detail); + res.json({ ...detail, canChangeResponsibleUser: await canChangeResponsibleUser(req, detail.companyId) }); }); router.get("/routines/:id/revisions", async (req, res) => { @@ -370,6 +381,12 @@ export function routineRoutes( const assigneeWillChange = req.body.assigneeAgentId !== undefined && req.body.assigneeAgentId !== routine.assigneeAgentId; + if (req.body.responsibleUserId !== undefined) { + if (req.actor.type !== "board") { + throw forbidden("Only people can change routine responsibility"); + } + await assertBoardCanAssignTasks(req, routine.companyId); + } if (assigneeWillChange) { await assertBoardCanAssignTasks(req, routine.companyId); } @@ -403,7 +420,13 @@ export function routineRoutes( action: "routine.updated", entityType: "routine", entityId: routine.id, - details: { title: updated?.title ?? routine.title }, + details: { + title: updated?.title ?? routine.title, + ...(req.body.responsibleUserId !== undefined ? { + previousResponsibleUserId: updated?.previousResponsibleUserId, + responsibleUserId: updated?.responsibleUserId, + } : {}), + }, }); if (updated && updated.latestRevisionId !== routine.latestRevisionId) { await remapRoutineDescriptionAnnotations(req, routine.id); @@ -416,7 +439,12 @@ export function routineRoutes( triggerCount: null, }); } - res.json(updated); + if (updated) { + const { previousResponsibleUserId: _previousResponsibleUserId, ...response } = updated; + res.json(response); + } else { + res.json(null); + } }); router.post("/routines/:id/revisions/:revisionId/restore", async (req, res) => { diff --git a/server/src/services/routines.ts b/server/src/services/routines.ts index d5b0b369f40..3046fdb2384 100644 --- a/server/src/services/routines.ts +++ b/server/src/services/routines.ts @@ -2279,7 +2279,7 @@ export function routineService( return createdRoutine; }, - update: async (id: string, patch: UpdateRoutine, actor: Actor): Promise => { + update: async (id: string, patch: UpdateRoutine, actor: Actor): Promise<(Routine & { previousResponsibleUserId: string | null }) | null> => { const existing = await getRoutineById(id); if (!existing) return null; const nextProjectId = patch.projectId === undefined ? existing.projectId : patch.projectId; @@ -2330,7 +2330,7 @@ export function routineService( if (enabledScheduleTriggers) { assertScheduleCompatibleVariables(nextVariables); } - const responsibleUserId = await resolveRoutineResponsibleUserId( + const responsibleUserId = patch.responsibleUserId ?? existing.responsibleUserId ?? await resolveRoutineResponsibleUserId( db, existing.companyId, actor.userId, @@ -2339,6 +2339,7 @@ export function routineService( if (!responsibleUserId) { throw unprocessable("Routine requires a responsible user"); } + let previousResponsibleUserId: string | null = null; const updatedRoutine = await db.transaction(async (tx) => { const txDb = tx as unknown as Db; await notifyDeliveryWork(txDb, DELIVERY_QUEUES.routineSchedules); @@ -2349,6 +2350,7 @@ export function routineService( .where(eq(routines.id, id)) .then((rows) => rows[0] ?? null); if (!locked) return null; + previousResponsibleUserId = locked.responsibleUserId; if (patch.baseRevisionId && patch.baseRevisionId !== locked.latestRevisionId) { throw conflict("Routine was updated by someone else", { @@ -2356,6 +2358,20 @@ export function routineService( }); } + if (patch.responsibleUserId !== undefined) { + const member = await txDb.select({ id: companyMemberships.id }) + .from(companyMemberships) + .where(and( + eq(companyMemberships.companyId, locked.companyId), + eq(companyMemberships.principalType, "user"), + eq(companyMemberships.principalId, patch.responsibleUserId), + eq(companyMemberships.status, "active"), + )) + .for("share") + .then((rows) => rows[0] ?? null); + if (!member) throw unprocessable("Responsible user must be an active company member"); + } + const candidate: RoutineRow = { ...locked, projectId: nextProjectId, @@ -2373,7 +2389,7 @@ export function routineService( activityGateScope: patch.activityGateScope ?? locked.activityGateScope, variables: nextVariables, env: nextEnv, - responsibleUserId: locked.responsibleUserId ?? responsibleUserId, + responsibleUserId: patch.responsibleUserId ?? locked.responsibleUserId ?? responsibleUserId, updatedByAgentId: actor.agentId ?? null, updatedByUserId: actor.userId ?? null, }; @@ -2459,7 +2475,7 @@ export function routineService( } return routine; }); - return updatedRoutine; + return updatedRoutine ? { ...updatedRoutine, previousResponsibleUserId } : null; }, createTrigger: async ( diff --git a/ui/src/components/RoutineOverview.test.tsx b/ui/src/components/RoutineOverview.test.tsx index 240b9eb0b36..2562a0e1100 100644 --- a/ui/src/components/RoutineOverview.test.tsx +++ b/ui/src/components/RoutineOverview.test.tsx @@ -14,6 +14,10 @@ import { const issueRowRender = vi.hoisted(() => vi.fn()); +vi.mock("./RoutineResponsibleUser", () => ({ + RoutineResponsibleUser: ({ routine }: { routine: RoutineDetail }) =>
Responsible user
, +})); + vi.mock("@/components/IssueRow", () => ({ IssueRow: (props: { issue: { title: string }; presentation?: string }) => { issueRowRender(props); diff --git a/ui/src/components/RoutineOverview.tsx b/ui/src/components/RoutineOverview.tsx index 5baf0930bb7..a4a5d84d652 100644 --- a/ui/src/components/RoutineOverview.tsx +++ b/ui/src/components/RoutineOverview.tsx @@ -17,6 +17,7 @@ import { routineDetailHref, } from "./RoutineContextualSidebar"; import { useRoutineDetail } from "./routine-sections/context"; +import { RoutineResponsibleUser } from "./RoutineResponsibleUser"; export type RoutineScheduleSummary = { label: string; @@ -186,20 +187,23 @@ export function RoutineOverview() { /> -
-

Default agent

- {currentAssignee ? ( - -
+
+
+

Default agent

+ {currentAssignee ? ( + +
+ +

Description

diff --git a/ui/src/components/RoutineResponsibleUser.test.tsx b/ui/src/components/RoutineResponsibleUser.test.tsx new file mode 100644 index 00000000000..55e75f6825e --- /dev/null +++ b/ui/src/components/RoutineResponsibleUser.test.tsx @@ -0,0 +1,147 @@ +// @vitest-environment jsdom +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider, useQuery } from "@tanstack/react-query"; +import type { RoutineDetail } from "@paperclipai/shared"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { accessApi } from "@/api/access"; +import { ApiError } from "@/api/client"; +import { routinesApi } from "@/api/routines"; +import { queryKeys } from "@/lib/queryKeys"; +import { RoutineResponsibleUser } from "./RoutineResponsibleUser"; + +const routine = { + id: "routine-1", companyId: "company-1", responsibleUserId: "user-alex", + latestRevisionId: "revision-1", canChangeResponsibleUser: true, +} as RoutineDetail; +const users = ["Alex", "Sam"].map((name) => ({ + principalId: `user-${name.toLowerCase()}`, status: "active" as const, + user: { id: `user-${name.toLowerCase()}`, name, email: `${name.toLowerCase()}@example.test`, image: null }, +})); + +describe("RoutineResponsibleUser", () => { + let root: Root; + let container: HTMLDivElement; + let client: QueryClient; + + beforeEach(() => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal("ResizeObserver", class { observe() {} unobserve() {} disconnect() {} }); + HTMLElement.prototype.scrollIntoView = vi.fn(); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + client = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false } } }); + vi.spyOn(accessApi, "listUserDirectory").mockResolvedValue({ users }); + vi.spyOn(routinesApi, "update").mockResolvedValue({ ...routine, responsibleUserId: "user-sam" }); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + client.clear(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + }); + + async function render(value = routine) { + await act(async () => root.render()); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 20)); }); + } + + async function selectSam() { + await act(async () => container.querySelector("button")!.click()); + const option = [...document.querySelectorAll('[role="option"]')].find((item) => item.textContent?.includes("Sam")); + expect(option).toBeDefined(); + await act(async () => option!.click()); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 20)); }); + } + + it("shows the person and inheritance rules to read-only viewers", async () => { + await render({ ...routine, canChangeResponsibleUser: false }); + expect(container.textContent).toContain("Alex"); + expect(container.textContent).toContain("Automatic tasks use this person. Manual runs use the person who starts them."); + expect(container.querySelector("button")).toBeNull(); + }); + + it("saves only responsibility and its base revision, then refreshes routine data", async () => { + const invalidate = vi.spyOn(client, "invalidateQueries"); + await render(); + await selectSam(); + expect(routinesApi.update).toHaveBeenCalledWith("routine-1", { responsibleUserId: "user-sam", baseRevisionId: "revision-1" }); + expect(invalidate).toHaveBeenCalledWith({ queryKey: queryKeys.routines.detail("routine-1") }); + expect(invalidate).toHaveBeenCalledWith({ queryKey: queryKeys.routines.revisions("routine-1") }); + expect(invalidate).toHaveBeenCalledWith({ queryKey: queryKeys.routines.activity("company-1", "routine-1") }); + expect(document.querySelector('[role="option"]')).toBeNull(); + }); + + it("keeps the picker disabled until the saved detail is refreshed, then uses the new revision", async () => { + let resolveRefresh!: (value: RoutineDetail) => void; + const refresh = new Promise((resolve) => { resolveRefresh = resolve; }); + const saved = { ...routine, responsibleUserId: "user-sam", latestRevisionId: "revision-2" }; + vi.mocked(routinesApi.update).mockResolvedValue(saved); + function Detail() { + const detail = useQuery({ + queryKey: queryKeys.routines.detail(routine.id), + queryFn: () => refresh, + initialData: routine, + staleTime: Infinity, + }); + return ; + } + await act(async () => root.render()); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 20)); }); + await selectSam(); + expect(container.querySelector("button")?.disabled).toBe(true); + expect(container.textContent).toContain("Saving…"); + await act(async () => resolveRefresh(saved)); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 20)); }); + expect(container.querySelector("button")?.disabled).toBe(false); + expect(container.textContent).toContain("Sam"); + await act(async () => container.querySelector("button")!.click()); + const alex = [...document.querySelectorAll('[role="option"]')].find((item) => item.textContent?.includes("Alex")); + await act(async () => alex!.click()); + expect(routinesApi.update).toHaveBeenLastCalledWith(routine.id, { responsibleUserId: "user-alex", baseRevisionId: "revision-2" }); + }); + + it("does not save when the current person is selected", async () => { + await render(); + await act(async () => container.querySelector("button")!.click()); + const option = [...document.querySelectorAll('[role="option"]')].find((item) => item.textContent?.includes("Alex")); + await act(async () => option!.click()); + expect(routinesApi.update).not.toHaveBeenCalled(); + }); + + it.each([403, 409])("keeps the saved person and shows the %s failure", async (status) => { + vi.mocked(routinesApi.update).mockRejectedValue(new ApiError("Missing permission: tasks:assign", status, null)); + await render(); + await selectSam(); + expect(container.textContent).toContain("Alex"); + expect(container.querySelector('[role="alert"]')?.textContent).toContain(status === 409 ? "Reload before changing responsibility" : "Missing permission"); + }); + + it("shows an unavailable person without inventing a name and permits repair", async () => { + await render({ ...routine, responsibleUserId: "former-user" }); + expect(container.textContent).toContain("Unavailable user"); + await selectSam(); + expect(routinesApi.update).toHaveBeenCalledWith(routine.id, expect.objectContaining({ responsibleUserId: "user-sam" })); + }); + + it("disables the picker and explains a directory failure", async () => { + vi.mocked(accessApi.listUserDirectory).mockRejectedValue(new Error("Unavailable")); + await render(); + expect(container.querySelector("button")?.disabled).toBe(true); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("Unavailable"); + expect(container.textContent).toContain("Retry"); + }); + + it("keeps a cached person visible without a local alert during a transient directory failure", async () => { + await render(); + vi.mocked(accessApi.listUserDirectory).mockRejectedValue(new ApiError("tenant_app_unavailable", 503, { error: "tenant_app_unavailable" })); + await act(async () => client.invalidateQueries({ queryKey: queryKeys.access.companyUserDirectory(routine.companyId) })); + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 20)); }); + expect(container.textContent).toContain("Alex"); + expect(container.querySelector("button")?.disabled).toBe(false); + expect(container.querySelector('[role="alert"]')).toBeNull(); + }); +}); diff --git a/ui/src/components/RoutineResponsibleUser.tsx b/ui/src/components/RoutineResponsibleUser.tsx new file mode 100644 index 00000000000..b7684f0f2cc --- /dev/null +++ b/ui/src/components/RoutineResponsibleUser.tsx @@ -0,0 +1,105 @@ +import { useState } from "react"; +import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import type { RoutineDetail } from "@paperclipai/shared"; +import { Check, ChevronDown, UserRound } from "lucide-react"; +import { accessApi } from "@/api/access"; +import { ApiError } from "@/api/client"; +import { describeError } from "@/api/errors"; +import { routinesApi } from "@/api/routines"; +import { buildCompanyUserProfileMap } from "@/lib/company-members"; +import { queryKeys } from "@/lib/queryKeys"; +import { Command, CommandEmpty, CommandInput, CommandItem, CommandList } from "./ui/command"; +import { Popover, PopoverContent, PopoverTrigger } from "./ui/popover"; +import { QueryErrorState, useQueryView } from "./QueryView"; + +export function RoutineResponsibleUser({ routine }: { routine: RoutineDetail }) { + const [open, setOpen] = useState(false); + const queryClient = useQueryClient(); + const directory = useQuery({ + queryKey: queryKeys.access.companyUserDirectory(routine.companyId), + queryFn: () => accessApi.listUserDirectory(routine.companyId), + }); + const directoryView = useQueryView(directory); + const profiles = buildCompanyUserProfileMap(directory.data?.users); + const label = routine.responsibleUserId + ? profiles.get(routine.responsibleUserId)?.label + ?? (directory.isPending ? "Loading…" : "Unavailable user") + : "No responsible user"; + const change = useMutation({ + mutationFn: (responsibleUserId: string) => routinesApi.update(routine.id, { + responsibleUserId, + baseRevisionId: routine.latestRevisionId, + }), + onSuccess: async (saved) => { + queryClient.setQueryData(queryKeys.routines.detail(routine.id), (current) => ( + current ? { ...current, ...saved } : current + )); + await Promise.all([ + queryClient.invalidateQueries({ queryKey: queryKeys.routines.detail(routine.id) }), + queryClient.invalidateQueries({ queryKey: queryKeys.routines.list(routine.companyId) }), + queryClient.invalidateQueries({ queryKey: queryKeys.routines.revisions(routine.id) }), + queryClient.invalidateQueries({ queryKey: queryKeys.routines.activity(routine.companyId, routine.id) }), + ]); + }, + }); + + return ( +
+

Responsible user

+ {routine.canChangeResponsibleUser ? ( + + + + + + + + + No people found. + {(directory.data?.users ?? []).map((member) => ( + { + setOpen(false); + if (member.principalId !== routine.responsibleUserId) change.mutate(member.principalId); + }} + > + + ))} + + + + + ) : ( +

+

+ )} +

Automatic tasks use this person. Manual runs use the person who starts them.

+ {directoryView.kind === "error" ? ( + + ) : null} + {change.error ? ( +

+ {change.error instanceof ApiError && change.error.status === 409 + ? "This routine changed. Reload before changing responsibility." + : describeError(change.error, { action: "change responsibility" }).body} +

+ ) : null} +
+ ); +}