fix: Query history view button in SqlLab (#36540)

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(<QueryTable {...propsWithResults} />, {
+      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(<QueryTable {...propsWithoutResults} />, {
+      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(<QueryTable {...propsWithResults} />, {
+      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(<QueryTable {...propsWithResults} />, {
+      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();
+    });
+  });
 });

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<QueryResponse | null>(
+    null,
+  );
+  const selectedQueryRef = useRef<QueryResponse | null>(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<SqlLabRootState, User>(state => state.user);
+  const reduxQueries = useSelector<
+    SqlLabRootState,
+    Record<string, QueryResponse>
+  >(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 = (
-            <ModalTrigger
-              className="ResultsModal"
-              triggerNode={
-                <Button buttonSize="xsmall" buttonStyle="secondary">
-                  {t('View')}
-                </Button>
-              }
-              modalTitle={t('Data preview')}
-              beforeOpen={() => openAsyncResults(query, displayLimit)}
-              onExit={() => dispatch(clearQueryResults(query))}
-              modalBody={
-                <ResultSet
-                  showSql
-                  queryId={query.id}
-                  displayLimit={displayLimit}
-                  defaultQueryLimit={1000}
-                />
-              }
-              responsive
-            />
+            <Button
+              buttonSize="xsmall"
+              buttonStyle="secondary"
+              onClick={(e: React.MouseEvent) => {
+                selectedQueryRef.current = query;
+                setSelectedQuery(query);
+                modalRef.current?.open(e);
+              }}
+            >
+              {t('View')}
+            </Button>
           );
         } else {
           q.results = <></>;
@@ -365,6 +370,55 @@ const QueryTable = ({
 
   return (
     <div className="QueryTable">
+      <ModalTrigger
+        ref={modalRef}
+        triggerNode={null}
+        className="ResultsModal"
+        modalTitle={t('Data preview')}
+        beforeOpen={() => {
+          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 ? (
+            <ModalResultSetWrapper>
+              {(() => {
+                const height =
+                  reduxQueries[selectedQuery.id]?.state ===
+                    QueryState.Success &&
+                  reduxQueries[selectedQuery.id]?.results
+                    ? Math.floor(window.innerHeight * 0.5)
+                    : undefined;
+                return (
+                  <ResultSet
+                    showSql
+                    queryId={selectedQuery.id}
+                    displayLimit={displayLimit}
+                    defaultQueryLimit={1000}
+                    useFixedHeight
+                    height={height}
+                  />
+                );
+              })()}
+            </ModalResultSetWrapper>
+          ) : null
+        }
+        responsive
+      />
       <TableView
         columns={columnsOfTable}
         data={data}

superset-frontend/src/SqlLab/components/QueryTable/styles.ts

@@ -39,3 +39,10 @@ export const StyledTooltip = styled(IconTooltip)`
     }
   }
 `;
+
+export const ModalResultSetWrapper = styled.div`
+  display: flex;
+  flex-direction: column;
+  overflow: hidden;
+  max-height: 50vh;
+`;

superset-frontend/src/SqlLab/components/ResultSet/index.tsx

@@ -104,12 +104,14 @@ export interface ResultSetProps {
   csv?: boolean;
   database?: Record<string, any>;
   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 (
         <>
           <ResultContainer>
@@ -753,27 +767,21 @@ const ResultSet = ({
                 {sql}
               </>
             )}
-            <div
-              css={css`
-                flex: 1 1 auto;
-              `}
-            >
-              <AutoSizer disableWidth>
-                {({ height }) => (
-                  <ResultTable
-                    data={data}
-                    queryId={query.id}
-                    orderedColumnKeys={results.columns.map(
-                      col => col.column_name,
-                    )}
-                    height={height}
-                    filterText={searchText}
-                    expandedColumns={expandedColumns}
-                    allowHTML={allowHTML}
-                  />
-                )}
-              </AutoSizer>
-            </div>
+            {useFixedHeight && height !== undefined ? (
+              <ResultTable {...tableProps} height={height} />
+            ) : (
+              <div
+                css={css`
+                  flex: 1 1 auto;
+                `}
+              >
+                <AutoSizer disableWidth>
+                  {({ height: autoHeight }) => (
+                    <ResultTable {...tableProps} height={autoHeight} />
+                  )}
+                </AutoSizer>
+              </div>
+            )}
           </ResultContainer>
           <StreamingExportModal
             visible={showStreamingModal}