Skip to content

Commit

Permalink
[VisBuilder] Avoid prop passing by extracting custom hooks
Browse files Browse the repository at this point in the history
- refactor meta field identification

Signed-off-by: Josh Romero <rmerqg@amazon.com>
  • Loading branch information
joshuarrrr committed Nov 3, 2022
1 parent ebc16a3 commit db5bffb
Show file tree
Hide file tree
Showing 9 changed files with 175 additions and 186 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,10 @@
* under the License.
*/

import React, { useCallback, useState } from 'react';
import React, { useState } from 'react';
import { EuiPopover } from '@elastic/eui';

import {
FilterManager,
IndexPattern,
IndexPatternField,
opensearchFilters,
} from '../../../../../data/public';
import { IndexPatternField } from '../../../../../data/public';
import {
FieldButton,
FieldButtonProps,
Expand All @@ -50,31 +45,13 @@ import './field.scss';

export interface FieldProps {
field: IndexPatternField;
filterManager: FilterManager;
indexPattern?: IndexPattern;
getDetails: (field) => FieldDetails;
}

// TODO: Add field sections (Available fields, popular fields from src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx)
export const Field = ({ field, filterManager, indexPattern, getDetails }: FieldProps) => {
const { id: indexPatternId = '', metaFields = [] } = indexPattern ?? {};
const isMetaField = metaFields.includes(field.name);
export const Field = ({ field, getDetails }: FieldProps) => {
const [infoIsOpen, setOpen] = useState(false);

const onAddFilter = useCallback(
(fieldToFilter, value, operation) => {
const newFilters = opensearchFilters.generateFilters(
filterManager,
fieldToFilter,
value,
operation,
indexPatternId
);
return filterManager.addFilters(newFilters);
},
[filterManager, indexPatternId]
);

function togglePopover() {
setOpen(!infoIsOpen);
}
Expand All @@ -91,14 +68,7 @@ export const Field = ({ field, filterManager, indexPattern, getDetails }: FieldP
repositionOnScroll
data-test-subj="field-popover"
>
{infoIsOpen && (
<FieldDetailsView
field={field}
isMetaField={isMetaField}
details={getDetails(field)}
onAddFilter={onAddFilter}
/>
)}
{infoIsOpen && <FieldDetailsView field={field} details={getDetails(field)} />}
</EuiPopover>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,19 @@ import { IndexPatternField } from '../../../../../data/public';

import { Bucket } from './types';
import './field_bucket.scss';
import { useOnAddFilter } from '../../utils/use';

interface FieldBucketProps {
bucket: Bucket;
field: IndexPatternField;
onAddFilter: (field: IndexPatternField | string, value: string, type: '+' | '-') => void;
}

export function FieldBucket({ bucket, field, onAddFilter }: FieldBucketProps) {
export function FieldBucket({ bucket, field }: FieldBucketProps) {
const { count, display, percent, value } = bucket;
const { filterable: isFilterableField, name: fieldName } = field;

const onAddFilter = useOnAddFilter();

const emptyText = i18n.translate('visBuilder.fieldSelector.detailsView.emptyStringText', {
// We need this to communicate to users when a top value is actually an empty string
defaultMessage: 'Empty string',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,17 @@ import { IndexPatternField } from '../../../../../data/public';

import { FieldDetailsView } from './field_details';

const mockOnAddFilter = jest.fn();
const mockUseIndexPatterns = jest.fn(() => ({ selected: 'mockIndexPattern' }));
const mockUseOnAddFilter = jest.fn();
jest.mock('../../utils/use', () => ({
useIndexPatterns: jest.fn(() => mockUseIndexPatterns),
useOnAddFilter: jest.fn(() => mockUseOnAddFilter),
}));

describe('visBuilder field details', function () {
const defaultProps = {
isMetaField: false,
details: { buckets: [], error: '', exists: 1, total: 1 },
onAddFilter: mockOnAddFilter,
};

const defaultDetails = { buckets: [], error: '', exists: 1, total: 1 };
function mountComponent(field: IndexPatternField, props?: Record<string, any>) {
const compProps = { ...defaultProps, ...props, field };
const compProps = { details: defaultDetails, ...props, field };
return mountWithIntl(<FieldDetailsView {...compProps} />);
}

Expand All @@ -75,7 +75,7 @@ describe('visBuilder field details', function () {
count: 100,
}));
const comp = mountComponent(field, {
details: { ...defaultProps.details, buckets },
details: { ...defaultDetails, buckets },
});
expect(findTestSubject(comp, 'fieldDetailsContainer').length).toBe(1);
expect(findTestSubject(comp, 'fieldDetailsError').length).toBe(0);
Expand Down Expand Up @@ -124,7 +124,7 @@ describe('visBuilder field details', function () {
);
const errText = 'Some error';
const comp = mountComponent(field, {
details: { ...defaultProps.details, error: errText },
details: { ...defaultDetails, error: errText },
});
expect(findTestSubject(comp, 'fieldDetailsContainer').length).toBe(1);
expect(findTestSubject(comp, 'fieldDetailsBucketsContainer').children().length).toBe(0);
Expand Down Expand Up @@ -152,27 +152,4 @@ describe('visBuilder field details', function () {
expect(findTestSubject(comp, 'fieldDetailsError').length).toBe(0);
expect(findTestSubject(comp, 'fieldDetailsExistsLink').length).toBe(0);
});

it('should not render an exists filter link for meta fields', async function () {
const field = new IndexPatternField(
{
name: 'bytes',
type: 'number',
esTypes: ['long'],
count: 10,
scripted: true,
searchable: true,
aggregatable: true,
readFromDocValues: true,
},
'bytes'
);
const comp = mountComponent(field, {
...defaultProps,
isMetaField: true,
});
expect(findTestSubject(comp, 'fieldDetailsContainer').length).toBe(1);
expect(findTestSubject(comp, 'fieldDetailsError').length).toBe(0);
expect(findTestSubject(comp, 'fieldDetailsExistsLink').length).toBe(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,25 @@ import { i18n } from '@osd/i18n';

import { IndexPatternField } from '../../../../../data/public';

import { useIndexPatterns, useOnAddFilter } from '../../utils/use';
import { FieldBucket } from './field_bucket';
import { Bucket, FieldDetails } from './types';

interface FieldDetailsProps {
field: IndexPatternField;
isMetaField: boolean;
details: FieldDetails;
onAddFilter: (field: IndexPatternField | string, value: string, type: '+' | '-') => void;
}

export function FieldDetailsView({ field, isMetaField, details, onAddFilter }: FieldDetailsProps) {
export function FieldDetailsView({ field, details }: FieldDetailsProps) {
const { buckets, error, exists, total } = details;

const onAddFilter = useOnAddFilter();
const indexPattern = useIndexPatterns().selected;

const { metaFields = [] } = indexPattern ?? {};
const isMetaField = metaFields.includes(field.name);
const shouldAllowExistsFilter = !isMetaField && !field.scripted;

const bucketsTitle =
buckets.length > 1
? i18n.translate('visBuilder.fieldSelector.detailsView.fieldTopValuesLabel', {
Expand All @@ -45,8 +51,6 @@ export function FieldDetailsView({ field, isMetaField, details, onAddFilter }: F

const title = buckets.length ? bucketsTitle : errorTitle;

const shouldAllowExistsFilter = !isMetaField && !field.scripted;

return (
<>
<EuiPopoverTitle>{title}</EuiPopoverTitle>
Expand All @@ -61,12 +65,7 @@ export function FieldDetailsView({ field, isMetaField, details, onAddFilter }: F
data-test-subj="fieldDetailsBucketsContainer"
>
{buckets.map((bucket: Bucket, idx: number) => (
<FieldBucket
key={`bucket${idx}`}
bucket={bucket}
field={field}
onAddFilter={onAddFilter}
/>
<FieldBucket key={`bucket${idx}`} bucket={bucket} field={field} />
))}
</div>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@ import { fireEvent, render, screen } from '@testing-library/react';
import { FilterManager, IndexPatternField } from '../../../../../data/public';
import { FieldGroup } from './field_selector';

const mockGetDetails = jest.fn(() => ({
const mockUseIndexPatterns = jest.fn(() => ({ selected: 'mockIndexPattern' }));
const mockUseOnAddFilter = jest.fn();
jest.mock('../../utils/use', () => ({
useIndexPatterns: jest.fn(() => mockUseIndexPatterns),
useOnAddFilter: jest.fn(() => mockUseOnAddFilter),
}));

const mockGetDetailsByField = jest.fn(() => ({
buckets: [1, 2, 3].map((n) => ({
display: `display-${n}`,
value: `value-${n}`,
Expand Down Expand Up @@ -39,7 +46,7 @@ const getFields = (name) => {
describe('visBuilder sidebar field selector', function () {
const defaultProps = {
filterManager: {} as FilterManager,
getDetails: mockGetDetails,
getDetailsByField: mockGetDetailsByField,
header: 'mockHeader',
id: 'mockID',
};
Expand All @@ -53,7 +60,7 @@ describe('visBuilder sidebar field selector', function () {

await fireEvent.click(screen.getByText(defaultProps.header));

expect(mockGetDetails).not.toHaveBeenCalled();
expect(mockGetDetailsByField).not.toHaveBeenCalled();
});

it('renders an accordion with Fields if fields provided', async () => {
Expand All @@ -69,7 +76,7 @@ describe('visBuilder sidebar field selector', function () {

await fireEvent.click(screen.getByText('memory'));

expect(mockGetDetails).toHaveBeenCalledTimes(1);
expect(mockGetDetailsByField).toHaveBeenCalledTimes(1);
});
});
});
Loading

0 comments on commit db5bffb

Please sign in to comment.