Skip to content

Commit 01ef8fd

Browse files
committed
fix(webapp): navigation filters speak the run page's language
1 parent b73599b commit 01ef8fd

2 files changed

Lines changed: 120 additions & 3 deletions

File tree

apps/webapp/app/components/dashboard-agent/navigate-target.test.ts

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,39 @@
11
import { describe, expect, it } from "vitest";
2+
import { getRunFiltersFromSearchParams } from "~/components/runs/v3/RunFilters";
23
import { appendRunFilters, navigateDestination, sameOriginPath } from "./navigate-target";
34

45
const RUNS_PATH = "/orgs/acme/projects/api/env/prod/runs";
56

7+
const FAILING = [
8+
"COMPLETED_WITH_ERRORS",
9+
"SYSTEM_FAILURE",
10+
"CRASHED",
11+
"EXPIRED",
12+
"TIMED_OUT",
13+
"INTERRUPTED",
14+
];
15+
16+
/** What the runs page makes of a URL this module produced. */
17+
function pageReads(path: string) {
18+
return getRunFiltersFromSearchParams(
19+
new URLSearchParams(new URL(path, "https://x.invalid").search)
20+
);
21+
}
22+
623
describe("appendRunFilters", () => {
724
it("returns the path untouched with no filters", () => {
825
expect(appendRunFilters(RUNS_PATH)).toBe(RUNS_PATH);
926
});
1027

1128
it("writes arrays as repeated params and keeps existing ones", () => {
1229
const result = appendRunFilters(`${RUNS_PATH}?query=payments`, {
13-
statuses: ["FAILED", "CRASHED"],
30+
statuses: ["COMPLETED_WITH_ERRORS", "CRASHED"],
1431
tasks: "send-email",
1532
period: "1d",
1633
});
1734

1835
expect(result).toBe(
19-
`${RUNS_PATH}?query=payments&statuses=FAILED&statuses=CRASHED&tasks=send-email&period=1d`
36+
`${RUNS_PATH}?query=payments&statuses=COMPLETED_WITH_ERRORS&statuses=CRASHED&tasks=send-email&period=1d`
2037
);
2138
});
2239

@@ -30,6 +47,51 @@ describe("appendRunFilters", () => {
3047
expect(appendRunFilters(RUNS_PATH, { search: "", rootOnly: false, tags: [] })).toBe(RUNS_PATH);
3148
expect(appendRunFilters(RUNS_PATH, { rootOnly: true })).toBe(`${RUNS_PATH}?rootOnly=true`);
3249
});
50+
51+
it("expands FAILED into the statuses the page calls failures", () => {
52+
const result = appendRunFilters(RUNS_PATH, { statuses: ["FAILED"], period: "1d" });
53+
54+
expect(pageReads(result)).toEqual({ statuses: FAILING, period: "1d" });
55+
});
56+
57+
it("takes the status the user said, however they cased it", () => {
58+
expect(pageReads(appendRunFilters(RUNS_PATH, { statuses: "failed" }))).toEqual({
59+
statuses: FAILING,
60+
});
61+
});
62+
63+
it("passes a page-native status through untranslated", () => {
64+
const result = appendRunFilters(RUNS_PATH, { statuses: ["COMPLETED_SUCCESSFULLY"] });
65+
66+
expect(result).toBe(`${RUNS_PATH}?statuses=COMPLETED_SUCCESSFULLY`);
67+
expect(pageReads(result)).toEqual({ statuses: ["COMPLETED_SUCCESSFULLY"] });
68+
});
69+
70+
it("translates the other API status names the model borrows", () => {
71+
expect(pageReads(appendRunFilters(RUNS_PATH, { statuses: ["QUEUED", "COMPLETED"] }))).toEqual({
72+
statuses: ["PENDING", "COMPLETED_SUCCESSFULLY"],
73+
});
74+
});
75+
76+
it("drops a status the page cannot parse rather than losing every filter with it", () => {
77+
const result = appendRunFilters(RUNS_PATH, { statuses: ["NONSENSE"], period: "1d" });
78+
79+
expect(result).toBe(`${RUNS_PATH}?period=1d`);
80+
expect(pageReads(result)).toEqual({ period: "1d" });
81+
});
82+
83+
// The page's parser has no `search`, and an unread param is only noise in the URL.
84+
it("leaves search out of the URL", () => {
85+
expect(appendRunFilters(RUNS_PATH, { search: "boom", period: "1d" })).toBe(
86+
`${RUNS_PATH}?period=1d`
87+
);
88+
});
89+
90+
// Control: the untranslated URL is what the live failure looked like. One status the
91+
// page cannot parse and it discards everything, the period included.
92+
it("pins why translation is needed: raw FAILED wipes the whole filter set", () => {
93+
expect(pageReads(`${RUNS_PATH}?statuses=FAILED&period=1d`)).toEqual({});
94+
});
3395
});
3496

