Skip to content

Commit

Permalink
[Backport 2.x][Chore] Refactor and improve Discover field summaries
Browse files Browse the repository at this point in the history
* [Chore] Refactor and improve field summaries
* Convert to typescript
* Fix types
* Add tests
* Groups are now naturally sorted by key, which requires selecting a different date filter
* [Chore] Add changelog entry
* [Chore] Refactor columns passing, fix bugs
* pass columns explicitly as props
* fix branding in core mocks
* fix `toBeUndefined()` usage in tests
* remove leftover comment
* fix test subject
* condense types

Backport PR:
#2391

Signed-off-by: Josh Romero <rmerqg@amazon.com>

Signed-off-by: Josh Romero <rmerqg@amazon.com>
(cherry picked from commit b53d4d8)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
github-actions[bot] authored and ananzh committed Nov 3, 2022
1 parent 18e9f52 commit 4b7df81
Show file tree
Hide file tree
Showing 14 changed files with 524 additions and 265 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### 🪛 Refactoring
* [MD] Refactor data source error handling ([#2661](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2661))
- Refactor and improve Discover field summaries ([#2391](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2391))

### 🔩 Tests
* [Multi DataSource] Add unit test coverage for Update Data source management stack ([#2567](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2567))
Expand Down
1 change: 1 addition & 0 deletions src/core/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ function createCoreSetupMock({
uiSettings: uiSettingsServiceMock.createSetupContract(),
injectedMetadata: {
getInjectedVar: injectedMetadataServiceMock.createSetupContract().getInjectedVar,
getBranding: injectedMetadataServiceMock.createSetupContract().getBranding,
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/

import React from 'react';
// @ts-ignore
import { findTestSubject } from '@elastic/eui/lib/test';
// @ts-ignore
import stubbedLogstashFields from 'fixtures/logstash_fields';
Expand Down Expand Up @@ -99,8 +100,9 @@ function getComponent({

const props = {
indexPattern,
columns: [],
field: finalField,
getDetails: jest.fn(() => ({ buckets: [], error: '', exists: 1, total: true, columns: [] })),
getDetails: jest.fn(() => ({ buckets: [], error: '', exists: 1, total: 1 })),
onAddFilter: jest.fn(),
onAddField: jest.fn(),
onRemoveField: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ import { getFieldTypeName } from './lib/get_field_type_name';
import './discover_field.scss';

export interface DiscoverFieldProps {
/**
* the selected columns displayed in the doc table in discover
*/
columns: string[];
/**
* The displayed field
*/
Expand Down Expand Up @@ -76,6 +80,7 @@ export interface DiscoverFieldProps {
}

export function DiscoverField({
columns,
field,
indexPattern,
onAddField,
Expand Down Expand Up @@ -228,9 +233,10 @@ export function DiscoverField({
</EuiPopoverTitle>
{infoIsOpen && (
<DiscoverFieldDetails
indexPattern={indexPattern}
field={field}
columns={columns}
details={getDetails(field)}
field={field}
indexPattern={indexPattern}
onAddFilter={onAddFilter}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export function DiscoverFieldBucket({ field, bucket, onAddFilter }: Props) {
title={
bucket.display === ''
? emptyTxt
: `${bucket.display}: ${bucket.count} (${bucket.percent}%)`
: `${bucket.display}: ${bucket.count} (${bucket.percent.toFixed(1)}%)`
}
size="xs"
className="eui-textTruncate"
Expand All @@ -78,7 +78,7 @@ export function DiscoverFieldBucket({ field, bucket, onAddFilter }: Props) {
</EuiFlexItem>
<EuiFlexItem grow={false} className="eui-textTruncate">
<EuiText color="secondary" size="xs" className="eui-textTruncate">
{bucket.percent}%
{bucket.percent.toFixed(1)}%
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,26 @@
*/

import React from 'react';
// @ts-ignore
import { findTestSubject } from '@elastic/eui/lib/test';
import { act } from '@testing-library/react';
// @ts-ignore
import stubbedLogstashFields from 'fixtures/logstash_fields';
import { mountWithIntl } from 'test_utils/enzyme_helpers';
import { mountWithIntl, nextTick } from 'test_utils/enzyme_helpers';
import { DiscoverFieldDetails } from './discover_field_details';
import { coreMock } from '../../../../../../core/public/mocks';
import { IndexPatternField } from '../../../../../data/public';
import { getStubIndexPattern } from '../../../../../data/public/test_utils';

const mockGetHref = jest.fn();
const mockGetTriggerCompatibleActions = jest.fn();

jest.mock('../../../opensearch_dashboards_services', () => ({
getUiActions: () => ({
getTriggerCompatibleActions: mockGetTriggerCompatibleActions,
}),
}));

const indexPattern = getStubIndexPattern(
'logstash-*',
(cfg: any) => cfg,
Expand All @@ -48,17 +59,187 @@ const indexPattern = getStubIndexPattern(

describe('discover sidebar field details', function () {
const defaultProps = {
columns: [],
details: { buckets: [], error: '', exists: 1, total: 1 },
indexPattern,
details: { buckets: [], error: '', exists: 1, total: true, columns: [] },
onAddFilter: jest.fn(),
};

function mountComponent(field: IndexPatternField) {
const compProps = { ...defaultProps, field };
beforeEach(() => {
mockGetHref.mockReturnValue('/foo/bar');
mockGetTriggerCompatibleActions.mockReturnValue([
{
getHref: mockGetHref,
},
]);
});

function mountComponent(field: IndexPatternField, props?: Record<string, any>) {
const compProps = { ...defaultProps, ...props, field };
return mountWithIntl(<DiscoverFieldDetails {...compProps} />);
}

it('should enable the visualize link for a number field', function () {
it('should render buckets if they exist', async function () {
const visualizableField = new IndexPatternField(
{
name: 'bytes',
type: 'number',
esTypes: ['long'],
count: 10,
scripted: false,
searchable: true,
aggregatable: true,
readFromDocValues: true,
},
'bytes'
);
const buckets = [1, 2, 3].map((n) => ({
display: `display-${n}`,
value: `value-${n}`,
percent: 25,
count: 100,
}));
const comp = mountComponent(visualizableField, {
details: { ...defaultProps.details, buckets },
});
expect(findTestSubject(comp, 'fieldVisualizeError').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualizeBucketContainer').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualizeBucketContainer').children().length).toBe(
buckets.length
);
// Visualize link should not be rendered until async hook update
expect(findTestSubject(comp, 'fieldVisualizeLink').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualize-bytes').length).toBe(0);

// Complete async hook
await act(async () => {
await nextTick();
comp.update();
});
expect(findTestSubject(comp, 'fieldVisualizeError').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualizeBucketContainer').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualizeBucketContainer').children().length).toBe(
buckets.length
);
expect(findTestSubject(comp, 'fieldVisualizeLink').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualize-bytes').length).toBe(1);
});

it('should only render buckets if they exist', async function () {
const visualizableField = new IndexPatternField(
{
name: 'bytes',
type: 'number',
esTypes: ['long'],
count: 10,
scripted: false,
searchable: true,
aggregatable: true,
readFromDocValues: true,
},
'bytes'
);
const comp = mountComponent(visualizableField);
expect(findTestSubject(comp, 'fieldVisualizeContainer').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualizeError').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualizeBucketContainer').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualizeLink').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualize-bytes').length).toBe(0);

await act(async () => {
await nextTick();
comp.update();
});

expect(findTestSubject(comp, 'fieldVisualizeContainer').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualizeError').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualizeBucketContainer').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualizeLink').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualize-bytes').length).toBe(1);
});

it('should render a details error', async function () {
const visualizableField = new IndexPatternField(
{
name: 'bytes',
type: 'number',
esTypes: ['long'],
count: 10,
scripted: false,
searchable: true,
aggregatable: true,
readFromDocValues: true,
},
'bytes'
);
const errText = 'Some error';
const comp = mountComponent(visualizableField, {
details: { ...defaultProps.details, error: errText },
});
expect(findTestSubject(comp, 'fieldVisualizeContainer').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualizeBucketContainer').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualizeError').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualizeError').text()).toBe(errText);

await act(async () => {
await nextTick();
comp.update();
});
expect(findTestSubject(comp, 'fieldVisualizeLink').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualize-bytes').length).toBe(1);
});

it('should handle promise rejection from isFieldVisualizable', async function () {
mockGetTriggerCompatibleActions.mockRejectedValue(new Error('Async error'));
const visualizableField = new IndexPatternField(
{
name: 'bytes',
type: 'number',
esTypes: ['long'],
count: 10,
scripted: false,
searchable: true,
aggregatable: true,
readFromDocValues: true,
},
'bytes'
);
const comp = mountComponent(visualizableField);

await act(async () => {
await nextTick();
comp.update();
});
expect(findTestSubject(comp, 'fieldVisualizeLink').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualize-bytes').length).toBe(0);
});

it('should handle promise rejection from getVisualizeHref', async function () {
mockGetHref.mockRejectedValue(new Error('Async error'));
const visualizableField = new IndexPatternField(
{
name: 'bytes',
type: 'number',
esTypes: ['long'],
count: 10,
scripted: false,
searchable: true,
aggregatable: true,
readFromDocValues: true,
},
'bytes'
);
const comp = mountComponent(visualizableField);

await act(async () => {
await nextTick();
comp.update();
});
expect(findTestSubject(comp, 'fieldVisualizeLink').length).toBe(0);
expect(findTestSubject(comp, 'fieldVisualize-bytes').length).toBe(0);
});

it('should enable the visualize link for a number field', async function () {
const visualizableField = new IndexPatternField(
{
name: 'bytes',
Expand All @@ -73,10 +254,17 @@ describe('discover sidebar field details', function () {
'bytes'
);
const comp = mountComponent(visualizableField);
expect(findTestSubject(comp, 'fieldVisualize-bytes')).toBeTruthy();

await act(async () => {
await nextTick();
comp.update();
});
expect(findTestSubject(comp, 'fieldVisualizeLink').length).toBe(1);
expect(findTestSubject(comp, 'fieldVisualize-bytes').length).toBe(1);
});

it('should disable the visualize link for an _id field', function () {
it('should disable the visualize link for an _id field', async function () {
expect.assertions(1);
const conflictField = new IndexPatternField(
{
name: '_id',
Expand All @@ -91,10 +279,15 @@ describe('discover sidebar field details', function () {
'test'
);
const comp = mountComponent(conflictField);
expect(findTestSubject(comp, 'fieldVisualize-_id')).toEqual({});

await act(async () => {
await nextTick();
comp.update();
});
expect(findTestSubject(comp, 'fieldVisualize-_id').length).toBe(0);
});

it('should disable the visualize link for an unknown field', function () {
it('should disable the visualize link for an unknown field', async function () {
const unknownField = new IndexPatternField(
{
name: 'test',
Expand All @@ -109,6 +302,11 @@ describe('discover sidebar field details', function () {
'test'
);
const comp = mountComponent(unknownField);
expect(findTestSubject(comp, 'fieldVisualize-test')).toEqual({});

await act(async () => {
await nextTick();
comp.update();
});
expect(findTestSubject(comp, 'fieldVisualize-test').length).toBe(0);
});
});
Loading

0 comments on commit 4b7df81

Please sign in to comment.