From 910f023cf8e4d1a2a8d6983f4a20c0a41257d0c9 Mon Sep 17 00:00:00 2001 From: Meredith Heller Date: Mon, 19 Oct 2026 18:05:02 -0700 Subject: [PATCH] feat(admin): Make System Queries results readable and the editor easier to use Rebuild the System Queries results as a table that scrolls on its own, keep storage, host and Execute in view when the editor is closed, add copy formats and a way to clear saved results, and stop the Execute button and host warning from flickering. Co-Authored-By: Claude Opus 5.5 --- .../admin/static/clickhouse_queries/index.tsx | 29 +- .../clickhouse_queries/query_display.tsx | 559 +++++++++++------- .../query_history_entry.tsx | 107 +++- .../clickhouse_queries/result_table.tsx | 228 +++++++ .../admin/static/clickhouse_queries/types.tsx | 2 +- .../admin/static/common/components/theme.tsx | 6 + snuba/admin/static/query_editor.tsx | 33 +- snuba/admin/static/query_history.tsx | 4 + .../tests/clickhouse_queries/prefill.spec.tsx | 113 +++- .../tests/utils/execute_button.spec.tsx | 4 +- snuba/admin/static/utils/execute_button.tsx | 51 +- .../static/utils/query_result_copier.tsx | 148 ++++- 12 files changed, 969 insertions(+), 315 deletions(-) create mode 100644 snuba/admin/static/clickhouse_queries/result_table.tsx diff --git a/snuba/admin/static/clickhouse_queries/index.tsx b/snuba/admin/static/clickhouse_queries/index.tsx index ec2b8b8d7fe..4e7220b4433 100644 --- a/snuba/admin/static/clickhouse_queries/index.tsx +++ b/snuba/admin/static/clickhouse_queries/index.tsx @@ -1,10 +1,7 @@ import React, { useState, useEffect } from "react"; import Client from "SnubaAdmin/api_client"; -import { Table } from "SnubaAdmin/table"; -import QueryDisplay, { - STICKY_HEADER_TOP, -} from "SnubaAdmin/clickhouse_queries/query_display"; -import { COLORS } from "SnubaAdmin/theme"; +import QueryDisplay from "SnubaAdmin/clickhouse_queries/query_display"; +import { ResultTable } from "SnubaAdmin/clickhouse_queries/result_table"; import { parseSystemQueryPrefill } from "SnubaAdmin/clickhouse_queries/prefill"; import { QueryResult, PredefinedQuery } from "./types"; @@ -37,10 +34,9 @@ function ClickhouseQueries(props: { api: Client }) { function tablePopulator(queryResult: QueryResult) { return ( - ); } @@ -70,19 +66,4 @@ const selectStyle = { height: 30, }; -// Keeps column names in view below the editor pane while scrolling a long -// result. This needs no scrolling ancestor between the table and the page, -// so wide results wrap rather than scroll sideways. -const stickyHeaderStyles = { - thStyle: { - position: "sticky" as const, - top: STICKY_HEADER_TOP, - zIndex: 1, - backgroundColor: COLORS.TABLE_HEADER_BG, - // Collapsed borders don't move with a stuck cell, so draw its bottom - // edge here. - boxShadow: `inset 0 -1px 0 ${COLORS.TABLE_BORDER}`, - }, -}; - export default ClickhouseQueries; diff --git a/snuba/admin/static/clickhouse_queries/query_display.tsx b/snuba/admin/static/clickhouse_queries/query_display.tsx index 908d8861e1f..2c27eba4952 100644 --- a/snuba/admin/static/clickhouse_queries/query_display.tsx +++ b/snuba/admin/static/clickhouse_queries/query_display.tsx @@ -1,8 +1,9 @@ -import React, { useEffect, useRef, useState } from "react"; +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 { + ActionIcon, Alert, Button, SegmentedControl, @@ -10,8 +11,17 @@ import { Tooltip, UnstyledButton, } from "@mantine/core"; -import { IconChevronDown, IconX } from "@tabler/icons-react"; -import { getRecentHistory, setRecentHistory } from "SnubaAdmin/query_history"; +import { + IconInfoCircle, + IconPencil, + IconTrash, + IconX, +} from "@tabler/icons-react"; +import { + clearRecentHistory, + 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"; @@ -32,10 +42,13 @@ import { type QueryState = Partial; const HISTORY_KEY = "clickhouse_queries"; -// Set on the page to the editor pane's height; see STICKY_HEADER_TOP. -const EDITOR_PANEL_HEIGHT_VAR = "--system-queries-editor-height"; -// Where content stuck to the top of the results sits: just below the pane. -const STICKY_HEADER_TOP = `var(${EDITOR_PANEL_HEIGHT_VAR}, 0px)`; +// The storage's first node, to have a host ready as soon as a storage is +// picked. +function firstHost(nodeData: ClickhouseNodeData[], storage?: string) { + const [first] = getHostsForStorage(nodeData, storage); + return first ? parseHostPort(first.value) : {}; +} + function QueryDisplay(props: { api: Client; resultDataPopulator: (queryResult: QueryResult) => JSX.Element; @@ -49,6 +62,9 @@ function QueryDisplay(props: { prefill !== null ); const [nodeData, setNodeData] = useState([]); + // The host is filled in once the nodes arrive, so until then it isn't + // flagged as missing. + const [nodesLoaded, setNodesLoaded] = useState(false); const [manualHostInput, setManualHostInput] = useState(""); const [query, setQuery] = useState({ storage: prefill?.storage ?? getParamFromStorage("storage"), @@ -61,26 +77,13 @@ function QueryDisplay(props: { // Only a result the user just ran opens automatically; saved history loads // collapsed. const [justRan, setJustRan] = useState(null); - // The editor sits in a sticky panel so it can be reopened from anywhere in - // the results. It closes after a successful run to make room for them. + // The editor closes after a successful run to make room for the results. + // Closed, the panel sticks to the top so it can be reopened, or the query + // run again from beside the storage and host, from anywhere in them. const [editorOpen, setEditorOpen] = useState(true); - const resultsHeadingRef = useRef(null); - // Result tables pin their headers just below the pane, so they need its - // height, which changes as it opens, closes and the query grows. - const editorPanelRef = useRef(null); - const [editorPanelHeight, setEditorPanelHeight] = useState(0); - - useEffect(() => { - const panel = editorPanelRef.current; - if (!panel || typeof ResizeObserver === "undefined") { - return; - } - const observer = new ResizeObserver(() => - setEditorPanelHeight(panel.offsetHeight) - ); - observer.observe(panel); - return () => observer.disconnect(); - }, []); + // The query editor puts its predefined query picker here, beside Close. + const [predefinedQuerySlot, setPredefinedQuerySlot] = + useState(null); const [queryError, setQueryError] = useState(null); @@ -93,6 +96,7 @@ function QueryDisplay(props: { } props.api.getClickhouseNodes().then((res) => { setNodeData(res); + setNodesLoaded(true); // 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) { @@ -104,18 +108,17 @@ function QueryDisplay(props: { setManualHostInput(hostPort); setClusterlessHost(true); } + } else if (!prefill) { + // The saved storage comes back without a host. + setQuery((prevQuery) => + prevQuery.host + ? prevQuery + : { ...prevQuery, ...firstHost(res, prevQuery.storage) } + ); } }); }, []); - // Bring a fresh result into view, since it may be above where the user - // scrolled to. (jsdom has no scrollIntoView.) - useEffect(() => { - if (justRan) { - resultsHeadingRef.current?.scrollIntoView?.({ block: "start" }); - } - }, [justRan]); - function selectStorage(storage: string) { setQuery((prevQuery) => { // clear old host port @@ -125,6 +128,8 @@ function QueryDisplay(props: { return { ...prevQuery, storage: storage, + // A typed host isn't replaced; it's up to the user. + ...(prevQuery.clusterless ? {} : firstHost(nodeData, storage)), }; }); } @@ -199,8 +204,17 @@ function QueryDisplay(props: { }); } + function clearResults() { + clearRecentHistory(HISTORY_KEY); + setQueryResultHistory([]); + setJustRan(null); + } + function handleQueryError(error: Error) { setQueryError(error); + // Errors only show in the editor, so a failed run from the closed bar + // opens it. + setEditorOpen(true); } function convertResultsToCSV(queryResult: QueryResult) { @@ -228,53 +242,45 @@ function QueryDisplay(props: { host: "Select a host to run the query.", query: "Write or pick a query to run.", }; + // What to flag on the page; Execute stays disabled either way. + const flagged = nodesLoaded ? missing : null; - return ( -
+
+ +
+ + ); + + return ( +
- {!editorOpen && ( - // The whole bar opens the editor. The "button" inside is only a - // span, since a real button can't be nested in another. - setEditorOpen(true)} - aria-label="Open editor" - style={editorClosedBarStyle} - sx={{ "&:hover": { backgroundColor: COLORS.BG_GRAY_LIGHTER } }} - > - - - {query.sudo && ( - - SUDO{" "} - - )} - {query.storage ?? "(no storage)"} - {" · "} - {query.host ? `${query.host}:${query.port}` : "(no host)"} - {" · "} - {query.sql || "(no query)"} - - - )} - {/* Hidden rather than unmounted, so the editor keeps its state. */} -
+ {/* Storage and host stay in view while the editor is closed. */} +
{prefill && prefillWarningOpen && ( )} -
-

- Construct a ClickHouse System Query -

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

{missingHint.storage}

+ {flagged === "storage" && ( +

{missingHint.storage}

+ )} +
+
+ {/* The manual-entry rules live in a tooltip so they don't + make the row taller. */} + + Host + {query.clusterless && ( + + + + + )} -
-
- - Host - - {query.clusterless ? ( - <> - + {query.clusterless ? ( + <> + selectHostIp(evt.target.value)} + name="host" + placeholder="enter host or host:port..." + type="text" + /> + {flagged === "host" && ( +

{missingHint.host}

+ )} + + ) : ( + <> +
+ selectHostIp(evt.target.value)} - name="host" - placeholder="enter host or host:port..." - type="text" + onChange={selectHost} + name="Host" + options={getHostsForStorage(nodeData, query.storage)} /> - {missing === "host" && ( -

{missingHint.host}

- )} -

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

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

{missingHint.host}

- )} - - )} +
+ {flagged === "host" && ( +

{missingHint.host}

+ )} + + )} +
+
+ + Host entry + + setClusterlessHost(value === "manual")} + data={[ + { label: "Pick a node", value: "pick" }, + { label: "Manual", value: "manual" }, + ]} + /> +
+ {!editorOpen && ( +
+ {/* An empty label lines the button up with the selects. */} + +   + + {executeControl}
+ )} +
+
+ {!editorOpen && ( + // The whole bar opens the editor. The icon inside is only a span, + // since a real button can't be nested in another. + setEditorOpen(true)} + aria-label="Open editor" + style={editorClosedBarStyle} + sx={{ "&:hover": { backgroundColor: COLORS.BG_GRAY_LIGHTER } }} + > + + {query.sudo && ( + + SUDO{" "} + + )} + {query.sql || "(no query)"} + + {/* Where the editor's Close button sits, so opening and closing + are in the same place. */} + + + + + + + )} + {/* Hidden rather than unmounted, so the editor keeps its state. */} + +
+

+ Construct a ClickHouse System Query +

+
+ {/* What each mode allows is on hover, to keep the row short. */} + +
+ setSudo(value === "sudo")} + color={query.sudo ? "red" : undefined} + data={[ + { label: "User mode", value: "user" }, + { label: "Sudo mode", value: "sudo" }, + ]} + /> +
+ +
+
@@ -405,55 +489,34 @@ function QueryDisplay(props: { }} predefinedQueryOptions={props.predefinedQueryOptions} initialQuery={prefill?.sql} + predefinedQuerySlot={predefinedQuerySlot} />
{/* Sudo can change data, so it gets a red frame. */}
+ {/* The error sits beside Execute, so it never moves the button. */}
- setSudo(value === "sudo")} - color={query.sudo ? "red" : undefined} - data={[ - { label: "User mode", value: "user" }, - { label: "Sudo mode", value: "sudo" }, - ]} - style={{ flexShrink: 0 }} - /> -

