mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-22 16:51:28 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents coordinate through the server API. They find sub-tasks by filtering the company issues list by parent. > - `GET /api/companies/:companyId/issues` accepts `?parentId=`. Many callers send `?parentIssueId=` instead, which the handler never read. > - The mismatch is silent. The filter is dropped and the full company list comes back, so agents fetch everything and filter client-side. Issue #3846 reports this. > - `parentIssueId` is not an arbitrary spelling. It is the field name the wakeup payloads in this same route file already use, so callers expect it. > - This pull request accepts `parentIssueId` as an alias for `parentId` at the route boundary, on both the issues list and `issues/count`. > - The benefit is that parent filtering works for both spellings, and the list and its count cannot disagree. ## Linked Issues or Issue Description Fixes #3846 Related: #3870 proposes the same alias for the list route. ## What Changed - `server/src/routes/issues.ts`: `listFilters.parentId` in `GET /companies/:companyId/issues` now reads `req.query.parentId ?? req.query.parentIssueId`. - `server/src/routes/issues.ts`: `blockedCountFilters.parentId` in `GET /companies/:companyId/issues/count` reads the same alias, so the list and its count agree. - `server/src/__tests__/issues-parent-id-alias.test.ts`: new regression test for alias resolution, precedence, and absence. ## Verification - Run `pnpm run test:run -- server/src/__tests__/issues-parent-id-alias.test.ts`. - The test covers four query shapes: `?parentId=`, `?parentIssueId=`, both present (short form wins), and neither present (filter unset). - Existing callers are unaffected. The UI client `ui/src/api/issues.ts` only sets `parentId`. Nullish coalescing falls back only when the primary key is absent. - The service layer applies the filter with `if (filters?.parentId)` in `server/src/services/issues.ts`. This pull request does not change it. ## Risks - Low risk. The change only widens accepted query input. Both spellings resolve, and the short form still wins. - `?parentId=` with an empty value stays falsy and unfiltered, exactly as before. - This route has no validation middleware, and these list filters are not in the published OpenAPI surface. No contract needs an update. ## Model Used - Claude Opus 5 (`claude-opus-5`), extended thinking with tool use, run by the maintainer's triage agent. It rebased the original commit onto current `master`, extended the alias to `issues/count`, and wrote the regression test. @scokeepa authored the original one-line route change. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: josangmun <cmeia.ai02@cmeia.co.kr> Co-authored-by: Andrew Aymeloglu <aaymeloglu@gmail.com>
117 lines
4.0 KiB
TypeScript
117 lines
4.0 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import request from "supertest";
|
|
import { expect, it } from "vitest";
|
|
import { issues } from "@paperclipai/db";
|
|
import { issueRoutes } from "../routes/issues.js";
|
|
import {
|
|
describeEmbeddedPostgres,
|
|
resetCompanyIssueFixtures,
|
|
routeApp,
|
|
seedCompanyWithBoardAccess,
|
|
useEmbeddedPostgres,
|
|
} from "./helpers/route-test-harness.js";
|
|
|
|
describeEmbeddedPostgres("issue list parentIssueId query alias", () => {
|
|
const ctx = useEmbeddedPostgres("paperclip-issues-parent-id-alias-", {
|
|
resetEach: resetCompanyIssueFixtures,
|
|
});
|
|
|
|
async function seed() {
|
|
const company = await seedCompanyWithBoardAccess(ctx.db, "Parent alias");
|
|
const companyId = company.companyId;
|
|
const parentId = randomUUID();
|
|
const otherParentId = randomUUID();
|
|
const childId = randomUUID();
|
|
const blockedChildId = randomUUID();
|
|
|
|
await ctx.db.insert(issues).values([
|
|
{ id: parentId, companyId, title: "Parent", status: "todo", priority: "medium" },
|
|
{ id: otherParentId, companyId, title: "Other parent", status: "todo", priority: "medium" },
|
|
]);
|
|
await ctx.db.insert(issues).values([
|
|
{ id: childId, companyId, title: "Child", status: "todo", priority: "medium", parentId },
|
|
{ id: blockedChildId, companyId, title: "Blocked child", status: "blocked", priority: "medium", parentId },
|
|
{
|
|
id: randomUUID(),
|
|
companyId,
|
|
title: "Other child",
|
|
status: "todo",
|
|
priority: "medium",
|
|
parentId: otherParentId,
|
|
},
|
|
{
|
|
id: randomUUID(),
|
|
companyId,
|
|
title: "Blocked other child",
|
|
status: "blocked",
|
|
priority: "medium",
|
|
parentId: otherParentId,
|
|
},
|
|
]);
|
|
|
|
return { ...company, parentId, otherParentId, childId, blockedChildId };
|
|
}
|
|
|
|
type Seeded = Awaited<ReturnType<typeof seed>>;
|
|
|
|
function appFor(seeded: Seeded) {
|
|
return routeApp(ctx.db, seeded.actor, issueRoutes);
|
|
}
|
|
|
|
async function listIds(seeded: Seeded, query: Record<string, string>) {
|
|
const res = await request(appFor(seeded))
|
|
.get(`/api/companies/${seeded.companyId}/issues`)
|
|
.query(query)
|
|
.expect(200);
|
|
return (res.body as { id: string }[]).map((issue) => issue.id).sort();
|
|
}
|
|
|
|
async function blockedCount(seeded: Seeded, query: Record<string, string>) {
|
|
const res = await request(appFor(seeded))
|
|
.get(`/api/companies/${seeded.companyId}/issues/count`)
|
|
.query({ attention: "blocked", ...query })
|
|
.expect(200);
|
|
return res.body as { count: number };
|
|
}
|
|
|
|
function childrenOfParent(seeded: Seeded) {
|
|
return [seeded.childId, seeded.blockedChildId].sort();
|
|
}
|
|
|
|
it("filters children by ?parentId=", async () => {
|
|
const seeded = await seed();
|
|
expect(await listIds(seeded, { parentId: seeded.parentId })).toEqual(childrenOfParent(seeded));
|
|
});
|
|
|
|
it("filters children by ?parentIssueId=", async () => {
|
|
const seeded = await seed();
|
|
expect(await listIds(seeded, { parentIssueId: seeded.parentId })).toEqual(childrenOfParent(seeded));
|
|
});
|
|
|
|
it("prefers ?parentId= when both spellings are present", async () => {
|
|
const seeded = await seed();
|
|
const ids = await listIds(seeded, {
|
|
parentId: seeded.parentId,
|
|
parentIssueId: seeded.otherParentId,
|
|
});
|
|
expect(ids).toEqual(childrenOfParent(seeded));
|
|
});
|
|
|
|
it("returns the whole company list when neither spelling is present", async () => {
|
|
const seeded = await seed();
|
|
expect(await listIds(seeded, {})).toHaveLength(6);
|
|
});
|
|
|
|
it("filters the blocked count by ?parentIssueId=", async () => {
|
|
const seeded = await seed();
|
|
expect(await blockedCount(seeded, { parentIssueId: seeded.parentId })).toEqual({ count: 1 });
|
|
});
|
|
|
|
it("returns the same blocked count for both spellings", async () => {
|
|
const seeded = await seed();
|
|
const byShortForm = await blockedCount(seeded, { parentId: seeded.parentId });
|
|
const byAlias = await blockedCount(seeded, { parentIssueId: seeded.parentId });
|
|
expect(byAlias).toEqual(byShortForm);
|
|
});
|
|
});
|