From ace9422bfd3f54d9e25c5986dcfee53a4764b5fb Mon Sep 17 00:00:00 2001 From: Saxon Fletcher Date: Thu, 13 Aug 2026 20:59:30 +1000 Subject: [PATCH] refactor(studio): share Explorer query editor (#49041) ## Stack Depends on #49027. Followed by #49038. ## Summary - extract a controlled `QueryEditor` from the existing notebook query cell - reuse it from `QueryCell`, leaving notebook persistence and sortable-block behavior in the adapter - make table/chart result settings controlled so other query surfaces can share them - persist notebook SQL on editor blur and query execution ## To test 1. Open a notebook query cell, edit and rename it, then run the query and confirm results appear. 2. Switch between table and chart results and confirm notebook move/delete actions still work. ## Why Notebooks, query tabs, and future chat tabs need consistent query actions and result rendering without duplicating the notebook implementation. ## Impact This is primarily a refactor of the existing notebook query experience. It introduces no new query-tab routes or source-selection behavior. ## Validation - fresh non-incremental Studio TypeScript check - focused NotebookEditor component tests ## Summary by CodeRabbit * **New Features** * Added a shared query editor with SQL editing, execution, validation, visibility controls, editable titles, row limits, and loading/error states. * Added table and chart result views, including customizable bar and line charts. * Added support for switching display modes and updating chart settings. * **Improvements** * Improved query result handling and display-setting updates. * Repositioned the logarithmic-scale tooltip for better visibility. --------- Co-authored-by: Joshen Lim --- .../QueryCell/DisplaySettingsButton.tsx | 67 ++----- .../Explorer/QueryCell/QueryResultChart.tsx | 9 +- .../interfaces/Explorer/QueryCell/index.tsx | 177 ++++-------------- .../interfaces/Explorer/QueryEditor.tsx | 172 +++++++++++++++++ .../components/interfaces/Explorer/types.ts | 20 +- .../ui/QueryBlock/QueryBlock.utils.ts | 6 +- 6 files changed, 253 insertions(+), 198 deletions(-) create mode 100644 apps/studio/components/interfaces/Explorer/QueryEditor.tsx diff --git a/apps/studio/components/interfaces/Explorer/QueryCell/DisplaySettingsButton.tsx b/apps/studio/components/interfaces/Explorer/QueryCell/DisplaySettingsButton.tsx index 29a8ee38427..70c07a3bae5 100644 --- a/apps/studio/components/interfaces/Explorer/QueryCell/DisplaySettingsButton.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryCell/DisplaySettingsButton.tsx @@ -18,34 +18,29 @@ import { TooltipTrigger, } from 'ui' import { FormItemLayout } from 'ui-patterns/form/FormItemLayout/FormItemLayout' -import { type Snapshot } from 'valtio' import { ExplorerToolbarAction } from '../ExplorerToolbar' -import { type QueryResult } from '../types' +import { type QueryChartConfig, type QueryDisplay, type QueryResult } from '../types' import { checkHasNonPositiveValues } from '@/components/ui/QueryBlock/QueryBlock.utils' -import { type DatabaseCell as DatabaseCellSchema } from '@/data/content/notebooks/notebook-schema' -import { useCurrentNotebook, useNotebooksStateSnapshot } from '@/state/notebooks/notebooks-state' interface DisplaySettingsButtonProps { - cell: Snapshot + display: QueryDisplay result?: QueryResult columns: string[] disabled: boolean + onChange: (display: QueryDisplay) => void } // [Joshen] TODO support multiple y axis charts export const DisplaySettingsButton = ({ - cell, + display, result, columns, disabled, + onChange, }: DisplaySettingsButtonProps) => { - const snap = useNotebooksStateSnapshot() - const currentNotebook = useCurrentNotebook() - const cells = currentNotebook?.notebook.content?.cells ?? [] - - const { view, chart } = cell + const { view, chart } = display const { type = 'bar', x_column, @@ -66,44 +61,22 @@ export const DisplaySettingsButton = ({ }, [hasNonPositiveValues, result, y_columns.length]) const onChangeView = (view: 'table' | 'chart') => { - const notebookId = currentNotebook?.notebook.id - if (!notebookId) return - - const nextCells = cells.map((c) => - c.id === cell.id && c._tag === 'database_cell' ? { ...c, view } : c - ) - snap.updateCells({ id: notebookId, cells: nextCells }) + onChange({ ...display, view }) } - const onUpdateChartConfig = ( - payload: - | { type: 'bar' | 'line' } - | { x_column: string } - | { y_columns: string[] } - | { cumulative: boolean } - | { show_labels: boolean } - | { scale: 'linear' | 'log' } - ) => { - const notebookId = currentNotebook?.notebook.id - if (!notebookId) return - - const nextCells = cells.map((c) => { - if (c.id !== cell.id || c._tag !== 'database_cell') return c - - return { - ...c, - chart: { - type: c.chart?.type ?? 'bar', - x_column: c.chart?.x_column ?? '', - y_columns: c.chart?.y_columns ?? [], - cumulative: c.chart?.cumulative ?? false, - scale: c.chart?.scale ?? 'linear', - show_labels: c.chart?.show_labels ?? false, - ...payload, - }, - } + const onUpdateChartConfig = (payload: Partial) => { + onChange({ + ...display, + chart: { + type: chart?.type ?? 'bar', + x_column: chart?.x_column ?? '', + y_columns: chart?.y_columns ?? [], + cumulative: chart?.cumulative ?? false, + scale: chart?.scale ?? 'linear', + show_labels: chart?.show_labels ?? false, + ...payload, + }, }) - snap.updateCells({ id: notebookId, cells: nextCells }) } const resetToLinearScale = useEffectEvent(() => { @@ -230,7 +203,7 @@ export const DisplaySettingsButton = ({ {!canToggleLogScale && ( - + {y_columns.length === 0 ? 'Select a column for the Y axis first' : 'Data contains zero or negative values'} diff --git a/apps/studio/components/interfaces/Explorer/QueryCell/QueryResultChart.tsx b/apps/studio/components/interfaces/Explorer/QueryCell/QueryResultChart.tsx index ddee91b17b7..da90821dbd2 100644 --- a/apps/studio/components/interfaces/Explorer/QueryCell/QueryResultChart.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryCell/QueryResultChart.tsx @@ -1,14 +1,12 @@ import { useMemo } from 'react' import { Chart, ChartBar, ChartCard, ChartContent, ChartLine } from 'ui-patterns/Chart' -import { type Snapshot } from 'valtio' -import { type QueryResult } from '../types' +import { type QueryChartConfig, type QueryResult } from '../types' import NoDataPlaceholder from '@/components/ui/Charts/NoDataPlaceholder' import { formatLogTick, getCumulativeResults } from '@/components/ui/QueryBlock/QueryBlock.utils' -import { type DatabaseCell as DatabaseCellSchema } from '@/data/content/notebooks/notebook-schema' interface QueryResultChartProps { - cell: Snapshot + chart?: QueryChartConfig result?: QueryResult } @@ -21,8 +19,7 @@ const toChartValue = (value: unknown): string | number => { return String(value) } -export const QueryResultChart = ({ cell, result }: QueryResultChartProps) => { - const { chart } = cell +export const QueryResultChart = ({ chart, result }: QueryResultChartProps) => { const { type, x_column, y_columns = [], cumulative, show_labels, scale } = chart ?? {} const hasConfig = !!x_column && y_columns.length > 0 diff --git a/apps/studio/components/interfaces/Explorer/QueryCell/index.tsx b/apps/studio/components/interfaces/Explorer/QueryCell/index.tsx index 2a2a6a8ff1f..9553fa64262 100644 --- a/apps/studio/components/interfaces/Explorer/QueryCell/index.tsx +++ b/apps/studio/components/interfaces/Explorer/QueryCell/index.tsx @@ -1,177 +1,76 @@ -import { acceptUntrustedSql, untrustedSql } from '@supabase/pg-meta' -import { CodeSquare, Eye, EyeOff, Play } from 'lucide-react' +import { untrustedSql } from '@supabase/pg-meta' import { useState } from 'react' -import { cn } from 'ui' import { type Snapshot } from 'valtio' -import { - ExplorerQuery, - ExplorerQueryEditor, - ExplorerQueryFooter, - ExplorerQueryResults, -} from '../ExplorerQuery' -import { - ExplorerToolbar, - ExplorerToolbarAction, - ExplorerToolbarActions, - ExplorerToolbarIcon, - ExplorerToolbarTitle, -} from '../ExplorerToolbar' -import { QueryResultTable } from '../QueryResultTable' -import { type QueryResult } from '../types' -import { DisplaySettingsButton } from './DisplaySettingsButton' -import { QueryResultChart } from './QueryResultChart' -import { CodeEditor } from '@/components/ui/CodeEditor/CodeEditor' +import { QueryEditor } from '../QueryEditor' +import { type QueryDisplay, type QueryResult } from '../types' import { SortableSection } from '@/components/ui/SortableSection' import { type DatabaseCell as DatabaseCellSchema } from '@/data/content/notebooks/notebook-schema' -import { useExecuteSqlMutation } from '@/data/sql/execute-sql-mutation' -import { useLatest } from '@/hooks/misc/useLatest' -import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' import { useCurrentNotebook, useNotebooksStateSnapshot } from '@/state/notebooks/notebooks-state' -import { type ResponseError } from '@/types' interface QueryCellProps { cell: Snapshot } -/** - * [Joshen] Aiming to keep PRs small so the following are deliberating missing for now: - * - Auto limit logic - * - Database selection logic - * - Data display logic - * - * QueryCell atm minimally supports running queries and rendering results - */ +type QueryCellUpdate = { sql: string } | { title: string } | { display: QueryDisplay } +/** Notebook adapter around the shared QueryEditor. */ export const QueryCell = ({ cell }: QueryCellProps) => { const snap = useNotebooksStateSnapshot() const currentNotebook = useCurrentNotebook() - const { data: project } = useSelectedProjectQuery() const cells = currentNotebook?.notebook.content?.cells ?? [] - const { title = 'Untitled snippet', row_limit, view } = cell - - const [showQuery, setShowQuery] = useState(true) - const [value, setValue] = useState(cell.unchecked_sql) + const [sql, setSql] = useState(cell.unchecked_sql) const [result, setResult] = useState() - const columns = Object.keys(result?.rows?.[0] ?? {}) - const valueRef = useLatest(value) - - const { mutateAsync: executeQuery, isPending: isExecuting } = useExecuteSqlMutation({ - onSuccess: (data) => - setResult({ - rows: data.result, - error: undefined, - autoLimit: undefined, - }), - onError: (error) => - setResult({ - rows: undefined, - error: error as unknown as ResponseError, - autoLimit: undefined, - }), - }) - - const onRunQuery = async () => { - if (!project) return console.error('Project is required') - - handleUpdateCell({ sql: value }) - - executeQuery({ - projectRef: project?.ref, - connectionString: project?.connectionString, - sql: acceptUntrustedSql(untrustedSql(value)), - }) + const title = cell.title ?? 'Untitled snippet' + const display: QueryDisplay = { + view: cell.view ?? 'table', + chart: cell.chart ? { ...cell.chart, y_columns: [...cell.chart.y_columns] } : undefined, } - const handleUpdateCell = (payload: { sql: string } | { title: string }) => { + const handleUpdateCell = (payload: QueryCellUpdate) => { const notebookId = currentNotebook?.notebook.id if (!notebookId) return - const nextCells = cells.map((c) => { - if (c.id !== cell.id || c._tag !== 'database_cell') { - return c - } + const nextCells = cells.map((candidate) => { + if (candidate.id !== cell.id || candidate._tag !== 'database_cell') return candidate if ('sql' in payload) { - return { ...c, unchecked_sql: untrustedSql(payload.sql) } + return { ...candidate, unchecked_sql: untrustedSql(payload.sql) } } - const trimmedTitle = payload.title.trim() - return trimmedTitle ? { ...c, title: trimmedTitle } : c + if ('title' in payload) { + const nextTitle = payload.title.trim() + return nextTitle ? { ...candidate, title: nextTitle } : candidate + } + + return { + ...candidate, + view: payload.display.view, + chart: payload.display.chart, + } }) snap.updateCells({ id: notebookId, cells: nextCells }) } - const handleUpdateCellRef = useLatest(handleUpdateCell) - return ( - - - - - - handleUpdateCell({ title: newTitle })}> - {title} - - - - : } - tooltip={showQuery ? 'Hide query' : 'Show query'} - onClick={() => setShowQuery((prev) => !prev)} - /> - } - tooltip="Run query" - onClick={onRunQuery} - /> - - - - {showQuery && ( - - setValue(v ?? '')} - className="h-32" - actions={{ runQuery: { enabled: true, callback: onRunQuery } }} - onMount={(editor) => { - editor.onDidBlurEditorWidget(() => - handleUpdateCellRef.current({ sql: valueRef.current }) - ) - }} - /> - - )} - - - {view === 'table' && } - {view === 'chart' && } - - - -

{(result?.rows ?? []).length.toLocaleString()} rows

-

·

-

Limit {row_limit} rows

- - + handleUpdateCell({ title })} + onSqlChange={setSql} + onSqlCommit={(sql) => handleUpdateCell({ sql })} + onResultChange={setResult} + onDisplayChange={(display) => handleUpdateCell({ display })} + /> ) } diff --git a/apps/studio/components/interfaces/Explorer/QueryEditor.tsx b/apps/studio/components/interfaces/Explorer/QueryEditor.tsx new file mode 100644 index 00000000000..dc85d731fb8 --- /dev/null +++ b/apps/studio/components/interfaces/Explorer/QueryEditor.tsx @@ -0,0 +1,172 @@ +import { acceptUntrustedSql, untrustedSql } from '@supabase/pg-meta' +import { CodeSquare, Eye, EyeOff, Play } from 'lucide-react' +import { useState, type ReactNode } from 'react' +import { cn } from 'ui' + +import { + ExplorerQuery, + ExplorerQueryEditor, + ExplorerQueryFooter, + ExplorerQueryResults, + ExplorerQueryViewport, +} from './ExplorerQuery' +import { + ExplorerToolbar, + ExplorerToolbarAction, + ExplorerToolbarActions, + ExplorerToolbarIcon, + ExplorerToolbarTitle, +} from './ExplorerToolbar' +import { DisplaySettingsButton } from './QueryCell/DisplaySettingsButton' +import { QueryResultChart } from './QueryCell/QueryResultChart' +import { QueryResultTable } from './QueryResultTable' +import { type QueryDisplay, type QueryResult } from './types' +import { applyAutoLimit } from '@/components/interfaces/SQLEditor/SQLEditor.utils' +import { CodeEditor } from '@/components/ui/CodeEditor/CodeEditor' +import { useExecuteSqlMutation } from '@/data/sql/execute-sql-mutation' +import { useLatest } from '@/hooks/misc/useLatest' +import { useSelectedProjectQuery } from '@/hooks/misc/useSelectedProject' + +export type QueryEditorProps = { + id: string + variant: 'embedded' | 'viewport' + title: string + sql: string + result?: QueryResult + rowLimit: number + display?: QueryDisplay + toolbarActions?: ReactNode + onTitleChange: (title: string) => void + onSqlChange: (sql: string) => void + onSqlCommit?: (sql: string) => void + onResultChange: (result: QueryResult) => void + onDisplayChange?: (display: QueryDisplay) => void +} + +/** + * Shared query editor used by query tabs, notebook cells, and other Explorer surfaces. + * The consuming surface owns persistence and surrounding chrome; this component owns + * query-level UI and execution behavior. + */ +export const QueryEditor = ({ + id, + variant, + title, + sql, + result, + rowLimit, + display, + toolbarActions, + onTitleChange, + onSqlChange, + onSqlCommit, + onResultChange, + onDisplayChange, +}: QueryEditorProps) => { + const sqlRef = useLatest(sql) + const onSqlCommitRef = useLatest(onSqlCommit) + + const { data: project, isPending: isLoadingProject } = useSelectedProjectQuery() + + const view = display?.view ?? 'table' + const columns = Object.keys(result?.rows?.[0] ?? {}) + + const [showQuery, setShowQuery] = useState(true) + + const { mutate: executeSql, isPending: isExecuting } = useExecuteSqlMutation({ + onSuccess: (data) => onResultChange({ rows: data.result }), + onError: (error) => onResultChange({ error }), + }) + + const handleRunQuery = (sqlToRun: string = sql) => { + if (!project || isLoadingProject || isExecuting || sqlToRun.trim().length === 0) return + + onSqlCommit?.(sql) + + const safeSql = acceptUntrustedSql(untrustedSql(sqlToRun)) + const limitedSql = applyAutoLimit(safeSql, rowLimit) + + executeSql({ + projectRef: project.ref, + connectionString: project.connectionString, + sql: limitedSql.sql, + autoLimit: limitedSql.appendAutoLimit ? rowLimit : undefined, + contextualInvalidation: true, + isStatementTimeoutDisabled: true, + }) + } + + const Shell = variant === 'viewport' ? ExplorerQueryViewport : ExplorerQuery + + return ( + + + + + + {title} + + {toolbarActions} + {display && onDisplayChange && ( + + )} + : } + tooltip={showQuery ? 'Hide query' : 'Show query'} + onClick={() => setShowQuery((value) => !value)} + /> + } + tooltip="Run query" + disabled={isLoadingProject || isExecuting || sql.trim().length === 0} + onClick={() => handleRunQuery()} + > + Run + + + + + {showQuery && ( + + onSqlChange(value ?? '')} + onMount={(editor) => { + editor.onDidBlurEditorWidget(() => onSqlCommitRef.current?.(sqlRef.current)) + }} + /> + + )} + + + {view === 'table' && } + {view === 'chart' && } + + + +

