feat(sqllab): improve table metadata UI (apache#32051)

superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts

@@ -144,6 +144,13 @@ export interface SQLResultTableExtensionProps {
   allowHTML?: boolean;
 }
 
+export interface SQLTablePreviewExtensionProps {
+  dbId: number;
+  catalog?: string;
+  schema: string;
+  tableName: string;
+}
+
 /**
  * Interface for extensions to Slice Header
  */
@@ -229,4 +236,8 @@ export type Extensions = Partial<{
   'sqleditor.extension.customAutocomplete': (
     args: CustomAutoCompleteArgs,
   ) => CustomAutocomplete[] | undefined;
+  'sqleditor.extension.tablePreview': [
+    string,
+    ComponentType<SQLTablePreviewExtensionProps>,
+  ][];
 }>;

superset-frontend/src/SqlLab/actions/sqlLab.js

@@ -237,15 +237,15 @@ export function clearInactiveQueries(interval) {
   return { type: CLEAR_INACTIVE_QUERIES, interval };
 }
 
-export function startQuery(query) {
+export function startQuery(query, runPreviewOnly) {
   Object.assign(query, {
     id: query.id ? query.id : nanoid(11),
     progress: 0,
     startDttm: now(),
     state: query.runAsync ? 'pending' : 'running',
     cached: false,
   });
-  return { type: START_QUERY, query };
+  return { type: START_QUERY, query, runPreviewOnly };
 }
 
 export function querySuccess(query, results) {
@@ -327,9 +327,9 @@ export function fetchQueryResults(query, displayLimit, timeoutInMs) {
   };
 }
 
-export function runQuery(query) {
+export function runQuery(query, runPreviewOnly) {
   return function (dispatch) {
-    dispatch(startQuery(query));
+    dispatch(startQuery(query, runPreviewOnly));
     const postPayload = {
       client_id: query.id,
       database_id: query.dbId,
@@ -947,29 +947,25 @@ export function mergeTable(table, query, prepend) {
 
 export function addTable(queryEditor, tableName, catalogName, schemaName) {
   return function (dispatch, getState) {
-    const query = getUpToDateQuery(getState(), queryEditor, queryEditor.id);
+    const { dbId } = getUpToDateQuery(getState(), queryEditor, queryEditor.id);
     const table = {
-      dbId: query.dbId,
-      queryEditorId: query.id,
+      dbId,
+      queryEditorId: queryEditor.id,
       catalog: catalogName,
       schema: schemaName,
       name: tableName,
     };
     dispatch(
-      mergeTable(
-        {
-          ...table,
-          id: nanoid(11),
-          expanded: true,
-        },
-        null,
-        true,
-      ),
+      mergeTable({
+        ...table,
+        id: nanoid(11),
+        expanded: true,
+      }),
     );
   };
 }
 
-export function runTablePreviewQuery(newTable) {
+export function runTablePreviewQuery(newTable, runPreviewOnly) {
   return function (dispatch, getState) {
     const {
       sqlLab: { databases },
@@ -979,7 +975,7 @@ export function runTablePreviewQuery(newTable) {
 
     if (database && !database.disable_data_preview) {
       const dataPreviewQuery = {
-        id: nanoid(11),
+        id: newTable.previewQueryId ?? nanoid(11),
         dbId,
         catalog,
         schema,
@@ -991,6 +987,9 @@ export function runTablePreviewQuery(newTable) {
         ctas: false,
         isDataPreview: true,
       };
+      if (runPreviewOnly) {
+        return dispatch(runQuery(dataPreviewQuery, runPreviewOnly));
+      }
       return Promise.all([
         dispatch(
           mergeTable(
@@ -1024,17 +1023,14 @@ export function syncTable(table, tableMetadata) {
 
     return sync
       .then(({ json: resultJson }) => {
-        const newTable = { ...table, id: resultJson.id };
+        const newTable = { ...table, id: `${resultJson.id}` };
         dispatch(
           mergeTable({
             ...newTable,
             expanded: true,
             initialized: true,
           }),
         );
-        if (!table.dataPreviewQueryId) {
-          dispatch(runTablePreviewQuery({ ...tableMetadata, ...newTable }));
-        }
       })
       .catch(() =>
         dispatch(

superset-frontend/src/SqlLab/actions/sqlLab.test.js

@@ -1031,30 +1031,38 @@ describe('async actions', () => {
     });
 
     describe('runTablePreviewQuery', () => {
-      it('updates and runs data preview query when configured', () => {
-        expect.assertions(3);
+      const results = {
+        data: mockBigNumber,
+        query: { sqlEditorId: 'null', dbId: 1 },
+        query_id: 'efgh',
+      };
+      const tableName = 'table';
+      const catalogName = null;
+      const schemaName = 'schema';
+      const store = mockStore({
+        ...initialState,
+        sqlLab: {
+          ...initialState.sqlLab,
+          databases: {
+            1: { disable_data_preview: false },
+          },
+        },
+      });
 
-        const results = {
-          data: mockBigNumber,
-          query: { sqlEditorId: 'null', dbId: 1 },
-          query_id: 'efgh',
-        };
+      beforeEach(() => {
         fetchMock.post(runQueryEndpoint, JSON.stringify(results), {
           overwriteRoutes: true,
         });
+      });
+
+      afterEach(() => {
+        store.clearActions();
+        fetchMock.resetHistory();
+      });
+
+      it('updates and runs data preview query when configured', () => {
+        expect.assertions(3);
 
-        const tableName = 'table';
-        const catalogName = null;
-        const schemaName = 'schema';
-        const store = mockStore({
-          ...initialState,
-          sqlLab: {
-            ...initialState.sqlLab,
-            databases: {
-              1: { disable_data_preview: false },
-            },
-          },
-        });
         const expectedActionTypes = [
           actions.MERGE_TABLE, // addTable (data preview)
           actions.START_QUERY, // runQuery (data preview)
@@ -1075,6 +1083,30 @@ describe('async actions', () => {
           expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0);
         });
       });
+
+      it('runs data preview query only', () => {
+        const expectedActionTypes = [
+          actions.START_QUERY, // runQuery (data preview)
+          actions.QUERY_SUCCESS, // querySuccess
+        ];
+        const request = actions.runTablePreviewQuery(
+          {
+            dbId: 1,
+            name: tableName,
+            catalog: catalogName,
+            schema: schemaName,
+          },
+          true,
+        );
+        return request(store.dispatch, store.getState).then(() => {
+          expect(store.getActions().map(a => a.type)).toEqual(
+            expectedActionTypes,
+          );
+          expect(fetchMock.calls(runQueryEndpoint)).toHaveLength(1);
+          // tab state is not updated, since the query is a data preview
+          expect(fetchMock.calls(updateTabStateEndpoint)).toHaveLength(0);
+        });
+      });
     });
 
     describe('expandTable', () => {

superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx

@@ -16,8 +16,6 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import configureStore from 'redux-mock-store';
-import thunk from 'redux-thunk';
 import reducerIndex from 'spec/helpers/reducerIndex';
 import { render, waitFor, createStore } from 'spec/helpers/testing-library';
 import { QueryEditor } from 'src/SqlLab/types';
@@ -34,9 +32,6 @@ import {
 } from 'src/SqlLab/actions/sqlLab';
 import fetchMock from 'fetch-mock';
 
-const middlewares = [thunk];
-const mockStore = configureStore(middlewares);
-
 fetchMock.get('glob:*/api/v1/database/*/function_names/', {
   function_names: [],
 });
@@ -79,7 +74,8 @@ describe('AceEditorWrapper', () => {
   });
 
   it('renders ace editor including sql value', async () => {
-    const { getByTestId } = setup(defaultQueryEditor, mockStore(initialState));
+    const store = createStore(initialState, reducerIndex);
+    const { getByTestId } = setup(defaultQueryEditor, store);
     await waitFor(() => expect(getByTestId('react-ace')).toBeInTheDocument());
 
     expect(getByTestId('react-ace')).toHaveTextContent(
@@ -89,9 +85,8 @@ describe('AceEditorWrapper', () => {
 
   it('renders current sql for unrelated unsaved changes', () => {
     const expectedSql = 'SELECT updated_column\nFROM updated_table\nWHERE';
-    const { getByTestId } = setup(
-      defaultQueryEditor,
-      mockStore({
+    const store = createStore(
+      {
         ...initialState,
         sqlLab: {
           ...initialState.sqlLab,
@@ -100,8 +95,10 @@ describe('AceEditorWrapper', () => {
             sql: expectedSql,
           },
         },
-      }),
+      },
+      reducerIndex,
     );
+    const { getByTestId } = setup(defaultQueryEditor, store);
 
     expect(getByTestId('react-ace')).not.toHaveTextContent(
       JSON.stringify({ value: expectedSql }).slice(1, -1),
@@ -122,7 +119,7 @@ describe('AceEditorWrapper', () => {
       queryEditorSetCursorPosition(defaultQueryEditor, updatedCursorPosition),
     );
     expect(FullSQLEditor).toHaveBeenCalledTimes(renderCount);
-    store.dispatch(queryEditorSetDb(defaultQueryEditor, 1));
+    store.dispatch(queryEditorSetDb(defaultQueryEditor, 2));
     expect(FullSQLEditor).toHaveBeenCalledTimes(renderCount + 1);
   });
 });

superset-frontend/src/SqlLab/components/AceEditorWrapper/useKeywords.test.ts

@@ -202,6 +202,7 @@ test('returns column keywords among selected tables', async () => {
             {
               name: expectColumn,
               type: 'VARCHAR',
+              longType: 'VARCHAR',
             },
           ],
         },
@@ -223,6 +224,7 @@ test('returns column keywords among selected tables', async () => {
             {
               name: unexpectedColumn,
               type: 'VARCHAR',
+              longType: 'VARCHAR',
             },
           ],
         },

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

@@ -47,7 +47,7 @@ const SqlLabStyles = styled.div`
       left: 0;
       padding: 0 ${theme.gridUnit * 2}px;
 
-      pre {
+      pre:not(.code) {
         padding: 0 !important;
         margin: 0;
         border: none;

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

@@ -28,21 +28,25 @@ interface ShowSQLProps {
   sql: string;
   title: string;
   tooltipText: string;
+  triggerNode?: React.ReactNode;
 }
 
 export default function ShowSQL({
   tooltipText,
   title,
   sql: sqlString,
+  triggerNode,
 }: ShowSQLProps) {
   return (
     <ModalTrigger
       modalTitle={title}
       triggerNode={
-        <IconTooltip
-          className="fa fa-eye pull-left m-l-2"
-          tooltip={tooltipText}
-        />
+        triggerNode || (
+          <IconTooltip
+            className="fa fa-eye pull-left m-l-2"
+            tooltip={tooltipText}
+          />
+        )
       }
       modalBody={
         <div>

superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.tsx

@@ -135,7 +135,7 @@ test('should render empty result state when latestQuery is empty', () => {
   expect(resultPanel).toHaveTextContent('Run a query to display results');
 });
 
-test('should render tabs for table preview queries', () => {
+test('should render tabs for table metadata view', () => {
   const { getAllByRole } = render(<SouthPane {...mockedProps} />, {
     useRedux: true,
     initialState: mockState,
@@ -145,7 +145,7 @@ test('should render tabs for table preview queries', () => {
   expect(tabs).toHaveLength(mockState.sqlLab.tables.length + 2);
   expect(tabs[0]).toHaveTextContent('Results');
   expect(tabs[1]).toHaveTextContent('Query history');
-  mockState.sqlLab.tables.forEach(({ name }, index) => {
-    expect(tabs[index + 2]).toHaveTextContent(`Preview: \`${name}\``);
+  mockState.sqlLab.tables.forEach(({ name, schema }, index) => {
+    expect(tabs[index + 2]).toHaveTextContent(`${schema}.${name}`);
   });
 });