Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UnifiedFieldList] Add new fields ingested in background with valid mappings #172329

Merged
merged 45 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
26d3d6d
Add new fields to list with correct types
kertal Nov 30, 2023
7b999db
Improve code
kertal Dec 1, 2023
3a6c239
Remove redundant newFieldsByFieldNameMap
kertal Dec 1, 2023
307a16d
Add multi field support
kertal Dec 4, 2023
54daa2f
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 4, 2023
ca50f4d
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 7, 2023
999a6a9
Add support for Lens
kertal Dec 9, 2023
1a30c70
Merge remote-tracking branch 'origin/unified-field-list-add-new-field…
kertal Dec 9, 2023
fa237a4
Merge remote-tracking branch 'upstream/main' into unified-field-list-…
kertal Dec 12, 2023
15cc582
Add functional test
kertal Dec 13, 2023
3cea884
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 13, 2023
065a50c
Add documentation
kertal Dec 14, 2023
e9fd00e
Improve code
kertal Dec 14, 2023
63907e4
Merge branch 'unified-field-list-add-new-fields' of github.com:kertal…
kertal Dec 14, 2023
454592c
Add Lens test
kertal Dec 15, 2023
75cc235
Improve Lens code preventing an endless loop of update
kertal Dec 15, 2023
12616b4
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 18, 2023
3a74600
Update packages/kbn-unified-field-list/src/hooks/use_existing_fields.ts
kertal Dec 19, 2023
15e5168
Update packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts
kertal Dec 19, 2023
139b651
Update packages/kbn-unified-field-list/src/services/field_existing/fi…
kertal Dec 19, 2023
ab514e4
Update packages/kbn-unified-field-list/src/services/field_existing/fi…
kertal Dec 19, 2023
5cd087f
Merge remote-tracking branch 'upstream/main' into unified-field-list-…
kertal Dec 19, 2023
b424159
Improve code
kertal Dec 20, 2023
9b70dec
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Dec 20, 2023
30dfec6
Add isFieldLensCompatible for new fields in Lens
kertal Dec 20, 2023
f747c2e
Improve multi field code
kertal Dec 21, 2023
9ed0c49
Improve code
kertal Dec 21, 2023
5420c77
Remove redundant field list refreshing
kertal Dec 21, 2023
f340fae
Undo multi field changes
kertal Dec 22, 2023
26c4d72
Address feedback
kertal Dec 22, 2023
ab2c743
Merge remote-tracking branch 'upstream/main' into unified-field-list-…
kertal Dec 22, 2023
921a598
Address review feedback
kertal Dec 22, 2023
ec55f89
Fix error
kertal Dec 22, 2023
c76c87e
Add useMemo for new fields
kertal Dec 22, 2023
165c3cd
Address review feedback
kertal Dec 22, 2023
cbee199
Merge branch 'main' into unified-field-list-add-new-fields
jughosta Jan 2, 2024
9170300
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine Jan 2, 2024
d62dba7
Merge branch 'main' into unified-field-list-add-new-fields
jughosta Jan 3, 2024
02a4537
[Discover] Small fixes for types and memo deps. Add tests.
jughosta Jan 3, 2024
621a1df
Merge pull request #16 from jughosta/unified-field-list-add-new-field…
kertal Jan 8, 2024
8617b3d
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Jan 8, 2024
223f546
Merge branch 'main' into unified-field-list-add-new-fields
kibanamachine Jan 8, 2024
8549c3f
Merge remote-tracking branch 'upstream/main' into unified-field-list-…
kertal Jan 9, 2024
f751a4e
Address review feedback
kertal Jan 9, 2024
d024027
Apply useLatest feedback
kertal Jan 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
useEuiTheme,
} from '@elastic/eui';
import { ToolbarButton } from '@kbn/shared-ux-button-toolbar';
import { type DataViewField } from '@kbn/data-views-plugin/public';
import { DataViewField, 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';
Expand Down Expand Up @@ -191,25 +191,6 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
onSelectedFieldFilter,
]);

useEffect(() => {
if (
searchMode !== 'documents' ||
!useNewFieldsApi ||
stateService.creationOptions.disableMultiFieldsGroupingByParent
) {
setMultiFieldsMap(undefined); // we don't have to calculate multifields in this case
} else {
setMultiFieldsMap(calculateMultiFields(allFields, selectedFieldsState.selectedFieldsMap));
}
}, [
stateService.creationOptions.disableMultiFieldsGroupingByParent,
selectedFieldsState.selectedFieldsMap,
allFields,
useNewFieldsApi,
setMultiFieldsMap,
searchMode,
]);

