diff --git a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts index 3f0a55929..0e7e0c978 100644 --- a/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/ui-overrides/types.ts @@ -118,6 +118,15 @@ export interface SQLFormExtensionProps { startQuery: (ctasArg?: any, ctas_method?: any) => void; } +export interface SQLResultTableExtentionProps { + queryId: string; + orderedColumnKeys: string[]; + data: Record[]; + height: number; + filterText?: string; + expandedColumns?: string[]; +} + export type Extensions = Partial<{ 'alertsreports.header.icon': React.ComponentType; 'embedded.documentation.configuration_details': React.ComponentType; @@ -137,4 +146,5 @@ export type Extensions = Partial<{ 'database.delete.related': React.ComponentType; 'dataset.delete.related': React.ComponentType; 'sqleditor.extension.form': React.ComponentType; + 'sqleditor.extension.resultTable': React.ComponentType; }>; diff --git a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx index e16fc569e..35eac7804 100644 --- a/superset-frontend/src/SqlLab/components/ResultSet/index.tsx +++ b/superset-frontend/src/SqlLab/components/ResultSet/index.tsx @@ -32,6 +32,7 @@ import { usePrevious, css, getNumberFormatter, + getExtensionsRegistry, } from '@superset-ui/core'; import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; import { @@ -135,6 +136,8 @@ const ResultSetButtons = styled.div` const ROWS_CHIP_WIDTH = 100; const GAP = 8; +const extensionsRegistry = getExtensionsRegistry(); + const ResultSet = ({ cache = false, csv = true, @@ -149,6 +152,9 @@ const ResultSet = ({ user, defaultQueryLimit, }: ResultSetProps) => { + const ResultTable = + extensionsRegistry.get('sqleditor.extension.resultTable') ?? + FilterableTable; const theme = useTheme(); const [searchText, setSearchText] = useState(''); const [cachedData, setCachedData] = useState[]>([]); @@ -578,8 +584,9 @@ const ResultSet = ({ {sql} )} - col.column_name)} height={rowsHeight} filterText={searchText} diff --git a/superset-frontend/src/components/FilterableTable/FilterableTable.test.tsx b/superset-frontend/src/components/FilterableTable/FilterableTable.test.tsx index aebf2c44b..3cc04f8c2 100644 --- a/superset-frontend/src/components/FilterableTable/FilterableTable.test.tsx +++ b/superset-frontend/src/components/FilterableTable/FilterableTable.test.tsx @@ -17,9 +17,7 @@ * under the License. */ import React from 'react'; -import FilterableTable, { - convertBigIntStrToNumber, -} from 'src/components/FilterableTable'; +import FilterableTable from 'src/components/FilterableTable'; import { render, screen, within } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; @@ -383,19 +381,3 @@ describe('FilterableTable sorting - RTL', () => { ); }); }); - -test('renders bigInt value in a number format', () => { - expect(convertBigIntStrToNumber('123')).toBe('123'); - expect(convertBigIntStrToNumber('some string value')).toBe( - 'some string value', - ); - expect(convertBigIntStrToNumber('{ a: 123 }')).toBe('{ a: 123 }'); - expect(convertBigIntStrToNumber('"Not a Number"')).toBe('"Not a Number"'); - // trim quotes for bigint string format - expect(convertBigIntStrToNumber('"-12345678901234567890"')).toBe( - '-12345678901234567890', - ); - expect(convertBigIntStrToNumber('"12345678901234567890"')).toBe( - '12345678901234567890', - ); -}); diff --git a/superset-frontend/src/components/FilterableTable/index.tsx b/superset-frontend/src/components/FilterableTable/index.tsx index 9861d0af5..d89a65ad3 100644 --- a/superset-frontend/src/components/FilterableTable/index.tsx +++ b/superset-frontend/src/components/FilterableTable/index.tsx @@ -18,55 +18,12 @@ */ import JSONbig from 'json-bigint'; import React, { useEffect, useRef, useState, useMemo } from 'react'; -import { JSONTree } from 'react-json-tree'; -import { - getMultipleTextDimensions, - t, - safeHtmlSpan, - styled, -} from '@superset-ui/core'; +import { getMultipleTextDimensions, styled } from '@superset-ui/core'; import { useDebounceValue } from 'src/hooks/useDebounceValue'; -import { useJsonTreeTheme } from 'src/hooks/useJsonTreeTheme'; -import Button from '../Button'; -import CopyToClipboard from '../CopyToClipboard'; -import ModalTrigger from '../ModalTrigger'; +import { useCellContentParser } from './useCellContentParser'; +import { renderResultCell } from './utils'; import { Table, TableSize } from '../Table'; -function safeJsonObjectParse( - data: unknown, -): null | unknown[] | Record { - // First perform a cheap proxy to avoid calling JSON.parse on data that is clearly not a - // JSON object or array - if ( - typeof data !== 'string' || - ['{', '['].indexOf(data.substring(0, 1)) === -1 - ) { - return null; - } - - // We know `data` is a string starting with '{' or '[', so try to parse it as a valid object - try { - const jsonData = JSONbig({ storeAsString: true }).parse(data); - if (jsonData && typeof jsonData === 'object') { - return jsonData; - } - return null; - } catch (_) { - return null; - } -} - -export function convertBigIntStrToNumber(value: string | number) { - if (typeof value === 'string' && /^"-?\d+"$/.test(value)) { - return value.substring(1, value.length - 1); - } - return value; -} - -function renderBigIntStrToNumber(value: string | number) { - return <>{convertBigIntStrToNumber(value)}; -} - const SCROLL_BAR_HEIGHT = 15; // This regex handles all possible number formats in javascript, including ints, floats, // exponential notation, NaN, and Infinity. @@ -147,43 +104,10 @@ const FilterableTable = ({ const [fitted, setFitted] = useState(false); const [list] = useState(() => formatTableData(data)); - // columns that have complex type and were expanded into sub columns - const complexColumns = useMemo>( - () => - orderedColumnKeys.reduce( - (obj, key) => ({ - ...obj, - [key]: expandedColumns.some(name => name.startsWith(`${key}.`)), - }), - {}, - ), - [expandedColumns, orderedColumnKeys], - ); - - const getCellContent = ({ - cellData, - columnKey, - }: { - cellData: CellDataType; - columnKey: string; - }) => { - if (cellData === null) { - return 'NULL'; - } - const content = String(cellData); - const firstCharacter = content.substring(0, 1); - let truncated; - if (firstCharacter === '[') { - truncated = '[…]'; - } else if (firstCharacter === '{') { - truncated = '{…}'; - } else { - truncated = ''; - } - return complexColumns[columnKey] ? truncated : content; - }; - - const jsonTreeTheme = useJsonTreeTheme(); + const getCellContent = useCellContentParser({ + columnKeys: orderedColumnKeys, + expandedColumns, + }); const getWidthsForColumns = () => { const PADDING = 50; // accounts for cell padding and width of sorting icon @@ -259,29 +183,6 @@ const FilterableTable = ({ return values.some(v => v.includes(lowerCaseText)); }; - const renderJsonModal = ( - node: React.ReactNode, - jsonObject: Record | unknown[], - jsonString: CellDataType, - ) => ( - - } - modalFooter={ - - } - modalTitle={t('Cell content')} - triggerNode={node} - /> - ); - // Parse any numbers from strings so they'll sort correctly const parseNumberFromString = (value: string | number | null) => { if (typeof value === 'string') { @@ -321,21 +222,6 @@ const FilterableTable = ({ [list, keyword], ); - const renderTableCell = (cellData: CellDataType, columnKey: string) => { - const cellNode = getCellContent({ cellData, columnKey }); - if (cellData === null) { - return {cellNode}; - } - const jsonObject = safeJsonObjectParse(cellData); - if (jsonObject) { - return renderJsonModal(cellNode, jsonObject, cellData); - } - if (allowHTML && typeof cellData === 'string') { - return safeHtmlSpan(cellNode); - } - return cellNode; - }; - // exclude the height of the horizontal scroll bar from the height of the table // and the height of the table container if the content overflows const totalTableHeight = @@ -349,7 +235,13 @@ const FilterableTable = ({ dataIndex: key, width: widthsForColumnsByKey[key], sorter: (a: Datum, b: Datum) => sortResults(key, a, b), - render: (text: CellDataType) => renderTableCell(text, key), + render: (text: CellDataType) => + renderResultCell({ + cellData: text, + columnKey: key, + allowHTML, + getCellContent, + }), })); return ( diff --git a/superset-frontend/src/components/FilterableTable/useCellContentParser.test.ts b/superset-frontend/src/components/FilterableTable/useCellContentParser.test.ts new file mode 100644 index 000000000..7851dd093 --- /dev/null +++ b/superset-frontend/src/components/FilterableTable/useCellContentParser.test.ts @@ -0,0 +1,58 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { renderHook } from '@testing-library/react-hooks'; +import { useCellContentParser } from './useCellContentParser'; + +test('should return NULL for null cell data', () => { + const { result } = renderHook(() => + useCellContentParser({ columnKeys: [], expandedColumns: [] }), + ); + const parser = result.current; + expect(parser({ cellData: null, columnKey: '' })).toBe('NULL'); +}); + +test('should return truncated string for complex columns', () => { + const { result } = renderHook(() => + useCellContentParser({ + columnKeys: ['a'], + expandedColumns: ['a.b'], + }), + ); + const parser = result.current; + + expect( + parser({ + cellData: 'this is a very long string', + columnKey: 'a.b', + }), + ).toBe('this is a very long string'); + expect( + parser({ + cellData: '["this is a very long string"]', + columnKey: 'a', + }), + ).toBe('[…]'); + expect( + parser({ + cellData: '{ "b": "this is a very long string" }', + columnKey: 'a', + }), + ).toBe('{…}'); +}); diff --git a/superset-frontend/src/components/FilterableTable/useCellContentParser.ts b/superset-frontend/src/components/FilterableTable/useCellContentParser.ts new file mode 100644 index 000000000..3724691c7 --- /dev/null +++ b/superset-frontend/src/components/FilterableTable/useCellContentParser.ts @@ -0,0 +1,69 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useCallback, useMemo } from 'react'; + +export type CellDataType = string | number | null; + +export const NULL_STRING = 'NULL'; + +type Params = { + columnKeys: string[]; + expandedColumns?: string[]; +}; + +export function useCellContentParser({ columnKeys, expandedColumns }: Params) { + // columns that have complex type and were expanded into sub columns + const complexColumns = useMemo>( + () => + columnKeys.reduce( + (obj, key) => ({ + ...obj, + [key]: expandedColumns?.some(name => name.startsWith(`${key}.`)), + }), + {}, + ), + [expandedColumns, columnKeys], + ); + + return useCallback( + ({ + cellData, + columnKey, + }: { + cellData: CellDataType; + columnKey: string; + }) => { + if (cellData === null) { + return NULL_STRING; + } + const content = String(cellData); + const firstCharacter = content.substring(0, 1); + let truncated; + if (firstCharacter === '[') { + truncated = '[…]'; + } else if (firstCharacter === '{') { + truncated = '{…}'; + } else { + truncated = ''; + } + return complexColumns[columnKey] ? truncated : content; + }, + [complexColumns], + ); +} diff --git a/superset-frontend/src/components/FilterableTable/utils.test.tsx b/superset-frontend/src/components/FilterableTable/utils.test.tsx new file mode 100644 index 000000000..9d81b2cdc --- /dev/null +++ b/superset-frontend/src/components/FilterableTable/utils.test.tsx @@ -0,0 +1,79 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import { render } from 'spec/helpers/testing-library'; +import { renderResultCell } from './utils'; + +jest.mock('src/components/JsonModal', () => ({ + ...jest.requireActual('src/components/JsonModal'), + default: () =>
, +})); + +const unexpectedGetCellContent = () => 'none'; + +test('should render NULL for null cell data', () => { + const { container } = render( + <> + {renderResultCell({ + cellData: null, + columnKey: 'column1', + getCellContent: unexpectedGetCellContent, + })} + , + ); + expect(container).toHaveTextContent('NULL'); +}); + +test('should render JsonModal for json cell data', () => { + const { getByTestId } = render( + <> + {renderResultCell({ + cellData: '{ "a": 1 }', + columnKey: 'a', + getCellContent: unexpectedGetCellContent, + })} + , + ); + expect(getByTestId('mock-json-modal')).toBeInTheDocument(); +}); + +test('should render cellData value for default cell data', () => { + const { container } = render( + <> + {renderResultCell({ + cellData: 'regular_text', + columnKey: 'a', + })} + , + ); + expect(container).toHaveTextContent('regular_text'); +}); + +test('should transform cell data by getCellContent for the regular text', () => { + const { container } = render( + <> + {renderResultCell({ + cellData: 'regular_text', + columnKey: 'a', + getCellContent: ({ cellData, columnKey }) => `${cellData}:${columnKey}`, + })} + , + ); + expect(container).toHaveTextContent('regular_text:a'); +}); diff --git a/superset-frontend/src/components/FilterableTable/utils.tsx b/superset-frontend/src/components/FilterableTable/utils.tsx new file mode 100644 index 000000000..45bab99cc --- /dev/null +++ b/superset-frontend/src/components/FilterableTable/utils.tsx @@ -0,0 +1,59 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import JsonModal, { safeJsonObjectParse } from 'src/components/JsonModal'; +import { t, safeHtmlSpan } from '@superset-ui/core'; +import { NULL_STRING, CellDataType } from './useCellContentParser'; + +type CellParams = { + cellData: CellDataType; + columnKey: string; +}; + +type Params = CellParams & { + allowHTML?: boolean; + getCellContent?: (args: CellParams) => string; +}; + +export const renderResultCell = ({ + cellData, + getCellContent, + columnKey, + allowHTML = true, +}: Params) => { + const cellNode = + getCellContent?.({ cellData, columnKey }) ?? String(cellData); + if (cellData === null) { + return {NULL_STRING}; + } + const jsonObject = safeJsonObjectParse(cellData); + if (jsonObject) { + return ( + + ); + } + if (allowHTML && typeof cellData === 'string') { + return safeHtmlSpan(cellNode); + } + return cellNode; +}; diff --git a/superset-frontend/src/components/JsonModal/JsonModal.test.tsx b/superset-frontend/src/components/JsonModal/JsonModal.test.tsx new file mode 100644 index 000000000..98e8db463 --- /dev/null +++ b/superset-frontend/src/components/JsonModal/JsonModal.test.tsx @@ -0,0 +1,60 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import { fireEvent, render } from 'spec/helpers/testing-library'; +import JsonModal, { convertBigIntStrToNumber } from '.'; + +jest.mock('react-json-tree', () => ({ + JSONTree: () =>
, +})); + +test('renders JSON object in a tree view in a modal', () => { + const jsonData = { a: 1 }; + const jsonValue = JSON.stringify(jsonData); + const { getByText, getByTestId, queryByTestId } = render( + , + { + useRedux: true, + }, + ); + expect(queryByTestId('mock-json-tree')).not.toBeInTheDocument(); + const link = getByText(jsonValue); + fireEvent.click(link); + expect(getByTestId('mock-json-tree')).toBeInTheDocument(); +}); + +test('renders bigInt value in a number format', () => { + expect(convertBigIntStrToNumber('123')).toBe('123'); + expect(convertBigIntStrToNumber('some string value')).toBe( + 'some string value', + ); + expect(convertBigIntStrToNumber('{ a: 123 }')).toBe('{ a: 123 }'); + expect(convertBigIntStrToNumber('"Not a Number"')).toBe('"Not a Number"'); + // trim quotes for bigint string format + expect(convertBigIntStrToNumber('"-12345678901234567890"')).toBe( + '-12345678901234567890', + ); + expect(convertBigIntStrToNumber('"12345678901234567890"')).toBe( + '12345678901234567890', + ); +}); diff --git a/superset-frontend/src/components/JsonModal/index.tsx b/superset-frontend/src/components/JsonModal/index.tsx new file mode 100644 index 000000000..c0a541a42 --- /dev/null +++ b/superset-frontend/src/components/JsonModal/index.tsx @@ -0,0 +1,112 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import JSONbig from 'json-bigint'; +import React from 'react'; +import { JSONTree } from 'react-json-tree'; +import { useJsonTreeTheme } from 'src/hooks/useJsonTreeTheme'; +import Button from '../Button'; +import CopyToClipboard from '../CopyToClipboard'; +import ModalTrigger from '../ModalTrigger'; + +export function safeJsonObjectParse( + data: unknown, +): null | unknown[] | Record { + // First perform a cheap proxy to avoid calling JSON.parse on data that is clearly not a + // JSON object or array + if ( + typeof data !== 'string' || + ['{', '['].indexOf(data.substring(0, 1)) === -1 + ) { + return null; + } + + // We know `data` is a string starting with '{' or '[', so try to parse it as a valid object + try { + const jsonData = JSONbig({ storeAsString: true }).parse(data); + if (jsonData && typeof jsonData === 'object') { + return jsonData; + } + return null; + } catch (_) { + return null; + } +} + +export function convertBigIntStrToNumber(value: string | number) { + if (typeof value === 'string' && /^"-?\d+"$/.test(value)) { + return value.substring(1, value.length - 1); + } + return value; +} + +function renderBigIntStrToNumber(value: string | number) { + return <>{convertBigIntStrToNumber(value)}; +} + +type CellDataType = string | number | null; + +export interface Props { + modalTitle: string; + jsonObject: Record | unknown[]; + jsonValue: CellDataType; +} + +const JsonModal: React.FC = ({ modalTitle, jsonObject, jsonValue }) => { + const jsonTreeTheme = useJsonTreeTheme(); + + return ( + + } + modalFooter={ + + } + modalTitle={modalTitle} + triggerNode={<>{jsonValue}} + /> + ); +}; + +export default JsonModal;