You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by be...@apache.org on 2021/08/24 21:48:05 UTC
[superset] branch master updated: fix: copy to Clipboard order
(#16299)
This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 631ad02 fix: copy to Clipboard order (#16299)
631ad02 is described below
commit 631ad02a765e55f54c77b6cd0b8b6948cf013118
Author: AAfghahi <48...@users.noreply.github.com>
AuthorDate: Tue Aug 24 17:47:09 2021 -0400
fix: copy to Clipboard order (#16299)
* copy to Clipboard order
* centralized copyToClipboard
* fixed table order
* fixed tests
* added colnames to all viz types
* added colnames to all viz types
* added colnames to all viz types
---
.../src/SqlLab/components/ResultSet.tsx | 4 +-
.../src/components/TableView/TableView.tsx | 1 -
.../explore/components/DataTableControl/index.tsx | 45 +++++++++++++---------
.../DataTableControl/useTableColumns.test.ts | 9 +++--
.../DataTablesPane/DataTablesPane.test.tsx | 5 +++
.../explore/components/DataTablesPane/index.tsx | 24 ++++++++++--
.../src/explore/components/ExploreChartPanel.jsx | 4 +-
superset-frontend/src/utils/common.js | 15 +++++++-
superset-frontend/src/utils/common.test.jsx | 9 +++--
superset/viz.py | 6 ++-
10 files changed, 84 insertions(+), 38 deletions(-)
diff --git a/superset-frontend/src/SqlLab/components/ResultSet.tsx b/superset-frontend/src/SqlLab/components/ResultSet.tsx
index b5a6e4f..4f1c788 100644
--- a/superset-frontend/src/SqlLab/components/ResultSet.tsx
+++ b/superset-frontend/src/SqlLab/components/ResultSet.tsx
@@ -448,7 +448,7 @@ export default class ResultSet extends React.PureComponent<
if (this.props.cache && this.props.query.cached) {
({ data } = this.state);
}
-
+ const { columns } = this.props.query.results;
// Added compute logic to stop user from being able to Save & Explore
const {
saveDatasetRadioBtnState,
@@ -508,7 +508,7 @@ export default class ResultSet extends React.PureComponent<
)}
<CopyToClipboard
- text={prepareCopyToClipboardTabularData(data)}
+ text={prepareCopyToClipboardTabularData(data, columns)}
wrapped={false}
copyNode={
<Button buttonSize="small">
diff --git a/superset-frontend/src/components/TableView/TableView.tsx b/superset-frontend/src/components/TableView/TableView.tsx
index ff3fee1..67da3fe 100644
--- a/superset-frontend/src/components/TableView/TableView.tsx
+++ b/superset-frontend/src/components/TableView/TableView.tsx
@@ -158,7 +158,6 @@ const TableView = ({
useSortBy,
usePagination,
);
-
useEffect(() => {
if (serverPagination && pageIndex !== initialState.pageIndex) {
onServerPagination({
diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx
index 16a6c64..dc29adc 100644
--- a/superset-frontend/src/explore/components/DataTableControl/index.tsx
+++ b/superset-frontend/src/explore/components/DataTableControl/index.tsx
@@ -55,11 +55,15 @@ const CopyNode = (
export const CopyToClipboardButton = ({
data,
+ columns,
}: {
data?: Record<string, any>;
+ columns?: string[];
}) => (
<CopyToClipboard
- text={data ? prepareCopyToClipboardTabularData(data) : ''}
+ text={
+ data && columns ? prepareCopyToClipboardTabularData(data, columns) : ''
+ }
wrapped={false}
copyNode={CopyNode}
/>
@@ -113,29 +117,32 @@ export const useFilteredTableData = (
}, [data, filterText]);
export const useTableColumns = (
+ colnames?: string[],
data?: Record<string, any>[],
moreConfigs?: { [key: string]: Partial<Column> },
) =>
useMemo(
() =>
- data?.length
- ? Object.keys(data[0]).map(
- key =>
- ({
- accessor: row => row[key],
- Header: key,
- Cell: ({ value }) => {
- if (value === true) {
- return BOOL_TRUE_DISPLAY;
- }
- if (value === false) {
- return BOOL_FALSE_DISPLAY;
- }
- return String(value);
- },
- ...moreConfigs?.[key],
- } as Column),
- )
+ colnames && data?.length
+ ? colnames
+ .filter((column: string) => Object.keys(data[0]).includes(column))
+ .map(
+ key =>
+ ({
+ accessor: row => row[key],
+ Header: key,
+ Cell: ({ value }) => {
+ if (value === true) {
+ return BOOL_TRUE_DISPLAY;
+ }
+ if (value === false) {
+ return BOOL_FALSE_DISPLAY;
+ }
+ return String(value);
+ },
+ ...moreConfigs?.[key],
+ } as Column),
+ )
: [],
[data, moreConfigs],
);
diff --git a/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts b/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts
index 952b817..537f12b 100644
--- a/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts
+++ b/superset-frontend/src/explore/components/DataTableControl/useTableColumns.test.ts
@@ -42,9 +42,10 @@ const data = [
[unicodeKey]: unicodeKey,
},
];
+const all_columns = ['col01', 'col02', 'col03', asciiKey, unicodeKey];
test('useTableColumns with no options', () => {
- const hook = renderHook(() => useTableColumns(data));
+ const hook = renderHook(() => useTableColumns(all_columns, data));
expect(hook.result.current).toEqual([
{
Cell: expect.any(Function),
@@ -83,7 +84,7 @@ test('useTableColumns with no options', () => {
test('use only the first record columns', () => {
const newData = [data[3], data[0]];
- const hook = renderHook(() => useTableColumns(newData));
+ const hook = renderHook(() => useTableColumns(all_columns, newData));
expect(hook.result.current).toEqual([
{
Cell: expect.any(Function),
@@ -134,7 +135,9 @@ test('use only the first record columns', () => {
});
test('useTableColumns with options', () => {
- const hook = renderHook(() => useTableColumns(data, { col01: { id: 'ID' } }));
+ const hook = renderHook(() =>
+ useTableColumns(all_columns, data, { col01: { id: 'ID' } }),
+ );
expect(hook.result.current).toEqual([
{
Cell: expect.any(Function),
diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx
index 7df5190..3af49e0 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx
@@ -58,6 +58,11 @@ const createProps = () => ({
tableSectionHeight: 156.9,
chartStatus: 'rendered',
onCollapseChange: jest.fn(),
+ queriesResponse: [
+ {
+ colnames: [],
+ },
+ ],
});
afterAll(() => {
diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx
index 3d83089..4a1172d 100644
--- a/superset-frontend/src/explore/components/DataTablesPane/index.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane/index.tsx
@@ -112,6 +112,7 @@ export const DataTablesPane = ({
chartStatus,
ownState,
errorMessage,
+ queriesResponse,
}: {
queryFormData: Record<string, any>;
tableSectionHeight: number;
@@ -119,6 +120,7 @@ export const DataTablesPane = ({
ownState?: JsonObject;
onCollapseChange: (openPanelName: string) => void;
errorMessage?: JSX.Element;
+ queriesResponse: Record<string, any>;
}) => {
const [data, setData] = useState<{
[RESULT_TYPES.results]?: Record<string, any>[];
@@ -128,6 +130,7 @@ export const DataTablesPane = ({
[RESULT_TYPES.results]: true,
[RESULT_TYPES.samples]: true,
});
+ const [columnNames, setColumnNames] = useState<string[]>([]);
const [error, setError] = useState(NULLISH_RESULTS_STATE);
const [filterText, setFilterText] = useState('');
const [activeTabKey, setActiveTabKey] = useState<string>(
@@ -221,6 +224,13 @@ export const DataTablesPane = ({
}, [queryFormData.adhoc_filters, queryFormData.datasource]);
useEffect(() => {
+ if (queriesResponse) {
+ const { colnames } = queriesResponse[0];
+ setColumnNames([...colnames]);
+ }
+ }, [queriesResponse]);
+
+ useEffect(() => {
if (panelOpen && isRequestPending[RESULT_TYPES.results]) {
if (errorMessage) {
setIsRequestPending(prevState => ({
@@ -277,9 +287,17 @@ export const DataTablesPane = ({
),
};
+ // this is to preserve the order of the columns, even if there are integer values,
+ // while also only grabbing the first column's keys
const columns = {
- [RESULT_TYPES.results]: useTableColumns(data[RESULT_TYPES.results]),
- [RESULT_TYPES.samples]: useTableColumns(data[RESULT_TYPES.samples]),
+ [RESULT_TYPES.results]: useTableColumns(
+ columnNames,
+ data[RESULT_TYPES.results],
+ ),
+ [RESULT_TYPES.samples]: useTableColumns(
+ columnNames,
+ data[RESULT_TYPES.samples],
+ ),
};
const renderDataTable = (type: string) => {
@@ -316,7 +334,7 @@ export const DataTablesPane = ({
const TableControls = (
<TableControlsWrapper>
<RowCount data={data[activeTabKey]} loading={isLoading[activeTabKey]} />
- <CopyToClipboardButton data={data[activeTabKey]} />
+ <CopyToClipboardButton data={data[activeTabKey]} columns={columnNames} />
<FilterInput onChangeHandler={setFilterText} />
</TableControlsWrapper>
);
diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
index bd2213f..db35dd7 100644
--- a/superset-frontend/src/explore/components/ExploreChartPanel.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
@@ -116,7 +116,6 @@ const ExploreChartPanel = props => {
const theme = useTheme();
const gutterMargin = theme.gridUnit * GUTTER_SIZE_FACTOR;
const gutterHeight = theme.gridUnit * GUTTER_SIZE_FACTOR;
-
const { height: hHeight, ref: headerRef } = useResizeDetector({
refreshMode: 'debounce',
refreshRate: 300,
@@ -128,7 +127,6 @@ const ExploreChartPanel = props => {
const [splitSizes, setSplitSizes] = useState(
getFromLocalStorage(STORAGE_KEYS.sizes, INITIAL_SIZES),
);
-
const { slice } = props;
const updateQueryContext = useCallback(
async function fetchChartData() {
@@ -211,7 +209,6 @@ const ExploreChartPanel = props => {
}
setSplitSizes(splitSizes);
};
-
const renderChart = useCallback(() => {
const { chart, vizType } = props;
const newHeight =
@@ -317,6 +314,7 @@ const ExploreChartPanel = props => {
onCollapseChange={onCollapseChange}
chartStatus={props.chart.chartStatus}
errorMessage={props.errorMessage}
+ queriesResponse={props.chart.queriesResponse}
/>
</Split>
)}
diff --git a/superset-frontend/src/utils/common.js b/superset-frontend/src/utils/common.js
index 974268c..567ec3d 100644
--- a/superset-frontend/src/utils/common.js
+++ b/superset-frontend/src/utils/common.js
@@ -87,10 +87,21 @@ export function optionFromValue(opt) {
return { value: optionValue(opt), label: optionLabel(opt) };
}
-export function prepareCopyToClipboardTabularData(data) {
+export function prepareCopyToClipboardTabularData(data, columns) {
let result = '';
for (let i = 0; i < data.length; i += 1) {
- result += `${Object.values(data[i]).join('\t')}\n`;
+ const row = {};
+ for (let j = 0; j < columns.length; j += 1) {
+ // JavaScript does not mantain the order of a mixed set of keys (i.e integers and strings)
+ // the below function orders the keys based on the column names.
+ const key = columns[j].name || columns[j];
+ if (data[i][key]) {
+ row[j] = data[i][key];
+ } else {
+ row[j] = data[i][parseFloat(key)];
+ }
+ }
+ result += `${Object.values(row).join('\t')}\n`;
}
return result;
}
diff --git a/superset-frontend/src/utils/common.test.jsx b/superset-frontend/src/utils/common.test.jsx
index b47f423..5542648 100644
--- a/superset-frontend/src/utils/common.test.jsx
+++ b/superset-frontend/src/utils/common.test.jsx
@@ -46,15 +46,18 @@ describe('utils/common', () => {
describe('prepareCopyToClipboardTabularData', () => {
it('converts empty array', () => {
const array = [];
- expect(prepareCopyToClipboardTabularData(array)).toEqual('');
+ const column = [];
+ expect(prepareCopyToClipboardTabularData(array, column)).toEqual('');
});
it('converts non empty array', () => {
const array = [
{ column1: 'lorem', column2: 'ipsum' },
{ column1: 'dolor', column2: 'sit', column3: 'amet' },
];
- expect(prepareCopyToClipboardTabularData(array)).toEqual(
- 'lorem\tipsum\ndolor\tsit\tamet\n',
+ const column = ['column1', 'column2', 'column3'];
+ console.log(prepareCopyToClipboardTabularData(array, column));
+ expect(prepareCopyToClipboardTabularData(array, column)).toEqual(
+ 'lorem\tipsum\t\ndolor\tsit\tamet\n',
);
});
});
diff --git a/superset/viz.py b/superset/viz.py
index 57eb1a8..357b6c8 100644
--- a/superset/viz.py
+++ b/superset/viz.py
@@ -450,7 +450,8 @@ class BaseViz:
payload = self.get_df_payload(query_obj)
- df = payload.get("df")
+ # if payload does not have a df, we are raising an error here.
+ df = cast(Optional[pd.DataFrame], payload["df"])
if self.status != utils.QueryStatus.FAILED:
payload["data"] = self.get_data(df)
@@ -482,7 +483,8 @@ class BaseViz:
for col in filter_columns
if col not in columns and col not in filter_values_columns
] + rejected_time_columns
-
+ if df is not None:
+ payload["colnames"] = list(df.columns)
return payload
def get_df_payload(