fix(ui): harden issue breadcrumb source routing
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
0f9faa297b
commit
3986eb615c
6 changed files with 101 additions and 15 deletions
|
|
@ -2,6 +2,7 @@ import type { ReactNode } from "react";
|
|||
import type { Issue } from "@paperclipai/shared";
|
||||
import { Link } from "@/lib/router";
|
||||
import { X } from "lucide-react";
|
||||
import { createIssueDetailPath } from "../lib/issueDetailBreadcrumb";
|
||||
import { cn } from "../lib/utils";
|
||||
import { StatusIcon } from "./StatusIcon";
|
||||
|
||||
|
|
@ -45,7 +46,7 @@ export function IssueRow({
|
|||
|
||||
return (
|
||||
<Link
|
||||
to={`/issues/${issuePathId}`}
|
||||
to={createIssueDetailPath(issuePathId, issueLinkState)}
|
||||
state={issueLinkState}
|
||||
className={cn(
|
||||
"group flex items-start gap-2 border-b border-border py-2.5 pl-2 pr-3 text-sm no-underline text-inherit transition-colors hover:bg-accent/50 last:border-b-0 sm:items-center sm:py-2 sm:pl-1",
|
||||
|
|
|
|||
34
ui/src/lib/issueDetailBreadcrumb.test.ts
Normal file
34
ui/src/lib/issueDetailBreadcrumb.test.ts
Normal file
|
|
@ -0,0 +1,34 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
createIssueDetailLocationState,
|
||||
createIssueDetailPath,
|
||||
readIssueDetailBreadcrumb,
|
||||
} from "./issueDetailBreadcrumb";
|
||||
|
||||
describe("issueDetailBreadcrumb", () => {
|
||||
it("prefers the full breadcrumb from route state", () => {
|
||||
const state = createIssueDetailLocationState("Inbox", "/inbox/mine", "inbox");
|
||||
|
||||
expect(readIssueDetailBreadcrumb(state, "?from=issues")).toEqual({
|
||||
label: "Inbox",
|
||||
href: "/inbox/mine",
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back to the source query param when route state is unavailable", () => {
|
||||
expect(readIssueDetailBreadcrumb(null, "?from=inbox")).toEqual({
|
||||
label: "Inbox",
|
||||
href: "/inbox",
|
||||
});
|
||||
});
|
||||
|
||||
it("adds the source query param when building an issue detail path", () => {
|
||||
const state = createIssueDetailLocationState("Inbox", "/inbox/mine", "inbox");
|
||||
|
||||
expect(createIssueDetailPath("PAP-465", state)).toBe("/issues/PAP-465?from=inbox");
|
||||
});
|
||||
|
||||
it("reuses the current source query param when state has been dropped", () => {
|
||||
expect(createIssueDetailPath("PAP-465", null, "?from=issues")).toBe("/issues/PAP-465?from=issues");
|
||||
});
|
||||
});
|
||||
|
|
@ -1,3 +1,5 @@
|
|||
type IssueDetailSource = "issues" | "inbox";
|
||||
|
||||
type IssueDetailBreadcrumb = {
|
||||
label: string;
|
||||
href: string;
|
||||
|
|
@ -5,20 +7,64 @@ type IssueDetailBreadcrumb = {
|
|||
|
||||
type IssueDetailLocationState = {
|
||||
issueDetailBreadcrumb?: IssueDetailBreadcrumb;
|
||||
issueDetailSource?: IssueDetailSource;
|
||||
};
|
||||
|
||||
const ISSUE_DETAIL_SOURCE_QUERY_PARAM = "from";
|
||||
|
||||
function isIssueDetailBreadcrumb(value: unknown): value is IssueDetailBreadcrumb {
|
||||
if (typeof value !== "object" || value === null) return false;
|
||||
const candidate = value as Partial<IssueDetailBreadcrumb>;
|
||||
return typeof candidate.label === "string" && typeof candidate.href === "string";
|
||||
}
|
||||
|
||||
export function createIssueDetailLocationState(label: string, href: string): IssueDetailLocationState {
|
||||
return { issueDetailBreadcrumb: { label, href } };
|
||||
function isIssueDetailSource(value: unknown): value is IssueDetailSource {
|
||||
return value === "issues" || value === "inbox";
|
||||
}
|
||||
|
||||
export function readIssueDetailBreadcrumb(state: unknown): IssueDetailBreadcrumb | null {
|
||||
function readIssueDetailSource(state: unknown): IssueDetailSource | null {
|
||||
if (typeof state !== "object" || state === null) return null;
|
||||
const candidate = (state as IssueDetailLocationState).issueDetailBreadcrumb;
|
||||
return isIssueDetailBreadcrumb(candidate) ? candidate : null;
|
||||
const source = (state as IssueDetailLocationState).issueDetailSource;
|
||||
return isIssueDetailSource(source) ? source : null;
|
||||
}
|
||||
|
||||
function readIssueDetailSourceFromSearch(search?: string): IssueDetailSource | null {
|
||||
if (!search) return null;
|
||||
const params = new URLSearchParams(search);
|
||||
const source = params.get(ISSUE_DETAIL_SOURCE_QUERY_PARAM);
|
||||
return isIssueDetailSource(source) ? source : null;
|
||||
}
|
||||
|
||||
function breadcrumbForSource(source: IssueDetailSource): IssueDetailBreadcrumb {
|
||||
if (source === "inbox") return { label: "Inbox", href: "/inbox" };
|
||||
return { label: "Issues", href: "/issues" };
|
||||
}
|
||||
|
||||
export function createIssueDetailLocationState(
|
||||
label: string,
|
||||
href: string,
|
||||
source?: IssueDetailSource,
|
||||
): IssueDetailLocationState {
|
||||
return {
|
||||
issueDetailBreadcrumb: { label, href },
|
||||
issueDetailSource: source,
|
||||
};
|
||||
}
|
||||
|
||||
export function createIssueDetailPath(issuePathId: string, state?: unknown, search?: string): string {
|
||||
const source = readIssueDetailSource(state) ?? readIssueDetailSourceFromSearch(search);
|
||||
if (!source) return `/issues/${issuePathId}`;
|
||||
const params = new URLSearchParams();
|
||||
params.set(ISSUE_DETAIL_SOURCE_QUERY_PARAM, source);
|
||||
return `/issues/${issuePathId}?${params.toString()}`;
|
||||
}
|
||||
|
||||
export function readIssueDetailBreadcrumb(state: unknown, search?: string): IssueDetailBreadcrumb | null {
|
||||
if (typeof state === "object" && state !== null) {
|
||||
const candidate = (state as IssueDetailLocationState).issueDetailBreadcrumb;
|
||||
if (isIssueDetailBreadcrumb(candidate)) return candidate;
|
||||
}
|
||||
|
||||
const source = readIssueDetailSourceFromSearch(search);
|
||||
return source ? breadcrumbForSource(source) : null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ import { heartbeatsApi } from "../api/heartbeats";
|
|||
import { useCompany } from "../context/CompanyContext";
|
||||
import { useBreadcrumbs } from "../context/BreadcrumbContext";
|
||||
import { queryKeys } from "../lib/queryKeys";
|
||||
import { createIssueDetailLocationState } from "../lib/issueDetailBreadcrumb";
|
||||
import { createIssueDetailLocationState, createIssueDetailPath } from "../lib/issueDetailBreadcrumb";
|
||||
import { EmptyState } from "../components/EmptyState";
|
||||
import { PageSkeleton } from "../components/PageSkeleton";
|
||||
import { IssueRow } from "../components/IssueRow";
|
||||
|
|
@ -526,6 +526,7 @@ export function Inbox() {
|
|||
createIssueDetailLocationState(
|
||||
"Inbox",
|
||||
`${location.pathname}${location.search}${location.hash}`,
|
||||
"inbox",
|
||||
),
|
||||
[location.pathname, location.search, location.hash],
|
||||
);
|
||||
|
|
@ -1019,7 +1020,7 @@ export function Inbox() {
|
|||
const item = workItemsToRender[selectedIndex];
|
||||
if (item.kind === "issue") {
|
||||
const pathId = item.issue.identifier ?? item.issue.id;
|
||||
navigate(`/issues/${pathId}`, { state: issueLinkState });
|
||||
navigate(createIssueDetailPath(pathId, issueLinkState), { state: issueLinkState });
|
||||
} else if (item.kind === "approval") {
|
||||
navigate(`/approvals/${item.approval.id}`);
|
||||
} else if (item.kind === "failed_run") {
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import { useToast } from "../context/ToastContext";
|
|||
import { useBreadcrumbs } from "../context/BreadcrumbContext";
|
||||
import { assigneeValueFromSelection, suggestedCommentAssigneeValue } from "../lib/assignees";
|
||||
import { queryKeys } from "../lib/queryKeys";
|
||||
import { readIssueDetailBreadcrumb } from "../lib/issueDetailBreadcrumb";
|
||||
import { createIssueDetailPath, readIssueDetailBreadcrumb } from "../lib/issueDetailBreadcrumb";
|
||||
import { useProjectOrder } from "../hooks/useProjectOrder";
|
||||
import { relativeTime, cn, formatTokens, visibleRunCostUsd } from "../lib/utils";
|
||||
import { InlineEditor } from "../components/InlineEditor";
|
||||
|
|
@ -270,8 +270,8 @@ export function IssueDetail() {
|
|||
|
||||
const hasLiveRuns = (liveRuns ?? []).length > 0 || !!activeRun;
|
||||
const sourceBreadcrumb = useMemo(
|
||||
() => readIssueDetailBreadcrumb(location.state) ?? { label: "Issues", href: "/issues" },
|
||||
[location.state],
|
||||
() => readIssueDetailBreadcrumb(location.state, location.search) ?? { label: "Issues", href: "/issues" },
|
||||
[location.state, location.search],
|
||||
);
|
||||
|
||||
// Filter out runs already shown by the live widget to avoid duplication
|
||||
|
|
@ -581,9 +581,12 @@ export function IssueDetail() {
|
|||
// Redirect to identifier-based URL if navigated via UUID
|
||||
useEffect(() => {
|
||||
if (issue?.identifier && issueId !== issue.identifier) {
|
||||
navigate(`/issues/${issue.identifier}`, { replace: true, state: location.state });
|
||||
navigate(createIssueDetailPath(issue.identifier, location.state, location.search), {
|
||||
replace: true,
|
||||
state: location.state,
|
||||
});
|
||||
}
|
||||
}, [issue, issueId, navigate, location.state]);
|
||||
}, [issue, issueId, navigate, location.state, location.search]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!issue?.id) return;
|
||||
|
|
@ -695,7 +698,7 @@ export function IssueDetail() {
|
|||
<span key={ancestor.id} className="flex items-center gap-1">
|
||||
{i > 0 && <ChevronRight className="h-3 w-3 shrink-0" />}
|
||||
<Link
|
||||
to={`/issues/${ancestor.identifier ?? ancestor.id}`}
|
||||
to={createIssueDetailPath(ancestor.identifier ?? ancestor.id, location.state, location.search)}
|
||||
state={location.state}
|
||||
className="hover:text-foreground transition-colors truncate max-w-[200px]"
|
||||
title={ancestor.title}
|
||||
|
|
@ -1063,7 +1066,7 @@ export function IssueDetail() {
|
|||
{childIssues.map((child) => (
|
||||
<Link
|
||||
key={child.id}
|
||||
to={`/issues/${child.identifier ?? child.id}`}
|
||||
to={createIssueDetailPath(child.identifier ?? child.id, location.state, location.search)}
|
||||
state={location.state}
|
||||
className="flex items-center justify-between px-3 py-2 text-sm hover:bg-accent/20 transition-colors"
|
||||
>
|
||||
|
|
|
|||
|
|
@ -70,6 +70,7 @@ export function Issues() {
|
|||
createIssueDetailLocationState(
|
||||
"Issues",
|
||||
`${location.pathname}${location.search}${location.hash}`,
|
||||
"issues",
|
||||
),
|
||||
[location.pathname, location.search, location.hash],
|
||||
);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue