Skip to content

Commit

Permalink
[2.x Backport][Table Vis] move format table, consolidate types and ad…
Browse files Browse the repository at this point in the history
…d 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.

Backport PR:
opensearch-project#3397

Issue Resolved:
opensearch-project#3395
opensearch-project#2856

Signed-off-by: Anan Zhuang <ananzh@amazon.com>
  • Loading branch information
ananzh committed Apr 17, 2023
1 parent 15361cf commit 3a7fbe7
Show file tree
Hide file tree
Showing 28 changed files with 1,634 additions and 197 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Data] Add geo shape filter field ([#3605](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3605))
- [UI] Add support for comma delimiters in the global filter bar ([#3686](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3686))
- [VisBuilder] Add UI actions handler ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732))
- [Table Visualization] Move format table, consolidate types and add unit tests ([#3397](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3397))

### 🐛 Bug Fixes

Expand Down
23 changes: 19 additions & 4 deletions src/plugins/vis_type_table/public/components/table_vis_app.scss
Original file line number Diff line number Diff line change
@@ -1,14 +1,29 @@
// Container for the Table Visualization component
.visTable {
display: flex;
flex-direction: column;
flex: 1 0 0;
overflow: auto;

@include euiScrollBar;
}

// Group container for table visualization components
.visTable__group {
padding: $euiSizeS;
margin-bottom: $euiSizeL;
display: flex;
flex-direction: column;
flex: 0 0 auto;
}

> h3 {
text-align: center;
}
// Style for table component title
.visTable__component__title {
text-align: center;
}

// Modifier for visTables__group when displayed in columns
.visTable__groupInColumns {
display: flex;
flex-direction: row;
align-items: flex-start;
}
Original file line number Diff line number Diff line change
@@ -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();
});
});
17 changes: 7 additions & 10 deletions src/plugins/vis_type_table/public/components/table_vis_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,22 @@
*/

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 { getTableUIState, TableUiState } from '../utils';

interface TableVisAppProps {
services: CoreStart;
visData: TableContext;
visData: TableVisData;
visConfig: TableVisConfig;
handlers: IInterpreterRenderHandlers;
}
Expand All @@ -38,12 +40,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 = getTableUIState(handlers.uiState as PersistedState);

return (
<I18nProvider>
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
});
});
38 changes: 38 additions & 0 deletions src/plugins/vis_type_table/public/components/table_vis_cell.tsx
Original file line number Diff line number Diff line change
@@ -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 = row[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 formattedContent || null;
};
Loading

0 comments on commit 3a7fbe7

Please sign in to comment.