3597
describe("navigateDestination", () => {
@@ -38,7 +100,10 @@ describe("navigateDestination", () => {
38100
it("routes a dashboard path and applies the intent's filters", () => {
39101
expect(
40102
navigateDestination({ path: RUNS_PATH, external: false }, { statuses: ["FAILED"] })
41-
).toEqual({ kind: "route", path: `${RUNS_PATH}?statuses=FAILED` });
103+
).toEqual({
104+
kind: "route",
105+
path: `${RUNS_PATH}?${FAILING.map((s) => `statuses=${s}`).join("&")}`,
106+
});
42107
});
43108

44109
it("never routes a source file, it leaves the dashboard", () => {

apps/webapp/app/components/dashboard-agent/navigate-target.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,69 @@
11
// `trigger://` targets are resolved server-side in `resolveTriggerUri.server.ts`.
22
import { type RunFilters } from "@internal/dashboard-agent-contracts";
3+
import { allTaskRunStatuses } from "~/components/runs/v3/TaskRunStatus";
34

45
// Filter keys are the runs page's own URL params, except that the page reads these
56
// bounds as epoch milliseconds while an intent carries ISO strings.
67
const EPOCH_MS_KEYS = new Set(["from", "to"]);
78

9+
// The page's filter parser has no `search` param, so it would only clutter the URL.
10+
const UNSUPPORTED_KEYS = new Set(["search"]);
11+
12+
type PageStatus = (typeof allTaskRunStatuses)[number];
13+
14+
const PAGE_STATUSES = new Set<string>(allTaskRunStatuses);
15+
16+
/** What a user means by "failed runs": every terminal status that is not a success or a cancel. */
17+
const FAILING_STATUSES = [
18+
"COMPLETED_WITH_ERRORS",
19+
"SYSTEM_FAILURE",
20+
"CRASHED",
21+
"EXPIRED",
22+
"TIMED_OUT",
23+
"INTERRUPTED",
24+
] as const satisfies readonly PageStatus[];
25+
26+
/** Statuses the API (and so the model) uses that the runs page has never heard of. */
27+
const STATUS_ALIASES = {
28+
FAILED: FAILING_STATUSES,
29+
QUEUED: ["PENDING"],
30+
COMPLETED: ["COMPLETED_SUCCESSFULLY"],
31+
REATTEMPTING: ["RETRYING_AFTER_FAILURE"],
32+
FROZEN: ["WAITING_TO_RESUME"],
33+
} as const satisfies Record<string, readonly PageStatus[]>;
34+
35+
/**
36+
* Statuses in the page's own vocabulary. One value it cannot parse makes it discard
37+
* every filter — the period too — so anything unrecognized is dropped, not sent.
38+
*/
39+
function pageStatuses(values: readonly string[]): string[] {
40+
const translated = new Set<string>();
41+
for (const value of values) {
42+
const status = value.trim().toUpperCase();
43+
if (PAGE_STATUSES.has(status)) {
44+
translated.add(status);
45+
continue;
46+
}
47+
for (const alias of STATUS_ALIASES[status as keyof typeof STATUS_ALIASES] ?? []) {
48+
translated.add(alias);
49+
}
50+
}
51+
return [...translated];
52+
}
53+
854
export function appendRunFilters(path: string, filters?: RunFilters): string {
955
if (!filters) return path;
1056
// A base is needed to parse a relative path; only pathname + search is used.
1157
const url = new URL(path, "https://dashboard.invalid");
1258

1359
for (const [key, value] of Object.entries(filters)) {
1460
if (value === undefined || value === null || value === "") continue;
61+
if (UNSUPPORTED_KEYS.has(key)) continue;
62+
if (key === "statuses") {
63+
const statuses = Array.isArray(value) ? value : [String(value)];
64+
for (const status of pageStatuses(statuses)) url.searchParams.append(key, status);
65+
continue;
66+
}
1567
if (EPOCH_MS_KEYS.has(key)) {
1668
const epochMs = Date.parse(String(value));
1769
if (!Number.isNaN(epochMs)) url.searchParams.set(key, String(epochMs));

0 commit comments

Comments
 (0)