const popularFieldsLimit = useMemo(
() => core.uiSettings.get(FIELDS_LIMIT_SETTING),
[core.uiSettings]
Expand All @@ -226,24 +207,47 @@ export const UnifiedFieldListSidebarComponent: React.FC<UnifiedFieldListSidebarP
[searchMode, stateService.creationOptions.disableMultiFieldsGroupingByParent]
);

const { fieldListFiltersProps, fieldListGroupedProps } = useGroupedFields<DataViewField>({
dataViewId: (searchMode === 'documents' && dataView?.id) || null, // passing `null` for text-based queries
allFields,
popularFieldsLimit:
searchMode !== 'documents' || stateService.creationOptions.disablePopularFields
? 0
: popularFieldsLimit,
isAffectedByGlobalFilter,
services: {
dataViews,
core,
},
sortedSelectedFields: onSelectedFieldFilter ? undefined : selectedFieldsState.selectedFields,
onSelectedFieldFilter,
onSupportedFieldFilter:
stateService.creationOptions.onSupportedFieldFilter ?? onSupportedFieldFilter,
onOverrideFieldGroupDetails: stateService.creationOptions.onOverrideFieldGroupDetails,
});
const { fieldListFiltersProps, fieldListGroupedProps, allFieldsModified } =
useGroupedFields<DataViewField>({
dataViewId: (searchMode === 'documents' && dataView?.id) || null, // passing `null` for text-based queries
allFields,
popularFieldsLimit:
searchMode !== 'documents' || stateService.creationOptions.disablePopularFields
? 0
: popularFieldsLimit,
isAffectedByGlobalFilter,
services: {
dataViews,
core,
},
sortedSelectedFields: onSelectedFieldFilter ? undefined : selectedFieldsState.selectedFields,
onSelectedFieldFilter,
onSupportedFieldFilter:
stateService.creationOptions.onSupportedFieldFilter ?? onSupportedFieldFilter,
onOverrideFieldGroupDetails: stateService.creationOptions.onOverrideFieldGroupDetails,
getNewFieldsBySpec,
});

useEffect(() => {
Copy link
Member Author

@kertal kertal Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note, this code was just moved to have access to the new modified list of allFields

if (
searchMode !== 'documents' ||
!useNewFieldsApi ||
stateService.creationOptions.disableMultiFieldsGroupingByParent
) {
setMultiFieldsMap(undefined); // we don't have to calculate multifields in this case
} else {
setMultiFieldsMap(
calculateMultiFields(allFieldsModified, selectedFieldsState.selectedFieldsMap)
);
}
}, [
stateService.creationOptions.disableMultiFieldsGroupingByParent,
selectedFieldsState.selectedFieldsMap,
allFieldsModified,
useNewFieldsApi,
setMultiFieldsMap,
searchMode,
]);

const renderFieldItem: FieldListGroupedProps<DataViewField>['renderFieldItem'] = useCallback(
({ field, groupName, groupIndex, itemIndex, fieldSearchHighlight }) => (
Expand Down Expand Up @@ -456,3 +460,7 @@ function calculateMultiFields(
});
return map;
}

function getNewFieldsBySpec(fieldSpecArr: FieldSpec[]): DataViewField[] {
return fieldSpecArr.map((fieldSpec) => new DataViewField(fieldSpec));
}
23 changes: 20 additions & 3 deletions packages/kbn-unified-field-list/src/hooks/use_existing_fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { BehaviorSubject } from 'rxjs';
import type { CoreStart } from '@kbn/core/public';
import type { AggregateQuery, EsQueryConfig, Filter, Query } from '@kbn/es-query';
import { type DataPublicPluginStart } from '@kbn/data-plugin/public';
import type { DataView, DataViewsContract } from '@kbn/data-views-plugin/common';
import type { DataView, DataViewsContract, FieldSpec } from '@kbn/data-views-plugin/common';
import { getEsQueryConfig } from '@kbn/data-service/src/es_query';
import { reportPerformanceMetricEvent } from '@kbn/ebt-tools';
import { loadFieldExisting } from '../services/field_existing';
Expand All @@ -24,6 +24,7 @@ const generateId = htmlIdGenerator();
export interface ExistingFieldsInfo {
fetchStatus: ExistenceFetchStatus;
existingFieldsByFieldNameMap: Record<string, boolean>;
newFields?: FieldSpec[];
numberOfFetches: number;
hasDataViewRestrictions?: boolean;
}
Expand Down Expand Up @@ -54,6 +55,7 @@ export interface ExistingFieldsReader {
hasFieldData: (dataViewId: string, fieldName: string) => boolean;
getFieldsExistenceStatus: (dataViewId: string) => ExistenceFetchStatus;
isFieldsExistenceInfoUnavailable: (dataViewId: string) => boolean;
getNewFields: (dataViewId: string) => FieldSpec[];
}

