From 0b7853f9bc8dd07305c7637c10b322c515717266 Mon Sep 17 00:00:00 2001 From: Oliver Newland Date: Thu, 15 Oct 2026 09:13:41 -0700 Subject: [PATCH 1/5] feat(admin): Pre-fill System Queries from a link Opening /#system-queries?query=...&storage=...&host=host:port fills in the editor, storage and host, and shows a warning that whoever clicks Execute is responsible for the query. Nothing runs until the user clicks Execute, and sudo mode is never set from a link. Each history card also gets a "Share query" button that copies such a link for that query. This lets an agent point someone at a query to review and run, and makes it easier to share queries with teammates. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../static/clickhouse_queries/prefill.tsx | 97 +++++++++ .../clickhouse_queries/query_display.tsx | 78 +++++++- .../query_history_entry.tsx | 2 + snuba/admin/static/index.tsx | 3 +- snuba/admin/static/query_editor.tsx | 11 +- .../tests/clickhouse_queries/prefill.spec.tsx | 188 ++++++++++++++++++ .../static/utils/query_result_copier.tsx | 6 + 7 files changed, 374 insertions(+), 11 deletions(-) create mode 100644 snuba/admin/static/clickhouse_queries/prefill.tsx create mode 100644 snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx diff --git a/snuba/admin/static/clickhouse_queries/prefill.tsx b/snuba/admin/static/clickhouse_queries/prefill.tsx new file mode 100644 index 000000000..d83e4b7b0 --- /dev/null +++ b/snuba/admin/static/clickhouse_queries/prefill.tsx @@ -0,0 +1,97 @@ +// System Queries can be opened with the editor pre-filled from the URL, so a +// tool or agent can link someone straight to a query to review and run: +// +// /#system-queries?query=SELECT%201&storage=errors&host=clickhouse-01:8123 +// +// Params live inside the hash because the admin routes on the hash. Values +// are URL-encoded (encodeURIComponent / URLSearchParams). `host` may omit the +// port, in which case it defaults to ClickHouse HTTP (8123). Nothing here +// runs the query: the user still has to review it and click Execute. + +type SystemQueryPrefill = { + sql?: string; + storage?: string; + host?: string; + port?: number; +}; + +const DEFAULT_PORT = 8123; + +/** Splits "#page?a=b" into the route ("#page") and its params. */ +export function splitHash(hash: string): { + route: string; + params: URLSearchParams; +} { + const separator = hash.indexOf("?"); + if (separator === -1) { + return { route: hash, params: new URLSearchParams() }; + } + return { + route: hash.slice(0, separator), + params: new URLSearchParams(hash.slice(separator + 1)), + }; +} + +/** Parses "host" or "host:port", defaulting the port to ClickHouse HTTP. */ +export function parseHostPort(value: string): { host: string; port: number } { + const trimmed = value.trim(); + const separator = trimmed.lastIndexOf(":"); + const maybePort = separator === -1 ? "" : trimmed.slice(separator + 1); + if (/^\d+$/.test(maybePort)) { + return { + host: trimmed.slice(0, separator), + port: parseInt(maybePort, 10), + }; + } + return { host: trimmed, port: DEFAULT_PORT }; +} + +/** Returns the prefill encoded in the hash, or null if there is none. */ +export function parseSystemQueryPrefill( + hash: string +): SystemQueryPrefill | null { + const { params } = splitHash(hash); + const sql = params.get("query") || undefined; + const storage = params.get("storage") || undefined; + const hostParam = params.get("host")?.trim(); + if (!sql && !storage && !hostParam) { + return null; + } + return { + sql, + storage, + ...(hostParam ? parseHostPort(hostParam) : {}), + }; +} + +/** Builds a link that opens System Queries pre-filled with these values. */ +export function buildSystemQueryUrl(prefill: { + sql?: string; + storage?: string | null; + host?: string | null; +}): string { + const params = new URLSearchParams(); + if (prefill.sql) params.set("query", prefill.sql); + if (prefill.storage) params.set("storage", prefill.storage); + if (prefill.host) params.set("host", prefill.host); + // %20 reads better than "+" when the link is pasted somewhere. + const query = params.toString().replace(/\+/g, "%20"); + return `${window.location.origin}${window.location.pathname}#system-queries?${query}`; +} + +/** + * Drops the params from the address bar once they've been applied, so a + * refresh doesn't clobber edits by re-applying the link. + */ +export function clearHashParams() { + const { route } = splitHash(window.location.hash); + if (route !== window.location.hash) { + window.history.replaceState( + null, + "", + window.location.pathname + window.location.search + route + ); + } +} + +export { SystemQueryPrefill }; diff --git a/snuba/admin/static/clickhouse_queries/query_display.tsx b/snuba/admin/static/clickhouse_queries/query_display.tsx index 29af47221..c5a0aec4a 100644 --- a/snuba/admin/static/clickhouse_queries/query_display.tsx +++ b/snuba/admin/static/clickhouse_queries/query_display.tsx @@ -2,11 +2,16 @@ import React, { useEffect, useState } from "react"; import Client from "SnubaAdmin/api_client"; import QueryEditor from "SnubaAdmin/query_editor"; import ExecuteButton from "SnubaAdmin/utils/execute_button"; -import { Switch } from "@mantine/core"; +import { Alert, Switch } from "@mantine/core"; import { getRecentHistory, setRecentHistory } from "SnubaAdmin/query_history"; import { CustomSelect, getParamFromStorage } from "SnubaAdmin/select"; import { getHostsForStorage, getErrorDomElement } from "SnubaAdmin/utils/clickhouse_node_utils"; import { QueryHistoryEntry } from "SnubaAdmin/clickhouse_queries/query_history_entry"; +import { + clearHashParams, + parseHostPort, + parseSystemQueryPrefill, +} from "SnubaAdmin/clickhouse_queries/prefill"; import { ClickhouseNodeData, @@ -23,10 +28,19 @@ function QueryDisplay(props: { resultDataPopulator: (queryResult: QueryResult) => JSX.Element; predefinedQueryOptions: Array; }) { + // A link may pre-fill the query, storage and host (see prefill.tsx). + const [prefill] = useState(() => + parseSystemQueryPrefill(window.location.hash) + ); + const [prefillWarningOpen, setPrefillWarningOpen] = useState( + prefill !== null + ); const [nodeData, setNodeData] = useState([]); const [manualHostInput, setManualHostInput] = useState(""); const [query, setQuery] = useState({ - storage: getParamFromStorage("storage"), + storage: prefill?.storage ?? getParamFromStorage("storage"), + host: prefill?.host, + port: prefill?.port, }); const [queryResultHistory, setQueryResultHistory] = useState( getRecentHistory(HISTORY_KEY) @@ -41,8 +55,23 @@ function QueryDisplay(props: { const [collapseOpened, setCollapseOpened] = useState(false); useEffect(() => { + if (prefill) { + clearHashParams(); + } props.api.getClickhouseNodes().then((res) => { setNodeData(res); + // A linked host the storage's node list doesn't know about can only be + // shown (and checked) via manual host entry. + if (prefill?.host) { + const hostPort = `${prefill.host}:${prefill.port}`; + const isListed = getHostsForStorage(res, prefill.storage).some( + (option) => option.value === hostPort + ); + if (!isListed) { + setManualHostInput(hostPort); + setClusterlessHost(true); + } + } }); }, []); @@ -81,18 +110,14 @@ function QueryDisplay(props: { // Manual entry accepts "host" or "host:port". The raw text is kept in its // own state so a half-typed port ("host:9") is not clobbered by the split. setManualHostInput(hostStringIP); - - const trimmed = hostStringIP.trim(); - const separator = trimmed.lastIndexOf(":"); - const maybePort = separator === -1 ? "" : trimmed.slice(separator + 1); // Without a port we default to ClickHouse HTTP, not the old native port. - const hasPort = /^\d+$/.test(maybePort); + const { host, port } = parseHostPort(hostStringIP); setQuery((prevQuery) => { return { ...prevQuery, - host: hasPort ? trimmed.slice(0, separator) : trimmed, - port: hasPort ? parseInt(maybePort, 10) : 8123, + host, + port, }; }); } @@ -151,6 +176,31 @@ function QueryDisplay(props: {

