From d4ba44fce200cb21f0e3a2bb11e754e5fb0e0a6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Geid=C5=8D?= <60598000+geido@users.noreply.github.com> Date: Wed, 31 Dec 2025 15:18:59 +0100 Subject: [PATCH] fix: Query history view button in SqlLab (#36540) --- .../components/QueryTable/QueryTable.test.tsx | 97 ++++++++++++++- .../SqlLab/components/QueryTable/index.tsx | 110 +++++++++++++----- .../SqlLab/components/QueryTable/styles.ts | 7 ++ .../src/SqlLab/components/ResultSet/index.tsx | 50 ++++---- 4 files changed, 214 insertions(+), 50 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.tsx b/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.tsx index eeebc881eda..4d93638a292 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/QueryTable.test.tsx @@ -19,9 +19,10 @@ import { isValidElement } from 'react'; import thunk from 'redux-thunk'; import configureStore from 'redux-mock-store'; +import userEvent from '@testing-library/user-event'; import QueryTable from 'src/SqlLab/components/QueryTable'; import { runningQuery, successfulQuery, user } from 'src/SqlLab/fixtures'; -import { render, screen } from 'spec/helpers/testing-library'; +import { render, screen, waitFor } from 'spec/helpers/testing-library'; const mockedProps = { queries: [runningQuery, successfulQuery], @@ -29,6 +30,11 @@ const mockedProps = { latestQueryId: 'ryhMUZCGb', }; +const queryWithResults = { + ...successfulQuery, + resultsKey: 'test-results-key-123', +}; + // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks describe('QueryTable', () => { test('is valid', () => { @@ -92,4 +98,93 @@ describe('QueryTable', () => { ), ).toHaveLength(1); }); + + test('renders View button when query has resultsKey', () => { + const mockStore = configureStore([thunk]); + const propsWithResults = { + ...mockedProps, + columns: ['started', 'duration', 'rows', 'results'], + queries: [queryWithResults], + }; + render(, { + store: mockStore({ user, sqlLab: { queries: {} } }), + }); + + expect(screen.getByRole('button', { name: /view/i })).toBeInTheDocument(); + }); + + test('does not render View button when query has no resultsKey', () => { + const mockStore = configureStore([thunk]); + const queryWithoutResults = { + ...successfulQuery, + resultsKey: null, + }; + const propsWithoutResults = { + ...mockedProps, + columns: ['started', 'duration', 'rows', 'results'], + queries: [queryWithoutResults], + }; + render(, { + store: mockStore({ user, sqlLab: { queries: {} } }), + }); + + expect( + screen.queryByRole('button', { name: /view/i }), + ).not.toBeInTheDocument(); + }); + + test('clicking View button opens data preview modal', async () => { + const mockStore = configureStore([thunk]); + const propsWithResults = { + ...mockedProps, + columns: ['started', 'duration', 'rows', 'results'], + queries: [queryWithResults], + }; + render(, { + store: mockStore({ + user, + sqlLab: { + queries: { + [queryWithResults.id]: queryWithResults, + }, + }, + }), + }); + + const viewButton = screen.getByRole('button', { name: /view/i }); + await userEvent.click(viewButton); + + expect(await screen.findByText('Data preview')).toBeInTheDocument(); + }); + + test('modal closes when exiting', async () => { + const mockStore = configureStore([thunk]); + const propsWithResults = { + ...mockedProps, + columns: ['started', 'duration', 'rows', 'results'], + queries: [queryWithResults], + }; + render(, { + store: mockStore({ + user, + sqlLab: { + queries: { + [queryWithResults.id]: queryWithResults, + }, + }, + }), + }); + + const viewButton = screen.getByRole('button', { name: /view/i }); + await userEvent.click(viewButton); + + expect(await screen.findByText('Data preview')).toBeInTheDocument(); + + const closeButton = screen.getByRole('button', { name: /close/i }); + await userEvent.click(closeButton); + + await waitFor(() => { + expect(screen.queryByText('Data preview')).not.toBeInTheDocument(); + }); + }); }); diff --git a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx index 8e05d136d9c..344a0cb5135 100644 --- a/superset-frontend/src/SqlLab/components/QueryTable/index.tsx +++ b/superset-frontend/src/SqlLab/components/QueryTable/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { useMemo, ReactNode } from 'react'; +import { useMemo, ReactNode, useState, useRef } from 'react'; import { Card, Button, @@ -27,9 +27,9 @@ import { TableView, } from '@superset-ui/core/components'; import ProgressBar from '@superset-ui/core/components/ProgressBar'; -import { t, QueryResponse } from '@superset-ui/core'; +import { t, QueryResponse, QueryState } from '@superset-ui/core'; import { useTheme } from '@apache-superset/core/ui'; -import { useDispatch, useSelector } from 'react-redux'; +import { shallowEqual, useDispatch, useSelector } from 'react-redux'; import { queryEditorSetSql, @@ -37,6 +37,7 @@ import { fetchQueryResults, clearQueryResults, removeQuery, + startQuery, } from 'src/SqlLab/actions/sqlLab'; import { fDuration, extendedDayjs } from '@superset-ui/core/utils/dates'; import { SqlLabRootState } from 'src/SqlLab/types'; @@ -44,7 +45,7 @@ import { UserWithPermissionsAndRoles as User } from 'src/types/bootstrapTypes'; import { makeUrl } from 'src/utils/pathUtils'; import ResultSet from '../ResultSet'; import HighlightedSql from '../HighlightedSql'; -import { StaticPosition, StyledTooltip } from './styles'; +import { StaticPosition, StyledTooltip, ModalResultSetWrapper } from './styles'; interface QueryTableQuery extends Omit< QueryResponse, @@ -82,6 +83,15 @@ const QueryTable = ({ }: QueryTableProps) => { const theme = useTheme(); const dispatch = useDispatch(); + const [selectedQuery, setSelectedQuery] = useState( + null, + ); + const selectedQueryRef = useRef(null); + const modalRef = useRef<{ + close: () => void; + open: (e: React.MouseEvent) => void; + showModal: boolean; + } | null>(null); const QUERY_HISTORY_TABLE_HEADERS_LOCALIZED = { state: t('State'), @@ -116,6 +126,14 @@ const QueryTable = ({ ); const user = useSelector(state => state.user); + const reduxQueries = useSelector< + SqlLabRootState, + Record + >(state => state.sqlLab?.queries ?? {}, shallowEqual); + + const openAsyncResults = (query: QueryResponse, displayLimit: number) => { + dispatch(fetchQueryResults(query, displayLimit)); + }; const data = useMemo(() => { const restoreSql = (query: QueryResponse) => { @@ -128,10 +146,6 @@ const QueryTable = ({ dispatch(cloneQueryToNewTab(query, true)); }; - const openAsyncResults = (query: QueryResponse, displayLimit: number) => { - dispatch(fetchQueryResults(query, displayLimit)); - }; - const statusAttributes = { success: { config: { @@ -289,26 +303,17 @@ const QueryTable = ({ ); if (q.resultsKey) { q.results = ( - - {t('View')} - - } - modalTitle={t('Data preview')} - beforeOpen={() => openAsyncResults(query, displayLimit)} - onExit={() => dispatch(clearQueryResults(query))} - modalBody={ - - } - responsive - /> + ); } else { q.results = <>; @@ -365,6 +370,55 @@ const QueryTable = ({ return (
+ { + const query = selectedQueryRef.current; + if (query) { + const existingQuery = reduxQueries[query.id]; + if (!existingQuery?.sql && query.sql) { + dispatch(startQuery({ ...query, sql: query.sql }, false)); + } + openAsyncResults(query, displayLimit); + } + }} + onExit={() => { + const query = selectedQueryRef.current; + if (query) { + dispatch(clearQueryResults(query)); + selectedQueryRef.current = null; + setSelectedQuery(null); + } + }} + modalBody={ + selectedQuery ? ( + + {(() => { + const height = + reduxQueries[selectedQuery.id]?.state === + QueryState.Success && + reduxQueries[selectedQuery.id]?.results + ? Math.floor(window.innerHeight * 0.5) + : undefined; + return ( + + ); + })()} + + ) : null + } + responsive + /> ; displayLimit: number; + height?: number; queryId: string; search?: boolean; showSql?: boolean; showSqlInline?: boolean; visualize?: boolean; defaultQueryLimit: number; + useFixedHeight?: boolean; } const ResultContainer = styled.div` @@ -177,12 +179,14 @@ const ResultSet = ({ csv = true, database = {}, displayLimit, + height, queryId, search = true, showSql = false, showSqlInline = false, visualize = true, defaultQueryLimit, + useFixedHeight = false, }: ResultSetProps) => { const user = useSelector(({ user }: SqlLabRootState) => user, shallowEqual); const streamingThreshold = useSelector( @@ -711,6 +715,16 @@ const ResultSet = ({ LocalStorageKeys.SqllabIsRenderHtmlEnabled, true, ); + + const tableProps = { + data, + queryId: query.id, + orderedColumnKeys: results.columns.map(col => col.column_name), + filterText: searchText, + expandedColumns, + allowHTML, + }; + return ( <> @@ -753,27 +767,21 @@ const ResultSet = ({ {sql} )} -
- - {({ height }) => ( - col.column_name, - )} - height={height} - filterText={searchText} - expandedColumns={expandedColumns} - allowHTML={allowHTML} - /> - )} - -
+ {useFixedHeight && height !== undefined ? ( + + ) : ( +
+ + {({ height: autoHeight }) => ( + + )} + +
+ )}