From 02a4537c466e228687adaf78d2281740f37faf04 Mon Sep 17 00:00:00 2001 From: Julia Rechkunova Date: Wed, 3 Jan 2024 14:33:18 +0100 Subject: [PATCH] [Discover] Small fixes for types and memo deps. Add tests. --- .../field_list_sidebar.tsx | 2 +- .../src/hooks/use_existing_fields.test.tsx | 48 +++++++++++ .../src/hooks/use_existing_fields.ts | 5 +- .../src/hooks/use_grouped_fields.test.tsx | 78 ++++++++++++++++++ .../src/hooks/use_grouped_fields.ts | 6 +- .../src/hooks/use_new_fields.test.tsx | 80 +++++++++++++++++++ .../src/hooks/use_new_fields.ts | 47 +++++++---- 7 files changed, 243 insertions(+), 23 deletions(-) create mode 100644 packages/kbn-unified-field-list/src/hooks/use_new_fields.test.tsx diff --git a/packages/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx b/packages/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx index 17ebe5b19286f..f8438b0917577 100644 --- a/packages/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx +++ b/packages/kbn-unified-field-list/src/containers/unified_field_list_sidebar/field_list_sidebar.tsx @@ -22,7 +22,7 @@ import { useEuiTheme, } from '@elastic/eui'; import { ToolbarButton } from '@kbn/shared-ux-button-toolbar'; -import { DataViewField, FieldSpec } from '@kbn/data-views-plugin/common'; +import { DataViewField, type FieldSpec } from '@kbn/data-views-plugin/common'; import { getDataViewFieldSubtypeMulti } from '@kbn/es-query/src/utils'; import { FIELDS_LIMIT_SETTING, SEARCH_FIELDS_FROM_SOURCE } from '@kbn/discover-utils'; import { FieldList } from '../../components/field_list'; diff --git a/packages/kbn-unified-field-list/src/hooks/use_existing_fields.test.tsx b/packages/kbn-unified-field-list/src/hooks/use_existing_fields.test.tsx index 7cf22950a4a7d..0af0e4abc8e0d 100644 --- a/packages/kbn-unified-field-list/src/hooks/use_existing_fields.test.tsx +++ b/packages/kbn-unified-field-list/src/hooks/use_existing_fields.test.tsx @@ -125,6 +125,7 @@ describe('UnifiedFieldList useExistingFields', () => { expect(hookReader.result.current.getFieldsExistenceStatus(dataViewId)).toBe( ExistenceFetchStatus.succeeded ); + expect(hookReader.result.current.getNewFields(dataViewId)).toStrictEqual([]); // does not have existence info => works less restrictive const anotherDataViewId = 'test-id'; @@ -140,6 +141,7 @@ describe('UnifiedFieldList useExistingFields', () => { expect(hookReader.result.current.getFieldsExistenceStatus(anotherDataViewId)).toBe( ExistenceFetchStatus.unknown ); + expect(hookReader.result.current.getNewFields(dataViewId)).toStrictEqual([]); }); it('should work correctly with multiple readers', async () => { @@ -217,6 +219,7 @@ describe('UnifiedFieldList useExistingFields', () => { expect(currentResult.isFieldsExistenceInfoUnavailable(dataViewId)).toBe(true); expect(currentResult.hasFieldData(dataViewId, dataView.fields[0].name)).toBe(true); expect(currentResult.getFieldsExistenceStatus(dataViewId)).toBe(ExistenceFetchStatus.failed); + expect(currentResult.getNewFields(dataViewId)).toStrictEqual([]); }); it('should work correctly for multiple data views', async () => { @@ -533,4 +536,49 @@ describe('UnifiedFieldList useExistingFields', () => { expect(params.onNoData).toHaveBeenCalledTimes(1); // still 1 time }); + + it('should include newFields', async () => { + const newFields = [{ name: 'test', type: 'keyword', searchable: true, aggregatable: true }]; + + (ExistingFieldsServiceApi.loadFieldExisting as jest.Mock).mockImplementation( + async ({ dataView: currentDataView }) => { + return { + existingFieldNames: [currentDataView.fields[0].name], + newFields, + }; + } + ); + + const params: ExistingFieldsFetcherParams = { + dataViews: [dataView], + services: mockedServices, + fromDate: '2019-01-01', + toDate: '2020-01-01', + query: { query: '', language: 'lucene' }, + filters: [], + }; + const hookFetcher = renderHook(useExistingFieldsFetcher, { + initialProps: params, + }); + + const hookReader = renderHook(useExistingFieldsReader); + await hookFetcher.waitForNextUpdate(); + + expect(ExistingFieldsServiceApi.loadFieldExisting).toHaveBeenCalledWith( + expect.objectContaining({ + fromDate: '2019-01-01', + toDate: '2020-01-01', + dslQuery, + dataView, + timeFieldName: dataView.timeFieldName, + }) + ); + + expect(hookReader.result.current.getFieldsExistenceStatus(dataView.id!)).toBe( + ExistenceFetchStatus.succeeded + ); + + expect(hookReader.result.current.getNewFields(dataView.id!)).toBe(newFields); + expect(hookReader.result.current.getNewFields('another-id')).toStrictEqual([]); + }); }); diff --git a/packages/kbn-unified-field-list/src/hooks/use_existing_fields.ts b/packages/kbn-unified-field-list/src/hooks/use_existing_fields.ts index a733643008b11..e71d89d6b3626 100644 --- a/packages/kbn-unified-field-list/src/hooks/use_existing_fields.ts +++ b/packages/kbn-unified-field-list/src/hooks/use_existing_fields.ts @@ -20,6 +20,7 @@ import { ExistenceFetchStatus } from '../types'; const getBuildEsQueryAsync = async () => (await import('@kbn/es-query')).buildEsQuery; const generateId = htmlIdGenerator(); +const DEFAULT_EMPTY_NEW_FIELDS: FieldSpec[] = []; export interface ExistingFieldsInfo { fetchStatus: ExistenceFetchStatus; @@ -294,10 +295,10 @@ export const useExistingFieldsReader: () => ExistingFieldsReader = () => { const info = existingFieldsByDataViewMap[dataViewId]; if (info?.fetchStatus === ExistenceFetchStatus.succeeded) { - return info?.newFields ?? []; + return info?.newFields ?? DEFAULT_EMPTY_NEW_FIELDS; } - return []; + return DEFAULT_EMPTY_NEW_FIELDS; }, [existingFieldsByDataViewMap] ); diff --git a/packages/kbn-unified-field-list/src/hooks/use_grouped_fields.test.tsx b/packages/kbn-unified-field-list/src/hooks/use_grouped_fields.test.tsx index 78b34329e0bf0..4a937f86bd5c0 100644 --- a/packages/kbn-unified-field-list/src/hooks/use_grouped_fields.test.tsx +++ b/packages/kbn-unified-field-list/src/hooks/use_grouped_fields.test.tsx @@ -187,6 +187,8 @@ describe('UnifiedFieldList useGroupedFields()', () => { expect(fieldListGroupedProps.fieldsExistenceStatus).toBe(ExistenceFetchStatus.succeeded); expect(fieldListGroupedProps.fieldsExistInIndex).toBe(true); + expect(result.current.allFieldsModified).toBe(allFields); + expect(result.current.hasNewFields).toBe(false); rerender({ ...props, @@ -200,6 +202,82 @@ describe('UnifiedFieldList useGroupedFields()', () => { expect(result.current.fieldListGroupedProps.scrollToTopResetCounter).not.toBe( scrollToTopResetCounter1 ); + expect(result.current.allFieldsModified).toBe(allFields); + expect(result.current.hasNewFields).toBe(false); + + (ExistenceApi.useExistingFieldsReader as jest.Mock).mockRestore(); + }); + + it('should work correctly with new fields', async () => { + const props: GroupedFieldsParams = { + dataViewId: dataView.id!, + allFields, + services: mockedServices, + getNewFieldsBySpec: (spec) => spec.map((field) => new DataViewField(field)), + }; + + const newField = { name: 'test', type: 'keyword', searchable: true, aggregatable: true }; + + jest.spyOn(ExistenceApi, 'useExistingFieldsReader').mockImplementation( + (): ExistingFieldsReader => ({ + hasFieldData: (dataViewId) => { + return dataViewId === props.dataViewId; + }, + getFieldsExistenceStatus: (dataViewId) => + dataViewId === props.dataViewId + ? ExistenceFetchStatus.succeeded + : ExistenceFetchStatus.unknown, + isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== props.dataViewId, + getNewFields: () => [newField], + }) + ); + + const { result, waitForNextUpdate, rerender } = renderHook(useGroupedFields, { + initialProps: props, + }); + + await waitForNextUpdate(); + + let fieldListGroupedProps = result.current.fieldListGroupedProps; + const fieldGroups = fieldListGroupedProps.fieldGroups; + const scrollToTopResetCounter1 = fieldListGroupedProps.scrollToTopResetCounter; + + expect( + Object.keys(fieldGroups!).map( + (key) => `${key}-${fieldGroups![key as FieldsGroupNames]?.fields.length}` + ) + ).toStrictEqual([ + 'SpecialFields-0', + 'SelectedFields-0', + 'PopularFields-0', + 'AvailableFields-25', + 'UnmappedFields-1', + 'EmptyFields-0', + 'MetaFields-3', + ]); + + expect(fieldListGroupedProps.fieldsExistenceStatus).toBe(ExistenceFetchStatus.succeeded); + expect(fieldListGroupedProps.fieldsExistInIndex).toBe(true); + expect(result.current.allFieldsModified).toStrictEqual([ + ...allFields, + new DataViewField(newField), + ]); + expect(result.current.hasNewFields).toBe(true); + + rerender({ + ...props, + dataViewId: null, // for text-based queries + allFields, + }); + + fieldListGroupedProps = result.current.fieldListGroupedProps; + expect(fieldListGroupedProps.fieldsExistenceStatus).toBe(ExistenceFetchStatus.succeeded); + expect(fieldListGroupedProps.fieldsExistInIndex).toBe(true); + expect(result.current.fieldListGroupedProps.scrollToTopResetCounter).not.toBe( + scrollToTopResetCounter1 + ); + expect(result.current.allFieldsModified).toBe(allFields); + expect(result.current.hasNewFields).toBe(false); (ExistenceApi.useExistingFieldsReader as jest.Mock).mockRestore(); }); diff --git a/packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts b/packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts index a11d897c37199..7853c7e67800b 100644 --- a/packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts +++ b/packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts @@ -10,9 +10,9 @@ import { groupBy } from 'lodash'; import { useEffect, useMemo, useState } from 'react'; import { i18n } from '@kbn/i18n'; import { type CoreStart } from '@kbn/core-lifecycle-browser'; -import type { DataView, DataViewField, FieldSpec } from '@kbn/data-views-plugin/common'; +import type { DataView, DataViewField } from '@kbn/data-views-plugin/common'; import { type DataViewsContract } from '@kbn/data-views-plugin/public'; -import { useNewFields } from './use_new_fields'; +import { type UseNewFieldsParams, useNewFields } from './use_new_fields'; import { type FieldListGroups, type FieldsGroup, @@ -42,7 +42,7 @@ export interface GroupedFieldsParams { onOverrideFieldGroupDetails?: OverrideFieldGroupDetails; onSupportedFieldFilter?: (field: T) => boolean; onSelectedFieldFilter?: (field: T) => boolean; - getNewFieldsBySpec?: (fields: FieldSpec[], dataView: DataView | null) => T[]; + getNewFieldsBySpec?: UseNewFieldsParams['getNewFieldsBySpec']; } export interface GroupedFieldsResult { diff --git a/packages/kbn-unified-field-list/src/hooks/use_new_fields.test.tsx b/packages/kbn-unified-field-list/src/hooks/use_new_fields.test.tsx new file mode 100644 index 0000000000000..2be5d50764bc9 --- /dev/null +++ b/packages/kbn-unified-field-list/src/hooks/use_new_fields.test.tsx @@ -0,0 +1,80 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { renderHook } from '@testing-library/react-hooks'; +import { stubLogstashDataView as dataView } from '@kbn/data-views-plugin/common/data_view.stub'; +import { DataViewField } from '@kbn/data-views-plugin/common'; +import { useNewFields, type UseNewFieldsParams } from './use_new_fields'; +import { type ExistingFieldsReader } from './use_existing_fields'; +import { ExistenceFetchStatus } from '../types'; + +const fieldsExistenceReader: ExistingFieldsReader = { + hasFieldData: (dataViewId) => { + return dataViewId === dataView.id; + }, + getFieldsExistenceStatus: (dataViewId) => + dataViewId === dataView.id ? ExistenceFetchStatus.succeeded : ExistenceFetchStatus.unknown, + isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== dataView.id, + getNewFields: () => [], +}; + +describe('UnifiedFieldList useNewFields()', () => { + const allFields = dataView.fields; + + it('should work correctly in loading state', async () => { + const props: UseNewFieldsParams = { + dataView, + allFields: null, + fieldsExistenceReader, + }; + const { result } = renderHook(useNewFields, { + initialProps: props, + }); + + expect(result.current.allFieldsModified).toBe(null); + expect(result.current.hasNewFields).toBe(false); + }); + + it('should work correctly with empty new fields', async () => { + const props: UseNewFieldsParams = { + dataView, + allFields, + fieldsExistenceReader, + }; + const { result } = renderHook(useNewFields, { + initialProps: props, + }); + + expect(result.current.allFieldsModified).toBe(allFields); + expect(result.current.hasNewFields).toBe(false); + }); + + it('should work correctly with new fields', async () => { + const newField = { name: 'test', type: 'keyword', searchable: true, aggregatable: true }; + const newField2 = { ...newField, name: 'test2' }; + const props: UseNewFieldsParams = { + dataView, + allFields, + fieldsExistenceReader: { + ...fieldsExistenceReader, + getNewFields: () => [newField, newField2], + }, + getNewFieldsBySpec: (spec) => spec.map((field) => new DataViewField(field)), + }; + const { result } = renderHook(useNewFields, { + initialProps: props, + }); + + expect(result.current.allFieldsModified).toStrictEqual([ + ...allFields, + new DataViewField(newField), + new DataViewField(newField2), + ]); + expect(result.current.hasNewFields).toBe(true); + }); +}); diff --git a/packages/kbn-unified-field-list/src/hooks/use_new_fields.ts b/packages/kbn-unified-field-list/src/hooks/use_new_fields.ts index ee18f31363fab..51e143cc524c3 100644 --- a/packages/kbn-unified-field-list/src/hooks/use_new_fields.ts +++ b/packages/kbn-unified-field-list/src/hooks/use_new_fields.ts @@ -7,8 +7,22 @@ */ import { useMemo } from 'react'; +import type { FieldSpec } from '@kbn/data-views-plugin/common'; import type { DataView, DataViewField } from '@kbn/data-views-plugin/common'; -import { ExistingFieldsReader, type FieldListItem, GroupedFieldsParams } from '../..'; +import type { FieldListItem } from '../types'; +import type { ExistingFieldsReader } from './use_existing_fields'; + +export interface UseNewFieldsParams { + dataView?: DataView | null; + allFields: T[] | null; // `null` is for loading indicator + getNewFieldsBySpec?: (fields: FieldSpec[], dataView: DataView | null) => T[]; + fieldsExistenceReader: ExistingFieldsReader; +} + +export interface UseNewFieldsResult { + allFieldsModified: T[] | null; + hasNewFields: boolean; +} /** * This hook is used to get the new fields of previous fields for wildcards request, and merges those @@ -19,31 +33,30 @@ export function useNewFields({ allFields, getNewFieldsBySpec, fieldsExistenceReader, -}: { - dataView?: DataView | null; - allFields: GroupedFieldsParams['allFields']; - getNewFieldsBySpec: GroupedFieldsParams['getNewFieldsBySpec']; - fieldsExistenceReader: ExistingFieldsReader; -}) { +}: UseNewFieldsParams): UseNewFieldsResult { + const dataViewId = dataView?.id; + const newFields = useMemo(() => { - return allFields && dataView?.id && getNewFieldsBySpec - ? getNewFieldsBySpec(fieldsExistenceReader.getNewFields(dataView?.id), dataView) - : null; + const newLoadedFields = + allFields && dataView?.id && getNewFieldsBySpec + ? getNewFieldsBySpec(fieldsExistenceReader.getNewFields(dataView?.id), dataView) + : null; + + return newLoadedFields?.length ? newLoadedFields : null; }, [allFields, dataView, fieldsExistenceReader, getNewFieldsBySpec]); const hasNewFields = Boolean(allFields && newFields && newFields.length > 0); const allFieldsModified = useMemo(() => { - if (!allFields || !newFields?.length || !dataView) return allFields; + if (!allFields || !newFields?.length || !dataViewId) return allFields; // Filtering out fields that e.g. Discover provides with fields that were provided by the previous fieldsForWildcards request // These can be replaced by the new fields, which are mapped correctly, and therefore can be used in the right way - const allFieldsExlNew = - allFields && newFields - ? allFields.filter((field) => !newFields.some((newField) => newField.name === field.name)) - : allFields; + const allFieldsExlNew = allFields.filter( + (field) => !newFields.some((newField) => newField.name === field.name) + ); - return newFields ? [...allFieldsExlNew, ...newFields] : allFields; - }, [newFields, allFields, dataView]); + return [...allFieldsExlNew, ...newFields]; + }, [newFields, allFields, dataViewId]); return { allFieldsModified, hasNewFields }; }