- {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."} -

- {/* A disabled button gets no hover events, so the tooltip - hangs off a wrapper. */} - -
- -
- +
{editorOpen && errorElement}
+ {editorOpen && executeControl}
- {getErrorDomElement(queryError, collapseOpened, setCollapseOpened)} -
-
-

- Query results -

+
+

Query results

+ + + + + +
{queryResultHistory.map((queryResult, idx) => ( + row.map(cellText) + ); +} + +// Tab-separated, which spreadsheets and docs paste as a table. +function toTsv(queryResult: QueryResult) { + return tableRows(queryResult) + .map((row) => row.join("\t")) + .join("\n"); +} + +// A Markdown table, padded so it also lines up as plain text. Pipes would +// end a cell early, so they're escaped. +function toMarkdown(queryResult: QueryResult) { + const [header, ...rows] = tableRows(queryResult).map((row) => + row.map((cell) => cell.replace(/\|/g, "\\|")) + ); + // Markdown needs at least three dashes under each heading. + const widths = header.map((name, col) => + Math.max(3, name.length, ...rows.map((row) => (row[col] ?? "").length)) + ); + const line = (row: string[]) => + `| ${widths + .map((width, col) => (row[col] ?? "").padEnd(width)) + .join(" | ")} |`; + return [ + line(header), + line(widths.map((width) => "-".repeat(width))), + ...rows.map(line), + ].join("\n"); +} + function QueryHistoryEntry(props: { queryResult: QueryResult; isLatest: boolean; @@ -23,6 +70,16 @@ function QueryHistoryEntry(props: { }) { const { queryResult, isLatest } = props; const [expanded, setExpanded] = useState(props.defaultExpanded); + // The full query is only shown on request, to keep results up top. + const [sqlShown, setSqlShown] = useState(false); + + // A newer run takes over, so this one folds away to make room for it. + useEffect(() => { + if (!isLatest) { + setExpanded(false); + } + }, [isLatest]); + const { sql, storage, host } = parseInputQuery(queryResult.input_query); const rowCount = queryResult.rows.length; @@ -58,27 +115,44 @@ function QueryHistoryEntry(props: { ? ` · ${new Date(queryResult.timestamp * 1000).toLocaleTimeString()}` : ""} - {/* Copying shouldn't toggle the entry. */} -
evt.stopPropagation()}> + {rowCount === 0 && No results} + {/* Showing the SQL or copying shouldn't toggle the entry. */} + evt.stopPropagation()} + > + -
+
- {expanded ? ( - <> - 
{sql}
-
- {props.resultDataPopulator(queryResult)} -
- + {sqlShown ? ( + 
{sql}
) : ( -
- {sql.replace(/\s+/g, " ")} + !expanded && ( +
+ {sql.replace(/\s+/g, " ")} +
+ ) + )} + {expanded && ( +
+ {props.resultDataPopulator(queryResult)}
)}
@@ -137,6 +211,13 @@ const latestBadgeStyle = { backgroundColor: "rgba(31, 111, 235, 0.1)", }; +// Calls out an empty result, which is easy to miss in the row count. +const emptyBadgeStyle = { + ...badgeStyle, + color: COLORS.WARNING, + backgroundColor: "rgba(179, 92, 0, 0.1)", +}; + const storageBadgeStyle = { ...badgeStyle, color: COLORS.TEXT_DEFAULT, diff --git a/snuba/admin/static/clickhouse_queries/result_table.tsx b/snuba/admin/static/clickhouse_queries/result_table.tsx new file mode 100644 index 00000000000..7d3ea015dca --- /dev/null +++ b/snuba/admin/static/clickhouse_queries/result_table.tsx @@ -0,0 +1,228 @@ +import React, { useState } from "react"; +import { Button } from "@mantine/core"; +import { IconArrowsMaximize, IconArrowsMinimize } from "@tabler/icons-react"; +import { COLORS } from "SnubaAdmin/theme"; + +// Past this many rows the table scrolls in a box of fixed height, unless the +// user expands it. +const COLLAPSED_ROW_LIMIT = 12; +const COLLAPSED_MAX_HEIGHT = 480; +// Longer values are cut off; clicking one shows all of it. +const CELL_MAX_CHARS = 40; + +function displayText(value: unknown) { + if (value === null || value === undefined) { + return null; + } + return typeof value === "object" ? JSON.stringify(value) : String(value); +} + +function isNumeric(value: unknown) { + return ( + typeof value === "number" || + (typeof value === "string" && /^-?\d+(\.\d+)?$/.test(value)) + ); +} + +function ResultTable(props: { columnNames: string[]; rows: unknown[][] }) { + const { columnNames, rows } = props; + const [expanded, setExpanded] = useState(false); + // "row:col" of each long value the user opened. + const [openCells, setOpenCells] = useState>(new Set()); + const tall = rows.length > COLLAPSED_ROW_LIMIT; + + function toggleCell(key: string) { + setOpenCells((prev) => { + const next = new Set(prev); + next.has(key) ? next.delete(key) : next.add(key); + return next; + }); + } + + return ( +
+
+
+ + + + {columnNames.map((name, col) => ( + + ))} + + + + {rows.map((row, r) => { + const rowBg = r % 2 ? COLORS.BG_GRAY_LIGHTER : COLORS.TABLE_BG; + return ( + + + {row.map((value, c) => { + const text = displayText(value); + const key = `${r}:${c}`; + const long = text !== null && text.length > CELL_MAX_CHARS; + const open = openCells.has(key); + return ( + + ); + })} + + ); + })} + +
+ # + + {name} +
+ {r + 1} + + {text === null ? ( + NULL + ) : long ? ( +
toggleCell(key)} + onKeyDown={(evt) => { + if (evt.key === "Enter" || evt.key === " ") { + evt.preventDefault(); + toggleCell(key); + } + }} + > + {text} +
+ ) : ( + text + )} +
+ + {tall && ( + + )} + + ); +} + +const border = `1px solid ${COLORS.TABLE_BORDER}`; + +// Scrolls both ways, so wide and long results stay inside the entry. +const scrollBoxStyle = { + overflow: "auto", + border, + borderRadius: 4, + marginBottom: 12, +}; + +// Separate borders, since collapsed ones don't move with stuck cells. +const tableStyle = { + borderCollapse: "separate" as const, + borderSpacing: 0, + minWidth: "100%", + fontSize: 13, +}; + +const thStyle = { + position: "sticky" as const, + top: 0, + zIndex: 1, + backgroundColor: COLORS.TABLE_HEADER_BG, + color: COLORS.TABLE_HEADER_TEXT, + fontWeight: 600, + textAlign: "left" as const, + whiteSpace: "nowrap" as const, + padding: "6px 10px", + borderBottom: border, + borderRight: border, +}; + +const tdStyle = { + fontFamily: "monospace", + whiteSpace: "nowrap" as const, + verticalAlign: "top" as const, + padding: "4px 10px", + borderBottom: `1px solid ${COLORS.BORDER_GRAY_LIGHT}`, + borderRight: `1px solid ${COLORS.BORDER_GRAY_LIGHT}`, +}; + +// Row numbers stay in view while scrolling sideways. +const rowNumberStyle = { + position: "sticky" as const, + left: 0, + textAlign: "right" as const, + color: COLORS.TEXT_LIGHTER, + borderRight: border, +}; + +// Above both the header and the row numbers. +const cornerStyle = { + zIndex: 2, + backgroundColor: COLORS.TABLE_HEADER_BG, +}; + +// Opening a value keeps its width and only adds lines. A wrappable value with +// no set width would let the table squeeze its column to a sliver. +const longValueWidth = `${CELL_MAX_CHARS}ch`; + +const clippedValueStyle = { + width: longValueWidth, + overflow: "hidden", + textOverflow: "ellipsis", + cursor: "pointer", +}; + +const openValueStyle = { + width: longValueWidth, + whiteSpace: "pre-wrap" as const, + overflowWrap: "anywhere" as const, + cursor: "pointer", +}; + +const nullStyle = { + color: COLORS.TEXT_LIGHTER, + fontStyle: "italic" as const, + opacity: 0.7, +}; + +const toggleStyle = { + marginBottom: 12, +}; + +export { ResultTable }; diff --git a/snuba/admin/static/clickhouse_queries/types.tsx b/snuba/admin/static/clickhouse_queries/types.tsx index 0bdfd2d5f40..9c1c2955a84 100644 --- a/snuba/admin/static/clickhouse_queries/types.tsx +++ b/snuba/admin/static/clickhouse_queries/types.tsx @@ -27,7 +27,7 @@ type QueryResult = { input_query: string; timestamp: number; column_names: QueryResultColumnMetadata; - rows: [QueryResultRow]; + rows: QueryResultRow[]; trace_output?: string; error?: string; }; diff --git a/snuba/admin/static/common/components/theme.tsx b/snuba/admin/static/common/components/theme.tsx index dfc05af57ff..7be5e9624b3 100644 --- a/snuba/admin/static/common/components/theme.tsx +++ b/snuba/admin/static/common/components/theme.tsx @@ -9,6 +9,12 @@ export const theme = EditorView.theme({ backgroundColor: COLORS.PANEL_BG, border: `1px solid ${COLORS.BORDER_GRAY}`, borderRadius: "4px", + // A long query scrolls inside the editor instead of pushing what's below + // it, like errors, off the screen. + maxHeight: "40vh", + }, + ".cm-scroller": { + overflow: "auto", }, }); diff --git a/snuba/admin/static/query_editor.tsx b/snuba/admin/static/query_editor.tsx index aed09eb3259..1b51e1f9011 100644 --- a/snuba/admin/static/query_editor.tsx +++ b/snuba/admin/static/query_editor.tsx @@ -1,4 +1,5 @@ import React, { useEffect, useState, ReactElement } from "react"; +import { createPortal } from "react-dom"; import { Box, @@ -8,6 +9,7 @@ import { Select, Stack, Text, + Tooltip, } from "@mantine/core"; import { DateTimePicker } from "@mantine/dates"; import { SQLEditor } from "SnubaAdmin/common/components/sql_editor"; @@ -110,6 +112,9 @@ function QueryEditor(props: { outcomeOptions?: Array; // Replaces the saved query on mount, e.g. when opened from a link. initialQuery?: string; + // Where to put the predefined query picker instead of above the editor. + // It's null until that element mounts, and the picker waits for it. + predefinedQuerySlot?: HTMLElement | null; }) { const [query, setQuery] = useState(""); const hash = splitHash(window.location.hash).route; @@ -449,11 +454,33 @@ function QueryEditor(props: { return setters; } + const predefinedQueriesSelectors = + props.predefinedQueryOptions != null && renderPredefinedQueriesSelectors(); + const description = selectedPredefinedQuery?.description; + // Moved into a toolbar, the picker shows its description on hover rather + // than as a line of text. + const inSlot = props.predefinedQuerySlot !== undefined; + return (
- {props.predefinedQueryOptions != null && - renderPredefinedQueriesSelectors()} - {selectedPredefinedQuery?.description ? ( + {!inSlot + ? predefinedQueriesSelectors + : props.predefinedQuerySlot && + createPortal( + +
+ {predefinedQueriesSelectors} +
+ , + props.predefinedQuerySlot + )} + {!inSlot && description ? ( {selectedPredefinedQuery?.description} diff --git a/snuba/admin/static/query_history.tsx b/snuba/admin/static/query_history.tsx index fbca903691c..edf7faa9862 100644 --- a/snuba/admin/static/query_history.tsx +++ b/snuba/admin/static/query_history.tsx @@ -25,6 +25,10 @@ export function setRecentHistory(key: string, query: any) { } } +export function clearRecentHistory(key: string) { + localStorage.removeItem(`${key}.history`); +} + export function getRecentHistory(key: string) { const storageKey = `${key}.history` const recentHistory = localStorage.getItem(storageKey) diff --git a/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx b/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx index c8491a912f5..aa258324a62 100644 --- a/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx +++ b/snuba/admin/static/tests/clickhouse_queries/prefill.spec.tsx @@ -81,6 +81,8 @@ describe("QueryDisplay opened from a link", () => { afterEach(() => { cleanup(); window.location.hash = ""; + // Runs are saved to history; start each test without the last one's. + localStorage.clear(); }); const nodes: [ClickhouseNodeData] = [ @@ -201,13 +203,15 @@ describe("QueryDisplay opened from a link", () => { await screen.findAllByDisplayValue("clickhouse-01:9000"); fireEvent.click(screen.getByText("Execute Query")); - await screen.findByText("Open Editor"); + await screen.findByText("1 row", { exact: false }); expect(api.executeSystemQuery).toBeCalled(); + expect(screen.getByLabelText("Open editor")).toBeTruthy(); expect(screen.queryByRole("textbox")).toBeNull(); - // The closed bar says what will run. - expect(screen.getByTestId("editor-panel").textContent).toContain( - "errors · clickhouse-01:9000 · SELECT 1" - ); + // Where it will run stays in view; the closed bar shows the query. + expect(screen.getAllByDisplayValue("clickhouse-01:9000")).not.toHaveLength(0); + expect( + within(screen.getByLabelText("Open editor")).getByText("SELECT 1") + ).toBeTruthy(); // Anywhere on the bar opens it, not just the button. fireEvent.click( @@ -216,7 +220,40 @@ describe("QueryDisplay opened from a link", () => { }) ); expect(screen.getByRole("textbox").textContent).toBe("SELECT 1"); - expect(screen.queryByText("Open Editor")).toBeNull(); + expect(screen.queryByLabelText("Open editor")).toBeNull(); + }); + + it("fills in the first host for the saved storage", async () => { + localStorage.setItem("select-storage", "errors"); + const api = renderWithHash("#system-queries"); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + expect(await screen.findAllByDisplayValue("clickhouse-01:9000")).not.toHaveLength(0); + }); + + it("leaves the host empty for a link that doesn't name one", async () => { + const api = renderWithHash("#system-queries?query=SELECT%201&storage=errors"); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + expect(screen.getAllByText("Select a host to run the query.")).not.toHaveLength(0); + expect(screen.queryAllByDisplayValue("clickhouse-01:9000")).toHaveLength(0); + }); + + it("hides the error with the editor, and opens it for a failed run", async () => { + const api = renderWithHash( + "#system-queries?query=SELECT%201&storage=errors&host=clickhouse-01%3A9000" + ); + api.executeSystemQuery.mockRejectedValue(new Error("Unknown table")); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + await screen.findAllByDisplayValue("clickhouse-01:9000"); + + fireEvent.click(screen.getByText("Execute Query")); + await screen.findByText("Unknown table"); + fireEvent.click(screen.getByLabelText("Close editor")); + expect(screen.queryByText("Unknown table")).toBeNull(); + + // Running again from the closed bar brings the editor back to show it. + fireEvent.click(screen.getByText("Execute Query")); + await waitFor(() => expect(screen.queryByLabelText("Open editor")).toBeNull()); + expect(screen.getByText("Unknown table")).toBeTruthy(); }); it("closes the editor with the X", async () => { @@ -224,10 +261,26 @@ describe("QueryDisplay opened from a link", () => { await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); fireEvent.click(screen.getByLabelText("Close editor")); - expect(screen.getByText("Open Editor")).toBeTruthy(); + expect(screen.getByLabelText("Open editor")).toBeTruthy(); expect(api.executeSystemQuery).not.toBeCalled(); }); + it("runs the query from beside the host while the editor is closed", async () => { + const api = renderWithHash( + "#system-queries?query=SELECT%201&storage=errors&host=clickhouse-01%3A9000" + ); + await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); + await screen.findAllByDisplayValue("clickhouse-01:9000"); + + fireEvent.click(screen.getByLabelText("Close editor")); + // Only the button beside the host is rendered. + expect(screen.getAllByText("Execute Query")).toHaveLength(1); + fireEvent.click(screen.getByText("Execute Query")); + await screen.findByText("1 row", { exact: false }); + expect(api.executeSystemQuery).toBeCalled(); + expect(screen.getByLabelText("Open editor")).toBeTruthy(); + }); + it("shows no warning without params", async () => { const api = renderWithHash("#system-queries"); await waitFor(() => expect(api.getClickhouseNodes).toBeCalled()); @@ -258,7 +311,7 @@ describe("QueryHistoryEntry", () => { /> ); - fireEvent.click(screen.getByText("Share query")); + fireEvent.click(screen.getByLabelText("Share query")); const url = writeText.mock.calls[0][0]; expect(parseSystemQueryPrefill(new URL(url).hash)).toEqual({ @@ -268,4 +321,48 @@ describe("QueryHistoryEntry", () => { port: 9000, }); }); + + it("copies the results as a table", () => { + const writeText = jest + .fn<(text: string) => Promise>() + .mockResolvedValue(undefined); + Object.assign(window.navigator, { clipboard: { writeText } }); + render( +
} + /> + ); + + fireEvent.click(screen.getByLabelText("Copy as table")); + expect(writeText).toBeCalledWith( + "name\tvalue\na\t1\ntab here\tline break\na|b\t" + ); + + fireEvent.click(screen.getByLabelText("Copy as Markdown")); + expect(writeText).toHaveBeenLastCalledWith( + [ + "| name | value |", + "| -------- | ---------- |", + "| a | 1 |", + "| tab here | line break |", + "| a\\|b | |", + ].join("\n") + ); + }); + }); diff --git a/snuba/admin/static/tests/utils/execute_button.spec.tsx b/snuba/admin/static/tests/utils/execute_button.spec.tsx index e1ed80bbd02..d2be7b75c48 100644 --- a/snuba/admin/static/tests/utils/execute_button.spec.tsx +++ b/snuba/admin/static/tests/utils/execute_button.spec.tsx @@ -48,5 +48,7 @@ it("should not call if loading", async () => { await waitFor(() => expect(mockCall).toBeCalledTimes(1)); expect(screen.getByRole("status").textContent).toBe("Query running…"); - expect(button.textContent).toContain("Running"); + expect(button.getAttribute("aria-busy")).toBe("true"); + // The label doesn't change, so the button keeps its size. + expect(button.textContent).toContain("Execute Query"); }); diff --git a/snuba/admin/static/utils/execute_button.tsx b/snuba/admin/static/utils/execute_button.tsx index 29ea3839438..b1e4c13f1b9 100644 --- a/snuba/admin/static/utils/execute_button.tsx +++ b/snuba/admin/static/utils/execute_button.tsx @@ -1,5 +1,8 @@ -import React, { useState } from "react"; -import { Button, Group, Text } from "@mantine/core"; +import React, { useEffect, useState } from "react"; +import { Button } from "@mantine/core"; + +// Fast queries finish before this, so they don't flash a spinner. +const SPINNER_DELAY_MS = 150; function ExecuteButton(props: { disabled: boolean; @@ -10,6 +13,16 @@ function ExecuteButton(props: { color?: string; }) { const [isExecuting, setIsExecuting] = useState(false); + const [showSpinner, setShowSpinner] = useState(false); + + useEffect(() => { + if (!isExecuting) { + setShowSpinner(false); + return; + } + const timer = setTimeout(() => setShowSpinner(true), SPINNER_DELAY_MS); + return () => clearTimeout(timer); + }, [isExecuting]); let label = props.label || "Execute Query"; @@ -31,8 +44,9 @@ function ExecuteButton(props: { } function executeQuery() { + // Until the spinner shows the button still looks clickable, so extra + // clicks are ignored rather than queued. if (isExecuting) { - window.alert("A query is already running"); return; } setIsExecuting(true); @@ -48,27 +62,38 @@ function ExecuteButton(props: { }); } + // The label stays put and the spinner covers it, so the button keeps its + // size and nothing around it moves while a query runs. return ( - + <> - {isExecuting ? ( - - Query running… - - ) : null} - + + {isExecuting ? "Query running…" : ""} + + ); } +// Read out by screen readers without taking up space. +const visuallyHiddenStyle = { + position: "absolute" as const, + width: 1, + height: 1, + overflow: "hidden", + clip: "rect(0 0 0 0)", + whiteSpace: "nowrap" as const, +}; + export default ExecuteButton; diff --git a/snuba/admin/static/utils/query_result_copier.tsx b/snuba/admin/static/utils/query_result_copier.tsx index 14fec22c097..9f39f1db6bf 100644 --- a/snuba/admin/static/utils/query_result_copier.tsx +++ b/snuba/admin/static/utils/query_result_copier.tsx @@ -1,45 +1,149 @@ -import React from "react"; -import { Button, Group } from "@mantine/core"; +import React, { useEffect, useState } from "react"; +import { ActionIcon, Button, Group, Tooltip } from "@mantine/core"; +import { + IconBraces, + IconCheck, + IconCopy, + IconFileSpreadsheet, + IconFileText, + IconLink, + IconMarkdown, +} from "@tabler/icons-react"; + +// The Clipboard API only exists on secure pages (https or localhost), so +// admin served over plain http falls back to the older copy command. +function copyText(text: string): Promise { + if (window.navigator.clipboard) { + try { + return Promise.resolve(window.navigator.clipboard.writeText(text)); + } catch (err) { + return Promise.reject(err); + } + } + const textarea = document.createElement("textarea"); + textarea.value = text; + textarea.style.position = "fixed"; + textarea.style.opacity = "0"; + document.body.appendChild(textarea); + textarea.select(); + const copied = document.execCommand("copy"); + document.body.removeChild(textarea); + return copied ? Promise.resolve() : Promise.reject(new Error("copy failed")); +} + +// An icon that copies on click and briefly turns into a check. +function CopyIcon(props: { + label: string; + icon: React.ReactNode; + text: string; +}) { + const [copied, setCopied] = useState(false); + + useEffect(() => { + if (!copied) { + return; + } + const timer = setTimeout(() => setCopied(false), 1500); + return () => clearTimeout(timer); + }, [copied]); + + return ( + + + copyText(props.text).then( + () => setCopied(true), + (err) => console.error("Copy failed: ", err) + ) + } + > + {copied ? : props.icon} + + + ); +} function QueryResultCopier(props: { rawInput?: string; jsonInput?: string; csvInput?: string; + // Tab-separated, which spreadsheets and docs paste as a table. + tableInput?: string; + // A Markdown table, for pasting into docs, PRs or chat. + markdownInput?: string; linkInput?: string; - // Small, quiet buttons with short labels, for tight spaces. + // Icons with tooltips instead of labelled buttons, for tight spaces. compact?: boolean; }) { - function copyText(text: string) { - window.navigator.clipboard.writeText(text); + if (props.compact) { + return ( + + {props.tableInput && ( + } + text={props.tableInput} + /> + )} + {props.markdownInput && ( + } + text={props.markdownInput} + /> + )} + {props.rawInput && ( + } + text={props.rawInput} + /> + )} + {props.jsonInput && ( + } + text={props.jsonInput} + /> + )} + {props.csvInput && ( + } + text={props.csvInput} + /> + )} + {props.linkInput && ( + } + text={props.linkInput} + /> + )} + + ); } - const buttonProps = props.compact - ? { size: "xs" as const, variant: "default" as const } - : {}; - const label = (format: string) => - props.compact ? `Copy ${format}` : `Copy to clipboard (${format})`; - - return + return {props.rawInput && ( - )} {props.jsonInput && ( - )} {props.csvInput && ( - )} {props.linkInput && ( - )}