[Table Vis] move format table, consolidate types and add unit tests

Currently, table data is formatted by a until function convertToFormattedData in TableVisComponent. In this PR, we moved the formatting data process to table_vis_response_handler.ts to combine with other data process logics. In this way, component render and data handling logics are completely isolated. This PR also solidate some types. Issue Resolved: opensearch-project#3395 opensearch-project#2856 Signed-off-by: Anan Zhuang <[email protected]>
ananzh · Apr 5, 2023 · 2611e06 · 2611e06
1 parent 6b42669
commit 2611e06
Show file tree

Hide file tree

Showing 26 changed files with 1,490 additions and 193 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -71,6 +71,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Use mirrors to download Node.js binaries to escape sporadic 404 errors ([#3619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3619))
 - [Multiple DataSource] Refactor dev tool console to use opensearch-js client to send requests ([#3544](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3544))
 - [Data] Add geo shape filter field ([#3605](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3605))
+- [Table Visualization] Move format table, consolidate types and add unit tests ([#3397](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3397))
 
 ### 🐛 Bug Fixes
 

diff --git a/src/plugins/vis_type_table/public/components/table_vis_app.test.tsx b/src/plugins/vis_type_table/public/components/table_vis_app.test.tsx
@@ -0,0 +1,98 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import React from 'react';
+import { render } from '@testing-library/react';
+import { coreMock } from '../../../../core/public/mocks';
+import { IInterpreterRenderHandlers } from 'src/plugins/expressions';
+import { TableVisApp } from './table_vis_app';
+import { TableVisConfig } from '../types';
+import { TableVisData } from '../table_vis_response_handler';
+
+jest.mock('./table_vis_component_group', () => ({
+  TableVisComponentGroup: () => (
+    <div data-test-subj="TableVisComponentGroup">TableVisComponentGroup</div>
+  ),
+}));
+
+jest.mock('./table_vis_component', () => ({
+  TableVisComponent: () => <div data-test-subj="TableVisComponent">TableVisComponent</div>,
+}));
+
+describe('TableVisApp', () => {
+  const serviceMock = coreMock.createStart();
+  const handlersMock = ({
+    done: jest.fn(),
+    uiState: {
+      get: jest.fn((key) => {
+        switch (key) {
+          case 'vis.sortColumn':
+            return {};
+          case 'vis.columnsWidth':
+            return [];
+          default:
+            return undefined;
+        }
+      }),
+      set: jest.fn(),
+    },
+    event: 'event',
+  } as unknown) as IInterpreterRenderHandlers;
+  const visConfigMock = ({} as unknown) as TableVisConfig;
+
+  it('should render TableVisComponent if no split table', () => {
+    const visDataMock = {
+      table: {
+        columns: [],
+        rows: [],
+        formattedColumns: [],
+      },
+      tableGroups: [],
+    } as TableVisData;
+    const { getByTestId } = render(
+      <TableVisApp
+        services={serviceMock}
+        visData={visDataMock}
+        visConfig={visConfigMock}
+        handlers={handlersMock}
+      />
+    );
+    expect(getByTestId('TableVisComponent')).toBeInTheDocument();
+  });
+
+  it('should render TableVisComponentGroup component if split direction is column', () => {
+    const visDataMock = {
+      tableGroups: [],
+      direction: 'column',
+    } as TableVisData;
+    const { container, getByTestId } = render(
+      <TableVisApp
+        services={serviceMock}
+        visData={visDataMock}
+        visConfig={visConfigMock}
+        handlers={handlersMock}
+      />
+    );
+    expect(container.outerHTML.includes('visTable visTable__groupInColumns')).toBe(true);
+    expect(getByTestId('TableVisComponentGroup')).toBeInTheDocument();
+  });
+
+  it('should render TableVisComponentGroup component if split direction is row', () => {
+    const visDataMock = {
+      tableGroups: [],
+      direction: 'row',
+    } as TableVisData;
+    const { container, getByTestId } = render(
+      <TableVisApp
+        services={serviceMock}
+        visData={visDataMock}
+        visConfig={visConfigMock}
+        handlers={handlersMock}
+      />
+    );
+    expect(container.outerHTML.includes('visTable')).toBe(true);
+    expect(getByTestId('TableVisComponentGroup')).toBeInTheDocument();
+  });
+});
diff --git a/src/plugins/vis_type_table/public/components/table_vis_app.tsx b/src/plugins/vis_type_table/public/components/table_vis_app.tsx
@@ -4,20 +4,23 @@
  */
 
 import './table_vis_app.scss';
-import React, { useEffect, useState } from 'react';
+import React, { useEffect } from 'react';
 import classNames from 'classnames';
 import { CoreStart } from 'opensearch-dashboards/public';
 import { I18nProvider } from '@osd/i18n/react';
 import { IInterpreterRenderHandlers } from 'src/plugins/expressions';
+import { PersistedState } from '../../../visualizations/public';
 import { OpenSearchDashboardsContextProvider } from '../../../opensearch_dashboards_react/public';
-import { TableContext } from '../table_vis_response_handler';
-import { TableVisConfig, ColumnSort, ColumnWidth, TableUiState } from '../types';
+import { TableVisData } from '../table_vis_response_handler';
+import { TableVisConfig } from '../types';
 import { TableVisComponent } from './table_vis_component';
 import { TableVisComponentGroup } from './table_vis_component_group';
+import { uiStateManagement } from '../utils/ui_state_management';
+import { TableUiState } from '../utils/ui_state_management';
 
 interface TableVisAppProps {
   services: CoreStart;
-  visData: TableContext;
+  visData: TableVisData;
   visConfig: TableVisConfig;
   handlers: IInterpreterRenderHandlers;
 }
@@ -38,12 +41,7 @@ export const TableVisApp = ({
     visTable__groupInColumns: direction === 'column',
   });
 
-  // TODO: remove duplicate sort and width state
-  // Issue: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2704#issuecomment-1299380818
-  const [sort, setSort] = useState<ColumnSort>({ colIndex: null, direction: null });
-  const [width, setWidth] = useState<ColumnWidth[]>([]);
-
-  const tableUiState: TableUiState = { sort, setSort, width, setWidth };
+  const tableUiState: TableUiState = uiStateManagement(handlers.uiState as PersistedState);
 
   return (
     <I18nProvider>

diff --git a/src/plugins/vis_type_table/public/components/table_vis_cell.test.tsx b/src/plugins/vis_type_table/public/components/table_vis_cell.test.tsx
@@ -0,0 +1,65 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import React from 'react';
+import { render } from '@testing-library/react';
+
+import { OpenSearchDashboardsDatatableRow } from 'src/plugins/expressions';
+import { FormattedColumn } from '../types';
+import { getTableVisCellValue } from './table_vis_cell';
+import { FieldFormat } from 'src/plugins/data/public';
+
+class MockFieldFormat extends FieldFormat {
+  convert = jest.fn();
+}
+
+describe('getTableVisCellValue', () => {
+  const mockFormatter = new MockFieldFormat();
+
+  const columns: FormattedColumn[] = [
+    {
+      id: 'testId',
+      title: 'Test Column',
+      formatter: mockFormatter,
+      filterable: true,
+    },
+  ];
+
+  const sortedRows: OpenSearchDashboardsDatatableRow[] = [
+    {
+      testId: 'Test Value 1',
+    },
+    {
+      testId: 'Test Value 2',
+    },
+  ];
+
+  const TableCell = ({ rowIndex, columnId }: { rowIndex: number; columnId: string }) => {
+    const getCellValue = getTableVisCellValue(sortedRows, columns);
+    return getCellValue({ rowIndex, columnId });
+  };
+
+  beforeEach(() => {
+    mockFormatter.convert.mockClear();
+  });
+
+  test('should render cell value with correct formatting', () => {
+    mockFormatter.convert.mockReturnValueOnce('<strong>Test Value 1</strong>');
+    const { getByText } = render(<TableCell rowIndex={0} columnId="testId" />);
+    expect(mockFormatter.convert).toHaveBeenCalledWith('Test Value 1', 'html');
+    expect(getByText('Test Value 1')).toBeInTheDocument();
+    expect(getByText('Test Value 1').closest('strong')).toBeInTheDocument();
+  });
+
+  test('should return null when rowIndex is out of bounds', () => {
+    const { container } = render(<TableCell rowIndex={2} columnId="testId" />);
+    expect(container.firstChild).toBeNull();
+  });
+
+  test('should return null when no matching columnId is found', () => {
+    const { container } = render(<TableCell rowIndex={0} columnId="nonexistent" />);
+    expect(container.firstChild).toBeNull();
+  });
+});
diff --git a/src/plugins/vis_type_table/public/components/table_vis_cell.tsx b/src/plugins/vis_type_table/public/components/table_vis_cell.tsx
@@ -0,0 +1,38 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import React from 'react';
+import dompurify from 'dompurify';
+
+import { OpenSearchDashboardsDatatableRow } from 'src/plugins/expressions';
+import { FormattedColumn } from '../types';
+
+export const getTableVisCellValue = (
+  sortedRows: OpenSearchDashboardsDatatableRow[],
+  columns: FormattedColumn[]
+) => ({ rowIndex, columnId }: { rowIndex: number; columnId: string }) => {
+  if (rowIndex < 0 || rowIndex >= sortedRows.length) {
+    return null;
+  }
+  const row = sortedRows[rowIndex];
+  if (!row || !row.hasOwnProperty(columnId)) {
+    return null;
+  }
+  const rawContent = sortedRows[rowIndex][columnId];
+  const colIndex = columns.findIndex((col) => col.id === columnId);
+  const htmlContent = columns[colIndex].formatter.convert(rawContent, 'html');
+  const formattedContent = (
+    /*
+     * Justification for dangerouslySetInnerHTML:
+     * This is one of the visualizations which makes use of the HTML field formatters.
+     * Since these formatters produce raw HTML, this visualization needs to be able to render them as-is, relying
+     * on the field formatter to only produce safe HTML.
+     * `htmlContent` is created by converting raw data via HTML field formatter, so we need to make sure this value never contains
+     * any unsafe HTML (e.g. by bypassing the field formatter).
+     */
+    <div dangerouslySetInnerHTML={{ __html: dompurify.sanitize(htmlContent) }} /> // eslint-disable-line react/no-danger
+  );
+  return sortedRows.hasOwnProperty(rowIndex) ? formattedContent || null : null;
+};