forked from farhoodlabs/paperclip
fix: count all descendants in collapsed badge and prune stale localStorage IDs
Address two Greptile review comments: 1. Collapsed parent badge now shows total descendant count at all depths rather than direct-child count only. Add `countDescendants` utility to issue-tree.ts (recursive, uses existing childMap) and replace `children.length` with it in the titleSuffix badge. 2. Add a useEffect that prunes stale IDs from `collapsedParents` whenever the issues prop changes. Deleted or reassigned issues previously left orphan IDs in localStorage indefinitely; the effect filters to only IDs that appear as a parentId in the current issue list and persists the cleaned array via updateView. Add four unit tests for countDescendants: leaf node, single-level, multi-level, and unknown ID. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -23,7 +23,7 @@ import { Checkbox } from "@/components/ui/checkbox";
|
||||
import { Collapsible, CollapsibleTrigger, CollapsibleContent } from "@/components/ui/collapsible";
|
||||
import { CircleDot, Plus, Filter, ArrowUpDown, Layers, Check, X, ChevronRight, List, Columns3, User, Search } from "lucide-react";
|
||||
import { KanbanBoard } from "./KanbanBoard";
|
||||
import { buildIssueTree } from "../lib/issue-tree";
|
||||
import { buildIssueTree, countDescendants } from "../lib/issue-tree";
|
||||
import type { Issue } from "@paperclipai/shared";
|
||||
|
||||
/* ── Helpers ── */
|
||||
@@ -249,6 +249,29 @@ export function IssuesList({
|
||||
return next;
|
||||
});
|
||||
}, [scopedKey]);
|
||||
|
||||
// Prune stale IDs from collapsedParents whenever the issue list changes.
|
||||
// Deleted or reassigned issues leave orphan IDs in localStorage; this keeps
|
||||
// the stored array bounded to only current parent IDs.
|
||||
useEffect(() => {
|
||||
const parentIds = new Set(issues.map((i) => i.parentId).filter(Boolean) as string[]);
|
||||
const pruned = viewState.collapsedParents.filter((id) => parentIds.has(id));
|
||||
if (pruned.length !== viewState.collapsedParents.length) {
|
||||
updateView({ collapsedParents: pruned });
|
||||
}
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [issues]);
|
||||
|
||||
const { data: searchedIssues = [] } = useQuery({
|
||||
queryKey: [
|
||||
...queryKeys.issues.search(selectedCompanyId!, normalizedIssueSearch, projectId),
|
||||
searchFilters ?? {},
|
||||
],
|
||||
queryFn: () => issuesApi.list(selectedCompanyId!, { q: normalizedIssueSearch, projectId, ...searchFilters }),
|
||||
enabled: !!selectedCompanyId && normalizedIssueSearch.length > 0,
|
||||
placeholderData: (previousData) => previousData,
|
||||
});
|
||||
|
||||
const agentName = useCallback((id: string | null) => {
|
||||
if (!id || !agents) return null;
|
||||
return agents.find((a) => a.id === id)?.name ?? null;
|
||||
@@ -673,6 +696,7 @@ export function IssuesList({
|
||||
const renderIssueRow = (issue: Issue, depth: number) => {
|
||||
const children = childMap.get(issue.id) ?? [];
|
||||
const hasChildren = children.length > 0;
|
||||
const totalDescendants = hasChildren ? countDescendants(issue.id, childMap) : 0;
|
||||
const isExpanded = !viewState.collapsedParents.includes(issue.id);
|
||||
const toggleCollapse = (e: { preventDefault: () => void; stopPropagation: () => void }) => {
|
||||
e.preventDefault();
|
||||
@@ -691,7 +715,7 @@ export function IssuesList({
|
||||
issueLinkState={issueLinkState}
|
||||
titleSuffix={hasChildren && !isExpanded ? (
|
||||
<span className="ml-1.5 text-xs text-muted-foreground">
|
||||
({children.length} sub-task{children.length !== 1 ? "s" : ""})
|
||||
({totalDescendants} sub-task{totalDescendants !== 1 ? "s" : ""})
|
||||
</span>
|
||||
) : undefined}
|
||||
mobileLeading={
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { Issue } from "@paperclipai/shared";
|
||||
import { buildIssueTree } from "./issue-tree";
|
||||
import { buildIssueTree, countDescendants } from "./issue-tree";
|
||||
|
||||
function makeIssue(id: string, parentId: string | null = null): Issue {
|
||||
return {
|
||||
@@ -96,3 +96,35 @@ describe("buildIssueTree", () => {
|
||||
expect(childMap.get("p1")?.map((c) => c.id)).toEqual(["c1", "c2"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("countDescendants", () => {
|
||||
it("returns 0 for a leaf node", () => {
|
||||
const { childMap } = buildIssueTree([makeIssue("a")]);
|
||||
expect(countDescendants("a", childMap)).toBe(0);
|
||||
});
|
||||
|
||||
it("returns direct child count for a single-level parent", () => {
|
||||
const { childMap } = buildIssueTree([
|
||||
makeIssue("p"),
|
||||
makeIssue("c1", "p"),
|
||||
makeIssue("c2", "p"),
|
||||
]);
|
||||
expect(countDescendants("p", childMap)).toBe(2);
|
||||
});
|
||||
|
||||
it("counts all descendants across multiple levels", () => {
|
||||
// P → C → G1, G2 (P has 3 total descendants: C, G1, G2)
|
||||
const { childMap } = buildIssueTree([
|
||||
makeIssue("p"),
|
||||
makeIssue("c", "p"),
|
||||
makeIssue("g1", "c"),
|
||||
makeIssue("g2", "c"),
|
||||
]);
|
||||
expect(countDescendants("p", childMap)).toBe(3);
|
||||
});
|
||||
|
||||
it("returns 0 for an id not in the childMap", () => {
|
||||
const { childMap } = buildIssueTree([makeIssue("a"), makeIssue("b")]);
|
||||
expect(countDescendants("nonexistent", childMap)).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -25,3 +25,12 @@ export function buildIssueTree(items: Issue[]): IssueTree {
|
||||
}
|
||||
return { roots, childMap };
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the total number of descendants (all depths) of `id` in `childMap`.
|
||||
* Used to accurately label collapsed parent badges like "(3 sub-tasks)".
|
||||
*/
|
||||
export function countDescendants(id: string, childMap: Map<string, Issue[]>): number {
|
||||
const children = childMap.get(id) ?? [];
|
||||
return children.reduce((sum, c) => sum + 1 + countDescendants(c.id, childMap), 0);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user