Construct a ClickHouse System Query

+ {prefill && prefillWarningOpen && ( + setPrefillWarningOpen(false)} + style={prefillAlertStyle} + data-testid="prefill-warning" + > +

+ Whoever clicks Execute is responsible for the query, no matter + who or what wrote it. Read the whole query and check where it + will run before you execute it. Don't run it just because an + agent or a link suggested it. +

+

+ Storage: {prefill.storage ?? "(not set)"} + {" ยท "} + Host:{" "} + + {prefill.host ? `${prefill.host}:${prefill.port}` : "(not set)"} + +

+ + )}
{query.clusterless ? @@ -260,6 +311,15 @@ const sudoForm = { const standardForm = {}; +const prefillAlertStyle = { + margin: "8px 0px", +}; + +const prefillAlertTextStyle = { + fontSize: 16, + margin: "4px 0px", +}; + const switchStyle = { margin: '8px 0px', }; diff --git a/snuba/admin/static/clickhouse_queries/query_history_entry.tsx b/snuba/admin/static/clickhouse_queries/query_history_entry.tsx index 9231f47b8..226573c30 100644 --- a/snuba/admin/static/clickhouse_queries/query_history_entry.tsx +++ b/snuba/admin/static/clickhouse_queries/query_history_entry.tsx @@ -2,6 +2,7 @@ import React, { useState } from "react"; import QueryResultCopier from "SnubaAdmin/utils/query_result_copier"; import { COLORS } from "SnubaAdmin/theme"; import { QueryResult } from "SnubaAdmin/clickhouse_queries/types"; +import { buildSystemQueryUrl } from "SnubaAdmin/clickhouse_queries/prefill"; // History entries store the query and its target together as // " (,:)"; split them back apart for display. @@ -63,6 +64,7 @@ function QueryHistoryEntry(props: { rawInput={queryResult.trace_output || ""} jsonInput={JSON.stringify(queryResult)} csvInput={props.csv} + linkInput={buildSystemQueryUrl({ sql, storage, host })} compact />
diff --git a/snuba/admin/static/index.tsx b/snuba/admin/static/index.tsx index bcf22e3c9..fb04dd2ad 100644 --- a/snuba/admin/static/index.tsx +++ b/snuba/admin/static/index.tsx @@ -113,7 +113,8 @@ function getTab(locationHash: string): string { throw new Error("invalid hash"); } - const hash = locationHash.split("/")[0]; + // Pages may carry params after the id, e.g. "#system-queries?query=...". + const hash = locationHash.split(/[/?]/)[0]; const navItem = NAV_ITEMS.find((item) => "#" + item.id === hash); diff --git a/snuba/admin/static/query_editor.tsx b/snuba/admin/static/query_editor.tsx index 0130a8924..68bb2a4fc 100644 --- a/snuba/admin/static/query_editor.tsx +++ b/snuba/admin/static/query_editor.tsx @@ -13,6 +13,7 @@ import { DateTimePicker } from "@mantine/dates"; import { SQLEditor } from "SnubaAdmin/common/components/sql_editor"; import { useLocalStorage } from "@mantine/hooks"; import { CustomSelect } from "SnubaAdmin/select"; +import { splitHash } from "SnubaAdmin/clickhouse_queries/prefill"; type PredefinedQuery = { name: string; @@ -107,9 +108,11 @@ function QueryEditor(props: { predefinedQueryOptions?: Array; categoryOptions?: Array; outcomeOptions?: Array; + // Replaces the saved query on mount, e.g. when opened from a link. + initialQuery?: string; }) { const [query, setQuery] = useState(""); - const hash = window.location.hash; + const hash = splitHash(window.location.hash).route; // Namespace the storage by the hash, which corresponds to the screen const [queryTemplate, setQueryTemplate] = useLocalStorage({ @@ -131,6 +134,12 @@ function QueryEditor(props: { const [absoluteStartTime, setAbsoluteStartTime] = useState(null); const [absoluteEndTime, setAbsoluteEndTime] = useState(null); + useEffect(() => { + if (props.initialQuery !== undefined) { + setQueryTemplate(props.initialQuery); + } + }, []); + const variableRegex = /{{([a-zA-Z0-9_]+)}}/; const hasTimeRangeParams = queryTemplate.includes(START_TIME_PARAM) && diff --git a/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx b/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx new file mode 100644 index 000000000..a06e7ece2 --- /dev/null +++ b/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx @@ -0,0 +1,188 @@ +import React from "react"; +import { it, expect, describe, jest, afterEach } from "@jest/globals"; +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; +import Client from "SnubaAdmin/api_client"; +import QueryDisplay from "SnubaAdmin/clickhouse_queries/query_display"; +import { QueryHistoryEntry } from "SnubaAdmin/clickhouse_queries/query_history_entry"; +import { + ClickhouseNodeData, + QueryRequest, + QueryResult, +} from "SnubaAdmin/clickhouse_queries/types"; +import { + buildSystemQueryUrl, + parseHostPort, + parseSystemQueryPrefill, + splitHash, +} from "SnubaAdmin/clickhouse_queries/prefill"; + +jest.mock("SnubaAdmin/common/components/sql_editor"); + +describe("parseSystemQueryPrefill", () => { + it("returns null without params", () => { + expect(parseSystemQueryPrefill("#system-queries")).toBeNull(); + expect(parseSystemQueryPrefill("#system-queries?")).toBeNull(); + }); + + it("decodes the query, storage and host", () => { + const sql = "SELECT count() FROM system.parts WHERE table = 'a+b'"; + const hash = + "#system-queries?query=" + + encodeURIComponent(sql) + + "&storage=errors&host=clickhouse-01%3A9000"; + expect(parseSystemQueryPrefill(hash)).toEqual({ + sql, + storage: "errors", + host: "clickhouse-01", + port: 9000, + }); + }); + + it("defaults the host port to 8123", () => { + expect(parseHostPort("clickhouse-01")).toEqual({ + host: "clickhouse-01", + port: 8123, + }); + }); + + it("round-trips a link built from a query", () => { + const sql = "SELECT 1 + 1\nFROM system.one WHERE x = 'a&b'"; + const url = buildSystemQueryUrl({ + sql, + storage: "errors", + host: "clickhouse-01:9000", + }); + expect(url).not.toContain("+"); + expect(parseSystemQueryPrefill(new URL(url).hash)).toEqual({ + sql, + storage: "errors", + host: "clickhouse-01", + port: 9000, + }); + }); + + it("splits the route from the params", () => { + expect(splitHash("#system-queries?query=x").route).toBe("#system-queries"); + expect(splitHash("#system-queries").route).toBe("#system-queries"); + }); +}); + +describe("QueryDisplay opened from a link", () => { + global.ResizeObserver = require("resize-observer-polyfill"); + afterEach(() => { + cleanup(); + window.location.hash = ""; + }); + + const nodes: [ClickhouseNodeData] = [ + { + storage_name: "errors", + local_table_name: "errors_local", + local_nodes: [{ host: "clickhouse-01", port: 9000 }], + dist_nodes: [], + query_node: { host: "clickhouse-01", port: 9000 }, + }, + ]; + + function renderWithHash(hash: string) { + window.location.hash = hash; + const api = { + ...Client(), + getClickhouseNodes: jest + .fn<() => Promise<[ClickhouseNodeData]>>() + .mockResolvedValue(nodes), + executeSystemQuery: jest + .fn<(req: QueryRequest) => Promise>() + .mockResolvedValue({ + input_query: "", + timestamp: 0, + column_names: ["x"], + rows: [["1"]], + } as QueryResult), + }; + render( +
} + predefinedQueryOptions={[]} + /> + ); + return api; + } + + it("fills the form, warns, and waits for the user to execute", async () => { + const api = renderWithHash( + "#system-queries?query=SELECT%201&storage=errors&host=clickhouse-01%3A9000" + ); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + + expect(screen.getByTestId("prefill-warning")).toBeTruthy(); + expect(screen.getByRole("textbox").textContent).toBe("SELECT 1"); + expect(await screen.findAllByDisplayValue("clickhouse-01:9000")).not.toHaveLength(0); + expect( + screen.getByText("Execute Query").closest("button")?.disabled + ).toBe(false); + expect(api.executeSystemQuery).not.toBeCalled(); + // The params are dropped so a refresh doesn't re-apply them. + expect(window.location.hash).toBe("#system-queries"); + }); + + it("uses manual host entry for a host the storage doesn't list", async () => { + const api = renderWithHash( + "#system-queries?query=SELECT%201&storage=errors&host=other-host" + ); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + + await waitFor(() => + expect(screen.getByPlaceholderText("enter host or host:port...")).toBeTruthy() + ); + expect(screen.getByDisplayValue("other-host:8123")).toBeTruthy(); + }); + + it("shows no warning without params", async () => { + const api = renderWithHash("#system-queries"); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + expect(screen.queryByTestId("prefill-warning")).toBeNull(); + }); +}); + +describe("QueryHistoryEntry", () => { + afterEach(cleanup); + + it("copies a link that pre-fills the entry's query", () => { + const writeText = jest.fn<(text: string) => Promise>(); + Object.assign(window.navigator, { clipboard: { writeText } }); + render( +
} + /> + ); + + fireEvent.click(screen.getByText("Share query")); + + const url = writeText.mock.calls[0][0]; + expect(parseSystemQueryPrefill(new URL(url).hash)).toEqual({ + sql: "SELECT 1", + storage: "errors", + host: "clickhouse-01", + port: 9000, + }); + }); +}); diff --git a/snuba/admin/static/utils/query_result_copier.tsx b/snuba/admin/static/utils/query_result_copier.tsx index 528349f80..14fec22c0 100644 --- a/snuba/admin/static/utils/query_result_copier.tsx +++ b/snuba/admin/static/utils/query_result_copier.tsx @@ -5,6 +5,7 @@ function QueryResultCopier(props: { rawInput?: string; jsonInput?: string; csvInput?: string; + linkInput?: string; // Small, quiet buttons with short labels, for tight spaces. compact?: boolean; }) { @@ -37,6 +38,11 @@ function QueryResultCopier(props: { {label("CSV")} )} + {props.linkInput && ( + + )} ; } From e4c397e6915bddea5c89ba35a5fd3ce3d0394903 Mon Sep 17 00:00:00 2001 From: Oliver Newland Date: Thu, 15 Oct 2026 09:18:18 -0700 Subject: [PATCH 2/5] fix(admin): Pick up a System Queries link pasted on the same page Pasting a share link while already on System Queries only changes the hash, so the page doesn't reload and the form ignored the new link. Remount the form when the hash changes to one with link params. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../admin/static/clickhouse_queries/index.tsx | 25 +++++++++++++++---- .../tests/clickhouse_queries/prefill.spec.tsx | 24 ++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/snuba/admin/static/clickhouse_queries/index.tsx b/snuba/admin/static/clickhouse_queries/index.tsx index d13fccb24..e41b9647c 100644 --- a/snuba/admin/static/clickhouse_queries/index.tsx +++ b/snuba/admin/static/clickhouse_queries/index.tsx @@ -2,12 +2,26 @@ import React, { useState, useEffect } from "react"; import Client from "SnubaAdmin/api_client"; import { Table } from "SnubaAdmin/table"; import QueryDisplay from "SnubaAdmin/clickhouse_queries/query_display"; +import { parseSystemQueryPrefill } from "SnubaAdmin/clickhouse_queries/prefill"; import { QueryResult, PredefinedQuery } from "./types"; function ClickhouseQueries(props: { api: Client }) { const [predefinedQueryOptions, setPredefinedQueryOptions] = useState< PredefinedQuery[] >([]); + // Pasting a share link while already on this page only changes the hash, + // which doesn't reload the page. Remount the form so it reads the new link. + const [linkKey, setLinkKey] = useState(0); + + useEffect(() => { + function onHashChange() { + if (parseSystemQueryPrefill(window.location.hash)) { + setLinkKey((key) => key + 1); + } + } + window.addEventListener("hashchange", onHashChange); + return () => window.removeEventListener("hashchange", onHashChange); + }, []); useEffect(() => { props.api.getPredefinedQueryOptions().then((res) => { @@ -39,11 +53,12 @@ function ClickhouseQueries(props: { api: Client }) { return (
- {QueryDisplay({ - api: props.api, - resultDataPopulator: tablePopulator, - predefinedQueryOptions: predefinedQueryOptions, - })} +
); } diff --git a/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx b/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx index a06e7ece2..65a96491a 100644 --- a/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx +++ b/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx @@ -8,10 +8,12 @@ import { waitFor, } from "@testing-library/react"; import Client from "SnubaAdmin/api_client"; +import ClickhouseQueries from "SnubaAdmin/clickhouse_queries"; import QueryDisplay from "SnubaAdmin/clickhouse_queries/query_display"; import { QueryHistoryEntry } from "SnubaAdmin/clickhouse_queries/query_history_entry"; import { ClickhouseNodeData, + PredefinedQuery, QueryRequest, QueryResult, } from "SnubaAdmin/clickhouse_queries/types"; @@ -145,6 +147,28 @@ describe("QueryDisplay opened from a link", () => { expect(screen.getByDisplayValue("other-host:8123")).toBeTruthy(); }); + it("picks up a link pasted while already on the page", async () => { + window.location.hash = "#system-queries"; + const api = { + ...Client(), + getPredefinedQueryOptions: jest + .fn<() => Promise<[PredefinedQuery]>>() + .mockResolvedValue([{ name: "q", sql: "SELECT 1", description: "" }]), + getClickhouseNodes: jest + .fn<() => Promise<[ClickhouseNodeData]>>() + .mockResolvedValue(nodes), + }; + render(); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + expect(screen.queryByTestId("prefill-warning")).toBeNull(); + + window.location.hash = "#system-queries?query=SELECT%202&storage=errors"; + window.dispatchEvent(new HashChangeEvent("hashchange")); + + expect(await screen.findByTestId("prefill-warning")).toBeTruthy(); + expect(screen.getByRole("textbox").textContent).toBe("SELECT 2"); + }); + it("shows no warning without params", async () => { const api = renderWithHash("#system-queries"); await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); From 060e0b1e2057cd7172c9f9e336ae5e23ee385568 Mon Sep 17 00:00:00 2001 From: Oliver Newland Date: Thu, 15 Oct 2026 09:39:23 -0700 Subject: [PATCH 3/5] feat(admin): Group the System Queries form into Where, What and How Split the form into labelled sections: storage and host (with the host-entry toggle), the query editor, and the mode toggle next to the Execute button. The on/off switches become two-option toggles so both choices are visible, and the mode toggle explains what user and sudo mode allow. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../clickhouse_queries/query_display.tsx | 196 ++++++++++-------- 1 file changed, 113 insertions(+), 83 deletions(-) diff --git a/snuba/admin/static/clickhouse_queries/query_display.tsx b/snuba/admin/static/clickhouse_queries/query_display.tsx index c5a0aec4a..f39beeb04 100644 --- a/snuba/admin/static/clickhouse_queries/query_display.tsx +++ b/snuba/admin/static/clickhouse_queries/query_display.tsx @@ -2,7 +2,7 @@ import React, { useEffect, useState } from "react"; import Client from "SnubaAdmin/api_client"; import QueryEditor from "SnubaAdmin/query_editor"; import ExecuteButton from "SnubaAdmin/utils/execute_button"; -import { Alert, Switch } from "@mantine/core"; +import { Alert, Box, SegmentedControl, Text } from "@mantine/core"; import { getRecentHistory, setRecentHistory } from "SnubaAdmin/query_history"; import { CustomSelect, getParamFromStorage } from "SnubaAdmin/select"; import { getHostsForStorage, getErrorDomElement } from "SnubaAdmin/utils/clickhouse_node_utils"; @@ -201,65 +201,51 @@ function QueryDisplay(props: {

)} -
- ) => - setSudo(evt.currentTarget.checked) - } - onLabel="Sudo mode" - offLabel="User mode" - size="xl" +
+ setClusterlessHost(value === "manual")} + data={[ + { label: "Pick a node", value: "pick" }, + { label: "Enter host manually", value: "manual" }, + ]} + style={segmentedStyle} /> -
-
- ) => - setClusterlessHost(evt.currentTarget.checked) - } - onLabel="Manual Host Entry" - offLabel="Auto Populate Nodes" - size="xl" - /> -
- { - updateQuerySql(sql); - }} - predefinedQueryOptions={props.predefinedQueryOptions} - initialQuery={prefill?.sql} - /> -
- {query.clusterless ? - (
+
+
+ + Storage + storage.storage_name)} /> - selectHostIp(evt.target.value)} - name="host" - placeholder="enter host or host:port..." - type="text" - /> -

