mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
Land checklist sub-issue QA fixes
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
bfd94d2a93
commit
0fe232b3b0
7 files changed
+232
-12
No files matched your search
@@ -895,6 +895,66 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("includes blockedBy summaries on list rows in one batched pass", async () => {
|
||||
const companyId = randomUUID();
|
||||
const blockerId = randomUUID();
|
||||
const blockedId = randomUUID();
|
||||
const unblockedId = randomUUID();
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
|
||||
await db.insert(issues).values([
|
||||
{
|
||||
id: blockerId,
|
||||
companyId,
|
||||
title: "Blocker issue",
|
||||
status: "todo",
|
||||
priority: "high",
|
||||
},
|
||||
{
|
||||
id: blockedId,
|
||||
companyId,
|
||||
title: "Blocked issue",
|
||||
status: "blocked",
|
||||
priority: "medium",
|
||||
},
|
||||
{
|
||||
id: unblockedId,
|
||||
companyId,
|
||||
title: "Unblocked issue",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
},
|
||||
]);
|
||||
|
||||
await db.insert(issueRelations).values({
|
||||
companyId,
|
||||
issueId: blockerId,
|
||||
relatedIssueId: blockedId,
|
||||
type: "blocks",
|
||||
});
|
||||
|
||||
const result = await svc.list(companyId);
|
||||
const byId = new Map(result.map((issue) => [issue.id, issue]));
|
||||
|
||||
expect(byId.get(blockedId)?.blockedBy).toEqual([
|
||||
expect.objectContaining({
|
||||
id: blockerId,
|
||||
identifier: null,
|
||||
title: "Blocker issue",
|
||||
status: "todo",
|
||||
priority: "high",
|
||||
}),
|
||||
]);
|
||||
expect(byId.get(blockerId)?.blockedBy).toEqual([]);
|
||||
expect(byId.get(unblockedId)?.blockedBy).toEqual([]);
|
||||
});
|
||||
|
||||
it("trims list payload fields that can grow large on issue index routes", async () => {
|
||||
const companyId = randomUUID();
|
||||
const issueId = randomUUID();
|
||||
|
||||
@@ -1296,6 +1296,62 @@ async function lastActivityStatsForIssues(
|
||||
return [...byIssueId.values()];
|
||||
}
|
||||
|
||||
async function blockedByMapForIssues(
|
||||
dbOrTx: any,
|
||||
companyId: string,
|
||||
issueIds: string[],
|
||||
): Promise<Map<string, IssueRelationIssueSummary[]>> {
|
||||
const map = new Map<string, IssueRelationIssueSummary[]>();
|
||||
const uniqueIssueIds = [...new Set(issueIds)];
|
||||
for (const issueId of uniqueIssueIds) {
|
||||
map.set(issueId, []);
|
||||
}
|
||||
if (uniqueIssueIds.length === 0) return map;
|
||||
|
||||
for (const issueIdChunk of chunkList(uniqueIssueIds, ISSUE_LIST_RELATED_QUERY_CHUNK_SIZE)) {
|
||||
const rows = await dbOrTx
|
||||
.select({
|
||||
currentIssueId: issueRelations.relatedIssueId,
|
||||
relatedId: issues.id,
|
||||
identifier: issues.identifier,
|
||||
title: issues.title,
|
||||
status: issues.status,
|
||||
priority: issues.priority,
|
||||
assigneeAgentId: issues.assigneeAgentId,
|
||||
assigneeUserId: issues.assigneeUserId,
|
||||
})
|
||||
.from(issueRelations)
|
||||
.innerJoin(issues, eq(issueRelations.issueId, issues.id))
|
||||
.where(
|
||||
and(
|
||||
eq(issueRelations.companyId, companyId),
|
||||
eq(issueRelations.type, "blocks"),
|
||||
inArray(issueRelations.relatedIssueId, issueIdChunk),
|
||||
),
|
||||
);
|
||||
|
||||
for (const row of rows) {
|
||||
const blockedBy = map.get(row.currentIssueId);
|
||||
if (!blockedBy) continue;
|
||||
blockedBy.push({
|
||||
id: row.relatedId,
|
||||
identifier: row.identifier,
|
||||
title: row.title,
|
||||
status: row.status as IssueRelationIssueSummary["status"],
|
||||
priority: row.priority as IssueRelationIssueSummary["priority"],
|
||||
assigneeAgentId: row.assigneeAgentId,
|
||||
assigneeUserId: row.assigneeUserId,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
for (const blockedBy of map.values()) {
|
||||
blockedBy.sort((a, b) => a.title.localeCompare(b.title));
|
||||
}
|
||||
|
||||
return map;
|
||||
}
|
||||
|
||||
export function issueService(db: Db) {
|
||||
const instanceSettings = instanceSettingsService(db);
|
||||
const treeControlSvc = issueTreeControlService(db);
|
||||
@@ -1914,7 +1970,7 @@ export function issueService(db: Db) {
|
||||
}
|
||||
|
||||
const issueIds = withRuns.map((row) => row.id);
|
||||
const [statsRows, readRows, lastActivityRows] = await Promise.all([
|
||||
const [statsRows, readRows, lastActivityRows, blockedByMap] = await Promise.all([
|
||||
contextUserId
|
||||
? userCommentStatsForIssues(db, companyId, contextUserId, issueIds)
|
||||
: Promise.resolve([]),
|
||||
@@ -1922,6 +1978,7 @@ export function issueService(db: Db) {
|
||||
? userReadStatsForIssues(db, companyId, contextUserId, issueIds)
|
||||
: Promise.resolve([]),
|
||||
lastActivityStatsForIssues(db, companyId, issueIds),
|
||||
blockedByMapForIssues(db, companyId, issueIds),
|
||||
]);
|
||||
const statsByIssueId = new Map(statsRows.map((row) => [row.issueId, row]));
|
||||
const lastActivityByIssueId = new Map(lastActivityRows.map((row) => [row.issueId, row]));
|
||||
@@ -1937,6 +1994,7 @@ export function issueService(db: Db) {
|
||||
) ?? row.updatedAt;
|
||||
return {
|
||||
...row,
|
||||
blockedBy: blockedByMap.get(row.id) ?? [],
|
||||
lastActivityAt,
|
||||
...(blockerAttentionByIssueId.has(row.id) ? { blockerAttention: blockerAttentionByIssueId.get(row.id) } : {}),
|
||||
};
|
||||
@@ -1954,6 +2012,7 @@ export function issueService(db: Db) {
|
||||
) ?? row.updatedAt;
|
||||
return {
|
||||
...row,
|
||||
blockedBy: blockedByMap.get(row.id) ?? [],
|
||||
lastActivityAt,
|
||||
...(blockerAttentionByIssueId.has(row.id) ? { blockerAttention: blockerAttentionByIssueId.get(row.id) } : {}),
|
||||
...deriveIssueUserContext(row, contextUserId, {
|
||||
|
||||
@@ -24,7 +24,7 @@ interface IssueRowProps {
|
||||
trailingMeta?: ReactNode;
|
||||
titleSuffix?: ReactNode;
|
||||
titleClassName?: string;
|
||||
checklistStepNumber?: number | null;
|
||||
checklistStepNumber?: number | string | null;
|
||||
checklistCurrentStep?: boolean;
|
||||
checklistDependencyChips?: ReactNode;
|
||||
checklistRowId?: string;
|
||||
@@ -84,7 +84,7 @@ export function IssueRow({
|
||||
>
|
||||
{hasChecklistStep ? (
|
||||
<span
|
||||
className="inline-block w-4 shrink-0 self-center text-right font-mono text-[10px] text-muted-foreground sm:w-8 sm:text-xs"
|
||||
className="inline-block w-7 shrink-0 self-center text-right font-mono text-[10px] text-muted-foreground sm:w-10 sm:text-xs"
|
||||
aria-hidden="true"
|
||||
>
|
||||
{checklistStepNumber}.
|
||||
|
||||
@@ -101,7 +101,7 @@ vi.mock("./IssueRow", () => ({
|
||||
desktopMetaLeading?: ReactNode;
|
||||
desktopTrailing?: ReactNode;
|
||||
titleClassName?: string;
|
||||
checklistStepNumber?: number | null;
|
||||
checklistStepNumber?: number | string | null;
|
||||
checklistCurrentStep?: boolean;
|
||||
checklistDependencyChips?: ReactNode;
|
||||
checklistRowId?: string;
|
||||
@@ -414,10 +414,17 @@ describe("IssuesList", () => {
|
||||
status: "blocked",
|
||||
createdAt: new Date("2026-04-03T00:00:00.000Z"),
|
||||
});
|
||||
const cancelledIssue = createIssue({
|
||||
id: "issue-cancelled",
|
||||
identifier: "PAP-4",
|
||||
title: "Cancelled follow-up",
|
||||
status: "cancelled",
|
||||
createdAt: new Date("2026-04-04T00:00:00.000Z"),
|
||||
});
|
||||
|
||||
const { root } = renderWithQueryClient(
|
||||
<IssuesList
|
||||
issues={[blockedIssue, nextIssue, doneIssue]}
|
||||
issues={[cancelledIssue, blockedIssue, nextIssue, doneIssue]}
|
||||
agents={[]}
|
||||
projects={[]}
|
||||
viewStateKey="paperclip:test-issues"
|
||||
@@ -439,6 +446,7 @@ describe("IssuesList", () => {
|
||||
expect(container.textContent).toContain("Next up");
|
||||
const link = container.querySelector('a[href="/issues/PAP-2"]');
|
||||
expect(link?.textContent).toContain("Implement next slice");
|
||||
expect(container.querySelector('[title="Cancelled: 1"]')).toBeNull();
|
||||
});
|
||||
|
||||
act(() => {
|
||||
@@ -497,6 +505,66 @@ describe("IssuesList", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("uses hierarchical checklist step numbers when nested rows render inline", async () => {
|
||||
const firstRoot = createIssue({
|
||||
id: "issue-first-root",
|
||||
identifier: "PAP-1",
|
||||
title: "First root",
|
||||
status: "done",
|
||||
createdAt: new Date("2026-04-01T00:00:00.000Z"),
|
||||
});
|
||||
const parent = createIssue({
|
||||
id: "issue-parent",
|
||||
identifier: "PAP-2",
|
||||
title: "Parent slice",
|
||||
status: "todo",
|
||||
createdAt: new Date("2026-04-02T00:00:00.000Z"),
|
||||
});
|
||||
const nextRoot = createIssue({
|
||||
id: "issue-next-root",
|
||||
identifier: "PAP-3",
|
||||
title: "Next root",
|
||||
status: "todo",
|
||||
createdAt: new Date("2026-04-03T00:00:00.000Z"),
|
||||
});
|
||||
const grandchild = createIssue({
|
||||
id: "issue-grandchild",
|
||||
identifier: "PAP-4",
|
||||
title: "Nested cancelled cleanup",
|
||||
status: "cancelled",
|
||||
parentId: "issue-parent",
|
||||
createdAt: new Date("2026-04-04T00:00:00.000Z"),
|
||||
});
|
||||
|
||||
const { root } = renderWithQueryClient(
|
||||
<IssuesList
|
||||
issues={[grandchild, nextRoot, firstRoot, parent]}
|
||||
agents={[]}
|
||||
projects={[]}
|
||||
viewStateKey="paperclip:test-issues"
|
||||
defaultSortField="workflow"
|
||||
onUpdateIssue={() => undefined}
|
||||
/>,
|
||||
container,
|
||||
);
|
||||
|
||||
await waitForAssertion(() => {
|
||||
const rows = Array.from(container.querySelectorAll('[data-testid="issue-row"]'));
|
||||
expect(rows).toHaveLength(4);
|
||||
expect(rows.map((row) => row.textContent)).toEqual([
|
||||
expect.stringContaining("First root"),
|
||||
expect.stringContaining("Parent slice"),
|
||||
expect.stringContaining("Nested cancelled cleanup"),
|
||||
expect.stringContaining("Next root"),
|
||||
]);
|
||||
expect(rows.map((row) => row.getAttribute("data-step"))).toEqual(["1", "2", "2.1", "3"]);
|
||||
});
|
||||
|
||||
act(() => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
it("hides the sub-issue progress summary unless it is enabled and populated", async () => {
|
||||
const { root } = renderWithQueryClient(
|
||||
<IssuesList
|
||||
|
||||
@@ -240,6 +240,35 @@ function isActionableWorkflowStatus(status: IssueStatus): boolean {
|
||||
return status !== "done" && status !== "cancelled" && status !== "blocked";
|
||||
}
|
||||
|
||||
function buildChecklistStepNumberMap(issues: Issue[], nestingEnabled: boolean): Map<string, string> {
|
||||
const stepNumberByIssueId = new Map<string, string>();
|
||||
|
||||
if (!nestingEnabled) {
|
||||
issues.forEach((issue, index) => {
|
||||
stepNumberByIssueId.set(issue.id, String(index + 1));
|
||||
});
|
||||
return stepNumberByIssueId;
|
||||
}
|
||||
|
||||
const { roots, childMap } = buildIssueTree(issues);
|
||||
const visit = (siblings: Issue[], prefix: string | null) => {
|
||||
siblings.forEach((issue, index) => {
|
||||
const stepNumber = prefix ? `${prefix}.${index + 1}` : String(index + 1);
|
||||
stepNumberByIssueId.set(issue.id, stepNumber);
|
||||
visit(childMap.get(issue.id) ?? [], stepNumber);
|
||||
});
|
||||
};
|
||||
visit(roots, null);
|
||||
|
||||
issues.forEach((issue, index) => {
|
||||
if (!stepNumberByIssueId.has(issue.id)) {
|
||||
stepNumberByIssueId.set(issue.id, String(index + 1));
|
||||
}
|
||||
});
|
||||
|
||||
return stepNumberByIssueId;
|
||||
}
|
||||
|
||||
/* ── Component ── */
|
||||
|
||||
interface Agent {
|
||||
@@ -413,6 +442,8 @@ function SubIssueProgressSummaryStrip({
|
||||
<span>{targetIssue.title}</span>
|
||||
</Link>
|
||||
</>
|
||||
) : summary.totalCount === 0 ? (
|
||||
<div className="text-sm font-medium text-foreground">No active sub-issues</div>
|
||||
) : summary.doneCount === summary.totalCount ? (
|
||||
<div className="text-sm font-medium text-foreground">All sub-issues done</div>
|
||||
) : (
|
||||
@@ -825,11 +856,10 @@ export function IssuesList({
|
||||
if (!checklistAffordanceEnabled) return null;
|
||||
|
||||
const visibleIssueIds = new Set(filtered.map((issue) => issue.id));
|
||||
const stepNumberByIssueId = new Map<string, number>();
|
||||
const stepNumberByIssueId = buildChecklistStepNumberMap(filtered, viewState.nestingEnabled);
|
||||
const unresolvedVisibleBlockersByIssueId = new Map<string, string[]>();
|
||||
|
||||
filtered.forEach((issue, index) => {
|
||||
stepNumberByIssueId.set(issue.id, index + 1);
|
||||
filtered.forEach((issue) => {
|
||||
const unresolvedVisible = (issue.blockedBy ?? [])
|
||||
.map((blocker) => blocker.id)
|
||||
.filter((blockerId) => {
|
||||
@@ -849,7 +879,7 @@ export function IssuesList({
|
||||
unresolvedVisibleBlockersByIssueId,
|
||||
currentStepIssueId: currentStepIssue?.id ?? null,
|
||||
};
|
||||
}, [checklistAffordanceEnabled, filtered, issueById]);
|
||||
}, [checklistAffordanceEnabled, filtered, issueById, viewState.nestingEnabled]);
|
||||
|
||||
const { data: labels } = useQuery({
|
||||
queryKey: queryKeys.issues.labels(selectedCompanyId!),
|
||||
|
||||
@@ -53,6 +53,7 @@ describe("buildSubIssueProgressSummary", () => {
|
||||
issue("1", "done", "2026-04-01T00:00:00.000Z"),
|
||||
issue("2", "in_progress", "2026-04-02T00:00:00.000Z", ["1"]),
|
||||
issue("4", "blocked", "2026-04-04T00:00:00.000Z"),
|
||||
issue("5", "cancelled", "2026-04-05T00:00:00.000Z"),
|
||||
]);
|
||||
|
||||
expect(summary.totalCount).toBe(4);
|
||||
@@ -60,6 +61,7 @@ describe("buildSubIssueProgressSummary", () => {
|
||||
expect(summary.inProgressCount).toBe(1);
|
||||
expect(summary.blockedCount).toBe(1);
|
||||
expect(summary.countsByStatus.todo).toBe(1);
|
||||
expect(summary.countsByStatus.cancelled).toBeUndefined();
|
||||
expect(summary.target?.kind).toBe("next");
|
||||
expect(summary.target?.issue.id).toBe("2");
|
||||
});
|
||||
|
||||
@@ -27,11 +27,12 @@ export function shouldRenderSubIssueProgressSummary(enabled: boolean | undefined
|
||||
|
||||
export function buildSubIssueProgressSummary(issues: Issue[]): SubIssueProgressSummary {
|
||||
const countsByStatus: Partial<Record<IssueStatus, number>> = {};
|
||||
for (const issue of issues) {
|
||||
const progressIssues = issues.filter((issue) => issue.status !== "cancelled");
|
||||
for (const issue of progressIssues) {
|
||||
countsByStatus[issue.status] = (countsByStatus[issue.status] ?? 0) + 1;
|
||||
}
|
||||
|
||||
const orderedIssues = workflowSort(issues);
|
||||
const orderedIssues = workflowSort(progressIssues);
|
||||
const nextIssue = orderedIssues.find((issue) => isActionableStatus(issue.status)) ?? null;
|
||||
const remainingIssues = orderedIssues.filter((issue) => !isTerminalStatus(issue.status));
|
||||
const blockedIssue =
|
||||
@@ -40,7 +41,7 @@ export function buildSubIssueProgressSummary(issues: Issue[]): SubIssueProgressS
|
||||
: null;
|
||||
|
||||
return {
|
||||
totalCount: issues.length,
|
||||
totalCount: progressIssues.length,
|
||||
doneCount: countsByStatus.done ?? 0,
|
||||
inProgressCount: countsByStatus.in_progress ?? 0,
|
||||
blockedCount: countsByStatus.blocked ?? 0,
|
||||
|
||||
Reference in new issue
Block a user