{(result?.rows ?? []).length.toLocaleString()} rows

+

·

+

Limit {rowLimit} rows

+ + + ) +} diff --git a/apps/studio/components/interfaces/Explorer/types.ts b/apps/studio/components/interfaces/Explorer/types.ts index 91c1087fd09..c3b07cad34e 100644 --- a/apps/studio/components/interfaces/Explorer/types.ts +++ b/apps/studio/components/interfaces/Explorer/types.ts @@ -1,7 +1,19 @@ -import { type ResponseError } from '@/types' - export type QueryResult = { - rows?: Record[] - error?: ResponseError + rows?: readonly Record[] + error?: { message: string; formattedError?: string } autoLimit?: number } + +export type QueryChartConfig = { + type: 'bar' | 'line' + x_column: string + y_columns: string[] + cumulative: boolean + scale: 'linear' | 'log' + show_labels: boolean +} + +export type QueryDisplay = { + view: 'table' | 'chart' + chart?: QueryChartConfig +} diff --git a/apps/studio/components/ui/QueryBlock/QueryBlock.utils.ts b/apps/studio/components/ui/QueryBlock/QueryBlock.utils.ts index 8a18bde4688..19fec3bbd95 100644 --- a/apps/studio/components/ui/QueryBlock/QueryBlock.utils.ts +++ b/apps/studio/components/ui/QueryBlock/QueryBlock.utils.ts @@ -1,5 +1,7 @@ -export const checkHasNonPositiveValues = (data: Record[], key: string): boolean => - data.some((row) => (row[key] as number) <= 0) +export const checkHasNonPositiveValues = ( + data: readonly Record[], + key: string +): boolean => data.some((row) => (row[key] as number) <= 0) export const formatYAxisTick = (value: number): string => { if (Math.abs(value) >= 1_000_000) {