diff --git a/snuba/admin/static/clickhouse_queries/query_display.tsx b/snuba/admin/static/clickhouse_queries/query_display.tsx index c5a0aec4ab..dab886da33 100644 --- a/snuba/admin/static/clickhouse_queries/query_display.tsx +++ b/snuba/admin/static/clickhouse_queries/query_display.tsx @@ -2,8 +2,9 @@ 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, 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"; import { getHostsForStorage, getErrorDomElement } from "SnubaAdmin/utils/clickhouse_node_utils"; import { QueryHistoryEntry } from "SnubaAdmin/clickhouse_queries/query_history_entry"; @@ -172,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 && ( )} -
- ) => - setSudo(evt.currentTarget.checked) - } - onLabel="Sudo mode" - offLabel="User mode" - size="xl" - /> -
-
- ) => - 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_name)} +

Construct a ClickHouse System Query

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

{missingHint.storage}

+ )} +
+
+ + Host + + {query.clusterless ? ( + <> + selectHostIp(evt.target.value)} + name="host" + 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 + 8123. +

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

{missingHint.host}

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

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

Query results

+

Query results

{queryResultHistory.map((queryResult, idx) => ( + + {props.title} + + {props.children} + + ); +} + +const sectionStyle = { + border: "1px solid #ced4da", + 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 standardForm = {}; +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, +}; + +// 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 = { @@ -320,20 +466,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 +480,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; diff --git a/snuba/admin/static/query_editor.tsx b/snuba/admin/static/query_editor.tsx index 68bb2a4fca..7129983858 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 65a96491a5..d3a6898988 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 668b63abb6..5b6a4d4d25 100644 --- a/snuba/admin/static/theme.tsx +++ b/snuba/admin/static/theme.tsx @@ -25,9 +25,21 @@ const COLORS = { BORDER_GRAY_LIGHT: "#dddddd", ERROR: "#c62828", DANGER: "#a8322d", + // Dark enough orange to read as text on white. + WARNING: "#b35c00", 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 +56,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 5ad1b13839..29ea383943 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 e486c2d22c..c7d0b29bc8 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 = {