const initialData: ExistingFieldsByDataViewMap = {};
Expand Down Expand Up @@ -118,7 +120,7 @@ export const useExistingFieldsFetcher = (

setActiveRequests((value) => value + 1);

const hasRestrictions = Boolean(dataView.getAggregationRestrictions?.());
const hasRestrictions = Boolean(dataView?.getAggregationRestrictions?.());
davismcphee marked this conversation as resolved.
Show resolved Hide resolved
const info: ExistingFieldsInfo = {
...unknownInfo,
numberOfFetches,
Expand Down Expand Up @@ -157,6 +159,7 @@ export const useExistingFieldsFetcher = (
}

info.existingFieldsByFieldNameMap = booleanMap(existingFieldNames);
info.newFields = result.newFields;
info.fetchStatus = ExistenceFetchStatus.succeeded;
} catch (error) {
info.fetchStatus = ExistenceFetchStatus.failed;
Expand Down Expand Up @@ -286,6 +289,19 @@ export const useExistingFieldsReader: () => ExistingFieldsReader = () => {
[existingFieldsByDataViewMap]
);

const getNewFields = useCallback(
(dataViewId: string) => {
const info = existingFieldsByDataViewMap[dataViewId];

if (info?.fetchStatus === ExistenceFetchStatus.succeeded) {
return info?.newFields ?? [];
}

return [];
},
[existingFieldsByDataViewMap]
);

const getFieldsExistenceInfo = useCallback(
(dataViewId: string) => {
return dataViewId ? existingFieldsByDataViewMap[dataViewId] : unknownInfo;
Expand Down Expand Up @@ -321,8 +337,9 @@ export const useExistingFieldsReader: () => ExistingFieldsReader = () => {
hasFieldData,
getFieldsExistenceStatus,
isFieldsExistenceInfoUnavailable,
getNewFields,
}),
[hasFieldData, getFieldsExistenceStatus, isFieldsExistenceInfoUnavailable]
[hasFieldData, getFieldsExistenceStatus, isFieldsExistenceInfoUnavailable, getNewFields]
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ describe('UnifiedFieldList useGroupedFields()', () => {
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== props.dataViewId,
getNewFields: () => [],
})
);

Expand Down Expand Up @@ -156,6 +157,7 @@ describe('UnifiedFieldList useGroupedFields()', () => {
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== props.dataViewId,
getNewFields: () => [],
})
);

Expand Down Expand Up @@ -438,6 +440,7 @@ describe('UnifiedFieldList useGroupedFields()', () => {
? ExistenceFetchStatus.succeeded
: ExistenceFetchStatus.unknown,
isFieldsExistenceInfoUnavailable: (dataViewId) => dataViewId !== knownDataViewId,
getNewFields: () => [],
})
);

Expand Down
24 changes: 21 additions & 3 deletions packages/kbn-unified-field-list/src/hooks/use_grouped_fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +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, type DataViewField } from '@kbn/data-views-plugin/common';
import type { DataView, DataViewField, FieldSpec } from '@kbn/data-views-plugin/common';
import { type DataViewsContract } from '@kbn/data-views-plugin/public';
import { useNewFields } from './use_new_fields';
import {
type FieldListGroups,
type FieldsGroup,
Expand Down Expand Up @@ -41,6 +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[];
}

export interface GroupedFieldsResult<T extends FieldListItem> {
Expand All @@ -52,6 +54,8 @@ export interface GroupedFieldsResult<T extends FieldListItem> {
fieldsExistInIndex: boolean;
screenReaderDescriptionId?: string;
};
allFieldsModified: T[] | null; // `null` is for loading indicator
hasNewFields: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it being used somewhere?

Copy link
Member Author

@kertal kertal Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it's here

const { fieldListFiltersProps, fieldListGroupedProps, hasNewFields } =
needed for Lens to indicate there are new fields

}

export function useGroupedFields<T extends FieldListItem = DataViewField>({
Expand All @@ -65,6 +69,7 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
onOverrideFieldGroupDetails,
onSupportedFieldFilter,
onSelectedFieldFilter,
getNewFieldsBySpec,
}: GroupedFieldsParams<T>): GroupedFieldsResult<T> {
const fieldsExistenceReader = useExistingFieldsReader();
const fieldListFilters = useFieldFilters<T>({
Expand All @@ -73,6 +78,7 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
getCustomFieldType,
onSupportedFieldFilter,
});

const onFilterFieldList = fieldListFilters.onFilterField;
const [dataView, setDataView] = useState<DataView | null>(null);
const isAffectedByTimeFilter = Boolean(dataView?.timeFieldName);
Expand Down Expand Up @@ -101,6 +107,13 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
// if field existence information changed, reload the data view too
}, [dataViewId, services.dataViews, setDataView, hasFieldDataHandler]);

const { allFieldsModified, hasNewFields } = useNewFields<T>({
kertal marked this conversation as resolved.
Show resolved Hide resolved
dataView,
allFields,
getNewFieldsBySpec,
fieldsExistenceReader,
});

// important when switching from a known dataViewId to no data view (like in text-based queries)
useEffect(() => {
if (dataView && !dataViewId) {
Expand All @@ -120,13 +133,16 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
};

const selectedFields = sortedSelectedFields || [];
const sortedFields = [...(allFields || [])].sort(sortFields);

const sortedFields = [...(allFieldsModified || [])].sort(sortFields);

const groupedFields = {
...getDefaultFieldGroups(),
...groupBy(sortedFields, (field) => {
if (!sortedSelectedFields && onSelectedFieldFilter && onSelectedFieldFilter(field)) {
selectedFields.push(field);
}

if (onSupportedFieldFilter && !onSupportedFieldFilter(field)) {
return 'skippedFields';
}
Expand Down Expand Up @@ -311,7 +327,7 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({

return fieldGroupDefinitions;
}, [
allFields,
allFieldsModified,
onSupportedFieldFilter,
onSelectedFieldFilter,
onOverrideFieldGroupDetails,
Expand Down Expand Up @@ -381,6 +397,8 @@ export function useGroupedFields<T extends FieldListItem = DataViewField>({
return {
fieldListGroupedProps,
fieldListFiltersProps: fieldListFilters.fieldListFiltersProps,
allFieldsModified,
hasNewFields,
};
}

Expand Down
51 changes: 51 additions & 0 deletions packages/kbn-unified-field-list/src/hooks/use_new_fields.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* 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 { useMemo } from 'react';
import type { DataView, DataViewField } from '@kbn/data-views-plugin/common';
import { ExistingFieldsReader, type FieldListItem, GroupedFieldsParams } from '../..';

/**
* This hook is used to get the new fields of previous fields for wildcards request, and merges those
* with the existing fields.
* @param dataView
* @param allFields
* @param getNewFieldsBySpec
* @param fieldsExistenceReader
*/
export function useNewFields<T extends FieldListItem = DataViewField>({
dataView,
allFields,
getNewFieldsBySpec,
fieldsExistenceReader,
}: {
dataView?: DataView | null;
kertal marked this conversation as resolved.
Show resolved Hide resolved
allFields: GroupedFieldsParams<T>['allFields'];
getNewFieldsBySpec: GroupedFieldsParams<T>['getNewFieldsBySpec'];
fieldsExistenceReader: ExistingFieldsReader;
}) {
const newFields =
allFields && dataView?.id && getNewFieldsBySpec
? getNewFieldsBySpec(fieldsExistenceReader.getNewFields(dataView?.id), dataView)
kertal marked this conversation as resolved.
Show resolved Hide resolved
: null;
const hasNewFields = Boolean(allFields && newFields && newFields.length > 0);

const allFieldsModified = useMemo(() => {
if (!allFields || !newFields?.length) 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;

return newFields ? [...allFieldsExlNew, ...newFields] : allFields;
}, [newFields, allFields]);

return { allFieldsModified, hasNewFields };
}
Loading