mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 21:07:40 +01:00
fix: retain partial reviews and open immutable attachments in both interfaces
This commit is contained in:
1 parent
faa52c7bf2
commit
57902152ac
10 files changed
+82
-14
No files matched your search
@@ -5,7 +5,7 @@ import { createRoot } from "react-dom/client";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
import { issuesApi } from "../api/issues";
|
||||
import { pendingRequestConfirmationInteraction } from "../fixtures/issueThreadInteractionFixtures";
|
||||
import { pendingRequestConfirmationInteraction, pendingRequestItemVerdictsInteraction } from "../fixtures/issueThreadInteractionFixtures";
|
||||
import { ThemeProvider } from "../context/ThemeContext";
|
||||
import { TooltipProvider } from "./ui/tooltip";
|
||||
import { AttentionInteractionResolver } from "./AttentionInteractionResolver";
|
||||
@@ -17,7 +17,7 @@ vi.mock("@/lib/router", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("../api/issues", () => ({
|
||||
issuesApi: { listInteractions: vi.fn(), acceptInteraction: vi.fn() },
|
||||
issuesApi: { listInteractions: vi.fn(), acceptInteraction: vi.fn(), submitInteractionVerdicts: vi.fn() },
|
||||
}));
|
||||
|
||||
afterEach(() => {
|
||||
@@ -25,6 +25,28 @@ afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("passes the canonical pending status back after a partial item review", async () => {
|
||||
const interaction = pendingRequestItemVerdictsInteraction;
|
||||
const partial = { ...interaction, status: "pending" as const, result: { version: 1 as const, outcome: "resolved" as const,
|
||||
complete: false, items: [{ id: interaction.payload.items[0]!.id, verdict: "approve" as const, resolvedAt: new Date().toISOString() }] } };
|
||||
vi.mocked(issuesApi.listInteractions).mockResolvedValue([interaction]);
|
||||
vi.mocked(issuesApi.submitInteractionVerdicts).mockResolvedValue(partial);
|
||||
const onResolved = vi.fn();
|
||||
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
const host = document.createElement("div"); document.body.append(host); const root = createRoot(host);
|
||||
try {
|
||||
await act(async () => root.render(<QueryClientProvider client={client}><TooltipProvider><ThemeProvider>
|
||||
<AttentionInteractionResolver companyId={interaction.companyId} issueId={interaction.issueId} interactionId={interaction.id} onResolved={onResolved} />
|
||||
</ThemeProvider></TooltipProvider></QueryClientProvider>));
|
||||
await act(async () => { await new Promise(resolve => setTimeout(resolve, 0)); });
|
||||
await act(async () => host.querySelector<HTMLButtonElement>('button[data-verdict="approve"]')!.click());
|
||||
const apply = [...host.querySelectorAll("button")].find(button => button.textContent?.startsWith("Apply"));
|
||||
expect(apply).toBeTruthy();
|
||||
await act(async () => apply!.click());
|
||||
expect(onResolved).toHaveBeenCalledWith(partial);
|
||||
} finally { await act(async () => root.unmount()); host.remove(); client.clear(); }
|
||||
});
|
||||
|
||||
it("refreshes a preparing approval automatically, then stops polling without accepting it", async () => {
|
||||
vi.useFakeTimers();
|
||||
const interaction = pendingRequestConfirmationInteraction;
|
||||
|
||||
@@ -28,8 +28,8 @@ interface AttentionInteractionResolverProps {
|
||||
userLabelMap?: ReadonlyMap<string, string> | null;
|
||||
footerSlot?: ReactNode;
|
||||
presentation?: "default" | "coordinator-decision";
|
||||
/** Called after a resolution so the parent can refresh the feed. */
|
||||
onResolved?: () => void;
|
||||
/** Successful updates include partial item reviews; inspect status before settling the whole card. */
|
||||
onResolved?: (interaction?: IssueThreadInteraction) => void;
|
||||
}
|
||||
|
||||
export function replaceResolvedInteraction(
|
||||
@@ -90,7 +90,7 @@ export function AttentionInteractionResolver({
|
||||
queryClient.invalidateQueries({ queryKey: queryKeys.attention(companyId) });
|
||||
queryClient.invalidateQueries({ queryKey: ["issues", "pending-questions"] });
|
||||
queryClient.invalidateQueries({ queryKey: ["issues", "decision-receipts"] });
|
||||
onResolved?.();
|
||||
onResolved?.(resolvedInteraction);
|
||||
};
|
||||
|
||||
const acceptMutation = useMutation({
|
||||
|
||||
@@ -1148,6 +1148,16 @@ describe("IssueThreadInteractionCard", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("opens a bound attachment directly without experimental panel routing", () => {
|
||||
const host = renderCard({ interaction: { ...pendingRequestItemVerdictsInteraction,
|
||||
payload: { ...pendingRequestItemVerdictsInteraction.payload, items: [{ id: "file", label: "Reviewed file", target: {
|
||||
type: "issue_attachment", attachmentId: "immutable-file", assetId: "asset", sha256: "hash",
|
||||
} }] },
|
||||
} });
|
||||
const link = host.querySelector('a[href="/api/attachments/immutable-file/content"]');
|
||||
expect(link?.getAttribute("target")).toBe("_blank");
|
||||
});
|
||||
|
||||
it("links a bound review item to its exact document without requiring href", () => {
|
||||
const host = renderCard({ interaction: { ...pendingRequestItemVerdictsInteraction,
|
||||
payload: { ...pendingRequestItemVerdictsInteraction.payload, items: [{ id: "copy", label: "Reviewed copy", target: {
|
||||
|
||||
@@ -3243,7 +3243,7 @@ function VerdictConsequenceChip({ verdict }: { verdict: RequestItemVerdictValue
|
||||
function ItemVerdictDeepLink({ item, interaction }: { item: RequestItemVerdictsItem; interaction: RequestItemVerdictsInteraction }) {
|
||||
const href = itemVerdictHref(item, interaction);
|
||||
if (!href) return null;
|
||||
const isInternal = href.startsWith("/") || href.startsWith("#");
|
||||
const isInternal = !href.startsWith("/api/") && (href.startsWith("/") || href.startsWith("#"));
|
||||
const className =
|
||||
"inline-flex items-center gap-1 text-xs font-medium text-primary hover:underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-1";
|
||||
const label = (
|
||||
|
||||
@@ -7,7 +7,7 @@ import type { AttentionItem, DecisionHistoryItem } from "@paperclipai/shared";
|
||||
import { CoordinatorNeedsYou } from "./CoordinatorNeedsYou";
|
||||
|
||||
(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
const fixture = vi.hoisted(() => ({ pending: [] as AttentionItem[], history: [] as DecisionHistoryItem[], historyFail: false, feedFail: false, openPanel: vi.fn(), closePanel: vi.fn(), setPanelVisible: vi.fn(), navigate: vi.fn(), breadcrumbs: vi.fn() }));
|
||||
const fixture = vi.hoisted(() => ({ pending: [] as AttentionItem[], history: [] as DecisionHistoryItem[], partial: false, historyFail: false, feedFail: false, openPanel: vi.fn(), closePanel: vi.fn(), setPanelVisible: vi.fn(), navigate: vi.fn(), breadcrumbs: vi.fn() }));
|
||||
vi.mock("@/context/CompanyContext", () => ({ useCompany: () => ({ selectedCompanyId: "company" }) }));
|
||||
vi.mock("@/context/BreadcrumbContext", () => ({ useBreadcrumbs: () => ({ setBreadcrumbs: fixture.breadcrumbs }) }));
|
||||
vi.mock("@/context/PanelContext", () => ({ usePanel: () => ({ openPanel: fixture.openPanel, closePanel: fixture.closePanel, setPanelVisible: fixture.setPanelVisible }) }));
|
||||
@@ -30,11 +30,12 @@ vi.mock("@/components/StalledReviewActions", () => ({ StalledReviewActions: ({ i
|
||||
fixture.pending = fixture.pending.filter((item) => item.subject.id !== issueId);
|
||||
onResolved();
|
||||
}}>{approveLabel}</button> }));
|
||||
vi.mock("@/components/AttentionInteractionResolver", () => ({ AttentionInteractionResolver: ({ issueId, interactionId, footerSlot, onResolved }: { issueId: string; interactionId: string; footerSlot?: ReactNode; onResolved: () => void }) => <>{footerSlot}<button data-issue-id={issueId} onClick={() => {
|
||||
vi.mock("@/components/AttentionInteractionResolver", () => ({ AttentionInteractionResolver: ({ issueId, interactionId, footerSlot, onResolved }: { issueId: string; interactionId: string; footerSlot?: ReactNode; onResolved: (interaction: { status: string }) => void }) => <>{footerSlot}<button data-issue-id={issueId} onClick={() => {
|
||||
if (fixture.partial) { onResolved({ status: "pending" }); return; }
|
||||
fixture.pending = fixture.pending.filter((item) => item.subject.id !== interactionId);
|
||||
fixture.history.push({ id: `issue_thread_interaction:${interactionId}`, sourceKind: "issue_thread_interaction", sourceId: interactionId,
|
||||
issueId: `task-${interactionId}`, issueIdentifier: `PAP-${interactionId}`, issueTitle: `Task ${interactionId}`, title: "Decision", label: interactionId === "3" ? "Answers recorded" : "Approved", outcome: interactionId === "3" ? "answered" : "accepted", resolvedAt: new Date().toISOString(), resolvedByUserId: "reviewer", resolvedByAgentId: null, agentId: null, agentName: null, projectId: null, projectName: null, href: `/PAP/issues/PAP-${interactionId}#interaction-${interactionId}`, target: null });
|
||||
onResolved();
|
||||
onResolved({ status: "answered" });
|
||||
}}>Resolve {interactionId}</button></> }));
|
||||
|
||||
let root: Root;
|
||||
@@ -42,7 +43,7 @@ let container: HTMLDivElement;
|
||||
beforeEach(() => {
|
||||
fixture.pending = ["1", "2", "3"].map((id) => ({ id, sourceKind: "issue_thread_interaction", inlineResolvable: true, subject: { id, kind: "interaction", title: `Review ${id}`, metadata: { issueId: `task-${id}` } }, relatedIssue: { id: `task-${id}`, identifier: `PAP-${id}`, title: `Task ${id}`, href: `/PAP/issues/PAP-${id}` }, activityAt: new Date().toISOString() } as unknown as AttentionItem));
|
||||
fixture.history = []; vi.clearAllMocks();
|
||||
fixture.historyFail = false; fixture.feedFail = false;
|
||||
fixture.historyFail = false; fixture.feedFail = false; fixture.partial = false;
|
||||
container = document.createElement("div"); document.body.appendChild(container); root = createRoot(container);
|
||||
});
|
||||
afterEach(async () => { await act(async () => root.unmount()); container.remove(); vi.useRealTimers(); vi.restoreAllMocks(); });
|
||||
@@ -53,6 +54,19 @@ async function click(text: string) {
|
||||
await act(async () => button!.click()); await flush();
|
||||
}
|
||||
describe("Coordinator Needs You", () => {
|
||||
it("keeps a partial review actionable even when history is unavailable", async () => {
|
||||
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
await act(async () => root.render(<QueryClientProvider client={client}><CoordinatorNeedsYou /></QueryClientProvider>));
|
||||
await flush();
|
||||
fixture.partial = true; fixture.historyFail = true;
|
||||
await click("Resolve 1"); await flush();
|
||||
expect([...container.querySelectorAll("button")].some(button => button.textContent === "Resolve 1")).toBe(true);
|
||||
expect(container.querySelector('[data-motion="gather"]')).toBeNull();
|
||||
fixture.partial = false;
|
||||
await click("Resolve 1"); await flush();
|
||||
expect([...container.querySelectorAll("button")].some(button => button.textContent === "Resolve 1")).toBe(false);
|
||||
client.clear();
|
||||
});
|
||||
it.each([false, true])("does not restore a saved decision when history refresh fails (stale feed: %s)", async staleFeed => {
|
||||
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
await act(async () => root.render(<QueryClientProvider client={client}><CoordinatorNeedsYou /></QueryClientProvider>));
|
||||
|
||||
@@ -200,7 +200,10 @@ export function CoordinatorNeedsYou() {
|
||||
<CoordinatorTaskMotion eventId={`decision:${selectedCompanyId}:${userId}:${id}`} phase={phase} createdAt={item.activityAt} className="coordinator-decision-motion"
|
||||
onSettled={() => setSettling(current => current.filter(entry => `${entry.sourceKind}:${entry.subject.id}` !== id))}>
|
||||
{receipt ? <Receipt item={receipt} onOpen={navigate} /> : <article className="coordinator-decision-card space-y-4 rounded-xl border border-border bg-card p-6">
|
||||
{interaction ? <AttentionInteractionResolver footerSlot={taskLink} presentation="coordinator-decision" companyId={selectedCompanyId} issueId={issueId} interactionId={item.subject.id} agentMap={agentMap} currentUserId={userId} onResolved={() => onResolved(item)} />
|
||||
{interaction ? <AttentionInteractionResolver footerSlot={taskLink} presentation="coordinator-decision" companyId={selectedCompanyId} issueId={issueId} interactionId={item.subject.id} agentMap={agentMap} currentUserId={userId} onResolved={resolved => {
|
||||
if (!resolved || resolved.status === "pending") { void feed.refetch(); void recent.refetch(); return; }
|
||||
onResolved(item);
|
||||
}} />
|
||||
: decision ? <DecisionResolver companyId={selectedCompanyId} decisionId={item.subject.id} originIssue={item.relatedIssue} agentMap={agentMap} onResolved={() => onResolved(item)} />
|
||||
: stalledReview ? <><h2 className="text-lg font-medium">Choose how this task continues</h2><p className="text-sm">This task is still in review with no active reviewer or continuation. Return it to work, request changes, or mark the task done.</p><StalledReviewActions footerSlot={taskLink} issueId={item.subject.id} companyId={selectedCompanyId} reviewPolicy={readIssueReviewPolicyMetadata(item.subject.metadata)} approveLabel="Mark task done" onResolved={() => { void feed.refetch(); void recent.refetch(); }} /></>
|
||||
: <><h2 className="text-lg font-medium">{item.subject.title ?? item.whyNow}</h2><p className="text-sm">{attentionDetailLine(item) ?? item.whyNow}</p><Button disabled={!item.subject.href} onClick={() => { if (item.subject.href) navigate(item.subject.href); }}>Open to review<ArrowUpRight className="size-4" /></Button></>}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { itemVerdictHref } from "@/lib/item-verdict-links";
|
||||
import { Link } from "@/lib/router";
|
||||
import {
|
||||
useEffect,
|
||||
useMemo,
|
||||
@@ -1773,9 +1774,13 @@ function ItemVerdictsCard({
|
||||
</p>
|
||||
) : null}
|
||||
</div>
|
||||
{itemHref ? (
|
||||
{itemHref && !itemHref.startsWith("/api/") && (itemHref.startsWith("/") || itemHref.startsWith("#")) ? (
|
||||
<Link to={itemHref} disableIssueQuicklook className="text-xs font-medium text-primary hover:underline">Open</Link>
|
||||
) : itemHref ? (
|
||||
<a
|
||||
href={itemHref}
|
||||
target="_blank"
|
||||
rel="noreferrer"
|
||||
className="text-xs font-medium text-primary hover:underline"
|
||||
>
|
||||
Open
|
||||
|
||||
@@ -18,6 +18,10 @@ import {
|
||||
pendingRequestItemVerdictsInteraction,
|
||||
} from "@/fixtures/issueThreadInteractionFixtures";
|
||||
import { TaskChatInteractionCard } from "./TaskChatInteractionCard";
|
||||
vi.mock("@/lib/router", async importOriginal => ({
|
||||
...await importOriginal<typeof import("@/lib/router")>(),
|
||||
Link: ({ to, children }: { to: string; children: React.ReactNode }) => <a href={to}>{children}</a>,
|
||||
}));
|
||||
import { TaskChatThreadView } from "./TaskChatThreadView";
|
||||
import type { TaskChatInteractionItem } from "./task-chat-model";
|
||||
|
||||
@@ -838,6 +842,14 @@ describe("TaskChatInteractionCard", () => {
|
||||
}]);
|
||||
});
|
||||
|
||||
it("opens a compact attachment item through the authenticated file endpoint", () => {
|
||||
const interaction = { ...pendingRequestItemVerdictsInteraction, payload: { ...pendingRequestItemVerdictsInteraction.payload,
|
||||
items: [{ id: "file", label: "Reviewed file", target: { type: "issue_attachment" as const, attachmentId: "immutable-file", assetId: "asset", sha256: "hash" } }],
|
||||
} };
|
||||
flushSync(() => root.render(<TooltipProvider><ThemeProvider><TaskChatInteractionCard item={interactionItem(interaction)} presentation="takeover" onSubmitInteractionVerdicts={() => undefined} /></ThemeProvider></TooltipProvider>));
|
||||
expect(container.querySelector('a[href="/api/attachments/immutable-file/content"]')?.getAttribute("target")).toBe("_blank");
|
||||
});
|
||||
|
||||
it("opens the bound output revision from a compact review item", () => {
|
||||
const interaction = { ...pendingRequestItemVerdictsInteraction, payload: { ...pendingRequestItemVerdictsInteraction.payload,
|
||||
items: [{ id: "copy", label: "Reviewed copy", target: { type: "issue_document" as const, issueId: "owner", key: "copy", revisionId: "reviewed-v1" } }],
|
||||
|
||||
@@ -10,7 +10,7 @@ describe("review output links", () => {
|
||||
});
|
||||
it("opens the immutable attachment in its owning task", () => {
|
||||
expect(itemVerdictHref({ id: "art", label: "Artwork", target: { type: "issue_attachment", issueId: "owner", attachmentId: "asset-file", assetId: "asset", sha256: "hash" } }, interaction))
|
||||
.toBe("/issues/owner#interaction-review&attachment=asset-file");
|
||||
.toBe("/api/attachments/asset-file/content");
|
||||
});
|
||||
it("keeps safe legacy links and rejects unsafe custom URLs", () => {
|
||||
expect(itemVerdictHref({ id: "legacy", label: "Legacy", href: "javascript:alert(1)" }, interaction)).toBeNull();
|
||||
|
||||
@@ -11,7 +11,9 @@ export function itemVerdictHref(item: RequestItemVerdictsItem, interaction: { is
|
||||
})}`;
|
||||
}
|
||||
if (target?.type === "issue_attachment") {
|
||||
return `/issues/${encodeURIComponent(target.issueId ?? interaction.issueId)}#interaction-${encodeURIComponent(interaction.id)}&attachment=${encodeURIComponent(target.attachmentId)}`;
|
||||
// Use the authenticated immutable-content route in both interface modes.
|
||||
// This does not depend on the experimental task attachment panel.
|
||||
return `/api/attachments/${encodeURIComponent(target.attachmentId)}/content`;
|
||||
}
|
||||
return item.href ? normalizeRequestConfirmationTargetHref(item.href) : null;
|
||||
}
|
||||
Reference in new issue
Block a user