mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 01:50:39 +09:00
Improve issue approval visibility
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
977e9f3e9a
commit
bd0f56e523
4 changed files with 162 additions and 61 deletions
|
|
@ -1,10 +1,11 @@
|
|||
import { CheckCircle2, XCircle, Clock } from "lucide-react";
|
||||
import { Link } from "@/lib/router";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Button, buttonVariants } from "@/components/ui/button";
|
||||
import { Identity } from "./Identity";
|
||||
import { approvalLabel, typeIcon, defaultTypeIcon, ApprovalPayloadRenderer } from "./ApprovalPayload";
|
||||
import { timeAgo } from "../lib/timeAgo";
|
||||
import type { Approval, Agent } from "@paperclipai/shared";
|
||||
import { cn } from "@/lib/utils";
|
||||
|
||||
function statusIcon(status: string) {
|
||||
if (status === "approved") return <CheckCircle2 className="h-3.5 w-3.5 text-green-600 dark:text-green-400" />;
|
||||
|
|
@ -96,9 +97,12 @@ export function ApprovalCard({
|
|||
{(detailLink || onOpen) ? (
|
||||
<div className="mt-3">
|
||||
{detailLink ? (
|
||||
<Button variant="ghost" size="sm" className="text-xs px-0" asChild>
|
||||
<Link to={detailLink}>View details</Link>
|
||||
</Button>
|
||||
<Link
|
||||
to={detailLink}
|
||||
className={cn(buttonVariants({ variant: "ghost", size: "sm" }), "px-0 text-xs")}
|
||||
>
|
||||
View details
|
||||
</Link>
|
||||
) : (
|
||||
<Button variant="ghost" size="sm" className="text-xs px-0" onClick={onOpen}>
|
||||
View details
|
||||
|
|
|
|||
62
ui/src/components/ApprovalPayload.test.tsx
Normal file
62
ui/src/components/ApprovalPayload.test.tsx
Normal file
|
|
@ -0,0 +1,62 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
import { act } from "react";
|
||||
import { createRoot } from "react-dom/client";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { ApprovalPayloadRenderer, approvalLabel } from "./ApprovalPayload";
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
describe("approvalLabel", () => {
|
||||
it("uses payload titles for generic board approvals", () => {
|
||||
expect(
|
||||
approvalLabel("request_board_approval", {
|
||||
title: "Reply with an ASCII frog",
|
||||
}),
|
||||
).toBe("Board Approval: Reply with an ASCII frog");
|
||||
});
|
||||
});
|
||||
|
||||
describe("ApprovalPayloadRenderer", () => {
|
||||
let container: HTMLDivElement;
|
||||
|
||||
beforeEach(() => {
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
container.remove();
|
||||
});
|
||||
|
||||
it("renders request_board_approval payload fields without falling back to raw JSON", () => {
|
||||
const root = createRoot(container);
|
||||
|
||||
act(() => {
|
||||
root.render(
|
||||
<ApprovalPayloadRenderer
|
||||
type="request_board_approval"
|
||||
payload={{
|
||||
title: "Reply with an ASCII frog",
|
||||
summary: "Board asked for approval before posting the frog.",
|
||||
recommendedAction: "Approve the frog reply.",
|
||||
nextActionOnApproval: "Post the frog comment on the issue.",
|
||||
proposedComment: "(o)<",
|
||||
}}
|
||||
/>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(container.textContent).toContain("Reply with an ASCII frog");
|
||||
expect(container.textContent).toContain("Board asked for approval before posting the frog.");
|
||||
expect(container.textContent).toContain("Approve the frog reply.");
|
||||
expect(container.textContent).toContain("Post the frog comment on the issue.");
|
||||
expect(container.textContent).toContain("(o)<");
|
||||
expect(container.textContent).not.toContain("\"recommendedAction\"");
|
||||
|
||||
act(() => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -8,11 +8,26 @@ export const typeLabel: Record<string, string> = {
|
|||
request_board_approval: "Board Approval",
|
||||
};
|
||||
|
||||
function firstNonEmptyString(...values: unknown[]): string | null {
|
||||
for (const value of values) {
|
||||
if (typeof value === "string" && value.trim().length > 0) {
|
||||
return value.trim();
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/** Build a contextual label for an approval, e.g. "Hire Agent: Designer" */
|
||||
export function approvalLabel(type: string, payload?: Record<string, unknown> | null): string {
|
||||
const base = typeLabel[type] ?? type;
|
||||
if (type === "hire_agent" && payload?.name) {
|
||||
return `${base}: ${String(payload.name)}`;
|
||||
const subject = firstNonEmptyString(
|
||||
payload?.title,
|
||||
payload?.name,
|
||||
payload?.summary,
|
||||
payload?.recommendedAction,
|
||||
);
|
||||
if (subject) {
|
||||
return `${base}: ${subject}`;
|
||||
}
|
||||
return base;
|
||||
}
|
||||
|
|
@ -129,8 +144,39 @@ export function BudgetOverridePayload({ payload }: { payload: Record<string, unk
|
|||
);
|
||||
}
|
||||
|
||||
export function BoardApprovalPayload({ payload }: { payload: Record<string, unknown> }) {
|
||||
return (
|
||||
<div className="mt-3 space-y-2 text-sm">
|
||||
<PayloadField label="Title" value={payload.title} />
|
||||
{!!payload.summary && (
|
||||
<div className="flex items-start gap-2">
|
||||
<span className="text-muted-foreground w-20 sm:w-24 shrink-0 text-xs pt-0.5">Summary</span>
|
||||
<span className="text-muted-foreground">{String(payload.summary)}</span>
|
||||
</div>
|
||||
)}
|
||||
{!!payload.recommendedAction && (
|
||||
<div className="rounded-md bg-muted/40 px-3 py-2 text-xs text-muted-foreground">
|
||||
Recommended: {String(payload.recommendedAction)}
|
||||
</div>
|
||||
)}
|
||||
{!!payload.nextActionOnApproval && (
|
||||
<div className="flex items-start gap-2">
|
||||
<span className="text-muted-foreground w-20 sm:w-24 shrink-0 text-xs pt-0.5">On approval</span>
|
||||
<span>{String(payload.nextActionOnApproval)}</span>
|
||||
</div>
|
||||
)}
|
||||
{!!payload.proposedComment && (
|
||||
<div className="rounded-md bg-muted/40 px-3 py-2 text-xs text-muted-foreground whitespace-pre-wrap font-mono max-h-48 overflow-y-auto">
|
||||
{String(payload.proposedComment)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export function ApprovalPayloadRenderer({ type, payload }: { type: string; payload: Record<string, unknown> }) {
|
||||
if (type === "hire_agent") return <HireAgentPayload payload={payload} />;
|
||||
if (type === "budget_override_required") return <BudgetOverridePayload payload={payload} />;
|
||||
if (type === "request_board_approval") return <BoardApprovalPayload payload={payload} />;
|
||||
return <CeoStrategyPayload payload={payload} />;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,8 +38,8 @@ import {
|
|||
} from "../lib/optimistic-issue-comments";
|
||||
import { useProjectOrder } from "../hooks/useProjectOrder";
|
||||
import { relativeTime, cn, formatTokens, visibleRunCostUsd } from "../lib/utils";
|
||||
import { InlineEditor } from "../components/InlineEditor";
|
||||
import { ApprovalCard } from "../components/ApprovalCard";
|
||||
import { InlineEditor } from "../components/InlineEditor";
|
||||
import { CommentThread } from "../components/CommentThread";
|
||||
import { IssueDocumentsSection } from "../components/IssueDocumentsSection";
|
||||
import { IssueProperties } from "../components/IssueProperties";
|
||||
|
|
@ -56,14 +56,12 @@ import { PluginLauncherOutlet } from "@/plugins/launchers";
|
|||
import { Separator } from "@/components/ui/separator";
|
||||
import { Popover, PopoverTrigger, PopoverContent } from "@/components/ui/popover";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Collapsible, CollapsibleContent, CollapsibleTrigger } from "@/components/ui/collapsible";
|
||||
import { Sheet, SheetContent, SheetHeader, SheetTitle } from "@/components/ui/sheet";
|
||||
import { ScrollArea } from "@/components/ui/scroll-area";
|
||||
import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs";
|
||||
import {
|
||||
Activity as ActivityIcon,
|
||||
Check,
|
||||
ChevronDown,
|
||||
ChevronRight,
|
||||
Copy,
|
||||
EyeOff,
|
||||
|
|
@ -303,9 +301,6 @@ export function IssueDetail() {
|
|||
const [copied, setCopied] = useState(false);
|
||||
const [mobilePropsOpen, setMobilePropsOpen] = useState(false);
|
||||
const [detailTab, setDetailTab] = useState("comments");
|
||||
const [secondaryOpen, setSecondaryOpen] = useState({
|
||||
approvals: false,
|
||||
});
|
||||
const [pendingApprovalAction, setPendingApprovalAction] = useState<{
|
||||
approvalId: string;
|
||||
action: "approve" | "reject";
|
||||
|
|
@ -498,6 +493,13 @@ export function IssueDetail() {
|
|||
.filter((i) => i.parentId === issue.id)
|
||||
.sort((a, b) => new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime());
|
||||
}, [allIssues, issue]);
|
||||
const childIssuesPanelKey = useMemo(
|
||||
() => childIssues.map((child) => `${child.id}:${String(child.updatedAt)}`).join("|"),
|
||||
[childIssues],
|
||||
);
|
||||
const issuePanelKey = issue
|
||||
? `${issue.id}:${String(issue.updatedAt)}:${childIssuesPanelKey}`
|
||||
: "";
|
||||
const openNewSubIssue = useCallback(() => {
|
||||
if (!issue) return;
|
||||
openNewIssue({
|
||||
|
|
@ -507,7 +509,7 @@ export function IssueDetail() {
|
|||
projectId: issue.projectId ?? undefined,
|
||||
goalId: issue.goalId ?? undefined,
|
||||
});
|
||||
}, [issue, openNewIssue]);
|
||||
}, [issue?.goalId, issue?.id, issue?.identifier, issue?.projectId, issue?.title, openNewIssue]);
|
||||
|
||||
const commentReassignOptions = useMemo(() => {
|
||||
const options: Array<{ id: string; label: string; searchText?: string }> = [];
|
||||
|
|
@ -675,6 +677,9 @@ export function IssueDetail() {
|
|||
invalidateIssue();
|
||||
},
|
||||
});
|
||||
const handleIssuePropertiesUpdate = useCallback((data: Record<string, unknown>) => {
|
||||
updateIssue.mutate(data);
|
||||
}, [updateIssue.mutate]);
|
||||
|
||||
const approvalDecision = useMutation({
|
||||
mutationFn: async ({ approvalId, action }: { approvalId: string; action: "approve" | "reject" }) => {
|
||||
|
|
@ -1052,18 +1057,20 @@ export function IssueDetail() {
|
|||
}, [issue?.id]); // eslint-disable-line react-hooks/exhaustive-deps
|
||||
|
||||
useEffect(() => {
|
||||
if (issue) {
|
||||
openPanel(
|
||||
<IssueProperties
|
||||
issue={issue}
|
||||
childIssues={childIssues}
|
||||
onAddSubIssue={openNewSubIssue}
|
||||
onUpdate={(data) => updateIssue.mutate(data)}
|
||||
/>
|
||||
);
|
||||
if (!issue) {
|
||||
closePanel();
|
||||
return;
|
||||
}
|
||||
openPanel(
|
||||
<IssueProperties
|
||||
issue={issue}
|
||||
childIssues={childIssues}
|
||||
onAddSubIssue={openNewSubIssue}
|
||||
onUpdate={handleIssuePropertiesUpdate}
|
||||
/>
|
||||
);
|
||||
return () => closePanel();
|
||||
}, [childIssues, closePanel, issue, openNewSubIssue, openPanel, updateIssue]);
|
||||
}, [closePanel, handleIssuePropertiesUpdate, issuePanelKey, openNewSubIssue, openPanel]);
|
||||
|
||||
const inboxQuickArchiveArmedRef = useRef(false);
|
||||
const canQuickArchiveFromInbox =
|
||||
|
|
@ -1699,6 +1706,26 @@ export function IssueDetail() {
|
|||
</TabsContent>
|
||||
|
||||
<TabsContent value="activity">
|
||||
{linkedApprovals && linkedApprovals.length > 0 && (
|
||||
<div className="mb-3 space-y-3">
|
||||
{linkedApprovals.map((approval) => (
|
||||
<ApprovalCard
|
||||
key={approval.id}
|
||||
approval={approval}
|
||||
requesterAgent={approval.requestedByAgentId ? agentMap.get(approval.requestedByAgentId) ?? null : null}
|
||||
onApprove={() => approvalDecision.mutate({ approvalId: approval.id, action: "approve" })}
|
||||
onReject={() => approvalDecision.mutate({ approvalId: approval.id, action: "reject" })}
|
||||
detailLink={`/approvals/${approval.id}`}
|
||||
isPending={pendingApprovalAction?.approvalId === approval.id}
|
||||
pendingAction={
|
||||
pendingApprovalAction?.approvalId === approval.id
|
||||
? pendingApprovalAction.action
|
||||
: null
|
||||
}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
{linkedRuns && linkedRuns.length > 0 && (
|
||||
<div className="mb-3 px-3 py-2 rounded-lg border border-border">
|
||||
<div className="text-sm font-medium text-muted-foreground mb-1">Cost Summary</div>
|
||||
|
|
@ -1754,44 +1781,6 @@ export function IssueDetail() {
|
|||
)}
|
||||
</Tabs>
|
||||
|
||||
{linkedApprovals && linkedApprovals.length > 0 && (
|
||||
<Collapsible
|
||||
open={secondaryOpen.approvals}
|
||||
onOpenChange={(open) => setSecondaryOpen((prev) => ({ ...prev, approvals: open }))}
|
||||
className="rounded-lg border border-border"
|
||||
>
|
||||
<CollapsibleTrigger className="flex w-full items-center justify-between px-3 py-2 text-left">
|
||||
<span className="text-sm font-medium text-muted-foreground">
|
||||
Linked Approvals ({linkedApprovals.length})
|
||||
</span>
|
||||
<ChevronDown
|
||||
className={cn("h-4 w-4 text-muted-foreground transition-transform", secondaryOpen.approvals && "rotate-180")}
|
||||
/>
|
||||
</CollapsibleTrigger>
|
||||
<CollapsibleContent>
|
||||
<div className="border-t border-border divide-y divide-border">
|
||||
{linkedApprovals.map((approval) => (
|
||||
<div key={approval.id} className="px-3 py-3">
|
||||
<ApprovalCard
|
||||
approval={approval}
|
||||
requesterAgent={approval.requestedByAgentId ? agentMap.get(approval.requestedByAgentId) ?? null : null}
|
||||
onApprove={() => approvalDecision.mutate({ approvalId: approval.id, action: "approve" })}
|
||||
onReject={() => approvalDecision.mutate({ approvalId: approval.id, action: "reject" })}
|
||||
detailLink={`/approvals/${approval.id}`}
|
||||
isPending={pendingApprovalAction?.approvalId === approval.id}
|
||||
pendingAction={
|
||||
pendingApprovalAction?.approvalId === approval.id
|
||||
? pendingApprovalAction.action
|
||||
: null
|
||||
}
|
||||
/>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
</CollapsibleContent>
|
||||
</Collapsible>
|
||||
)}
|
||||
|
||||
|
||||
{/* Mobile properties drawer */}
|
||||
<Sheet open={mobilePropsOpen} onOpenChange={setMobilePropsOpen}>
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue