mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 12:59:13 +01:00
fix: open bound review files and expose decision history on phones
Derive attachment confirmation links from immutable attachment IDs in both layouts. Use the existing full-screen sheet for mobile decision history and retain the desktop panel across breakpoint changes. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
618c5a2a3f
commit
22f69c3042
6 files changed
+65
-14
No files matched your search
@@ -1158,6 +1158,15 @@ describe("IssueThreadInteractionCard", () => {
|
||||
expect(link?.getAttribute("target")).toBe("_blank");
|
||||
});
|
||||
|
||||
it.each([undefined, "/stale-display-link"])("opens a confirmation's bound attachment in Classic when href is %s", href => {
|
||||
const host = renderCard({ interaction: { ...pendingRequestConfirmationInteraction, payload: {
|
||||
version: 1, prompt: "Review the attached copy", target: {
|
||||
type: "issue_attachment", attachmentId: "immutable-file", assetId: "asset", sha256: "hash", href,
|
||||
},
|
||||
} } });
|
||||
expect(host.querySelector('a[href="/api/attachments/immutable-file/content"]')?.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: {
|
||||
|
||||
@@ -1541,10 +1541,10 @@ function requestConfirmationTargetHref({
|
||||
if (target.type === "issue_document" && target.revisionId) {
|
||||
return `/issues/${target.issueId ?? interaction.issueId}${buildDocumentAnnotationHash({ documentKey: target.key, revisionId: target.revisionId, interactionId: interaction.id, threadId: null, commentId: null })}`;
|
||||
}
|
||||
if (target.href) return target.href;
|
||||
if (target.type === "issue_attachment") {
|
||||
return `/issues/${target.issueId ?? interaction.issueId}#interaction-${interaction.id}&attachment=${encodeURIComponent(target.attachmentId)}`;
|
||||
return `/api/attachments/${encodeURIComponent(target.attachmentId)}/content`;
|
||||
}
|
||||
if (target.href) return target.href;
|
||||
if (target.type === "issue_document") {
|
||||
const issueId = target.issueId ?? interaction.issueId;
|
||||
return `/issues/${issueId}#document-${encodeURIComponent(target.key)}`;
|
||||
@@ -1579,7 +1579,7 @@ function RequestConfirmationTargetChip({
|
||||
);
|
||||
|
||||
if (!href) return <span className={className}>{content}</span>;
|
||||
if (/^https?:\/\//i.test(href)) {
|
||||
if (target.type === "issue_attachment" || /^https?:\/\//i.test(href)) {
|
||||
return (
|
||||
<a href={href} target="_blank" rel="noreferrer" className={className}>
|
||||
{content}
|
||||
|
||||
@@ -7,10 +7,11 @@ 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[], partial: false, 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[], mobile: false, 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 }) }));
|
||||
vi.mock("@/context/SidebarContext", () => ({ useSidebar: () => ({ isMobile: fixture.mobile }) }));
|
||||
vi.mock("@/hooks/useCoordinatorWorkspaceEnabled", () => ({ useCoordinatorWorkspaceEnabled: () => ({ userId: "reviewer" }) }));
|
||||
vi.mock("@/lib/router", () => ({ useNavigate: () => fixture.navigate }));
|
||||
vi.mock("@/api/agents", () => ({ agentsApi: { list: async () => [] } }));
|
||||
@@ -43,7 +44,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.partial = false;
|
||||
fixture.historyFail = false; fixture.feedFail = false; fixture.partial = false; fixture.mobile = 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(); });
|
||||
@@ -54,6 +55,28 @@ async function click(text: string) {
|
||||
await act(async () => button!.click()); await flush();
|
||||
}
|
||||
describe("Coordinator Needs You", () => {
|
||||
it("opens searchable history from both phone buttons without the hidden desktop panel", async () => {
|
||||
fixture.mobile = true;
|
||||
fixture.pending = [];
|
||||
fixture.history = [{ id: "old-decision", sourceKind: "issue_thread_interaction", sourceId: "old", issueId: "task-1", issueIdentifier: "PAP-1", issueTitle: "Earlier launch", title: "Review", label: "Approved", outcome: "accepted", resolvedAt: new Date(Date.now() - 30 * 24 * 60 * 60 * 1000).toISOString(), resolvedByUserId: "reviewer", resolvedByAgentId: null, agentId: null, agentName: null, projectId: null, projectName: null, href: "/PAP/issues/PAP-1#interaction-old", target: null }];
|
||||
const client = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||
await act(async () => root.render(<QueryClientProvider client={client}><CoordinatorNeedsYou /></QueryClientProvider>));
|
||||
await flush();
|
||||
for (const label of ["Decision history", "Open decision history"]) {
|
||||
const trigger = [...container.querySelectorAll("button")].find(button => button.textContent?.trim() === label);
|
||||
expect(trigger).toBeTruthy();
|
||||
await act(async () => trigger!.click()); await flush();
|
||||
const dialog = document.querySelector('[role="dialog"]');
|
||||
expect(dialog).not.toBeNull();
|
||||
expect(dialog!.querySelector('[aria-label="Search decision history"]')).not.toBeNull();
|
||||
expect(dialog!.textContent).toContain("Earlier launch");
|
||||
expect(fixture.openPanel).not.toHaveBeenCalled();
|
||||
const close = [...dialog!.querySelectorAll("button")].find(button => button.textContent === "Close");
|
||||
await act(async () => close!.click()); await flush();
|
||||
expect(document.querySelector('[role="dialog"]')).toBeNull();
|
||||
}
|
||||
client.clear();
|
||||
});
|
||||
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>));
|
||||
|
||||
@@ -10,6 +10,7 @@ import { projectsApi } from "@/api/projects";
|
||||
import { useCompany } from "@/context/CompanyContext";
|
||||
import { useBreadcrumbs } from "@/context/BreadcrumbContext";
|
||||
import { usePanel } from "@/context/PanelContext";
|
||||
import { useSidebar } from "@/context/SidebarContext";
|
||||
import { useCoordinatorWorkspaceEnabled } from "@/hooks/useCoordinatorWorkspaceEnabled";
|
||||
import { useNavigate } from "@/lib/router";
|
||||
import { queryKeys } from "@/lib/queryKeys";
|
||||
@@ -23,6 +24,7 @@ import { SidePanelTab } from "@/components/side-panel";
|
||||
import { ToolbarSearch } from "@/components/ToolbarSearch";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { NativeSelect } from "@/components/ui/select";
|
||||
import { Sheet, SheetContent, SheetHeader, SheetTitle } from "@/components/ui/sheet";
|
||||
import { parseCssTimeMs } from "@/components/task-chat/motion-tokens";
|
||||
import { readIssueReviewPolicyMetadata } from "@/lib/review-policy";
|
||||
import { CoordinatorDecisionReceipt as Receipt } from "./CoordinatorDecisionReceipt";
|
||||
@@ -30,7 +32,7 @@ import { CoordinatorTaskMotion } from "./CoordinatorTaskCommitment";
|
||||
|
||||
type Group = "none" | "task" | "agent" | "project";
|
||||
|
||||
function HistoryPanel({ companyId, agents, onClose }: { companyId: string; agents: Agent[]; onClose: () => void }) {
|
||||
function HistoryPanel({ companyId, agents, onClose, mobile = false }: { companyId: string; agents: Agent[]; onClose: () => void; mobile?: boolean }) {
|
||||
const navigate = useNavigate();
|
||||
const [search, setSearch] = useState("");
|
||||
const [outcome, setOutcome] = useState("");
|
||||
@@ -54,8 +56,7 @@ function HistoryPanel({ companyId, agents, onClose }: { companyId: string; agent
|
||||
const slot = typeof document !== "undefined" ? document.getElementById(PROPERTIES_PANE_HEADER_SLOT_ID) : null;
|
||||
const tab = <SidePanelTab id="decision-history" label="Decision history" active onSelect={() => {}} onClose={onClose} />;
|
||||
return <div className="space-y-5" data-coordinator-decision-history>
|
||||
{slot ? createPortal(tab, slot) : tab}
|
||||
<h2 className="text-lg font-medium">Decision history</h2>
|
||||
{!mobile ? <>{slot ? createPortal(tab, slot) : tab}<h2 className="text-lg font-medium">Decision history</h2></> : null}
|
||||
<ToolbarSearch query={search} onQueryChange={setSearch} label="Search decision history" placeholder="Search decisions or task IDs…" />
|
||||
<div className="grid grid-cols-2 gap-2">
|
||||
<NativeSelect aria-label="Filter decision outcomes" value={outcome} onChange={(event) => setOutcome(event.target.value)}>
|
||||
@@ -79,6 +80,7 @@ export function CoordinatorNeedsYou() {
|
||||
const { userId } = useCoordinatorWorkspaceEnabled();
|
||||
const { setBreadcrumbs } = useBreadcrumbs();
|
||||
const { openPanel, closePanel, setPanelVisible } = usePanel();
|
||||
const { isMobile } = useSidebar();
|
||||
const navigate = useNavigate();
|
||||
const reducedMotion = useReducedMotion();
|
||||
const [query, setQuery] = useState("");
|
||||
@@ -147,16 +149,17 @@ export function CoordinatorNeedsYou() {
|
||||
]).entries()], [pendingItems, recentItems]);
|
||||
const filtersActive = Boolean(query.trim() || agentFilter || projectFilter);
|
||||
const closeHistory = useCallback(() => { setHistoryOpen(false); closePanel(); }, [closePanel]);
|
||||
const showHistory = () => { setHistoryOpen(true); setPanelVisible(true); };
|
||||
const showHistory = () => setHistoryOpen(true);
|
||||
useEffect(() => { setBreadcrumbs([{ label: "Needs You" }]); }, [setBreadcrumbs]);
|
||||
useEffect(() => {
|
||||
setSettling([]); setHistoryOpen(false); setNow(Date.now()); closePanel();
|
||||
return closePanel;
|
||||
}, [selectedCompanyId, closePanel]);
|
||||
useEffect(() => {
|
||||
if (!historyOpen || !selectedCompanyId) { closePanel(); return; }
|
||||
if (!historyOpen || !selectedCompanyId || isMobile) { closePanel(); return; }
|
||||
setPanelVisible(true);
|
||||
openPanel(<HistoryPanel companyId={selectedCompanyId} agents={agentQuery.data ?? []} onClose={closeHistory} />);
|
||||
}, [historyOpen, selectedCompanyId, agentQuery.data, openPanel, closePanel, closeHistory]);
|
||||
}, [historyOpen, selectedCompanyId, isMobile, agentQuery.data, openPanel, closePanel, closeHistory, setPanelVisible]);
|
||||
useEffect(() => {
|
||||
const next = recentItems.reduce((earliest, item) => Math.min(earliest, Date.parse(item.resolvedAt) + COORDINATOR_RECENT_DECISION_MS), Infinity);
|
||||
if (!Number.isFinite(next)) return;
|
||||
@@ -175,6 +178,12 @@ export function CoordinatorNeedsYou() {
|
||||
const duration = typeof document === "undefined" ? 0 : parseCssTimeMs(getComputedStyle(document.documentElement).getPropertyValue("--motion-duration-base")) / 1_000;
|
||||
if (!selectedCompanyId) return <p className="text-sm text-muted-foreground">Choose a company to see its decisions.</p>;
|
||||
return <LayoutGroup id="coordinator-decisions"><section className="coordinator-needs-you mx-auto w-full max-w-4xl space-y-6" aria-label="Needs You">
|
||||
{isMobile ? <Sheet open={historyOpen} onOpenChange={open => { if (!open) closeHistory(); }}>
|
||||
<SheetContent side="bottom" aria-describedby={undefined} className="mobile-task-side-panel inset-0 h-dvh max-h-dvh w-full max-w-none gap-0 border-0 p-0 pt-(--sz-safe-top) pb-(--sz-safe-bottom)">
|
||||
<SheetHeader><SheetTitle>Decision history</SheetTitle></SheetHeader>
|
||||
<div className="min-h-0 flex-1 overflow-y-auto p-4"><HistoryPanel companyId={selectedCompanyId} agents={agentQuery.data ?? []} onClose={closeHistory} mobile /></div>
|
||||
</SheetContent>
|
||||
</Sheet> : null}
|
||||
<header className="flex items-center justify-between gap-3"><h1 className="text-2xl font-medium">Needs You</h1><Button variant="ghost" onClick={showHistory}><History className="size-4" />Decision history</Button></header>
|
||||
<div role="toolbar" aria-label="Decisions controls" className="flex flex-wrap items-center gap-2">
|
||||
<ToolbarSearch query={query} onQueryChange={setQuery} label="Search decisions" placeholder="Search decisions or task IDs…" />
|
||||
|
||||
@@ -303,13 +303,15 @@ function CompactTarget({
|
||||
const target = interaction.payload.target;
|
||||
const label = targetLabel(interaction);
|
||||
if (!target || !label) return null;
|
||||
const href = target.href
|
||||
? normalizeRequestConfirmationTargetHref(target.href)
|
||||
: null;
|
||||
const href = target.type === "issue_attachment"
|
||||
? `/api/attachments/${encodeURIComponent(target.attachmentId)}/content`
|
||||
: target.href ? normalizeRequestConfirmationTargetHref(target.href) : null;
|
||||
if (href) {
|
||||
return (
|
||||
<a
|
||||
href={href}
|
||||
target={target.type === "issue_attachment" ? "_blank" : undefined}
|
||||
rel={target.type === "issue_attachment" ? "noreferrer" : undefined}
|
||||
className="inline-flex rounded-sm border border-border px-2 py-0.5 font-mono text-xs text-foreground hover:bg-muted/60"
|
||||
>
|
||||
{label}
|
||||
|
||||
@@ -850,6 +850,14 @@ describe("TaskChatInteractionCard", () => {
|
||||
expect(container.querySelector('a[href="/api/attachments/immutable-file/content"]')?.getAttribute("target")).toBe("_blank");
|
||||
});
|
||||
|
||||
it.each([undefined, "/stale-display-link"])("opens a compact confirmation's bound attachment when href is %s", href => {
|
||||
const interaction = createRequestConfirmation({ payload: { version: 1, prompt: "Review the attached copy", target: {
|
||||
type: "issue_attachment", attachmentId: "immutable-file", assetId: "asset", sha256: "hash", href,
|
||||
} } });
|
||||
flushSync(() => root.render(<TooltipProvider><ThemeProvider><TaskChatInteractionCard item={interactionItem(interaction)} presentation="takeover" onAcceptInteraction={() => 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" } }],
|
||||
|
||||
Reference in new issue
Block a user