fix(explore): timestamp format when copy datatable to clipboard (apac…

…he#17166) * fix(explore): timestamp format when copy datatable to clipboard * Fix test * Fix test * Use translation for aria label * Memoize stringified cells * Fix test
opus-42 · Nov 14, 2021 · 7bd09e2 · 7bd09e2
1 parent 42f0c0a
commit 7bd09e2
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 28 deletions.
diff --git a/superset-frontend/src/explore/components/DataTableControl/index.tsx b/superset-frontend/src/explore/components/DataTableControl/index.tsx
@@ -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';
 
@@ -48,7 +45,7 @@ export const CopyButton = styled(Button)`
 `;
 
 const CopyNode = (
-  <CopyButton buttonSize="xsmall">
+  <CopyButton buttonSize="xsmall" aria-label={t('Copy')}>
     <i className="fa fa-clipboard" />
   </CopyButton>
 );
@@ -103,18 +100,26 @@ export const RowCount = ({
 export const useFilteredTableData = (
   filterText: string,
   data?: Record<string, any>[],
-) =>
-  useMemo(() => {
+) => {
+  const rowsAsStrings = useMemo(
+    () =>
+      data?.map((row: Record<string, any>) =>
+        Object.values(row).map(value => value?.toString().toLowerCase()),
+      ) ?? [],
+    [data],
+  );
+
+  return useMemo(() => {
     if (!data?.length) {
       return [];
     }
-    const formattedData = applyFormattingToTabularData(data);
-    return formattedData.filter((row: Record<string, any>) =>
-      Object.values(row).some(value =>
-        value?.toString().toLowerCase().includes(filterText.toLowerCase()),
+    return data.filter((_, index: number) =>
+      rowsAsStrings[index].some(value =>
+        value?.includes(filterText.toLowerCase()),
       ),
     );
-  }, [data, filterText]);
+  }, [data, filterText, rowsAsStrings]);
+};
 
 export const useTableColumns = (
   colnames?: string[],

diff --git a/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx b/superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.test.tsx
@@ -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',
@@ -65,10 +61,6 @@ const createProps = () => ({
   ],
 });
 
-afterAll(() => {
-  fetchMock.done();
-});
-
 test('Rendering DataTablesPane correctly', () => {
   const props = createProps();
   render(<DataTablesPane {...props} />, { useRedux: true });
@@ -108,3 +100,38 @@ 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'));
+  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();
+});
diff --git a/superset-frontend/src/explore/components/DataTablesPane/index.tsx b/superset-frontend/src/explore/components/DataTablesPane/index.tsx
@@ -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';
@@ -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,
@@ -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 => ({
@@ -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],
     ),
   };
 
@@ -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>
   );

diff --git a/superset-frontend/src/utils/common.test.jsx b/superset-frontend/src/utils/common.test.jsx
@@ -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',
       );