Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(explore): timestamp format when copy datatable to clipboard #17166

Merged
merged 6 commits into from
Oct 21, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ import {
SLOW_DEBOUNCE,
} from 'src/constants';
import Button from 'src/components/Button';
import {
applyFormattingToTabularData,
prepareCopyToClipboardTabularData,
} from 'src/utils/common';
import { prepareCopyToClipboardTabularData } from 'src/utils/common';
import CopyToClipboard from 'src/components/CopyToClipboard';
import RowCountLabel from 'src/explore/components/RowCountLabel';

Expand All @@ -48,7 +45,7 @@ export const CopyButton = styled(Button)`
`;

const CopyNode = (
<CopyButton buttonSize="xsmall">
<CopyButton buttonSize="xsmall" aria-label="copy">
kgabryje marked this conversation as resolved.
Show resolved Hide resolved
<i className="fa fa-clipboard" />
</CopyButton>
);
Expand Down Expand Up @@ -108,8 +105,7 @@ export const useFilteredTableData = (
if (!data?.length) {
return [];
}
const formattedData = applyFormattingToTabularData(data);
return formattedData.filter((row: Record<string, any>) =>
return data.filter((row: Record<string, any>) =>
Object.values(row).some(value =>
value?.toString().toLowerCase().includes(filterText.toLowerCase()),
),
Copy link
Member

@zhaoyongjie zhaoyongjie Oct 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit performance optimization, could we process the row of data before calling useFilteredTableData? (in the other words, pre-process every cell into string)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Is that what you meant?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,13 @@
* under the License.
*/

import userEvent from '@testing-library/user-event';
import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import fetchMock from 'fetch-mock';
import * as copyUtils from 'src/utils/copy';
import { render, screen } from 'spec/helpers/testing-library';
import { DataTablesPane } from '.';

fetchMock.post(
'http://api/v1/chart/data?form_data=%7B%22slice_id%22%3A456%7D',
{ body: {} },
);

const createProps = () => ({
queryFormData: {
viz_type: 'heatmap',
Expand Down Expand Up @@ -65,10 +61,6 @@ const createProps = () => ({
],
});

afterAll(() => {
fetchMock.done();
});

test('Rendering DataTablesPane correctly', () => {
const props = createProps();
render(<DataTablesPane {...props} />, { useRedux: true });
Expand Down Expand Up @@ -108,3 +100,39 @@ test('Should show tabs: View samples', async () => {
userEvent.click(await screen.findByText('View samples'));
expect(await screen.findByText('0 rows retrieved')).toBeVisible();
});

test('Should copy data table content correctly', async () => {
fetchMock.post(
'glob:*/api/v1/chart/data?form_data=%7B%22slice_id%22%3A456%7D',
{
result: [{ data: [{ __timestamp: 1230768000000, genre: 'Action' }] }],
},
);
const copyToClipboardSpy = jest.spyOn(copyUtils, 'default');
const props = createProps();
render(
<DataTablesPane
{...{
...props,
chartStatus: 'success',
queriesResponse: [
{
colnames: ['__timestamp', 'genre'],
},
],
}}
/>,
{
useRedux: true,
},
);
userEvent.click(await screen.findByText('Data'));
userEvent.click(await screen.findByText('View samples'));
expect(await screen.findByText('1 rows retrieved')).toBeVisible();

userEvent.click(screen.getByRole('button', { name: 'copy' }));
expect(copyToClipboardSpy).toHaveBeenCalledWith(
'2009-01-01 00:00:00\tAction\n',
);
fetchMock.done();
});
24 changes: 20 additions & 4 deletions superset-frontend/src/explore/components/DataTablesPane/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import React, { useCallback, useEffect, useState } from 'react';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { JsonObject, styled, t } from '@superset-ui/core';
import Collapse from 'src/components/Collapse';
import Tabs from 'src/components/Tabs';
Expand All @@ -35,6 +35,7 @@ import {
useFilteredTableData,
useTableColumns,
} from 'src/explore/components/DataTableControl';
import { applyFormattingToTabularData } from 'src/utils/common';

const RESULT_TYPES = {
results: 'results' as const,
Expand Down Expand Up @@ -144,6 +145,18 @@ export const DataTablesPane = ({
getFromLocalStorage(STORAGE_KEYS.isOpen, false),
);

const formattedData = useMemo(
() => ({
[RESULT_TYPES.results]: applyFormattingToTabularData(
data[RESULT_TYPES.results],
),
[RESULT_TYPES.samples]: applyFormattingToTabularData(
data[RESULT_TYPES.samples],
),
}),
[data],
);

const getData = useCallback(
(resultType: string) => {
setIsLoading(prevIsLoading => ({
Expand Down Expand Up @@ -279,11 +292,11 @@ export const DataTablesPane = ({
const filteredData = {
[RESULT_TYPES.results]: useFilteredTableData(
filterText,
data[RESULT_TYPES.results],
formattedData[RESULT_TYPES.results],
),
[RESULT_TYPES.samples]: useFilteredTableData(
filterText,
data[RESULT_TYPES.samples],
formattedData[RESULT_TYPES.samples],
),
};

Expand Down Expand Up @@ -334,7 +347,10 @@ export const DataTablesPane = ({
const TableControls = (
<TableControlsWrapper>
<RowCount data={data[activeTabKey]} loading={isLoading[activeTabKey]} />
<CopyToClipboardButton data={data[activeTabKey]} columns={columnNames} />
<CopyToClipboardButton
data={formattedData[activeTabKey]}
columns={columnNames}
/>
<FilterInput onChangeHandler={setFilterText} />
</TableControlsWrapper>
);
Expand Down
1 change: 0 additions & 1 deletion superset-frontend/src/utils/common.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ describe('utils/common', () => {
{ column1: 'dolor', column2: 'sit', column3: 'amet' },
];
const column = ['column1', 'column2', 'column3'];
console.log(prepareCopyToClipboardTabularData(array, column));
expect(prepareCopyToClipboardTabularData(array, column)).toEqual(
'lorem\tipsum\t\ndolor\tsit\tamet\n',
);
Expand Down