[Discover] Small fixes for types and memo deps. Add tests.

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';

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([]);
+  });
 });

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]
   );

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<DataViewField> = {
+      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();
   });

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<T extends FieldListItem> {
   onOverrideFieldGroupDetails?: OverrideFieldGroupDetails;
   onSupportedFieldFilter?: (field: T) => boolean;
   onSelectedFieldFilter?: (field: T) => boolean;
-  getNewFieldsBySpec?: (fields: FieldSpec[], dataView: DataView | null) => T[];
+  getNewFieldsBySpec?: UseNewFieldsParams<T>['getNewFieldsBySpec'];
 }
 
 export interface GroupedFieldsResult<T extends FieldListItem> {

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<DataViewField> = {
+      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<DataViewField> = {
+      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<DataViewField> = {
+      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);
+  });
+});

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<T extends FieldListItem> {
+  dataView?: DataView | null;
+  allFields: T[] | null; // `null` is for loading indicator
+  getNewFieldsBySpec?: (fields: FieldSpec[], dataView: DataView | null) => T[];
+  fieldsExistenceReader: ExistingFieldsReader;
+}
+
+export interface UseNewFieldsResult<T extends FieldListItem> {
+  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<T extends FieldListItem = DataViewField>({
   allFields,
   getNewFieldsBySpec,
   fieldsExistenceReader,
-}: {
-  dataView?: DataView | null;
-  allFields: GroupedFieldsParams<T>['allFields'];
-  getNewFieldsBySpec: GroupedFieldsParams<T>['getNewFieldsBySpec'];
-  fieldsExistenceReader: ExistingFieldsReader;
-}) {
+}: UseNewFieldsParams<T>): UseNewFieldsResult<T> {
+  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 };
 }