From e249728462057632c43444e72cf0f0e952cdc1e6 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Tue, 27 Oct 2026 19:26:47 -0700 Subject: [PATCH] feat(web): page, filter, and restyle the SubmitQueue UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? Queue activity only showed the last 24 hours, so a quiet queue looked empty even when it had history; paging meant clicking through snapshot pages, and there was no way to see only requests in a given status or when each last changed. Queue names also sat at the root of the URL space, where a queue named `api` or `healthz` would collide with host routes, and the move to BaseUI (#888) had replaced SubmitQueue's original centered look with full-width defaults that read worse. ### What? - **Queue activity**: starts at the newest request however old and loads older pages automatically near the bottom (Load more button, retry on failure); loaded rows and scroll position survive request visits, and refreshes update statuses in place behind "Show latest requests". Stovepipe's infinite-scroll list moves to `web/platform` as `PagedRequests` / `RequestPagesProvider` and both modules use it. - **Status filter and Updated column**: a status multi-select restricts the list through the gateway's `statuses` filter, shareable as repeated `?status=` parameters; an Updated column and summary fact show `updated_at_ms`. - **URLs**: pages live under `/queue/`, `/queue//request/`, and `/queue//change/…`. Continuations read `GET /api/queue//requests?status=…&page=…`, a thin host route over the module's new `queueRequests` entry, which shares the queue page's authorization, filter canonicalization, and loader. - **Look**: header, page, and footer share a centered `min(1200px, 100% - 48px)` column with an 80px ruled header and dark SQ badge; links use the theme's `accent500` without underline until hover, change URIs are monospace, and secondary text is muted. The queue directory is a card grid. Request history is a two-column timeline with the time since the previous event. All styling uses BaseUI theme tokens and `withStyle`, so host themes still apply; Stovepipe is unchanged apart from the shared list. - Removed: the read-only labels, the displayed-page search box, and bullets in the Changes column. ## Test Plan ✅ `bazel test //web/platform/... //web/submitqueue/... //web/stovepipe/... //web/service/submitqueue:all //web/service/stovepipe:all //web/test/package/...` ✅ `bazel test //web/test/e2e/submitqueue:web_test` (status filter, JSON API authentication and filtering, layout, accessibility scans) ✅ Locally: `make local-submitqueue-start`, `make demo-requests COUNT=1000 WATCH=false`, then open the printed Web UX URL (user `test`), scroll `/queue/demo-queue`, and filter by status. --- web/platform/index.ts | 1 + .../paged-requests.test.tsx} | 41 +-- web/platform/paged-requests.tsx | 200 ++++++++++++++ web/platform/table.tsx | 5 +- .../stovepipe/src/next/queue-provider.tsx | 6 +- web/service/submitqueue/README.md | 4 +- .../src/app/(protected)/[[...path]]/page.tsx | 2 +- .../api/queue/[queue]/requests/route.test.ts | 48 ++++ .../app/api/queue/[queue]/requests/route.ts | 28 ++ web/service/submitqueue/src/app/layout.tsx | 5 +- .../submitqueue/src/next/queue-provider.tsx | 16 ++ .../submitqueue/src/server/instrumentation.ts | 18 ++ web/stovepipe/README.md | 2 +- web/stovepipe/index.ts | 3 +- web/stovepipe/view/queue.tsx | 176 +------------ web/submitqueue/README.md | 4 +- web/submitqueue/controller/mapping.ts | 6 + web/submitqueue/controller/queue.ts | 58 +++-- web/submitqueue/core/change.test.ts | 16 +- web/submitqueue/core/paths.test.ts | 11 +- web/submitqueue/core/paths.ts | 7 +- web/submitqueue/core/route.test.ts | 14 +- web/submitqueue/core/route.ts | 12 +- web/submitqueue/core/window.ts | 5 + web/submitqueue/entity/index.ts | 2 +- web/submitqueue/entity/page.ts | 20 +- web/submitqueue/entity/request.ts | 10 + web/submitqueue/extension/gateway/gateway.ts | 6 +- .../extension/gateway/mock/fake.ts | 1 + web/submitqueue/index.ts | 6 +- web/submitqueue/module.test.ts | 78 +++--- web/submitqueue/module.ts | 75 ++++-- web/submitqueue/server.ts | 6 +- web/submitqueue/view/components.test.tsx | 87 +++---- web/submitqueue/view/components.tsx | 243 +++++++++++++----- web/submitqueue/view/pages.test.tsx | 23 +- web/submitqueue/view/pages.tsx | 23 +- web/submitqueue/view/shell.tsx | 75 ++++-- web/submitqueue/view/web-app.tsx | 13 +- web/test/e2e/submitqueue/submitqueue.spec.ts | 50 ++-- web/test/package/consumer-runtime.test.mjs | 4 +- web/test/package/consumer-server-runtime.mjs | 2 +- web/test/package/consumer.ts | 2 +- .../package/react18/react18-runtime.test.mjs | 6 +- 44 files changed, 915 insertions(+), 505 deletions(-) rename web/{stovepipe/view/queue.test.tsx => platform/paged-requests.test.tsx} (71%) create mode 100644 web/platform/paged-requests.tsx create mode 100644 web/service/submitqueue/src/app/api/queue/[queue]/requests/route.test.ts create mode 100644 web/service/submitqueue/src/app/api/queue/[queue]/requests/route.ts create mode 100644 web/service/submitqueue/src/next/queue-provider.tsx diff --git a/web/platform/index.ts b/web/platform/index.ts index e0409d847..b891ff0c1 100644 --- a/web/platform/index.ts +++ b/web/platform/index.ts @@ -5,3 +5,4 @@ export * from "./auto-refresh.js"; export * from "./timestamp.js"; export * from "./table.js"; export * from "./facts.js"; +export * from "./paged-requests.js"; diff --git a/web/stovepipe/view/queue.test.tsx b/web/platform/paged-requests.test.tsx similarity index 71% rename from web/stovepipe/view/queue.test.tsx rename to web/platform/paged-requests.test.tsx index 14a116aac..a0e724a79 100644 --- a/web/stovepipe/view/queue.test.tsx +++ b/web/platform/paged-requests.test.tsx @@ -1,17 +1,24 @@ -import { act, cleanup, fireEvent, render, screen, waitFor, within } from "../../platform/test/rtl.js"; +import { act, cleanup, fireEvent, render, screen, waitFor, within } from "./test/rtl.js"; import { afterEach, beforeEach, expect, it, vi } from "vitest"; -import type { QueueRequestsModel } from "../entity/index.js"; -import { QueueRequests, StovepipeQueueProvider } from "./queue.js"; +import { PagedRequests, RequestPagesProvider, type RequestPage } from "./paged-requests.js"; + +interface Row { + id: string; + state: string; +} const nextHref = "/repo%2Fmain?page=opaque%2B%2F%3D"; -function page(ids: string[], next: string | null = nextHref): QueueRequestsModel { - return { requests: ids.map(id => ({ - id, href: `/repo%2Fmain/request/${id}`, changeUri: `git://repo/${id}`, baseUri: "", state: "accepted", - acceptedAtMs: "1000", updatedAtMs: "1000", outcomeReason: "", - })), nextHref: next }; +function page(ids: string[], next: string | null = nextHref): RequestPage { + return { requests: ids.map(id => ({ id, state: "accepted" })), nextHref: next }; +} +function RowList({ initial, routeKey }: { initial: RequestPage; routeKey: string }) { + return request.id}> + {requests => } + ; } function displayedIds() { - return within(screen.getByRole("table", { name: "Requests" })).getAllByRole("link").map(link => link.textContent); + return within(screen.getByRole("list", { name: "Requests" })).getAllByRole("link").map(link => link.textContent); } let intersect: () => void; beforeEach(() => { @@ -31,11 +38,11 @@ afterEach(() => { }); it("loads on intersection once, appends unique requests, and stops at the end", async () => { - let resolve!: (result: QueueRequestsModel) => void; - const loadPage = vi.fn(() => new Promise(complete => { + let resolve!: (result: RequestPage) => void; + const loadPage = vi.fn(() => new Promise>(complete => { resolve = complete; })); - render(); + render(); await act(async () => { intersect(); intersect(); @@ -58,7 +65,7 @@ it("loads on intersection once, appends unique requests, and stops at the end", it("retains rows after a failed load and retries the same continuation by button", async () => { const loadPage = vi.fn().mockRejectedValueOnce(new Error("unavailable")).mockResolvedValueOnce(page(["1"], null)); - render(); + render(); await act(async () => intersect()); expect(displayedIds()).toEqual(["2"]); expect(screen.getByRole("alert")).toHaveTextContent("Could not load older requests"); @@ -72,17 +79,17 @@ it("retains rows after a failed load and retries the same continuation by button it("aborts a departing queue, restores its loaded rows, and keeps new arrivals behind an explicit action", async () => { const initial = page(["3", "2"]); const loadPage = vi.fn().mockResolvedValueOnce(page(["1"], "/repo%2Fmain?page=last")); - function App({ visible = true, latest = initial }: { visible?: boolean; latest?: QueueRequestsModel }) { - return {visible && }; + function App({ visible = true, latest = initial }: { visible?: boolean; latest?: RequestPage }) { + return {visible && }; } const { rerender } = render(); await act(async () => intersect()); expect(displayedIds()).toEqual(["3", "2", "1"]); let signal: AbortSignal | undefined; - let finish!: (result: QueueRequestsModel) => void; + let finish!: (result: RequestPage) => void; loadPage.mockImplementationOnce((_href: string, abort: AbortSignal) => { signal = abort; - return new Promise(resolve => { + return new Promise>(resolve => { finish = resolve; }); }); diff --git a/web/platform/paged-requests.tsx b/web/platform/paged-requests.tsx new file mode 100644 index 000000000..0c304950c --- /dev/null +++ b/web/platform/paged-requests.tsx @@ -0,0 +1,200 @@ +"use client"; + +import { createContext, useCallback, useContext, useEffect, useLayoutEffect, useMemo, useRef, useState, type ReactNode } from "react"; +import { Block } from "baseui/block"; +import { Button, KIND, SIZE } from "baseui/button"; +import { StyledLink } from "baseui/link"; +import { Spinner } from "baseui/spinner"; +import { ParagraphSmall, ParagraphXSmall } from "baseui/typography"; +import { useWebNavigation } from "./navigation.js"; + +const useBrowserLayoutEffect = typeof window === "undefined" ? useEffect : useLayoutEffect; + +/** One page of a request list ordered newest first. */ +export interface RequestPage { + /** Requests on this page, newest first. */ + requests: T[]; + /** Opaque in-app URL of the next page; null on the final page. */ + nextHref: string | null; +} + +/** Fetches a continuation through the host's authenticated transport. */ +export type RequestPageLoader = (href: string, signal: AbortSignal) => Promise>; + +interface RequestSnapshot extends RequestPage { + latest: RequestPage; + firstPageKeys: string[]; + loadedHrefs: string[]; +} +interface SavedRequests { + snapshot: RequestSnapshot; + scrollY: number; +} +const RequestPagesContext = createContext<{ + loadPage: RequestPageLoader; + pages: Map; +} | null>(null); + +/** Mount above route changes to retain loaded request rows and scroll positions. */ +export function RequestPagesProvider({ loadPage, children }: { loadPage: RequestPageLoader; children: ReactNode }) { + const pages = useRef(new Map()); + const value = useMemo(() => ({ loadPage, pages: pages.current }), [loadPage]); + return {children}; +} + +function createSnapshot(page: RequestPage, requestKey: (request: T) => string): RequestSnapshot { + return { ...page, latest: page, firstPageKeys: page.requests.map(requestKey), loadedHrefs: [] }; +} + +/** + * Renders `initial` and appends older pages as the viewer scrolls near the end. A new `initial` updates displayed + * rows in place; arrivals stay behind an explicit action. Without a provider, continuation links navigate normally. + */ +export function PagedRequests({ initial, routeKey, requestKey, children }: { + initial: RequestPage; + /** Identity of the list for retained rows and scroll position; independent of refresh time. */ + routeKey: string; + requestKey: (request: T) => string; + /** `complete` is true once no older page remains. */ + children: (requests: T[], complete: boolean) => ReactNode; +}) { + const context = useContext(RequestPagesContext); + const { Link: HostLink } = useWebNavigation(); + const [snapshot, setSnapshot] = useState(() => + (context?.pages.get(routeKey)?.snapshot as RequestSnapshot | undefined) ?? createSnapshot(initial, requestKey)); + const [hydrated, setHydrated] = useState(false); + const [loading, setLoading] = useState(false); + const [error, setError] = useState(false); + const sentinel = useRef(null); + const inFlight = useRef(null); + const saved = useRef({ snapshot, scrollY: context?.pages.get(routeKey)?.scrollY ?? 0 }); + saved.current.snapshot = snapshot; + const keyOf = useRef(requestKey); + keyOf.current = requestKey; + + if (snapshot.latest !== initial) { + const current = new Map(initial.requests.map(request => [requestKey(request), request])); + setSnapshot({ ...snapshot, latest: initial, requests: snapshot.requests.map(request => current.get(requestKey(request)) ?? request) }); + } + + useBrowserLayoutEffect(() => { + setHydrated(true); + const previous = context?.pages.get(routeKey); + const frame = previous ? requestAnimationFrame(() => window.scrollTo(0, previous.scrollY)) : null; + const recordScroll = () => { + saved.current.scrollY = window.scrollY; + }; + window.addEventListener("scroll", recordScroll, { passive: true }); + return () => { + if (frame !== null) cancelAnimationFrame(frame); + window.removeEventListener("scroll", recordScroll); + // Save before navigation can shrink the document and reset its scroll offset. + inFlight.current?.abort(); + context?.pages.set(routeKey, saved.current); + }; + }, [context, routeKey]); + + const loadOlderRequests = useCallback(async () => { + const href = snapshot.nextHref; + if (!context || !href || inFlight.current) return; + const controller = new AbortController(); + inFlight.current = controller; + setLoading(true); + setError(false); + try { + const next = await context.loadPage(href, controller.signal) as RequestPage; + if (controller.signal.aborted) return; + if (next.nextHref && [href, ...snapshot.loadedHrefs].includes(next.nextHref)) throw new Error("Repeated continuation"); + setSnapshot(current => { + if (current.nextHref !== href) return current; + const existing = new Set(current.requests.map(keyOf.current)); + const additions = next.requests.filter(request => { + const key = keyOf.current(request); + if (existing.has(key)) return false; + existing.add(key); + return true; + }); + return { ...current, requests: [...current.requests, ...additions], nextHref: next.nextHref, loadedHrefs: [...current.loadedHrefs, href] }; + }); + } catch { + if (!controller.signal.aborted) setError(true); + } finally { + if (inFlight.current === controller) { + inFlight.current = null; + setLoading(false); + } + } + }, [context, snapshot.nextHref, snapshot.loadedHrefs]); + + useEffect(() => { + if (!context || !snapshot.nextHref || error || loading || !sentinel.current || typeof IntersectionObserver === "undefined") return; + const observer = new IntersectionObserver(entries => { + if (entries.some(entry => entry.isIntersecting)) void loadOlderRequests(); + }, { rootMargin: "400px" }); + observer.observe(sentinel.current); + return () => observer.disconnect(); + }, [context, snapshot.nextHref, error, loading, loadOlderRequests]); + + let paginationMessage: string; + if (loading) { + paginationMessage = "Loading older requests…"; + } else if (error) { + paginationMessage = "Loading paused"; + } else if (snapshot.nextHref && context && hydrated && typeof IntersectionObserver !== "undefined") { + paginationMessage = "Scroll for more requests"; + } else if (snapshot.nextHref) { + paginationMessage = "More requests available"; + } else if (snapshot.requests.length) { + paginationMessage = "All available requests loaded"; + } else { + paginationMessage = "No requests to load"; + } + + const newRequests = initial.requests.some(request => !snapshot.firstPageKeys.includes(requestKey(request))); + return <> + {newRequests && + + } + {children(snapshot.requests, snapshot.nextHref === null)} +
+ {error && Could not load older requests. Your displayed requests are unchanged.} + + + + + {loading && + } + + + {paginationMessage} + + + {snapshot.requests.length} displayed requests + + + {snapshot.nextHref && (context && hydrated + ? + : Older requests →)} + + +
+ ; +} diff --git a/web/platform/table.tsx b/web/platform/table.tsx index e6abe2735..99d6ec936 100644 --- a/web/platform/table.tsx +++ b/web/platform/table.tsx @@ -12,13 +12,14 @@ import type { ReactNode } from "react"; export function Table({ label, headers, children }: { label: string; - headers: string[]; + /** Column headings; strings, or nodes when a host styles them. */ + headers: ReactNode[]; children: ReactNode; }) { return - {headers.map(header => {header})} + {headers.map((header, index) => {header})} {children} diff --git a/web/service/stovepipe/src/next/queue-provider.tsx b/web/service/stovepipe/src/next/queue-provider.tsx index 56bfcfc43..f73f07e0a 100644 --- a/web/service/stovepipe/src/next/queue-provider.tsx +++ b/web/service/stovepipe/src/next/queue-provider.tsx @@ -1,9 +1,9 @@ "use client"; import type { ReactNode } from "react"; -import { StovepipeQueueProvider, type QueuePageLoader } from "@submitqueue/web-stovepipe"; +import { RequestPagesProvider, type RequestPageLoader } from "@submitqueue/web-stovepipe"; -const loadPage: QueuePageLoader = async (href, signal) => { +const loadPage: RequestPageLoader = async (href, signal) => { const continuation = new URL(href, window.location.origin); const search = new URLSearchParams({ path: continuation.pathname, page: continuation.searchParams.get("page") ?? "" }); const response = await fetch(`/api/queue-requests?${search}`, { signal, cache: "no-store" }); @@ -12,5 +12,5 @@ const loadPage: QueuePageLoader = async (href, signal) => { }; export function QueueProvider({ children }: { children: ReactNode }) { - return {children}; + return {children}; } diff --git a/web/service/submitqueue/README.md b/web/service/submitqueue/README.md index 0bb5f6be9..7b4716adb 100644 --- a/web/service/submitqueue/README.md +++ b/web/service/submitqueue/README.md @@ -5,8 +5,8 @@ A local/demo Next.js host for the SubmitQueue module, and the composition root t | Path | Holds | |---|---| | `src/server/` | Wiring, free of Next.js: environment config, demo Basic auth, the Connect gateway client and transport, pino, opt-in Prometheus, and `resolveReferenceWebHost` | -| `src/next/` | Everything Next.js-specific: catch-all page and metadata, the per-request auth check, navigation provider (`next/link` without prefetch, transition-aware refresh), readiness route | -| `src/app/` | Routes: one optional catch-all page, `/healthz`, layouts, not-found and unauthorized pages | +| `src/next/` | Everything Next.js-specific: catch-all page and metadata, the per-request auth check, navigation provider (`next/link` without prefetch, transition-aware refresh), queue-page loader for older requests, readiness route | +| `src/app/` | Routes: one optional catch-all page, `/healthz`, the authenticated uncached `/api/queue//requests` JSON route, layouts, not-found and unauthorized pages | | `src/proxy.ts` | Challenges anonymous browsers with HTTP Basic auth | Two Next.js specifics: `next.config.ts` sets `htmlLimitedBots: /.*/` so page titles resolve before streaming, and Next leaves pino unbundled, so the image's runtime layer declares it. diff --git a/web/service/submitqueue/src/app/(protected)/[[...path]]/page.tsx b/web/service/submitqueue/src/app/(protected)/[[...path]]/page.tsx index 29748b978..1cb5712a9 100644 --- a/web/service/submitqueue/src/app/(protected)/[[...path]]/page.tsx +++ b/web/service/submitqueue/src/app/(protected)/[[...path]]/page.tsx @@ -7,7 +7,7 @@ export const dynamic = "force-dynamic"; export const revalidate = 0; const page = createNextPage({ - shell: { environmentLabel: "Local demo · read-only", footer: "Read-only demo" }, + shell: { environmentLabel: "Local demo" }, web: resolveReferenceWeb, authenticate: requireAuthorization, onLog: resolveReferenceLogCallback(), diff --git a/web/service/submitqueue/src/app/api/queue/[queue]/requests/route.test.ts b/web/service/submitqueue/src/app/api/queue/[queue]/requests/route.test.ts new file mode 100644 index 000000000..11e377a9b --- /dev/null +++ b/web/service/submitqueue/src/app/api/queue/[queue]/requests/route.test.ts @@ -0,0 +1,48 @@ +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { GET } from "./route"; + +const { queueRequests } = vi.hoisted(() => ({ queueRequests: vi.fn() })); +vi.mock("../../../../../server/host", () => ({ resolveReferenceWeb: () => ({ queueRequests }) })); +vi.mock("../../../../../server/observability", () => ({ resolveReferenceLogCallback: () => vi.fn() })); +const token = "demo-token-with-at-least-thirty-two-characters"; +const authorization = `Basic ${Buffer.from(`test:${token}`).toString("base64")}`; +function request(target: string, authorized = true) { + return new Request(`http://localhost${target}`, { headers: authorized ? { authorization } : {} }); +} +beforeEach(() => { + vi.stubEnv("SUBMITQUEUE_WEB_TOKEN", token); + queueRequests.mockReset().mockResolvedValue({ kind: "ok", page: { requests: [], nextHref: null } }); +}); +afterEach(() => vi.unstubAllEnvs()); + +it("requires authentication before reading", async () => { + const response = await GET(request("/api/queue/demo-queue/requests", false)); + expect(response.status).toBe(401); + expect(response.headers.get("www-authenticate")).toContain("Basic"); + expect(queueRequests).not.toHaveBeenCalled(); +}); + +it("reads the decoded queue with its statuses and opaque cursor, uncached", async () => { + const response = await GET(request(`/api/queue/uber-code%2Fsqv2-dummy/requests?status=error&status=landed&page=${encodeURIComponent("opaque+/=")}`)); + expect(response.status).toBe(200); + expect(response.headers.get("cache-control")).toBe("no-store"); + expect(await response.json()).toEqual({ requests: [], nextHref: null }); + expect(queueRequests).toHaveBeenCalledWith(expect.objectContaining({ + queue: "uber-code/sqv2-dummy", search: { page: "opaque+/=", status: ["error", "landed"] }, signal: expect.any(AbortSignal), + })); +}); + +it("rejects a malformed queue segment without reading", async () => { + expect((await GET(request("/api/queue/demo%E0%A4%A/requests"))).status).toBe(400); + expect(queueRequests).not.toHaveBeenCalled(); +}); + +it.each([ + [{ kind: "not-found" }, 404], + [{ kind: "forbidden" }, 403], + [{ kind: "invalid" }, 400], + [{ kind: "unavailable", error: { kind: "transient" } }, 503], +])("maps %j to its status", async (result, status) => { + queueRequests.mockResolvedValueOnce(result); + expect((await GET(request("/api/queue/demo-queue/requests"))).status).toBe(status); +}); diff --git a/web/service/submitqueue/src/app/api/queue/[queue]/requests/route.ts b/web/service/submitqueue/src/app/api/queue/[queue]/requests/route.ts new file mode 100644 index 000000000..08cfcfc8a --- /dev/null +++ b/web/service/submitqueue/src/app/api/queue/[queue]/requests/route.ts @@ -0,0 +1,28 @@ +import { decodeQueuePathSegment, type WebSearchParams } from "@submitqueue/web-submitqueue/server"; +import { authenticationChallenge, isAuthorized, loadAuthConfiguration } from "../../../../../server/auth"; +import { resolveReferenceWeb } from "../../../../../server/host"; +import { resolveReferenceLogCallback } from "../../../../../server/observability"; + +export const dynamic = "force-dynamic"; + +const statusCodes = { "not-found": 404, forbidden: 403, invalid: 400, unavailable: 503 } as const; + +/** `GET /api/queue//requests?status=…&page=…`: the queue page's requests as JSON. */ +export async function GET(request: Request): Promise { + if (!isAuthorized(request.headers.get("authorization"), loadAuthConfiguration())) return authenticationChallenge(); + const url = new URL(request.url); + // The raw segment, not Next's params, so an encoded "/" in a queue name is decoded exactly once. + const queue = decodeQueuePathSegment(url.pathname.split("/")[3] ?? ""); + if (queue === null) return new Response(null, { status: 400 }); + const page = url.searchParams.get("page"); + const statuses = url.searchParams.getAll("status"); + const search: WebSearchParams = { + ...(page === null ? {} : { page }), + ...(statuses.length > 0 ? { status: statuses } : {}), + }; + const result = await resolveReferenceWeb().queueRequests({ + queue, search, signal: request.signal, onLog: resolveReferenceLogCallback(), + }); + if (result.kind !== "ok") return new Response(null, { status: statusCodes[result.kind] }); + return Response.json(result.page, { headers: { "cache-control": "no-store" } }); +} diff --git a/web/service/submitqueue/src/app/layout.tsx b/web/service/submitqueue/src/app/layout.tsx index 461122a7c..b7c878efd 100644 --- a/web/service/submitqueue/src/app/layout.tsx +++ b/web/service/submitqueue/src/app/layout.tsx @@ -1,6 +1,7 @@ import type { Metadata } from "next"; import type { ReactNode } from "react"; import { ThemeProvider } from "@submitqueue/web-service-shared/theme"; +import { QueueProvider } from "../next/queue-provider"; import "@submitqueue/web-submitqueue/styles.css"; @@ -9,13 +10,13 @@ export const metadata: Metadata = { default: "SubmitQueue", template: "%s · SubmitQueue", }, - description: "Read-only request status for the SubmitQueue demo queue.", + description: "Request status for the SubmitQueue demo queue.", }; export default function RootLayout({ children }: { children: ReactNode }) { return ( - {children} + {children} ); } diff --git a/web/service/submitqueue/src/next/queue-provider.tsx b/web/service/submitqueue/src/next/queue-provider.tsx new file mode 100644 index 000000000..d577ae0a7 --- /dev/null +++ b/web/service/submitqueue/src/next/queue-provider.tsx @@ -0,0 +1,16 @@ +"use client"; + +import type { ReactNode } from "react"; +import { RequestPagesProvider, type RequestPageLoader } from "@submitqueue/web-submitqueue"; + +// A queue page `/queue/?…` reads its requests from `/api/queue//requests?…`. +const loadPage: RequestPageLoader = async (href, signal) => { + const continuation = new URL(href, window.location.origin); + const response = await fetch(`/api${continuation.pathname}/requests${continuation.search}`, { signal, cache: "no-store" }); + if (!response.ok) throw new Error("Queue page unavailable"); + return response.json(); +}; + +export function QueueProvider({ children }: { children: ReactNode }) { + return {children}; +} diff --git a/web/service/submitqueue/src/server/instrumentation.ts b/web/service/submitqueue/src/server/instrumentation.ts index 6e0bfd459..090066394 100644 --- a/web/service/submitqueue/src/server/instrumentation.ts +++ b/web/service/submitqueue/src/server/instrumentation.ts @@ -29,6 +29,24 @@ export function instrumentSubmitQueueWeb(web: SubmitQueueWeb, meter: Meter, trac }).record(Math.max(0, performance.now() - started), { page, outcome })); } }), + queueRequests: request => withSpan(tracer, "web.queue_requests", async span => { + const started = performance.now(); + let outcome: string = "exception"; + try { + const result = await web.queueRequests(request); + outcome = result.kind === "unavailable" ? result.error.kind : result.kind; + if (result.kind === "unavailable") { + observeTelemetry(() => span?.setStatus({ code: SpanStatusCode.ERROR })); + } + return result; + } finally { + observeTelemetry(() => span?.setAttribute("web.outcome", outcome)); + observeTelemetry(() => meter.createHistogram("web.queue_requests.duration", { + unit: "ms", + advice: { explicitBucketBoundaries: LATENCY_BUCKETS_MS }, + }).record(Math.max(0, performance.now() - started), { outcome })); + } + }), }; } diff --git a/web/stovepipe/README.md b/web/stovepipe/README.md index 9bb53bd18..25327a185 100644 --- a/web/stovepipe/README.md +++ b/web/stovepipe/README.md @@ -12,7 +12,7 @@ Instrumentation belongs in the host: wrap `handle` to observe page results and i Use the server entry's `createStovepipeWeb` to load a page and render its model with `StovepipeApp`. Mount beneath `WebNavigationProvider` when the host supplies client navigation and refresh. Without it, links and refresh use ordinary document navigation. Import `@submitqueue/web-stovepipe/styles.css` for long-identifier wrapping, and supply BaseUI's `BaseProvider` and Styletron's `Provider`. The host owns the theme and styling engine, including SSR stylesheet extraction and hydration; the library creates neither. BaseUI, React, React DOM, and Styletron React are peer dependencies. -Mount `StovepipeQueueProvider` above route changes with a `loadPage(href, signal)` callback that returns a `QueueRequestsModel` through the host's authenticated transport. Queue lists then load older rows automatically near the bottom, with a keyboard-accessible Load more button and explicit retry after failure. The provider retains loaded rows and scroll positions across detail visits. Without JavaScript or a loader, continuation links navigate normally. The reference host supplies an authenticated, uncached JSON route. +Mount `RequestPagesProvider` above route changes with a `loadPage(href, signal)` callback that returns a `QueueRequestsModel` through the host's authenticated transport. Queue lists then load older rows automatically near the bottom, with a keyboard-accessible Load more button and explicit retry after failure. The provider retains loaded rows and scroll positions across detail visits. Without JavaScript or a loader, continuation links navigate normally. The reference host supplies an authenticated, uncached JSON route. ## Routes and data diff --git a/web/stovepipe/index.ts b/web/stovepipe/index.ts index 1ad6a82f5..f667eaf09 100644 --- a/web/stovepipe/index.ts +++ b/web/stovepipe/index.ts @@ -1,8 +1,9 @@ "use client"; export { StovepipeApp } from "./view/app.js"; export type { QueueRequestsModel, StovepipePageModel } from "./entity/index.js"; -export { StovepipeQueueProvider, type QueuePageLoader } from "./view/queue.js"; export { + RequestPagesProvider, + type RequestPageLoader, WebNavigationProvider, type WebLinkProps, type WebNavigation, diff --git a/web/stovepipe/view/queue.tsx b/web/stovepipe/view/queue.tsx index 1d5483d96..b9d61cf65 100644 --- a/web/stovepipe/view/queue.tsx +++ b/web/stovepipe/view/queue.tsx @@ -1,177 +1,13 @@ "use client"; -import { createContext, useCallback, useContext, useEffect, useLayoutEffect, useMemo, useRef, useState, type ReactNode } from "react"; -import { Block } from "baseui/block"; -import { Button, KIND, SIZE } from "baseui/button"; -import { Spinner } from "baseui/spinner"; -import { ParagraphSmall, ParagraphXSmall } from "baseui/typography"; -import type { QueueRequestsModel } from "../entity/index.js"; -import { Link } from "./components.js"; +import { PagedRequests } from "@submitqueue/web-platform"; +import type { QueueRequestsModel, RequestSummaryModel } from "../entity/index.js"; import { RequestList } from "./pages.js"; -const useBrowserLayoutEffect = typeof window === "undefined" ? useEffect : useLayoutEffect; - -/** Fetches a continuation through the host's authenticated transport. */ -export type QueuePageLoader = (href: string, signal: AbortSignal) => Promise; - -interface QueueSnapshot extends QueueRequestsModel { - latest: QueueRequestsModel; - firstPageIds: string[]; - loadedHrefs: string[]; -} -interface SavedQueue { - snapshot: QueueSnapshot; - scrollY: number; -} -const QueueContext = createContext<{ - loadPage: QueuePageLoader; - pages: Map; -} | null>(null); - -/** Mount above route changes to retain loaded queue rows and scroll positions. */ -export function StovepipeQueueProvider({ loadPage, children }: { loadPage: QueuePageLoader; children: ReactNode }) { - const pages = useRef(new Map()); - const value = useMemo(() => ({ loadPage, pages: pages.current }), [loadPage]); - return {children}; -} - -function createQueueSnapshot(page: QueueRequestsModel): QueueSnapshot { - return { ...page, latest: page, firstPageIds: page.requests.map(request => request.id), loadedHrefs: [] }; -} +const requestId = (request: RequestSummaryModel) => request.id; export function QueueRequests({ initial, routeKey }: { initial: QueueRequestsModel; routeKey: string }) { - const context = useContext(QueueContext); - const [snapshot, setSnapshot] = useState(() => context?.pages.get(routeKey)?.snapshot ?? createQueueSnapshot(initial)); - const [hydrated, setHydrated] = useState(false); - const [loading, setLoading] = useState(false); - const [error, setError] = useState(false); - const sentinel = useRef(null); - const inFlight = useRef(null); - const saved = useRef({ snapshot, scrollY: context?.pages.get(routeKey)?.scrollY ?? 0 }); - saved.current.snapshot = snapshot; - - if (snapshot.latest !== initial) { - const current = new Map(initial.requests.map(request => [request.id, request])); - setSnapshot({ ...snapshot, latest: initial, requests: snapshot.requests.map(request => current.get(request.id) ?? request) }); - } - - useBrowserLayoutEffect(() => { - setHydrated(true); - const previous = context?.pages.get(routeKey); - const frame = previous ? requestAnimationFrame(() => window.scrollTo(0, previous.scrollY)) : null; - const recordScroll = () => { - saved.current.scrollY = window.scrollY; - }; - window.addEventListener("scroll", recordScroll, { passive: true }); - return () => { - if (frame !== null) cancelAnimationFrame(frame); - window.removeEventListener("scroll", recordScroll); - // Save before navigation can shrink the document and reset its scroll offset. - inFlight.current?.abort(); - context?.pages.set(routeKey, saved.current); - }; - }, [context, routeKey]); - - const loadOlderRequests = useCallback(async () => { - const href = snapshot.nextHref; - if (!context || !href || inFlight.current) return; - const controller = new AbortController(); - inFlight.current = controller; - setLoading(true); - setError(false); - try { - const next = await context.loadPage(href, controller.signal); - if (controller.signal.aborted) return; - if (next.nextHref && [href, ...snapshot.loadedHrefs].includes(next.nextHref)) throw new Error("Repeated continuation"); - setSnapshot(current => { - if (current.nextHref !== href) return current; - const existing = new Set(current.requests.map(request => request.id)); - const additions = next.requests.filter(request => { - if (existing.has(request.id)) return false; - existing.add(request.id); - return true; - }); - return { ...current, requests: [...current.requests, ...additions], nextHref: next.nextHref, loadedHrefs: [...current.loadedHrefs, href] }; - }); - } catch { - if (!controller.signal.aborted) setError(true); - } finally { - if (inFlight.current === controller) { - inFlight.current = null; - setLoading(false); - } - } - }, [context, snapshot.nextHref, snapshot.loadedHrefs]); - - useEffect(() => { - if (!context || !snapshot.nextHref || error || loading || !sentinel.current || typeof IntersectionObserver === "undefined") return; - const observer = new IntersectionObserver(entries => { - if (entries.some(entry => entry.isIntersecting)) void loadOlderRequests(); - }, { rootMargin: "400px" }); - observer.observe(sentinel.current); - return () => observer.disconnect(); - }, [context, snapshot.nextHref, error, loading, loadOlderRequests]); - - let paginationMessage: string; - if (loading) { - paginationMessage = "Loading older requests…"; - } else if (error) { - paginationMessage = "Loading paused"; - } else if (snapshot.nextHref && context && hydrated && typeof IntersectionObserver !== "undefined") { - paginationMessage = "Scroll for more requests"; - } else if (snapshot.nextHref) { - paginationMessage = "More requests available"; - } else if (snapshot.requests.length) { - paginationMessage = "All available requests loaded"; - } else { - paginationMessage = "No requests to load"; - } - - const newRequests = initial.requests.some(request => !snapshot.firstPageIds.includes(request.id)); - return <> - {newRequests && - - } - -
- {error && Could not load older requests. Your displayed requests are unchanged.} - - - - - {loading && - } - - - {paginationMessage} - - - {snapshot.requests.length} displayed requests - - - {snapshot.nextHref && (context && hydrated - ? - : Older requests →)} - - -
- ; + return + {requests => } + ; } diff --git a/web/submitqueue/README.md b/web/submitqueue/README.md index e436d9cd9..e300cd12e 100644 --- a/web/submitqueue/README.md +++ b/web/submitqueue/README.md @@ -15,13 +15,13 @@ Entry points: the root is client-safe (`SubmitQueueApp`, page frames, `WebNaviga Process the UI entry with the host's bundler for both SSR and the browser. The `/server` entry runs directly in Node and does not load BaseUI. See the shared [package contract](../README.md#layers). -**Hosting it.** Give `createSubmitQueueWeb` a gateway client (any client matching the structural contract, so the module ships no generated code) and optionally a mount path, authorization check, error classifier, or cached queue catalog. Route every page path to `handle`, render `SubmitQueueApp` with each page model (it includes the shared header, time zone preference and refresh controls), and wrap it in a `WebNavigationProvider` for client-side navigation. Load `./styles.css` for long-identifier wrapping and supply BaseUI's `BaseProvider` and Styletron's `Provider`. The host owns the theme and styling engine, including SSR stylesheet extraction and hydration; BaseUI, React, React DOM, and Styletron React are peers. `WebShell` follows the provider's theme rather than taking a separate theme prop. +**Hosting it.** Give `createSubmitQueueWeb` a gateway client (any client matching the structural contract, so the module ships no generated code) and optionally a mount path, authorization check, error classifier, or cached queue catalog. Route every page path to `handle`, render `SubmitQueueApp` with each page model (it includes the shared header, time zone preference and refresh controls), and wrap it in a `WebNavigationProvider` for client-side navigation. Mount `RequestPagesProvider` above route changes with a `loadPage(href, signal)` callback that returns the queue page's `QueueRequestsModel` through the host's authenticated transport; queue lists then load older rows automatically near the bottom, with a Load more button and explicit retry after failure, and keep loaded rows and scroll position across request visits. Without it, continuation links navigate normally. Load `./styles.css` for long-identifier wrapping and supply BaseUI's `BaseProvider` and Styletron's `Provider`. The host owns the theme and styling engine, including SSR stylesheet extraction and hydration; BaseUI, React, React DOM, and Styletron React are peers. `WebShell` follows the provider's theme rather than taking a separate theme prop. Pass `onLog` on each `handle` request to deliver severity, message, and structured metadata to the host's request-scoped logger. Errors retain their original value under `error`, and logs include the correlation `requestId` and page `path`. The callback must be synchronous; omitting it is silent, and callback exceptions do not change page results. The module has no logging-library dependency. Instrumentation belongs in the host: wrap `handle` to observe page results and instrument the concrete gateway client for RPC traces and timings. A `render` result can contain a failed load, so check `model.result.ok` before reporting success. The reference host uses native OpenTelemetry APIs; other hosts can use their own telemetry without an adapter. The reusable module has no telemetry dependency or instrumentation API. -**Behavior worth knowing.** Older pages put the gateway's `List` continuation token in the URL as `?page=` unchanged; the token carries its own window, so continuing needs only the queue and token, and hosts hold no pagination secret. Expected outcomes (missing resources, page tokens the gateway rejects) are results, never exceptions. Raw gateway errors are logged, never rendered. Views keep the last good data on transient failures. Request IDs stay opaque in URLs; dot-only segments get a visible `~` escape. +**Behavior worth knowing.** Pages live under `/queue/` (`/request/` and `/change/…` below it), so queue names never collide with host routes. `queueRequests` returns the queue page's requests as data for a host's JSON route, with the same authorization and filters. A queue list starts at its newest request however old it is, with no time window in the URL. Repeated `?status=` parameters restrict it to those current statuses; the gateway applies the filter, so a filtered page can be short while older matches load as the viewer scrolls. Queue refreshes update the status of displayed requests in place; new arrivals appear behind a Show latest requests action. Older pages put the gateway's `List` continuation token in the URL as `?page=` unchanged; the token carries its own window, so continuing needs only the queue and token, and hosts hold no pagination secret. Expected outcomes (missing resources, page tokens the gateway rejects) are results, never exceptions. Raw gateway errors are logged, never rendered. Views keep the last good data on transient failures. Request IDs stay opaque in URLs; dot-only segments get a visible `~` escape. The header offers optional ten-second auto-refresh, off by default. Browser reload remains available for manual updates. Polling pauses while the page is hidden or offline and stops for terminal views. diff --git a/web/submitqueue/controller/mapping.ts b/web/submitqueue/controller/mapping.ts index 1f884c7f3..930ee1ce0 100644 --- a/web/submitqueue/controller/mapping.ts +++ b/web/submitqueue/controller/mapping.ts @@ -13,12 +13,18 @@ export function mapRequestSummary(value: GatewayRequestSummary): RequestSummaryM queue: value.queue, changeUris: [...value.changeUris], receivedAtMs, + updatedAtMs: positiveMs(value.updatedAtMs), status: value.status, lastError: value.lastError === "" ? null : value.lastError, metadata: { ...value.metadata }, }; } +function positiveMs(value: Int64Value): number | null { + const ms = safeInt64ToNumber(value); + return ms === null || ms <= 0 ? null : ms; +} + export function mapResponseWindow(value: { receivedAtOrAfterMs: Int64Value; receivedBeforeMs: Int64Value }): RequestWindow | null { const fromMs = safeInt64ToNumber(value.receivedAtOrAfterMs); const toMs = safeInt64ToNumber(value.receivedBeforeMs); diff --git a/web/submitqueue/controller/queue.ts b/web/submitqueue/controller/queue.ts index b8fa12a5c..ca3751ae4 100644 --- a/web/submitqueue/controller/queue.ts +++ b/web/submitqueue/controller/queue.ts @@ -1,9 +1,8 @@ import type { WebSearchParams } from "../core/route.js"; import type { LoadResult } from "../entity/index.js"; -import type { RequestListModel, RequestSummaryModel } from "../entity/index.js"; +import type { QueueRequestsModel, RequestListModel, RequestSummaryModel } from "../entity/index.js"; import type { GatewayReader } from "../extension/gateway/index.js"; -import { requestChangeLabels, requestChangeLinks } from "../core/change.js"; -import { defaultRequestWindow, REQUEST_PAGE_SIZE, type RequestPageStart } from "../core/window.js"; +import { allReceiptsWindow, REQUEST_PAGE_SIZE, type RequestPageStart } from "../core/window.js"; import { INTERNAL_ERROR, INVALID_INPUT } from "./error.js"; import { callGateway, type GatewayLoadOptions } from "./gateway-call.js"; import { mapRequestSummary, mapResponseWindow } from "./mapping.js"; @@ -12,6 +11,8 @@ import { redirect, refreshModel, render, type PageDependencies, type SubmitQueue export type LoadRequestListInput = RequestPageStart & { queue: string; pageSize?: number; + /** Current statuses to include; empty or absent includes every status. */ + statuses?: readonly string[]; }; function validListInput(input: LoadRequestListInput): boolean { @@ -22,13 +23,19 @@ function validListInput(input: LoadRequestListInput): boolean { return input.queue !== "" && validStart && Number.isInteger(pageSize) && pageSize > 0; } -/** The gateway `list` input for one page; bounds are sent only when no token carries them. */ -export function gatewayListInput(queue: string, start: RequestPageStart, pageSize = REQUEST_PAGE_SIZE): Parameters[0] { +/** + * The gateway `list` input for one page; bounds are sent only when no token carries them. Statuses are sent with a + * token too, so the gateway rejects a URL whose filter disagrees with its token. + */ +export function gatewayListInput( + queue: string, start: RequestPageStart, pageSize = REQUEST_PAGE_SIZE, statuses: readonly string[] = [], +): Parameters[0] { + const filter = statuses.length > 0 ? { statuses: [...statuses] } : {}; if ("pageToken" in start) { - return { queue, pageSize, pageToken: start.pageToken }; + return { queue, pageSize, pageToken: start.pageToken, ...filter }; } return { - queue, pageSize, pageToken: "", + queue, pageSize, pageToken: "", ...filter, receivedLowerBound: { case: "receivedAtOrAfterMs", value: BigInt(start.window.fromMs) }, receivedUpperBound: { case: "receivedBeforeMs", value: BigInt(start.window.toMs) }, }; @@ -43,7 +50,7 @@ export async function loadRequestList( return { ok: false, error: INVALID_INPUT }; } const call = await callGateway(options, "list", { queue: input.queue }, () => - gateway.list(gatewayListInput(input.queue, input, input.pageSize))); + gateway.list(gatewayListInput(input.queue, input, input.pageSize, input.statuses))); if (!call.ok) { return call; } @@ -77,26 +84,35 @@ export async function loadQueuePage( queue: string, search: WebSearchParams, ): Promise { - const latestHref = deps.paths.requests(queue); - if (search.from !== undefined || search.to !== undefined || + const requested = search.status === undefined ? [] : typeof search.status === "string" ? [search.status] : search.status; + const statuses = [...new Set(requested)].sort(); + const latestHref = deps.paths.requests(queue, { statuses }); + if (statuses.includes("") || search.from !== undefined || search.to !== undefined || (search.page !== undefined && (typeof search.page !== "string" || search.page === ""))) { - return redirect(latestHref); + return redirect(deps.paths.requests(queue, { statuses: statuses.filter(status => status !== "") })); + } + if (statuses.join("\u0000") !== requested.join("\u0000")) { + return redirect(deps.paths.requests(queue, { statuses, pageToken: typeof search.page === "string" ? search.page : null })); } const pageToken = search.page; - const liveWindow = defaultRequestWindow(deps.now()); - const result = deps.catalog.ok ? await loadRequestList(deps.gateway, { - queue, pageSize: REQUEST_PAGE_SIZE, ...(pageToken === undefined ? { window: liveWindow } : { pageToken }), + const loaded = deps.catalog.ok ? await loadRequestList(deps.gateway, { + queue, pageSize: REQUEST_PAGE_SIZE, statuses, + ...(pageToken === undefined ? { window: allReceiptsWindow(deps.now()) } : { pageToken }), }, deps.gatewayOptions) : deps.catalog; - // The gateway rejects a stale or foreign token; restart from the live window rather than show an error. - if (pageToken !== undefined && !result.ok && result.error === INVALID_INPUT) { + // The gateway rejects a stale or foreign token; restart from the newest requests rather than show an error. + if (pageToken !== undefined && !loaded.ok && loaded.error === INVALID_INPUT) { return redirect(latestHref); } - const window = result.ok ? { fromMs: result.data.receivedAtOrAfterMs, toMs: result.data.receivedBeforeMs } : liveWindow; + const result: LoadResult = loaded.ok ? { + ok: true, + data: { + requests: loaded.data.requests, + nextHref: loaded.data.nextPageToken === null ? null : deps.paths.requests(queue, { statuses, pageToken: loaded.data.nextPageToken }), + }, + } : loaded; return render({ - kind: "queue", key: `${queue}:${pageToken ?? "live"}`, title: queue, - basePath: deps.paths.basePath, queue, window, + kind: "queue", key: `${queue}:${statuses.join(",")}:${pageToken ?? "live"}`, title: queue, + basePath: deps.paths.basePath, queue, statuses, paged: pageToken !== undefined, latestHref, result, refresh: refreshModel(result, pageToken !== undefined ? latestHref : null), - changeLinks: result.ok ? requestChangeLinks(queue, result.data.requests, deps.paths) : {}, - changeLabels: result.ok ? requestChangeLabels(result.data.requests) : {}, }); } diff --git a/web/submitqueue/core/change.test.ts b/web/submitqueue/core/change.test.ts index 43db2c8ab..c10c36c6c 100644 --- a/web/submitqueue/core/change.test.ts +++ b/web/submitqueue/core/change.test.ts @@ -6,14 +6,14 @@ const sha = "0123456789abcdef0123456789abcdef01234567"; describe("readable change identity", () => { it.each([ - [`github://github.com/uber/submitqueue/pull/123/${sha}`, `/demo-queue/change/github/github.com/uber/submitqueue/pull/123/${sha}`], - ["phab://phabricator.example.com/D12345/67890", "/demo-queue/change/phab/phabricator.example.com/D12345/67890"], - [`git://git.example.com/uber/demo/refs%2Fheads%2Fmain/${sha}`, `/demo-queue/change/git/git.example.com/uber/demo/refs%2Fheads%2Fmain/${sha}`], + [`github://github.com/uber/submitqueue/pull/123/${sha}`, `/queue/demo-queue/change/github/github.com/uber/submitqueue/pull/123/${sha}`], + ["phab://phabricator.example.com/D12345/67890", "/queue/demo-queue/change/phab/phabricator.example.com/D12345/67890"], + [`git://git.example.com/uber/demo/refs%2Fheads%2Fmain/${sha}`, `/queue/demo-queue/change/git/git.example.com/uber/demo/refs%2Fheads%2Fmain/${sha}`], ])("preserves authority and path from %s", (uri, href) => { const reference = parseChangeUri(uri); expect(reference).not.toBeNull(); expect(changeHref("demo-queue", reference!, true)).toBe(href); - expect(parseChangePath(href.split("/").slice(3).map(decodeURIComponent))?.uri).toBe(uri); + expect(parseChangePath(href.split("/").slice(4).map(decodeURIComponent))?.uri).toBe(uri); }); it("distinguishes logical review identity from its submitted version", () => { @@ -37,12 +37,12 @@ describe("readable change identity", () => { }); it.each([ - [`git://git.example.com/demo/refs%2Fheads%2Fmain/${sha}`, "?sq-files=demo%2F00%2Ffile.txt", "/demo-queue/change/git/git.example.com/demo/refs%2Fheads%2Fmain"], - [`github://github.com/uber/submitqueue/pull/123/${sha}`, "?sq-fake=build-fail&attempt=2", "/demo-queue/change/github/github.com/uber/submitqueue/pull/123"], + [`git://git.example.com/demo/refs%2Fheads%2Fmain/${sha}`, "?sq-files=demo%2F00%2Ffile.txt", "/queue/demo-queue/change/git/git.example.com/demo/refs%2Fheads%2Fmain"], + [`github://github.com/uber/submitqueue/pull/123/${sha}`, "?sq-fake=build-fail&attempt=2", "/queue/demo-queue/change/github/github.com/uber/submitqueue/pull/123"], ])("hides URI query markers in labels/links without changing the submitted URI: %s", (clean, query, expectedHref) => { const raw = `${clean}${query}`; const request: RequestSummaryModel = { - sqid: "42", queue: "demo-queue", changeUris: [raw], receivedAtMs: 1, + sqid: "42", queue: "demo-queue", changeUris: [raw], receivedAtMs: 1, updatedAtMs: null, status: "landed", lastError: null, metadata: {}, }; expect(requestChangeLabels([request])[raw]).toBe(clean); @@ -53,7 +53,7 @@ describe("readable change identity", () => { it("does not relabel a URI that is already canonical", () => { const uri = `github://github.com/uber/submitqueue/pull/123/${sha}`; const request: RequestSummaryModel = { - sqid: "42", queue: "demo-queue", changeUris: [uri], receivedAtMs: 1, + sqid: "42", queue: "demo-queue", changeUris: [uri], receivedAtMs: 1, updatedAtMs: null, status: "landed", lastError: null, metadata: {}, }; expect(requestChangeLabels([request])).toEqual({}); diff --git a/web/submitqueue/core/paths.test.ts b/web/submitqueue/core/paths.test.ts index 470fff634..dc887c08d 100644 --- a/web/submitqueue/core/paths.test.ts +++ b/web/submitqueue/core/paths.test.ts @@ -7,7 +7,7 @@ describe("WebPaths", () => { const paths = new WebPaths(); expect(paths.request("demo-queue", "demo-queue/42")).toBe( - "/demo-queue/request/demo-queue/42", + "/queue/demo-queue/request/demo-queue/42", ); }); @@ -18,7 +18,7 @@ describe("WebPaths", () => { }); expect(href).toBe( - "/submitqueue/demo%20queue?page=opaque%2B%2F%3D", + "/submitqueue/queue/demo%20queue?page=opaque%2B%2F%3D", ); }); @@ -26,20 +26,21 @@ describe("WebPaths", () => { const paths = new WebPaths(); expect(paths.request("demo-queue", "opaque/id?query#fragment")).toBe( - "/demo-queue/request/opaque/id%3Fquery%23fragment", + "/queue/demo-queue/request/opaque/id%3Fquery%23fragment", ); }); it.each([".", "..", "~.", "~value"])("round-trips %s without URL dot normalization", (id) => { const paths = new WebPaths(); const url = new URL(paths.request(id, `${id}/42`), "https://example.test"); - const [queue, resource, ...sqid] = url.pathname.slice(1).split("/").map(decodeURIComponent); + const [scope, queue, resource, ...sqid] = url.pathname.slice(1).split("/").map(decodeURIComponent); + expect(scope).toBe("queue"); expect(resource).toBe("request"); expect(decodePathSegment(queue!)).toBe(id); expect(sqid.map(decodePathSegment).join("/")).toBe(`${id}/42`); }); it("makes the history view shareable without consuming part of an opaque ID", () => { - expect(new WebPaths().request("demo-queue", "42", { view: "history" })).toBe("/demo-queue/request/42?view=history"); + expect(new WebPaths().request("demo-queue", "42", { view: "history" })).toBe("/queue/demo-queue/request/42?view=history"); }); }); diff --git a/web/submitqueue/core/paths.ts b/web/submitqueue/core/paths.ts index d01976452..2829b5cbc 100644 --- a/web/submitqueue/core/paths.ts +++ b/web/submitqueue/core/paths.ts @@ -2,6 +2,8 @@ import { encodePathSegment } from "@submitqueue/web-platform/path-segment"; export interface RequestsPathOptions { pageToken?: string | null; + /** Statuses a queue list is restricted to, as repeated `status` parameters. */ + statuses?: readonly string[]; } export interface WebPathsOptions { @@ -29,11 +31,14 @@ export class WebPaths { requests(queue: string, options: RequestsPathOptions = {}): string { const parameters = new URLSearchParams(); + for (const status of options.statuses ?? []) { + parameters.append("status", status); + } if (options.pageToken) { parameters.set("page", options.pageToken); } const query = parameters.toString(); - const queuePath = `${this.basePath}/${encodePathSegment(queue)}`; + const queuePath = `${this.basePath}/queue/${encodePathSegment(queue)}`; return query === "" ? queuePath : `${queuePath}?${query}`; } diff --git a/web/submitqueue/core/route.test.ts b/web/submitqueue/core/route.test.ts index 6de00150a..bdf932d13 100644 --- a/web/submitqueue/core/route.test.ts +++ b/web/submitqueue/core/route.test.ts @@ -19,21 +19,23 @@ describe("parseRoute", () => { { kind: "request", queue: "demo-queue", sqid: "demo-queue/42", search: {} }], ["an escaped sqid", paths.request("demo-queue", "opaque/id?query#~."), { kind: "request", queue: "demo-queue", sqid: "opaque/id?query#~.", search: {} }], - ["a change", "/demo-queue/change/git/git.example.com/demo/refs%2Fheads%2Fmain", + ["a change", "/queue/demo-queue/change/git/git.example.com/demo/refs%2Fheads%2Fmain", { kind: "change", queue: "demo-queue", reference: ["git", "git.example.com", "demo", "refs/heads/main"], search: {} }], ])("round-trips %s produced by WebPaths", (_name, href, route) => { expect(parseRoute(segments(href))).toEqual(route); }); it("carries search parameters to the page", () => { - expect(parseRoute(["demo-queue"], { page: "cursor" })).toEqual({ kind: "queue", queue: "demo-queue", search: { page: "cursor" } }); + expect(parseRoute(["queue", "demo-queue"], { page: "cursor" })).toEqual({ kind: "queue", queue: "demo-queue", search: { page: "cursor" } }); }); it.each([ - ["an unknown resource", ["demo-queue", "unknown", "1"]], - ["a request without an ID", ["demo-queue", "request"]], - ["an empty segment", ["demo-queue", "", "1"]], - ["malformed percent-encoding", ["demo%E0%A4%A"]], + ["a queue outside /queue", ["demo-queue"]], + ["/queue without a queue", ["queue"]], + ["an unknown resource", ["queue", "demo-queue", "unknown", "1"]], + ["a request without an ID", ["queue", "demo-queue", "request"]], + ["an empty segment", ["queue", "demo-queue", "", "1"]], + ["malformed percent-encoding", ["queue", "demo%E0%A4%A"]], ])("does not route %s", (_name, value) => { expect(parseRoute(value)).toBeNull(); }); diff --git a/web/submitqueue/core/route.ts b/web/submitqueue/core/route.ts index 60de0be50..f22b33a81 100644 --- a/web/submitqueue/core/route.ts +++ b/web/submitqueue/core/route.ts @@ -19,7 +19,8 @@ function decodeComponent(value: string): string | null { } } -function decodeResourceSegment(value: string): string | null { +/** Decodes one still percent-encoded queue or request-ID path segment; `null` when malformed. */ +export function decodeResourceSegment(value: string): string | null { const decoded = decodeComponent(value); return decoded === null ? null : decodePathSegment(decoded); } @@ -32,11 +33,14 @@ export function parseRoute(segments: readonly string[], search: WebSearchParams if (segments.length === 0) { return { kind: "queues" }; } - if (segments.some(segment => segment === "")) { + if (segments.some(segment => segment === "") || segments[0] !== "queue") { return null; } - const [encodedQueue, resource, ...rest] = segments; - const queue = decodeResourceSegment(encodedQueue!); + const [, encodedQueue, resource, ...rest] = segments; + if (encodedQueue === undefined) { + return null; + } + const queue = decodeResourceSegment(encodedQueue); if (queue === null || queue === "") { return null; } diff --git a/web/submitqueue/core/window.ts b/web/submitqueue/core/window.ts index ba5ac55ab..276169c37 100644 --- a/web/submitqueue/core/window.ts +++ b/web/submitqueue/core/window.ts @@ -13,3 +13,8 @@ export type RequestPageStart = Readonly<{ window: RequestWindow }> | Readonly<{ export function defaultRequestWindow(nowMs = Date.now()): RequestWindow { return { fromMs: nowMs - REQUEST_WINDOW_MS, toMs: nowMs }; } + +/** Every receipt before `nowMs`, so a queue list starts at its newest request however old it is. */ +export function allReceiptsWindow(nowMs = Date.now()): RequestWindow { + return { fromMs: 0, toMs: nowMs }; +} diff --git a/web/submitqueue/entity/index.ts b/web/submitqueue/entity/index.ts index 3ad00d404..c57ec9654 100644 --- a/web/submitqueue/entity/index.ts +++ b/web/submitqueue/entity/index.ts @@ -5,7 +5,7 @@ export type { } from "./page.js"; export type { QueueModel } from "./queue.js"; export type { - HistoryEventModel, HistoryEventType, RequestDetailModel, RequestListModel, RequestSummaryModel, + HistoryEventModel, HistoryEventType, QueueRequestsModel, RequestDetailModel, RequestListModel, RequestSummaryModel, } from "./request.js"; export type { StatusDisplay, StatusTone } from "./status.js"; export type { LoadResult, WebError, WebErrorKind } from "./result.js"; diff --git a/web/submitqueue/entity/page.ts b/web/submitqueue/entity/page.ts index 23ab52a02..3b480c6d7 100644 --- a/web/submitqueue/entity/page.ts +++ b/web/submitqueue/entity/page.ts @@ -1,7 +1,7 @@ import type { LoadResult } from "./result.js"; import type { ChangeDetailModel } from "./change.js"; import type { QueueModel } from "./queue.js"; -import type { RequestDetailModel, RequestListModel } from "./request.js"; +import type { QueueRequestsModel, RequestDetailModel } from "./request.js"; /** How the client keeps a rendered page current. */ export interface WebRefreshModel { @@ -37,24 +37,20 @@ export interface QueueDirectoryPageModel extends SubmitQueuePagePresentation { result: LoadResult; } -/** Requests one queue received in a 24-hour window. */ +/** Requests one queue received, newest first. */ export interface QueueRequestsPageModel extends SubmitQueuePagePresentation { /** Page discriminator. */ kind: "queue"; /** Queue name. */ queue: string; - /** Half-open receipt window shown, Unix epoch milliseconds. */ - window: { fromMs: number; toMs: number }; - /** Whether this is an older snapshot page rather than the live first page. */ + /** Statuses the list is restricted to, sorted; empty includes every status. */ + statuses: string[]; + /** Whether this page starts at a continuation rather than the newest request. */ paged: boolean; - /** In-app link to the live first page. */ + /** In-app link to the newest requests under the same status filter. */ latestHref: string; - /** The page of requests, or why it could not be loaded. */ - result: LoadResult; - /** In-app links for change URIs that have a change page, keyed by URI. */ - changeLinks: Record; - /** Display labels for change URIs, keyed by URI; unlabeled URIs show verbatim. */ - changeLabels: Record; + /** The first displayed page of requests, or why it could not be loaded. */ + result: LoadResult; } /** One request's summary or history. */ diff --git a/web/submitqueue/entity/request.ts b/web/submitqueue/entity/request.ts index cd91ef6f3..46abbbfee 100644 --- a/web/submitqueue/entity/request.ts +++ b/web/submitqueue/entity/request.ts @@ -10,6 +10,8 @@ export interface RequestSummaryModel { changeUris: string[]; /** Receipt time, Unix epoch milliseconds. */ receivedAtMs: number; + /** Time the current status was recorded, Unix epoch milliseconds, or `null` when the gateway does not report it. */ + updatedAtMs: number | null; /** Current lifecycle status; unknown values are kept verbatim. */ status: string; /** Most recent failure message, or `null` when none was recorded. */ @@ -51,6 +53,14 @@ export interface RequestListModel { nextPageToken: string | null; } +/** One page of a queue's requests, newest received first. */ +export interface QueueRequestsModel { + /** Requests on this page, newest received first. */ + requests: RequestSummaryModel[]; + /** Opaque in-app URL of the next page; null on the final page. */ + nextHref: string | null; +} + /** A request with its retained history. */ export interface RequestDetailModel { /** The request's current summary. */ diff --git a/web/submitqueue/extension/gateway/gateway.ts b/web/submitqueue/extension/gateway/gateway.ts index 7a4edecdf..00b367951 100644 --- a/web/submitqueue/extension/gateway/gateway.ts +++ b/web/submitqueue/extension/gateway/gateway.ts @@ -18,6 +18,8 @@ export interface GatewayRequestSummary { queue: string; changeUris: readonly string[]; receivedAtMs: Int64Value; + /** Time the current status was recorded; zero from gateways that predate the field. */ + updatedAtMs: Int64Value; status: string; lastError: string; metadata: Readonly>; @@ -33,7 +35,7 @@ export interface GatewayHistoryEvent { } export interface GatewayReader { - /** Bounds are sent on a first page only; a `pageToken` carries its own window. */ + /** Bounds are sent on a first page only; a `pageToken` carries its own window and statuses. */ list( input: { queue: string; @@ -41,6 +43,8 @@ export interface GatewayReader { receivedUpperBound?: { case: "receivedBeforeMs"; value: bigint }; pageSize: number; pageToken: string; + /** Current statuses to include; absent includes every status. */ + statuses?: string[]; }, options?: GatewayCallOptions, ): Promise<{ diff --git a/web/submitqueue/extension/gateway/mock/fake.ts b/web/submitqueue/extension/gateway/mock/fake.ts index 3ab712b2a..b2f16ea33 100644 --- a/web/submitqueue/extension/gateway/mock/fake.ts +++ b/web/submitqueue/extension/gateway/mock/fake.ts @@ -11,6 +11,7 @@ export function gatewayRequestFixture( queue: "demo-queue", changeUris: ["github://github.com/uber/submitqueue/pull/1/0123456789012345678901234567890123456789"], receivedAtMs: DEFAULT_TIMESTAMP, + updatedAtMs: DEFAULT_TIMESTAMP, status: "speculating", lastError: "", metadata: {}, diff --git a/web/submitqueue/index.ts b/web/submitqueue/index.ts index 9a5220430..b512878e6 100644 --- a/web/submitqueue/index.ts +++ b/web/submitqueue/index.ts @@ -5,10 +5,12 @@ export { SubmitQueueApp, SubmitQueuePage, SubmitQueueShell, SubmitQueueStatePage, type SubmitQueuePageProps, type SubmitQueueShellProps, } from "./view/index.js"; -export { WebNavigationProvider, type WebLinkProps, type WebNavigation } from "@submitqueue/web-platform"; +export { + RequestPagesProvider, WebNavigationProvider, type RequestPageLoader, type WebLinkProps, type WebNavigation, +} from "@submitqueue/web-platform"; export type { ChangeDetailModel, ChangePageModel, ChangeSubmissionModel, HistoryEventModel, HistoryEventType, LoadResult, - QueueDirectoryPageModel, QueueModel, QueueRequestsPageModel, RequestDetailModel, RequestListModel, + QueueDirectoryPageModel, QueueModel, QueueRequestsModel, QueueRequestsPageModel, RequestDetailModel, RequestListModel, RequestPageModel, RequestSummaryModel, StatusDisplay, StatusTone, SubmitQueuePageModel, WebError, WebErrorKind, WebPageModel, WebRefreshModel, } from "./entity/index.js"; diff --git a/web/submitqueue/module.test.ts b/web/submitqueue/module.test.ts index 1c8587e05..cb2e60186 100644 --- a/web/submitqueue/module.test.ts +++ b/web/submitqueue/module.test.ts @@ -10,8 +10,8 @@ const now = 1_700_000_000_000; const paths = new WebPaths({ basePath: "/sq" }); const sha = "0123456789abcdef0123456789abcdef01234567"; const gitUri = `git://git.example.com/demo/refs%2Fheads%2Fmain/${sha}`; -const changePath = "/sq/demo-queue/change/git/git.example.com/demo/refs%2Fheads%2Fmain"; -const githubPath = "/sq/demo-queue/change/github/github.com/uber/submitqueue/pull/123"; +const changePath = "/sq/queue/demo-queue/change/git/git.example.com/demo/refs%2Fheads%2Fmain"; +const githubPath = "/sq/queue/demo-queue/change/github/github.com/uber/submitqueue/pull/123"; function newWeb(options: Partial = {}) { const web = createSubmitQueueWeb({ gateway: createFakeGatewayReader(), paths, ...options }); @@ -19,11 +19,11 @@ function newWeb(options: Partial = {}) { web.handle({ path, search, ...extra }); } -function nextPageToken(result: Awaited>>): string { +function nextHref(result: Awaited>>): string { if (result.kind !== "render" || result.model.kind !== "queue" || !result.model.result.ok) { throw new Error("Expected a queue page"); } - return result.model.result.data.nextPageToken!; + return result.model.result.data.nextHref!; } describe("createSubmitQueueWeb", () => { @@ -39,38 +39,56 @@ describe("createSubmitQueueWeb", () => { it.each([ ["outside the mount", "/elsewhere"], - ["an unknown page", "/sq/demo-queue/unknown/x"], - ["malformed percent-encoding", "/sq/%E0%A4%A"], + ["an unknown page", "/sq/queue/demo-queue/unknown/x"], + ["malformed percent-encoding", "/sq/queue/%E0%A4%A"], ])("reports not-found for %s", async (_name, path) => { expect(await newWeb()(path)).toEqual({ kind: "not-found" }); }); - it("links the raw gateway token and resumes it without resending bounds", async () => { + it("starts at the newest request however old, links the raw gateway token, and resumes it without resending bounds", async () => { let clock = now; const gateway = createFakeGatewayReader({ - nextPageToken: "opaque/gateway-token", window: { fromMs: BigInt(now - REQUEST_WINDOW_MS), toMs: BigInt(now) }, + nextPageToken: "opaque/gateway-token", window: { fromMs: 0n, toMs: BigInt(now) }, }); const list = vi.spyOn(gateway, "list"); const get = newWeb({ gateway, now: () => clock }); - const first = await get("/sq/demo-queue"); + const first = await get("/sq/queue/demo-queue"); expect(first).toMatchObject({ - kind: "render", model: { kind: "queue", paged: false, latestHref: "/sq/demo-queue", refresh: { terminal: false } }, + kind: "render", model: { kind: "queue", paged: false, latestHref: "/sq/queue/demo-queue", refresh: { terminal: false } }, }); expect(list).toHaveBeenLastCalledWith(expect.objectContaining({ - receivedLowerBound: { case: "receivedAtOrAfterMs", value: BigInt(now - REQUEST_WINDOW_MS) }, + receivedLowerBound: { case: "receivedAtOrAfterMs", value: 0n }, receivedUpperBound: { case: "receivedBeforeMs", value: BigInt(now) }, })); - const page = nextPageToken(first); - expect(page).toBe("opaque/gateway-token"); + const href = nextHref(first); + expect(href).toBe("/sq/queue/demo-queue?page=opaque%2Fgateway-token"); clock += 10_000; - expect(await get("/sq/demo-queue", { page })).toMatchObject({ - kind: "render", model: { - kind: "queue", paged: true, window: { fromMs: now - REQUEST_WINDOW_MS, toMs: now }, - refresh: { terminal: true, refreshHref: "/sq/demo-queue" }, - }, + const page = new URL(href, "http://host.invalid").searchParams.get("page")!; + expect(await get("/sq/queue/demo-queue", { page })).toMatchObject({ + kind: "render", model: { kind: "queue", paged: true, refresh: { terminal: true, refreshHref: "/sq/queue/demo-queue" } }, }); expect(list).toHaveBeenLastCalledWith({ queue: "demo-queue", pageSize: 50, pageToken: "opaque/gateway-token" }); - expect(await get("/sq/demo-queue")).toMatchObject({ kind: "render", model: { window: { toMs: clock } } }); + await get("/sq/queue/demo-queue"); + expect(list).toHaveBeenLastCalledWith(expect.objectContaining({ + receivedUpperBound: { case: "receivedBeforeMs", value: BigInt(clock) }, + })); + }); + + it("restricts the list to sorted statuses, keeps them on continuation links, and canonicalizes the URL", async () => { + const gateway = createFakeGatewayReader({ nextPageToken: "next" }); + const list = vi.spyOn(gateway, "list"); + const get = newWeb({ gateway, now: () => now }); + expect(await get("/sq/queue/demo-queue", { status: ["landed", "error", "landed"] })) + .toEqual({ kind: "redirect", href: "/sq/queue/demo-queue?status=error&status=landed" }); + const first = await get("/sq/queue/demo-queue", { status: ["error", "landed"] }); + expect(first).toMatchObject({ + kind: "render", model: { kind: "queue", statuses: ["error", "landed"], latestHref: "/sq/queue/demo-queue?status=error&status=landed" }, + }); + expect(list).toHaveBeenLastCalledWith(expect.objectContaining({ statuses: ["error", "landed"], pageToken: "" })); + expect(nextHref(first)).toBe("/sq/queue/demo-queue?status=error&status=landed&page=next"); + await get("/sq/queue/demo-queue", { status: ["error", "landed"], page: "next" }); + expect(list).toHaveBeenLastCalledWith({ queue: "demo-queue", pageSize: 50, pageToken: "next", statuses: ["error", "landed"] }); + expect(await get("/sq/queue/demo-queue", { status: "" })).toEqual({ kind: "redirect", href: "/sq/queue/demo-queue" }); }); it.each([ @@ -78,11 +96,11 @@ describe("createSubmitQueueWeb", () => { { page: ["multiple", "tokens"] }, { page: "" }, ])("normalizes stale or invalid queue inputs to a live URL: %j", async search => { - expect(await newWeb()("/sq/demo-queue", search)).toEqual({ kind: "redirect", href: "/sq/demo-queue" }); + expect(await newWeb()("/sq/queue/demo-queue", search)).toEqual({ kind: "redirect", href: "/sq/queue/demo-queue" }); }); it.each([ - ["queue", "/sq/demo-queue"], + ["queue", "/sq/queue/demo-queue"], ["change", changePath], ])("restarts a %s page from the live window when the gateway rejects its token", async (_name, path) => { const gateway = createFakeGatewayReader({ listError: new ConnectError("bad token", Code.InvalidArgument) }); @@ -96,12 +114,12 @@ describe("createSubmitQueueWeb", () => { history: [gatewayHistoryFixture({ status: "landed" })], }); const summary = vi.spyOn(gateway, "getRequestSummaryByID"); - const result = await newWeb({ gateway })("/sq/demo-queue/request/legacy/id%252Fvalue", { view: "history" }); + const result = await newWeb({ gateway })("/sq/queue/demo-queue/request/legacy/id%252Fvalue", { view: "history" }); expect(summary).toHaveBeenCalledWith({ sqid, queue: "demo-queue" }); expect(result).toMatchObject({ kind: "render", model: { kind: "request", view: "history", title: `${sqid} · History`, - historyHref: "/sq/demo-queue/request/legacy/id%252Fvalue?view=history", + historyHref: "/sq/queue/demo-queue/request/legacy/id%252Fvalue?view=history", refresh: { terminal: true }, result: { ok: true, data: { request: { sqid } } }, }, }); @@ -109,14 +127,14 @@ describe("createSubmitQueueWeb", () => { it("returns explicit not-found decisions without exceptions for absent resources", async () => { const get = newWeb(); - expect(await get("/sq/unknown")).toEqual({ kind: "not-found" }); - expect(await get("/sq/demo-queue/change/unknown")).toEqual({ kind: "not-found" }); + expect(await get("/sq/queue/unknown")).toEqual({ kind: "not-found" }); + expect(await get("/sq/queue/demo-queue/change/unknown")).toEqual({ kind: "not-found" }); }); it("keeps catalog failures visible rather than declaring a configured queue missing", async () => { const gateway = createFakeGatewayReader({ queuesError: new ConnectError("upstream failure", Code.Unavailable) }); const list = vi.spyOn(gateway, "list"); - expect(await newWeb({ gateway })("/sq/demo-queue")).toMatchObject({ + expect(await newWeb({ gateway })("/sq/queue/demo-queue")).toMatchObject({ kind: "render", model: { result: { ok: false, error: { kind: "transient" } }, refresh: { terminal: false } }, }); expect(list).not.toHaveBeenCalled(); @@ -175,18 +193,18 @@ describe("createSubmitQueueWeb", () => { let gateway: SubmitQueueGateway = createFakeGatewayReader({ queues: [{ name: "first" }], requests: [] }); const resolve = vi.fn(() => gateway); const get = newWeb({ gateway: resolve }); - expect(await get("/sq/first", {}, { principal: "viewer" })).toMatchObject({ kind: "render", model: { result: { ok: true } } }); + expect(await get("/sq/queue/first", {}, { principal: "viewer" })).toMatchObject({ kind: "render", model: { result: { ok: true } } }); expect(resolve).toHaveBeenCalledWith(expect.objectContaining({ principal: "viewer" })); gateway = createFakeGatewayReader({ queues: [{ name: "second" }], requests: [] }); - expect(await get("/sq/first")).toEqual({ kind: "not-found" }); - expect(await get("/sq/second")).toMatchObject({ kind: "render", model: { result: { ok: true } } }); + expect(await get("/sq/queue/first")).toEqual({ kind: "not-found" }); + expect(await get("/sq/queue/second")).toMatchObject({ kind: "render", model: { result: { ok: true } } }); }); it("reports forbidden without reading the gateway when authorization fails", async () => { const gateway = createFakeGatewayReader(); const listQueues = vi.spyOn(gateway, "listQueues"); const authorize = vi.fn(() => false); - expect(await newWeb({ gateway, authorize })("/sq/demo-queue", {}, { principal: "viewer" })).toEqual({ kind: "forbidden" }); + expect(await newWeb({ gateway, authorize })("/sq/queue/demo-queue", {}, { principal: "viewer" })).toEqual({ kind: "forbidden" }); expect(authorize).toHaveBeenCalledWith("viewer", { kind: "queue", queue: "demo-queue", search: {} }); expect(listQueues).not.toHaveBeenCalled(); }); diff --git a/web/submitqueue/module.ts b/web/submitqueue/module.ts index be3876767..337f99008 100644 --- a/web/submitqueue/module.ts +++ b/web/submitqueue/module.ts @@ -7,7 +7,7 @@ import type { GatewayLoadOptions } from "./controller/gateway-call.js"; import { notFound, type PageDependencies, type SubmitQueueWebResult } from "./controller/page.js"; import { loadQueuePage } from "./controller/queue.js"; import { loadRequestPage } from "./controller/request.js"; -import type { LoadResult, QueueModel } from "./entity/index.js"; +import type { LoadResult, QueueModel, QueueRequestsModel, WebError } from "./entity/index.js"; import type { GatewayErrorClassifier, SubmitQueueGateway } from "./extension/gateway/index.js"; export type { SubmitQueueWebResult } from "./controller/page.js"; @@ -58,9 +58,25 @@ export interface SubmitQueueWebOptions { newRequestId?: () => string; } +/** One page of a queue's requests as data, such as a continuation an older-requests loader fetches. */ +export interface SubmitQueueQueueRequestsRequest extends Omit { + /** Queue name, decoded. */ + queue: string; +} + +/** A page of a queue's requests, or why there is none; `invalid` covers a stale token or malformed filter. */ +export type QueueRequestsResult = + | { kind: "ok"; page: QueueRequestsModel } + | { kind: "not-found" } + | { kind: "forbidden" } + | { kind: "invalid" } + | { kind: "unavailable"; error: WebError }; + export interface SubmitQueueWeb { /** Serves one page request; never throws for expected outcomes such as missing resources. */ handle(request: SubmitQueueWebRequest): Promise; + /** Reads the same page of requests as the queue page for `search`, with the same authorization. */ + queueRequests(request: SubmitQueueQueueRequestsRequest): Promise; } type QueueCatalogLoader = (gateway: SubmitQueueGateway, context: SubmitQueueRequestContext) => Promise>; @@ -138,6 +154,27 @@ export function createSubmitQueueWeb(options: SubmitQueueWebOptions): SubmitQueu } } + async function serve(route: SubmitQueueWebRoute, request: SubmitQueueWebRequest): Promise { + const requestId = request.requestId ?? newRequestId(); + const onLog = createLogCallback(request.onLog, { requestId, path: request.path }); + if (options.authorize && !(await options.authorize(request.principal, route))) { + onLog("info", "Web page request was not authorized", {}); + return { kind: "forbidden" }; + } + const context: SubmitQueueRequestContext = { + requestId, + onLog, + ...(request.principal === undefined ? {} : { principal: request.principal }), + ...(request.signal ? { signal: request.signal } : {}), + }; + try { + return await load(route, context); + } catch (cause) { + onLog("error", "Web page load failed", { error: cause }); + throw cause; + } + } + return { async handle(request) { const segments = pathSegments(request.path); @@ -145,26 +182,24 @@ export function createSubmitQueueWeb(options: SubmitQueueWebOptions): SubmitQueu return notFound; } const route = parseRoute(segments.slice(mount.length), request.search); - if (route === null) { - return notFound; - } - const requestId = request.requestId ?? newRequestId(); - const onLog = createLogCallback(request.onLog, { requestId, path: request.path }); - if (options.authorize && !(await options.authorize(request.principal, route))) { - onLog("info", "Web page request was not authorized", {}); - return { kind: "forbidden" }; + return route === null ? notFound : serve(route, request); + }, + async queueRequests({ queue, ...request }) { + if (queue === "") { + return { kind: "not-found" }; } - const context: SubmitQueueRequestContext = { - requestId, - onLog, - ...(request.principal === undefined ? {} : { principal: request.principal }), - ...(request.signal ? { signal: request.signal } : {}), - }; - try { - return await load(route, context); - } catch (cause) { - onLog("error", "Web page load failed", { error: cause }); - throw cause; + const result = await serve({ kind: "queue", queue, search: request.search }, { ...request, path: paths.requests(queue) }); + switch (result.kind) { + case "render": + if (result.model.kind !== "queue") { + return { kind: "not-found" }; + } + return result.model.result.ok ? { kind: "ok", page: result.model.result.data } : { kind: "unavailable", error: result.model.result.error }; + // The queue page redirects a stale token or a non-canonical filter; as data there is nothing to redirect to. + case "redirect": + return { kind: "invalid" }; + default: + return result; } }, }; diff --git a/web/submitqueue/server.ts b/web/submitqueue/server.ts index f92686233..f2d11a7e0 100644 --- a/web/submitqueue/server.ts +++ b/web/submitqueue/server.ts @@ -1,11 +1,11 @@ // Server entry: the page handler and the contracts a host implements for it. export { createSubmitQueueWeb, - type QueueCatalogCache, type SubmitQueueRequestContext, type SubmitQueueWeb, type SubmitQueueWebOptions, - type SubmitQueueWebRequest, type SubmitQueueWebResult, + type QueueCatalogCache, type QueueRequestsResult, type SubmitQueueQueueRequestsRequest, type SubmitQueueRequestContext, + type SubmitQueueWeb, type SubmitQueueWebOptions, type SubmitQueueWebRequest, type SubmitQueueWebResult, } from "./module.js"; export * from "./extension/gateway/index.js"; export { grpcStatusErrorCode } from "./extension/gateway/grpcstatus/index.js"; export type * from "./entity/index.js"; -export type { SubmitQueueWebRoute, WebSearchParams } from "./core/route.js"; +export { decodeResourceSegment as decodeQueuePathSegment, type SubmitQueueWebRoute, type WebSearchParams } from "./core/route.js"; export { WebPaths, type RequestPathOptions, type RequestsPathOptions, type WebPathsOptions } from "./core/paths.js"; diff --git a/web/submitqueue/view/components.test.tsx b/web/submitqueue/view/components.test.tsx index c90cc2486..cd7940add 100644 --- a/web/submitqueue/view/components.test.tsx +++ b/web/submitqueue/view/components.test.tsx @@ -2,14 +2,15 @@ import { act, fireEvent, render, screen, within } from "../../platform/test/rtl. import { describe, expect, it, vi } from "vitest"; import { WebNavigationProvider } from "@submitqueue/web-platform"; import { WebApp } from "./web-app.js"; -import { ChangeSubmissions, QueueDirectory, RequestDetail, RequestList, RequestListView, RequestStatus } from "./components.js"; -import type { ChangeDetailModel, RequestDetailModel, RequestListModel } from "../entity/index.js"; +import { ChangeSubmissions, QueueDirectory, QueueRequests, RequestDetail, RequestList, RequestStatus } from "./components.js"; +import type { ChangeDetailModel, RequestDetailModel } from "../entity/index.js"; const request = { sqid: "demo-queue/1", queue: "demo-queue", changeUris: ["github://github.com/uber/submitqueue/pull/1/abc"], receivedAtMs: 1_700_000_000_000, + updatedAtMs: 1_700_000_065_000, status: "speculating", lastError: null, metadata: { batch: "batch-1" }, @@ -17,39 +18,30 @@ const request = { describe("request components", () => { it("renders a linked queue request with its complete sqid", () => { - const model: RequestListModel = { - queue: "demo-queue", - receivedAtOrAfterMs: 1, - receivedBeforeMs: 2, - requests: [request], - nextPageToken: "opaque-token", - }; - render(); + render(); const link = screen.getByRole("link", { name: "demo-queue/1" }); expect(link.getAttribute("href")).toBe( - "/demo-queue/request/demo-queue/1", + "/queue/demo-queue/request/demo-queue/1", ); expect(screen.getByText("Speculating").closest("[data-tone]")?.getAttribute("data-tone")).toBe("progress"); expect(screen.getByText("Nov 14, 2023, 10:13:20 PM UTC")).toBeTruthy(); - expect(screen.getByRole("link", { name: "Next page" }).getAttribute("href")).toBe( - "/demo-queue?page=opaque-token", - ); + expect(screen.getByText("Nov 14, 2023, 10:14:25 PM UTC")).toBeTruthy(); }); - it("renders an accessible empty state", () => { - render( - , - ); - expect(screen.getByText("No requests were received in this window.")).toBeTruthy(); + it("shows an unreported update time as a dash", () => { + render(); + const row = screen.getAllByRole("row")[1]!; + expect(within(row).getAllByRole("cell").at(-1)?.textContent).toBe("—"); + }); + + it.each([ + { name: "an empty queue", filtered: false, complete: true, message: "No requests have been received on this queue." }, + { name: "no matches for a filter", filtered: true, complete: true, message: "No requests on this queue have the selected statuses." }, + { name: "a scan still in progress", filtered: true, complete: false, message: null }, + ])("describes $name accurately", ({ filtered, complete, message }) => { + const { container } = render(); + expect(container.textContent).toBe(message ?? ""); }); it("renders ordered status/build history with expandable metadata", () => { @@ -90,44 +82,27 @@ describe("request components", () => { it("lists configured queues even when there is only one", () => { render(); - expect(screen.getByRole("link", { name: "demo-queue" }).getAttribute("href")).toBe("/demo-queue"); - }); - - it("filters only the displayed page without discarding its pagination link", () => { - render(); - fireEvent.change(screen.getByRole("searchbox"), { target: { value: "landed" } }); - expect(screen.queryByRole("link", { name: request.sqid })).toBeNull(); - expect(screen.getByRole("link", { name: "42" })).toBeTruthy(); - expect(screen.getByRole("link", { name: "Next page" })).toBeTruthy(); + expect(screen.getByRole("link", { name: "demo-queue" }).getAttribute("href")).toBe("/queue/demo-queue"); }); - it("uses host-supplied change labels and links without exposing fake file hints", () => { - const raw = "git://git.example.com/demo/refs%2Fheads%2Fmain/sha?sq-files=demo%2Ffile.txt"; + it("links change URIs to their change page by canonical label without exposing fake file hints", () => { + const sha = "0123456789abcdef0123456789abcdef01234567"; + const raw = `git://git.example.com/demo/refs%2Fheads%2Fmain/${sha}?sq-files=demo%2Ffile.txt`; const clean = raw.split("?")[0]!; - render(); - expect(screen.getByRole("link", { name: clean }).getAttribute("href")).toBe("/change"); + render(); + expect(screen.getByRole("link", { name: clean }).getAttribute("href")) + .toBe("/sq/queue/demo-queue/change/git/git.example.com/demo/refs%2Fheads%2Fmain"); expect(screen.queryByText(/sq-files=/)).toBeNull(); - fireEvent.change(screen.getByRole("searchbox"), { target: { value: "file.txt" } }); - expect(screen.getByText("No displayed requests match.")).toBeTruthy(); }); it("retains the last successful list on a transient failure, but not a permanent failure", () => { - const view = render(); + const view = render(); const error = { kind: "transient" as const, title: "Unavailable", message: "Retry", retryable: true }; - view.rerender(); + view.rerender(); expect(screen.getByRole("link", { name: request.sqid })).toBeTruthy(); expect(screen.getByText(/last successful snapshot/)).toBeTruthy(); - view.rerender(); + view.rerender(); expect(screen.queryByRole("link", { name: request.sqid })).toBeNull(); }); @@ -156,7 +131,7 @@ describe("request components", () => { }} />); const table = screen.getByRole("table", { name: "Change submissions" }); expect(within(table).getAllByText("same-sha")).toHaveLength(2); - expect(within(table).getByRole("link", { name: "38" }).getAttribute("href")).toBe("/demo-queue/request/38"); + expect(within(table).getByRole("link", { name: "38" }).getAttribute("href")).toBe("/queue/demo-queue/request/38"); }); it("navigates between versions through the enclosing app and keeps full versions in links", () => { diff --git a/web/submitqueue/view/components.tsx b/web/submitqueue/view/components.tsx index e54826f72..1eeb08aca 100644 --- a/web/submitqueue/view/components.tsx +++ b/web/submitqueue/view/components.tsx @@ -2,23 +2,23 @@ import { ErrorState } from "./error-state.js"; import { useLastSuccessfulData } from "./last-successful-data.js"; -import { Fact, Table, Timestamp } from "@submitqueue/web-platform"; -import { styled } from "baseui"; +import { Fact, PagedRequests, Table, Timestamp, useWebNavigation } from "@submitqueue/web-platform"; +import { styled, withStyle } from "baseui"; import { Block } from "baseui/block"; -import { Button, SIZE } from "baseui/button"; -import { Input } from "baseui/input"; +import { Button, KIND as BUTTON_KIND, SIZE } from "baseui/button"; import { KIND, Notification } from "baseui/notification"; import { Select, StyledInput } from "baseui/select"; import { Tag } from "baseui/tag"; import { StyledTableBodyCell, StyledTableBodyRow } from "baseui/table-semantic"; -import { HeadingLarge, HeadingSmall, LabelXSmall, ParagraphSmall, ParagraphXSmall } from "baseui/typography"; -import { useWebApp, WebLink } from "./web-app.js"; +import { HeadingMedium, HeadingXSmall, LabelXSmall, ParagraphSmall, ParagraphXSmall } from "baseui/typography"; +import { AccentLink, useWebApp, WebLink } from "./web-app.js"; import { forwardRef, useState, type ComponentPropsWithoutRef, type ReactNode } from "react"; import type { - ChangeDetailModel, HistoryEventModel, LoadResult, QueueModel, RequestDetailModel, RequestListModel, RequestSummaryModel, + ChangeDetailModel, HistoryEventModel, LoadResult, QueueModel, QueueRequestsModel, RequestDetailModel, RequestSummaryModel, } from "../entity/index.js"; +import { requestChangeLabels, requestChangeLinks } from "../core/change.js"; import { WebPaths } from "../core/paths.js"; -import { statusDisplay } from "../core/status.js"; +import { knownRequestStatuses, statusDisplay } from "../core/status.js"; // Closing removes the options; suppress the old ID until the menu reports a new one. const SelectInput = forwardRef>((props, ref) => { @@ -53,8 +53,16 @@ const RequestViewLink = styled(WebLink, ({ paddingBottom: $theme.sizing.scale400, textDecoration: "none", borderBottom: `2px solid ${$active ? $theme.colors.contentPrimary : "transparent"}`, + ":hover": { textDecoration: "none" }, })); +const ChangeLink = styled(WebLink, ({ $theme }) => ({ ...$theme.typography.MonoParagraphXSmall })); + +/** Secondary text for timestamps and column labels, smaller than the values beside it. */ +function Secondary({ children }: { children: ReactNode }) { + return {children}; +} + function Metadata({ values }: { values: Record }) { return {Object.entries(values).sort(([a], [b]) => a.localeCompare(b)).map(([key, value]) => {key}{value} @@ -70,56 +78,140 @@ export function RequestStatus({ status }: { status: string }) { function RequestChanges({ request, links = {}, labels = {} }: { request: RequestSummaryModel; links?: Record; labels?: Record; }) { - return {request.changeUris.map(uri => - {links[uri] ? {labels[uri] ?? uri} : {labels[uri] ?? uri}} + return {request.changeUris.map(uri => + {links[uri] ? {labels[uri] ?? uri} : {labels[uri] ?? uri}} )}; } +const QueueGrid = styled("ul", ({ $theme }) => ({ + display: "grid", + gridTemplateColumns: "repeat(auto-fill, minmax(280px, 1fr))", + gap: $theme.sizing.scale600, + listStyleType: "none", + padding: 0, + margin: 0, +})); + +const QueueCard = styled("li", ({ $theme }) => ({ + position: "relative", + display: "flex", + flexDirection: "column", + gap: $theme.sizing.scale300, + padding: $theme.sizing.scale700, + border: `1px solid ${$theme.colors.borderOpaque}`, + borderRadius: "8px", + transitionProperty: "border-color, box-shadow", + transitionDuration: $theme.animation.timing200, + ":hover": { borderColor: $theme.colors.borderSelected, boxShadow: $theme.lighting.shadow400 }, + ":focus-within": { borderColor: $theme.colors.borderSelected, boxShadow: $theme.lighting.shadow400 }, +})); + +// Stretched over its card so the whole card is clickable while the queue name stays the one link and tab stop. +const QueueCardLink = withStyle(AccentLink, ({ $theme }) => ({ + color: $theme.colors.contentPrimary, + ":visited": { color: $theme.colors.contentPrimary }, + ":hover": { color: $theme.colors.contentPrimary, textDecoration: "none" }, + "::after": { content: '""', position: "absolute", inset: 0, borderRadius: "8px" }, +})); + export function QueueDirectory({ queues, basePath }: { queues: readonly QueueModel[]; basePath?: string }) { + const { Link } = useWebNavigation(); const paths = new WebPaths(basePath === undefined ? {} : { basePath }); return
- QueuesAvailable submit queues - {queues.length === 0 ? No queues are configured. : -
    {queues.map(queue =>
  • - {queue.name}{queue.description ? {queue.description} : null} -
    • )}
} + +
+ Queues + Choose a queue to see its requests, newest first. +
+ {queues.length > 0 ? {queues.length === 1 ? "1 queue" : `${queues.length} queues`} : null} + + {queues.length === 0 ? No queues are configured. : + {queues.map(queue => + {queue.name} + {queue.description ? {queue.description} : null} + + )}}
; } +/** Trail from the queue directory to a queue, and on to `page` within it when given. */ +export function QueueBreadcrumb({ queue, page, queueHref, basePath }: { + queue: string; page?: string; queueHref?: string; basePath?: string; +}) { + const paths = new WebPaths(basePath === undefined ? {} : { basePath }); + return + Queues› + {page ? <>{queue}› {page} : {queue}} + ; +} + export interface RequestListProps { - model: RequestListModel; + queue: string; + requests: RequestSummaryModel[]; basePath?: string; - empty?: ReactNode; - changeLinks?: Record; - changeLabels?: Record; + /** Whether no older requests remain to load; an empty list says so only once complete. */ + complete?: boolean; + /** Whether the list is restricted to some statuses. */ + filtered?: boolean; } -export function RequestList({ model, basePath, empty, changeLinks, changeLabels }: RequestListProps) { - const [search, setSearch] = useState(""); +export function RequestList({ queue, requests, basePath, complete = true, filtered = false }: RequestListProps) { const paths = new WebPaths(basePath === undefined ? {} : { basePath }); - const requests = model.requests.filter(request => - [request.sqid, request.status, ...request.changeUris.map(uri => changeLabels?.[uri] ?? uri)] - .some(value => value.toLowerCase().includes(search.toLowerCase())) - ); - return
- - Find in displayed requests setSearch(event.currentTarget.value)} /> - Displayed page - - {model.requests.length === 0 ? {empty ?? "No requests were received in this window."} : - requests.length === 0 ? No displayed requests match. : - - {requests.map(request => - {request.sqid} - {request.lastError ? {request.lastError} : null} - - - )} -
} - {requests.length} displayed requests · newest received first - {model.nextPageToken ? : null} - -
; + const changeLinks = requestChangeLinks(queue, requests, paths); + const changeLabels = requestChangeLabels(requests); + if (requests.length === 0) { + return complete ? + {filtered ? "No requests on this queue have the selected statuses." : "No requests have been received on this queue."} + : null; + } + return {header})}> + {requests.map(request => + {request.sqid} + {request.lastError ? {request.lastError} : null} + + + {request.updatedAtMs === null ? "—" : } + )} +
; +} + +const requestSqid = (request: RequestSummaryModel) => request.sqid; + +/** A queue's requests from `result`, loading older pages through the enclosing `RequestPagesProvider`. */ +export function QueueRequests({ queue, result, routeKey, basePath, filtered = false }: { + queue: string; + result: LoadResult; + /** Identity of the list for retained rows and scroll position. */ + routeKey: string; + basePath?: string; + filtered?: boolean; +}) { + return {data => + + {(requests, complete) => } + } + ; +} + +const filterableStatuses = knownRequestStatuses.filter(status => status !== "accepting"); + +/** + * Restricts a queue list to selected statuses by navigating to its filtered URL; renders nothing outside a `WebApp`. + * The menu renders in BaseUI's layer rather than inline: the request table's sticky header would otherwise cover it. + */ +export function QueueStatusFilter({ queue, statuses, basePath }: { queue: string; statuses: readonly string[]; basePath?: string }) { + const app = useWebApp(); + if (!app) { + return null; + } + const paths = new WebPaths(basePath === undefined ? {} : { basePath }); + const options = [...new Set([...filterableStatuses, ...statuses])].map(status => ({ id: status, label: statusDisplay(status).label })); + return Statuses + Request historyShow option.id === selectedVersion)} diff --git a/web/submitqueue/view/pages.test.tsx b/web/submitqueue/view/pages.test.tsx index d3b7fcdb3..e79fb4da0 100644 --- a/web/submitqueue/view/pages.test.tsx +++ b/web/submitqueue/view/pages.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen } from "../../platform/test/rtl.js"; +import { fireEvent, render, screen, within } from "../../platform/test/rtl.js"; import { describe, expect, it, vi } from "vitest"; import { WebNavigationProvider } from "@submitqueue/web-platform"; import { SubmitQueueApp } from "./app.js"; @@ -31,11 +31,11 @@ describe("default reusable application views", () => { }); it.each([ - { path: "/sq", heading: "Queues" }, - { path: "/sq/demo-queue", heading: "demo-queue" }, - { path: "/sq/demo-queue/request/12", heading: "12" }, - { path: "/sq/demo-queue/change/github/github.com/uber/submitqueue/pull/123", heading: "PR #123" }, - ])("renders a complete page at $path without Next.js", async ({ path, heading }) => { + { path: "/sq", heading: "Queues", breadcrumb: false }, + { path: "/sq/queue/demo-queue", heading: "demo-queue", breadcrumb: true }, + { path: "/sq/queue/demo-queue/request/12", heading: "12", breadcrumb: true }, + { path: "/sq/queue/demo-queue/change/github/github.com/uber/submitqueue/pull/123", heading: "PR #123", breadcrumb: true }, + ])("renders a complete page at $path without Next.js", async ({ path, heading, breadcrumb }) => { const result = await web.handle({ path, search: {} }); if (result.kind !== "render") { throw new Error("Expected a rendered page"); @@ -46,7 +46,12 @@ describe("default reusable application views", () => { expect(screen.getByRole("checkbox", { name: "Auto refresh every 10s" })).not.toBeChecked(); expect(refresh).not.toHaveBeenCalled(); const queueLinks = screen.queryAllByRole("link", { name: "demo-queue" }); - queueLinks.forEach(link => expect(link.getAttribute("href")).toBe("/sq/demo-queue")); + queueLinks.forEach(link => expect(link.getAttribute("href")).toBe("/sq/queue/demo-queue")); + const trail = screen.queryByRole("navigation", { name: "Breadcrumb" }); + expect(trail !== null).toBe(breadcrumb); + if (trail !== null) { + expect(within(trail).getByRole("link", { name: "Queues" }).getAttribute("href")).toBe("/sq"); + } }); it("renders a page without refresh controls outside an app", async () => { @@ -70,7 +75,7 @@ describe("default reusable application views", () => { }); it("delegates change-version navigation through the host adapter", async () => { - const result = await web.handle({ path: "/sq/demo-queue/change/github/github.com/uber/submitqueue/pull/123", search: {} }); + const result = await web.handle({ path: "/sq/queue/demo-queue/change/github/github.com/uber/submitqueue/pull/123", search: {} }); if (result.kind !== "render") { throw new Error("Expected a change page"); } @@ -78,7 +83,7 @@ describe("default reusable application views", () => { render(); fireEvent.click(screen.getByRole("combobox")); fireEvent.click(await screen.findByRole("option", { name: sha })); - expect(navigate).toHaveBeenCalledWith(`/sq/demo-queue/change/github/github.com/uber/submitqueue/pull/123/${sha}`); + expect(navigate).toHaveBeenCalledWith(`/sq/queue/demo-queue/change/github/github.com/uber/submitqueue/pull/123/${sha}`); }); it.each(["not-found", "unauthorized"] as const)("renders the shared %s state", kind => { diff --git a/web/submitqueue/view/pages.tsx b/web/submitqueue/view/pages.tsx index fe263ed07..e65019a03 100644 --- a/web/submitqueue/view/pages.tsx +++ b/web/submitqueue/view/pages.tsx @@ -1,11 +1,10 @@ "use client"; import { WebShell, type WebShellProps } from "./shell.js"; -import { Timestamp } from "@submitqueue/web-platform"; import { Block } from "baseui/block"; -import { HeadingLarge, ParagraphSmall, ParagraphXSmall } from "baseui/typography"; +import { HeadingMedium, ParagraphSmall, ParagraphXSmall } from "baseui/typography"; import { WebLink } from "./web-app.js"; -import { ChangeSubmissions, LastSuccessfulData, QueueDirectory, RequestDetailView, RequestListView } from "./components.js"; +import { ChangeSubmissions, LastSuccessfulData, QueueBreadcrumb, QueueDirectory, QueueRequests, QueueStatusFilter, RequestDetailView } from "./components.js"; import type { SubmitQueuePageModel } from "../entity/index.js"; export type SubmitQueueShellProps = Omit; @@ -20,18 +19,20 @@ export interface SubmitQueuePageProps { /** Renders one page model; version controls appear only inside a `WebApp` such as `SubmitQueueApp`. */ export function SubmitQueuePage({ model }: SubmitQueuePageProps) { - return + return {model.kind === "queue" ? <> - + +
- Queue activity{model.queue} - {model.paged ? "Older requests · snapshot" : "Last 24 hours · updates on refresh"}: — + {model.queue} + Requests ordered by receipt time, newest first.
- Latest 24 hours + + {model.paged ? Latest requests : null}
- + 0} />
: model.kind === "request" ?
{notFound ? "Request lookup" : "SubmitQueue"} - {notFound ? "Request not found" : "Authentication required"} + {notFound ? "Request not found" : "Authentication required"} {message ?? (notFound ? "The queue or request may not exist, or its retained data may have expired." : "Use the credentials configured for this host.")} {notFound ? Return to queues : null}
diff --git a/web/submitqueue/view/shell.tsx b/web/submitqueue/view/shell.tsx index 1b925fe75..7d5b6ab1d 100644 --- a/web/submitqueue/view/shell.tsx +++ b/web/submitqueue/view/shell.tsx @@ -1,11 +1,11 @@ "use client"; import type { ReactNode } from "react"; -import { TimeZoneProvider, TimeZoneToggle } from "@submitqueue/web-platform"; +import { TimeZoneProvider, TimeZoneToggle, useWebNavigation } from "@submitqueue/web-platform"; +import { styled, withStyle } from "baseui"; import { Block } from "baseui/block"; -import { StyledLink } from "baseui/link"; -import { Tag } from "baseui/tag"; -import { LabelMedium, LabelXSmall } from "baseui/typography"; +import { LabelXSmall } from "baseui/typography"; +import { AccentLink } from "./web-app.js"; export interface WebShellProps { children: ReactNode; @@ -20,25 +20,68 @@ export interface WebShellProps { controls?: ReactNode; } +/** Header, page, and footer share one centered column so their edges line up at every width. */ +const Column = styled("div", { + width: "min(1200px, calc(100% - 48px))", + marginLeft: "auto", + marginRight: "auto", +}); + +const Header = withStyle(Column, ({ $theme }) => ({ + minHeight: "80px", + display: "flex", + alignItems: "center", + justifyContent: "space-between", + flexWrap: "wrap", + gap: $theme.sizing.scale600, + borderBottom: `1px solid ${$theme.colors.borderOpaque}`, +})); + +const Footer = withStyle(Column, ({ $theme }) => ({ + ...$theme.typography.ParagraphXSmall, + color: $theme.colors.contentSecondary, + paddingTop: $theme.sizing.scale600, + paddingBottom: $theme.sizing.scale600, + borderTop: `1px solid ${$theme.colors.borderOpaque}`, +})); + +const BrandLink = withStyle(AccentLink, ({ $theme }) => ({ + display: "inline-flex", + alignItems: "center", + gap: $theme.sizing.scale300, + color: $theme.colors.contentPrimary, + fontSize: "18px", + fontWeight: 600, + ":visited": { color: $theme.colors.contentPrimary }, + ":hover": { color: $theme.colors.contentPrimary, textDecoration: "none" }, +})); + +const BrandMark = styled("span", ({ $theme }) => ({ + backgroundColor: $theme.colors.contentPrimary, + color: $theme.colors.backgroundPrimary, + borderRadius: "6px", + padding: "5px 7px", + fontSize: "12px", + lineHeight: 1, +})); + export function WebShell({ - children, homeHref = "/", homeLabel, brand, brandMark = "", environmentLabel = "Read-only", - footer = "Read-only", controls, + children, homeHref = "/", homeLabel, brand, brandMark = "", environmentLabel, footer, controls, }: WebShellProps) { + const { Link } = useWebNavigation(); return - - - {brandMark ? : null}{brand} - + minHeight="100vh" display="grid" gridTemplateColumns="minmax(0, 1fr)" gridTemplateRows="auto 1fr auto"> +
+ + {brandMark ? : null}{brand} + {environmentLabel ? {environmentLabel} : null} {controls} - - {children} - {footer ? {footer} : null} +
+ {children} + {footer ?
{footer}
: null} ; } diff --git a/web/submitqueue/view/web-app.tsx b/web/submitqueue/view/web-app.tsx index c2ac7a01f..103b43482 100644 --- a/web/submitqueue/view/web-app.tsx +++ b/web/submitqueue/view/web-app.tsx @@ -3,6 +3,7 @@ import { createContext, useContext, useMemo, useRef, type ReactNode } from "react"; import type { WebPageModel } from "../entity/index.js"; import { useWebNavigation, type WebLinkProps } from "@submitqueue/web-platform"; +import { styled } from "baseui"; import { StyledLink } from "baseui/link"; /** Client-side controls of the enclosing {@link WebApp}. */ @@ -58,8 +59,16 @@ export function WebApp({ model, children }: WebAppProps) { return {children}; } -/** An in-app link styled by BaseUI, using the host's router when supplied. */ +/** BaseUI link in the theme accent; extend it with `withStyle` so overrides take precedence. */ +export const AccentLink = styled(StyledLink, ({ $theme }) => ({ + color: $theme.colors.accent500, + textDecoration: "none", + ":visited": { color: $theme.colors.accent500 }, + ":hover": { color: $theme.colors.accent500, textDecoration: "underline" }, +})); + +/** An in-app link in the theme's accent color, underlined on hover, using the host's router when supplied. */ export function WebLink({ className = "", ...props }: WebLinkProps) { const { Link } = useWebNavigation(); - return ; + return ; } diff --git a/web/test/e2e/submitqueue/submitqueue.spec.ts b/web/test/e2e/submitqueue/submitqueue.spec.ts index 99796b481..38743ba42 100644 --- a/web/test/e2e/submitqueue/submitqueue.spec.ts +++ b/web/test/e2e/submitqueue/submitqueue.spec.ts @@ -48,13 +48,23 @@ function expectOrderedHistory(values: readonly string[], expected: readonly stri } test("challenges anonymous requests", async () => { - const response = await fetch(new URL(`/${queue}`, webURL), { + const response = await fetch(new URL(`/queue/${queue}`, webURL), { redirect: "manual", }); expect(response.status).toBe(401); expect(response.headers.get("www-authenticate")).toContain("Basic"); }); +test("serves a queue's filtered requests as JSON behind the same authentication", async ({ request }) => { + expect((await fetch(new URL(`/api/queue/${queue}/requests`, webURL))).status).toBe(401); + const response = await request.get(`/api/queue/${queue}/requests?status=landed`); + expect(response.status()).toBe(200); + expect(response.headers()["cache-control"]).toBe("no-store"); + const page = await response.json() as { requests: { status: string }[]; nextHref: string | null }; + expect(page.requests.every(request => request.status === "landed")).toBe(true); + expect((await request.get("/api/queue/not-a-configured-queue/requests")).status()).toBe(404); +}); + test("discovers gateway queues including empty queues and rejects unknown queues", async ({ page }) => { const expected = (await gateway.listQueues({})).queues.map(value => value.name); expect(expected).toContain(queue); @@ -62,13 +72,14 @@ test("discovers gateway queues including empty queues and rejects unknown queues expect(emptyQueue).toBeDefined(); await page.goto("/"); await expect(page.locator(".sq-directory h2 a")).toHaveText(expected); + await page.screenshot({ path: test.info().outputPath("directory.png"), fullPage: true }); expect((await new AxeBuilder({ page }).analyze()).violations).toEqual([]); await page.getByRole("link", { name: emptyQueue, exact: true }).click(); await expect(page.getByRole("heading", { name: emptyQueue, exact: true })).toBeVisible(); - await expect(page.getByText("No requests were received in this window.")).toBeVisible(); - await page.goto(`/${emptyQueue}/request/999999`); + await expect(page.getByText("No requests have been received on this queue.")).toBeVisible(); + await page.goto(`/queue/${emptyQueue}/request/999999`); await expect(page.getByRole("heading", { name: "Request not found", exact: true })).toBeVisible(); - await page.goto("/not-a-configured-queue"); + await page.goto("/queue/not-a-configured-queue"); await expect(page.getByRole("heading", { name: "Request not found", exact: true })).toBeVisible(); }); @@ -76,7 +87,7 @@ test("shows newest requests and an accessible readable detail history", async ({ await page.goto("/"); await expect(page.getByRole("heading", { name: "Queues", exact: true })).toBeVisible(); await page.getByRole("link", { name: queue, exact: true }).click(); - await expect(page.getByText("No requests were received in this window.")).toBeVisible(); + await expect(page.getByText("No requests have been received on this queue.")).toBeVisible(); expect(new URL(page.url()).search).toBe(""); const firstSqid = await submitRequest( @@ -102,21 +113,24 @@ test("shows newest requests and an accessible readable detail history", async ({ await page.screenshot({ path: test.info().outputPath("queue.png"), fullPage: true }); await page.setViewportSize({ width: 1920, height: 1080 }); const table = await page.getByRole("table", { name: `Requests in ${queue}` }).boundingBox(); - expect(table?.width).toBeGreaterThan(1920 * 0.95); + const column = await page.getByRole("main").boundingBox(); + expect(column?.width).toBe(1200); + expect(table?.width).toBeGreaterThan(1200 * 0.95); await page.setViewportSize({ width: 390, height: 844 }); expect(await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth)).toBe(true); await page.setViewportSize({ width: 1280, height: 720 }); - await page.getByRole("searchbox").fill("1111111111111111111111111111111111111111"); - await expect(requestLinks).toHaveCount(1); - await page.getByRole("searchbox").fill(""); + await page.getByRole("combobox", { name: "Filter requests by status" }).click(); + await page.getByRole("option", { name: "Landed", exact: true }).click(); + await expect(page).toHaveURL(new RegExp(`/queue/${queue}\\?status=landed$`)); + await page.goto(`/queue/${queue}`); await requestLinks.nth(0).click(); await expect(page.getByRole("heading", { name: repeatedSqid, exact: true })).toBeVisible(); await expect(page.locator(".sq-request-detail .sq-status").first()).toHaveText("Landed"); expect(new URL(page.url()).pathname).toBe( - `/${queue}/request/${repeatedSqid}`, + `/queue/${queue}/request/${repeatedSqid}`, ); await expect(page.getByRole("button", { name: "Copy ID" })).toBeVisible(); @@ -127,7 +141,7 @@ test("shows newest requests and an accessible readable detail history", async ({ expect(actionBoxes.map(box => box!.y)).toEqual([actionBoxes[0]!.y, actionBoxes[0]!.y]); expect(actionBoxes.map(box => box!.height)).toEqual([actionBoxes[0]!.height, actionBoxes[0]!.height]); const facts = page.locator(".sq-request-detail dl").first().locator(":scope > div"); - await expect(facts).toHaveCount(5); + await expect(facts).toHaveCount(6); const factLayout = await facts.evaluateAll(rows => rows.map(row => { const label = row.querySelector("dt")!.getBoundingClientRect(); const value = row.querySelector("dd")!.getBoundingClientRect(); @@ -214,7 +228,7 @@ test("shows newest requests and an accessible readable detail history", async ({ await expect(page.getByRole("table", { name: "Change submissions" }).locator("tbody tr")).toHaveCount(2); expect((await new AxeBuilder({ page }).analyze()).violations).toEqual([]); - await page.goto(`/${queue}?from=1&to=2`); + await page.goto(`/queue/${queue}?from=1&to=2`); await expect(page.getByRole("heading", { name: queue, exact: true })).toBeVisible(); expect(new URL(page.url()).search).toBe(""); const gitUri = "git://git.example.com/demo/refs%2Fheads%2Fweb-demo/3333333333333333333333333333333333333333"; @@ -223,7 +237,7 @@ test("shows newest requests and an accessible readable detail history", async ({ await expect(page.getByRole("link", { name: gitRequest, exact: true })).toBeVisible(); const changeLink = page.getByRole("link", { name: gitUri, exact: true }); await expect(changeLink).toBeVisible(); - await expect(changeLink).toHaveAttribute("href", `/${queue}/change/git/git.example.com/demo/refs%2Fheads%2Fweb-demo`); + await expect(changeLink).toHaveAttribute("href", `/queue/${queue}/change/git/git.example.com/demo/refs%2Fheads%2Fweb-demo`); await expect(page.getByText(/sq-files=/)).toHaveCount(0); await page.screenshot({ path: test.info().outputPath("queue-live.png"), fullPage: true }); await changeLink.click(); @@ -246,7 +260,7 @@ test("shows newest requests and an accessible readable detail history", async ({ test("preserves literal percent escapes in Git refs when opening change history", async ({ page }) => { const uri = "git://git.example.com/demo/refs%2Fheads%2Fweb-demo%252Fpercent/4444444444444444444444444444444444444444"; const sqid = await submitRequest(`${uri}?sq-files=demo%2F00%2Fpercent.txt`); - await page.goto(`/${queue}`); + await page.goto(`/queue/${queue}`); await page.getByRole("link", { name: uri, exact: true }).click(); await expect(page.getByRole("heading", { name: "refs/heads/web-demo%2Fpercent", exact: true })).toBeVisible(); await expect(page.getByRole("table", { name: "Change submissions" }).getByRole("link", { name: sqid, exact: true })).toBeVisible(); @@ -255,7 +269,7 @@ test("preserves literal percent escapes in Git refs when opening change history" }); test("paginates change history through more than 500 queue requests", async ({ page }) => { - const changePath = `/${queue}/change/git/git.example.com/demo/refs%2Fheads%2Fpagination`; + const changePath = `/queue/${queue}/change/git/git.example.com/demo/refs%2Fheads%2Fpagination`; const oldest = await submitRequest("git://git.example.com/demo/refs%2Fheads%2Fpagination/5555555555555555555555555555555555555555"); const expectedIds = [oldest]; for (let offset = 0; offset < 501; offset += 25) { @@ -307,7 +321,7 @@ test("paginates change history through more than 500 queue requests", async ({ p }); test("refreshes from the header only while auto-refresh is enabled", async ({ page }) => { - await page.goto(`/${queue}`); + await page.goto(`/queue/${queue}`); const automatic = page.getByRole("banner").getByRole("checkbox", { name: "Auto refresh every 10s" }); await expect(automatic).not.toBeChecked(); const now = new Date(); @@ -345,7 +359,7 @@ test.describe("time zone selection", () => { }, zone)).toBe(true); }; const timeZones = page.getByRole("banner").getByRole("listbox", { name: "Time zone", exact: true }); - await page.goto(`/${queue}`); + await page.goto(`/queue/${queue}`); await expect(timeZones.getByRole("option", { name: "Browser", exact: true })).toHaveAttribute("aria-selected", "true"); await expectTimeZone("America/New_York"); await timeZones.getByRole("option", { name: "UTC", exact: true }).click(); @@ -358,7 +372,7 @@ test.describe("time zone selection", () => { await expect(page.getByRole("list", { name: "Request history", exact: true })).toBeVisible(); await expect(timeZones.getByRole("option", { name: "UTC", exact: true })).toHaveAttribute("aria-selected", "true"); await expectTimeZone("UTC"); - const changePath = `/${queue}/change/git/git.example.com/demo/refs%2Fheads%2Fpagination`; + const changePath = `/queue/${queue}/change/git/git.example.com/demo/refs%2Fheads%2Fpagination`; for (const path of [changePath, `${changePath}/7777777777777777777777777777777777777777`]) { await page.goto(path); await expect(page.getByRole("table", { name: "Change submissions" })).toBeVisible(); diff --git a/web/test/package/consumer-runtime.test.mjs b/web/test/package/consumer-runtime.test.mjs index 21c46ecbb..d152138b7 100644 --- a/web/test/package/consumer-runtime.test.mjs +++ b/web/test/package/consumer-runtime.test.mjs @@ -15,7 +15,7 @@ test("renders a packaged application with its host providers", async () => { gateway: createFakeGatewayReader(), paths: new WebPaths({ basePath: "/queues" }), }); - const result = await web.handle({ path: "/queues/demo-queue", search: {} }); + const result = await web.handle({ path: "/queues/queue/demo-queue", search: {} }); assert.equal(result.kind, "render"); assert.equal(result.model.result.ok, true); assert.doesNotThrow(() => JSON.stringify(result.model)); @@ -24,7 +24,7 @@ test("renders a packaged application with its host providers", async () => { createElement(BaseProvider, { theme: LightTheme }, createElement(SubmitQueueApp, { model: result.model })))); assert.ok(engine.getCss().length > 0); - assert.ok(html.includes('href="/queues/demo-queue/request/demo-queue/1"')); + assert.ok(html.includes('href="/queues/queue/demo-queue/request/demo-queue/1"')); assert.ok(html.includes('aria-label="Queue requests"')); const stylesheet = await readFile(new URL(import.meta.resolve("@submitqueue/web-submitqueue/styles.css")), "utf8"); assert.ok(stylesheet.includes("@submitqueue/web-platform/styles.css")); diff --git a/web/test/package/consumer-server-runtime.mjs b/web/test/package/consumer-server-runtime.mjs index 4e0b1e6fa..637933648 100644 --- a/web/test/package/consumer-server-runtime.mjs +++ b/web/test/package/consumer-server-runtime.mjs @@ -4,7 +4,7 @@ import { createFakeGatewayReader } from "@submitqueue/web-submitqueue/extension/ import { createStovepipeWeb } from "@submitqueue/web-stovepipe/server"; const submitqueue = createSubmitQueueWeb({ gateway: createFakeGatewayReader() }); -const requests = await submitqueue.handle({ path: "/demo-queue", search: {} }); +const requests = await submitqueue.handle({ path: "/queue/demo-queue", search: {} }); assert.equal(requests.kind, "render"); assert.equal(requests.model.result.ok, true); assert.doesNotThrow(() => JSON.stringify(requests.model)); diff --git a/web/test/package/consumer.ts b/web/test/package/consumer.ts index 82feb6017..6768e5b24 100644 --- a/web/test/package/consumer.ts +++ b/web/test/package/consumer.ts @@ -13,7 +13,7 @@ const web = createSubmitQueueWeb({ gateway }); export async function render() { const result = await web.handle({ - path: "/demo-queue", search: {}, + path: "/queue/demo-queue", search: {}, onLog: (level, message, metadata) => { console[level](message, metadata); }, }); if (result.kind !== "render") { diff --git a/web/test/package/react18/react18-runtime.test.mjs b/web/test/package/react18/react18-runtime.test.mjs index 302b4ace4..2cfc154f7 100644 --- a/web/test/package/react18/react18-runtime.test.mjs +++ b/web/test/package/react18/react18-runtime.test.mjs @@ -21,9 +21,9 @@ test("renders and hydrates packaged pages with React 18 and host themes", async }); const routes = [ { path: "/", search: {} }, - { path: "/demo-queue", search: {} }, - { path: "/demo-queue/request/demo-queue/1", search: {} }, - { path: "/demo-queue/request/demo-queue/1", search: { view: "history" } }, + { path: "/queue/demo-queue", search: {} }, + { path: "/queue/demo-queue/request/demo-queue/1", search: {} }, + { path: "/queue/demo-queue/request/demo-queue/1", search: { view: "history" } }, ]; const pages = []; const themes = [LightTheme, DarkTheme, createTheme({ colors: {