- Must be a known cluster node or listed in - admin.copy_tables_allowed_target_hosts. Defaults to port 8123. -

-
) : ( -
- storage.storage_name)} - /> +
+
+ + Host + + {query.clusterless ? ( + <> + selectHostIp(evt.target.value)} + name="host" + placeholder="enter host or host:port..." + type="text" + /> +

+ Must be a known cluster node or listed in + admin.copy_tables_allowed_target_hosts. Defaults to port + 8123. +

+ + ) : ( -
- )} -
+ )} +
+
+ +
+ { + updateQuerySql(sql); + }} + predefinedQueryOptions={props.predefinedQueryOptions} + initialQuery={prefill?.sql} + /> +
+
+
+
+ setSudo(value === "sudo")} + color={query.sudo ? "red" : undefined} + data={[ + { label: "User mode", value: "user" }, + { label: "Sudo mode", value: "sudo" }, + ]} + style={segmentedStyle} + /> +

+ {query.sudo + ? "Sudo only runs SYSTEM, ALTER, OPTIMIZE and DROP statements. It needs the sudo role, and every sudo query is audit-logged with a notification." + : "SELECT, SHOW and DESCRIBE. SELECTs may only read system tables."} +

+
-
+
{getErrorDomElement(queryError, collapseOpened, setCollapseOpened)} @@ -305,6 +321,44 @@ function QueryDisplay(props: { ); } +function Section(props: { title: string; children: React.ReactNode }) { + return ( + + + {props.title} + + {props.children} + + ); +} + +const sectionStyle = { + border: "1px solid #ced4da", + borderRadius: 4, +}; + +const segmentedStyle = { + marginBottom: 8, +}; + +const targetRowStyle = { + display: "flex", + flexWrap: "wrap" as const, + gap: 16, +}; + +const targetFieldStyle = { + width: "20em", + display: "flex", + flexDirection: "column" as const, + gap: 4, +}; + +const modeHelpTextStyle = { + fontSize: "0.85em", + margin: 0, +}; + const sudoForm = { border: "8px solid red", }; @@ -320,20 +374,6 @@ const prefillAlertTextStyle = { margin: "4px 0px", }; -const switchStyle = { - margin: '8px 0px', -}; - -const hostSelectStyle = { - width: '20em', - minHeight: '4em', - margin: '8px 0px', - display: 'flex', - flexDirection: 'column' as const, - justifyContent: 'space-between', - // padding: "4px 20px", -} - const inputStyle = { fontSize: 16, minHeight: '2.25rem', @@ -348,18 +388,8 @@ const hostHelpTextStyle = { const executeActionsStyle = { display: "flex", justifyContent: "space-between", - marginTop: 8, -}; - -const executeButtonStyle = { - height: 30, - border: 0, - padding: "4px 20px", -}; - -const selectStyle = { - marginRight: 8, - height: 30, + alignItems: "flex-start", + gap: 16, }; export default QueryDisplay; From a64e7feeb601e86e3c0203019f1a26dca536d300 Mon Sep 17 00:00:00 2001 From: Oliver Newland Date: Thu, 15 Oct 2026 09:44:31 -0700 Subject: [PATCH 4/5] feat(admin): Restyle System Queries headings and sudo highlighting Use the home page's section heading style for the System Queries headings (now shared via theme.tsx), with a smaller version for the Where/What/How labels. Bootstrap's legend styles made those labels oversized. In sudo mode, highlight only the How section in red and make the Execute button red ("Execute Sudo Query") instead of putting a thick red border around the whole form. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../clickhouse_queries/query_display.tsx | 59 ++++++++++++++----- snuba/admin/static/theme.tsx | 12 +++- snuba/admin/static/utils/execute_button.tsx | 3 + snuba/admin/static/welcome/index.tsx | 11 +--- 4 files changed, 61 insertions(+), 24 deletions(-) diff --git a/snuba/admin/static/clickhouse_queries/query_display.tsx b/snuba/admin/static/clickhouse_queries/query_display.tsx index f39beeb04..39e0c8554 100644 --- a/snuba/admin/static/clickhouse_queries/query_display.tsx +++ b/snuba/admin/static/clickhouse_queries/query_display.tsx @@ -4,6 +4,7 @@ import QueryEditor from "SnubaAdmin/query_editor"; import ExecuteButton from "SnubaAdmin/utils/execute_button"; import { Alert, Box, SegmentedControl, Text } from "@mantine/core"; import { getRecentHistory, setRecentHistory } from "SnubaAdmin/query_history"; +import { COLORS, SECTION_HEADING_STYLE } from "SnubaAdmin/theme"; import { CustomSelect, getParamFromStorage } from "SnubaAdmin/select"; import { getHostsForStorage, getErrorDomElement } from "SnubaAdmin/utils/clickhouse_node_utils"; import { QueryHistoryEntry } from "SnubaAdmin/clickhouse_queries/query_history_entry"; @@ -174,8 +175,8 @@ function QueryDisplay(props: { return (
-
-

Construct a ClickHouse System Query

+ +

Construct a ClickHouse System Query

{prefill && prefillWarningOpen && ( -
+
-

Query results

+

Query results

{queryResultHistory.map((queryResult, idx) => ( - + + {props.title} - + {props.children} ); @@ -337,6 +356,24 @@ const sectionStyle = { borderRadius: 4, }; +const dangerSectionStyle = { + ...sectionStyle, + border: `2px solid ${COLORS.DANGER}`, + backgroundColor: "rgba(168, 50, 45, 0.06)", +}; + +// A step smaller than the page headings, so sections read as nested. +// Bootstrap makes legends large, full-width and floated; undo that so the +// label sits in the border. +const sectionLegendStyle = { + ...SECTION_HEADING_STYLE, + fontSize: 12, + float: "none" as const, + width: "auto", + padding: "0 4px", + margin: 0, +}; + const segmentedStyle = { marginBottom: 8, }; @@ -359,12 +396,6 @@ const modeHelpTextStyle = { margin: 0, }; -const sudoForm = { - border: "8px solid red", -}; - -const standardForm = {}; - const prefillAlertStyle = { margin: "8px 0px", }; diff --git a/snuba/admin/static/theme.tsx b/snuba/admin/static/theme.tsx index 668b63abb..66f27c7a6 100644 --- a/snuba/admin/static/theme.tsx +++ b/snuba/admin/static/theme.tsx @@ -28,6 +28,16 @@ const COLORS = { SUCCESS: "#1b5e20", }; +// Small uppercase label used for page section headings, e.g. on the home page. +const SECTION_HEADING_STYLE = { + fontSize: 14, + fontWeight: 700, + letterSpacing: "0.1em", + textTransform: "uppercase" as const, + color: COLORS.TEXT_LIGHTER, + margin: "0 0 12px 0", +}; + // Diagonal, staggered tiling of the slogan. The fill is only a few shades off // the page background so the pattern stays visible without competing with text. function sloganTiling(): string { @@ -44,4 +54,4 @@ function sloganTiling(): string { return `url("data:image/svg+xml,${encodeURIComponent(svg)}")`; } -export { COLORS, sloganTiling }; +export { COLORS, SECTION_HEADING_STYLE, sloganTiling }; diff --git a/snuba/admin/static/utils/execute_button.tsx b/snuba/admin/static/utils/execute_button.tsx index 5ad1b1383..29ea38394 100644 --- a/snuba/admin/static/utils/execute_button.tsx +++ b/snuba/admin/static/utils/execute_button.tsx @@ -6,6 +6,8 @@ function ExecuteButton(props: { onClick: () => Promise; onError?: (error: Error) => any; label?: string; + // Mantine color, e.g. "red" for actions that need extra care. + color?: string; }) { const [isExecuting, setIsExecuting] = useState(false); @@ -53,6 +55,7 @@ function ExecuteButton(props: { evt.preventDefault(); executeQuery(); }} + color={props.color} loading={isExecuting} disabled={isExecuting || props.disabled} aria-busy={isExecuting} diff --git a/snuba/admin/static/welcome/index.tsx b/snuba/admin/static/welcome/index.tsx index e486c2d22..c7d0b29bc 100644 --- a/snuba/admin/static/welcome/index.tsx +++ b/snuba/admin/static/welcome/index.tsx @@ -1,6 +1,6 @@ import React, { useEffect, useState } from "react"; import Client from "SnubaAdmin/api_client"; -import { COLORS } from "SnubaAdmin/theme"; +import { COLORS, SECTION_HEADING_STYLE } from "SnubaAdmin/theme"; import ToolGrid from "SnubaAdmin/tool_grid"; import { AdminRegion } from "SnubaAdmin/types"; import { regionColor } from "SnubaAdmin/utils/region_color"; @@ -105,14 +105,7 @@ const sectionStyle = { marginBottom: 30, }; -const headingStyle = { - fontSize: 14, - fontWeight: 700, - letterSpacing: "0.1em", - textTransform: "uppercase" as const, - color: COLORS.TEXT_LIGHTER, - margin: "0 0 12px 0", -}; +const headingStyle = SECTION_HEADING_STYLE; // Heading and pills share one line. const regionSectionStyle = { From 95a94fc8661d1b1580419d04456f3dc36eb77f31 Mon Sep 17 00:00:00 2001 From: Oliver Newland Date: Thu, 15 Oct 2026 09:51:51 -0700 Subject: [PATCH 5/5] feat(admin): Flag missing System Queries fields and tidy the layout Flag the next field needed before a query can run (storage, then host) with an orange outline and hint, and explain the disabled Execute button with a tooltip. Also put the host-entry toggle on the same line as storage and host, give the predefined query picker a proper label and smaller description, and move the share-link warning above the page heading. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../clickhouse_queries/query_display.tsx | 137 +++++++++++++----- snuba/admin/static/query_editor.tsx | 17 ++- .../tests/clickhouse_queries/prefill.spec.tsx | 23 +++ snuba/admin/static/theme.tsx | 2 + 4 files changed, 136 insertions(+), 43 deletions(-) diff --git a/snuba/admin/static/clickhouse_queries/query_display.tsx b/snuba/admin/static/clickhouse_queries/query_display.tsx index 39e0c8554..dab886da3 100644 --- a/snuba/admin/static/clickhouse_queries/query_display.tsx +++ b/snuba/admin/static/clickhouse_queries/query_display.tsx @@ -2,7 +2,7 @@ import React, { useEffect, useState } from "react"; import Client from "SnubaAdmin/api_client"; import QueryEditor from "SnubaAdmin/query_editor"; import ExecuteButton from "SnubaAdmin/utils/execute_button"; -import { Alert, Box, SegmentedControl, Text } from "@mantine/core"; +import { Alert, Box, SegmentedControl, Text, Tooltip } from "@mantine/core"; import { getRecentHistory, setRecentHistory } from "SnubaAdmin/query_history"; import { COLORS, SECTION_HEADING_STYLE } from "SnubaAdmin/theme"; import { CustomSelect, getParamFromStorage } from "SnubaAdmin/select"; @@ -173,10 +173,24 @@ function QueryDisplay(props: { return output; } + // The first thing still needed before the query can run, if any. It's + // flagged on its field and explains why Execute is disabled. + const missing = !query.storage + ? "storage" + : !query.host || !query.port + ? "host" + : !query.sql + ? "query" + : null; + const missingHint = { + storage: "Select a storage to run the query.", + host: "Select a host to run the query.", + query: "Write or pick a query to run.", + }; + return (
-

Construct a ClickHouse System Query

{prefill && prefillWarningOpen && ( )} +

Construct a ClickHouse System Query

- setClusterlessHost(value === "manual")} - data={[ - { label: "Pick a node", value: "pick" }, - { label: "Enter host manually", value: "manual" }, - ]} - style={segmentedStyle} - />
+
+ + Host entry + + setClusterlessHost(value === "manual")} + data={[ + { label: "Pick a node", value: "pick" }, + { label: "Enter host manually", value: "manual" }, + ]} + /> +
Storage - storage.storage_name)} - /> +
+ storage.storage_name)} + /> +
+ {missing === "storage" && ( +

{missingHint.storage}

+ )}
@@ -232,7 +256,11 @@ function QueryDisplay(props: { {query.clusterless ? ( <> selectHostIp(evt.target.value)} @@ -240,6 +268,9 @@ function QueryDisplay(props: { placeholder="enter host or host:port..." type="text" /> + {missing === "host" && ( +

{missingHint.host}

+ )}

Must be a known cluster node or listed in admin.copy_tables_allowed_target_hosts. Defaults to port @@ -247,15 +278,24 @@ function QueryDisplay(props: {

) : ( - + <> +
+ +
+ {missing === "host" && ( +

{missingHint.host}

+ )} + )}
@@ -289,15 +329,23 @@ function QueryDisplay(props: { : "SELECT, SHOW and DESCRIBE. SELECTs may only read system tables."}

- + {/* A disabled button gets no hover events, so the tooltip + hangs off a wrapper. */} + +
+ +
+
@@ -391,13 +439,26 @@ const targetFieldStyle = { gap: 4, }; +// Marks the field the user needs to fill in next. +const needsInputStyle = { + outline: `2px solid ${COLORS.WARNING}`, + outlineOffset: 1, + borderRadius: 4, +}; + +const needsInputHintStyle = { + fontSize: "0.85em", + color: COLORS.WARNING, + margin: 0, +}; + const modeHelpTextStyle = { fontSize: "0.85em", margin: 0, }; const prefillAlertStyle = { - margin: "8px 0px", + margin: "0 0 16px 0", }; const prefillAlertTextStyle = { diff --git a/snuba/admin/static/query_editor.tsx b/snuba/admin/static/query_editor.tsx index 68bb2a4fc..712998385 100644 --- a/snuba/admin/static/query_editor.tsx +++ b/snuba/admin/static/query_editor.tsx @@ -209,9 +209,11 @@ function QueryEditor(props: { function renderPredefinedQueriesSelectors() { return ( -
- -
+
+ + Predefined query + +
{ @@ -452,7 +454,9 @@ function QueryEditor(props: { {props.predefinedQueryOptions != null && renderPredefinedQueriesSelectors()} {selectedPredefinedQuery?.description ? ( -

{selectedPredefinedQuery?.description}

+ + {selectedPredefinedQuery?.description} + ) : null} @@ -472,7 +476,10 @@ function QueryEditor(props: { } const predefinedQueryStyle = { - display: "inline-block", + width: "20em", + display: "flex", + flexDirection: "column" as const, + gap: 4, }; const timeRangeStyle = { diff --git a/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx b/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx index 65a96491a..d3a689898 100644 --- a/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx +++ b/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx @@ -169,6 +169,29 @@ describe("QueryDisplay opened from a link", () => { expect(screen.getByRole("textbox").textContent).toBe("SELECT 2"); }); + it("flags a missing host and explains the disabled Execute", async () => { + const api = renderWithHash("#system-queries?query=SELECT%201&storage=errors"); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + + const hint = "Select a host to run the query."; + expect(screen.getAllByText(hint)).toHaveLength(1); + expect( + screen.getByText("Execute Query").closest("button")?.disabled + ).toBe(true); + + fireEvent.mouseEnter(screen.getByTestId("execute-wrapper")); + // The field hint plus the tooltip. + await waitFor(() => expect(screen.getAllByText(hint)).toHaveLength(2)); + }); + + it("drops the host cue once a host is set", async () => { + const api = renderWithHash( + "#system-queries?query=SELECT%201&storage=errors&host=clickhouse-01%3A9000" + ); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + expect(screen.queryByText("Select a host to run the query.")).toBeNull(); + }); + it("shows no warning without params", async () => { const api = renderWithHash("#system-queries"); await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); diff --git a/snuba/admin/static/theme.tsx b/snuba/admin/static/theme.tsx index 66f27c7a6..5b6a4d4d2 100644 --- a/snuba/admin/static/theme.tsx +++ b/snuba/admin/static/theme.tsx @@ -25,6 +25,8 @@ const COLORS = { BORDER_GRAY_LIGHT: "#dddddd", ERROR: "#c62828", DANGER: "#a8322d", + // Dark enough orange to read as text on white. + WARNING: "#b35c00", SUCCESS: "#1b5e20", };