Skip to content

Commit

Permalink
[One Discover] Fix document comparison mode and the field statistics …
Browse files Browse the repository at this point in the history
…tab when using Smart Fields (#184172)

## Summary

This addresses the bugs listed in
#181674
(#181621 and
#178970).

Smart Fields are compound fields, as such there is no "real" backing
field for them. In these scenarios where we can't use them directly we
fall back to the `fallbackFields` configured for them.

[@amyjtechwriter please could you check the
copy](#181621 (comment)).
I think `derived from...` would have sounded better, but it felt at odds
with the scenario where you might specifically pick a field that is also
a fallback field for a smart field.

## Reviewer notes

- There were quite a lot of cross cutting concerns here between the
`unified-field-list`, `unified-data-table`, core Discover code,
`discover-utils`, and the `data_vizualizer` plugin for the field
statistics table. Where I'd have liked to potentially put more things in
the discover-utils it wasn't possible due to circular project
references.

- I've not added any functional tests for now as we'd be adding them to
the Observability Logs Explorer which is going to be deprecated in due
course. This ties in with part three of
#181674.

- A `renderFieldName` prop has been added to the field statistics table,
this seemed to be the least intrusive way forward rather than a new /
comprehensive cell rendering change (similar to the external cell
renderers).

- I've used the original
[`fallbackFields`](https://github.com/elastic/kibana/blob/main/src/plugins/discover/common/data_types/logs/constants.ts#L35)
configurations but the [virtual column components actually render a bit
extra](https://github.com/elastic/kibana/blob/main/packages/kbn-discover-utils/src/utils/get_message_field_with_fallbacks.ts#L13).
Am I missing anything with regards to potentially extending the
`fallbackFields` definitions?

## Screenshots

<img width="1253" alt="Screenshot 2024-05-23 at 20 25 57"
src="https://github.com/elastic/kibana/assets/471693/c2982f45-c8fa-4caa-935d-7007745ae2ce">

<img width="1477" alt="Screenshot 2024-05-23 at 20 26 21"
src="https://github.com/elastic/kibana/assets/471693/3145fbd1-071e-42fd-a94d-beb15c29c739">

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Davis McPhee <davis.mcphee@elastic.co>
  • Loading branch information
3 people committed Jun 3, 2024
1 parent 76837c0 commit 105ba94
Show file tree
Hide file tree
Showing 25 changed files with 492 additions and 36 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* 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 { DataViewField } from '@kbn/data-views-plugin/common';

const smartFields = [
new DataViewField({
name: 'content',
type: 'smart_field',
searchable: false,
aggregatable: false,
}),
];
const fallbackFields = {
content: ['message'],
};

export const additionalFieldGroups = {
smartFields,
fallbackFields,
};
1 change: 1 addition & 0 deletions packages/kbn-discover-utils/src/__mocks__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@

export * from './data_view';
export * from './es_hits';
export * from './additional_field_groups';
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import type { DataView } from '@kbn/data-views-plugin/common';
import type { DataTableRecord } from '@kbn/discover-utils/types';
import type { FieldFormatsStart } from '@kbn/field-formats-plugin/public';
import { AdditionalFieldGroups } from '@kbn/unified-field-list';
import { memoize } from 'lodash';
import React, { useMemo, useState } from 'react';
import useLocalStorage from 'react-use/lib/useLocalStorage';
Expand Down Expand Up @@ -49,6 +50,7 @@ export interface CompareDocumentsProps {
getDocById: (id: string) => DataTableRecord | undefined;
setSelectedDocs: (selectedDocs: string[]) => void;
setIsCompareActive: (isCompareActive: boolean) => void;
additionalFieldGroups?: AdditionalFieldGroups;
}

const COMPARISON_ROW_HEIGHT: EuiDataGridRowHeightsOptions = { defaultHeight: 'auto' };
Expand All @@ -66,6 +68,7 @@ const CompareDocuments = ({
dataView,
isPlainRecord,
selectedFieldNames,
additionalFieldGroups,
selectedDocs,
schemaDetectors,
forceShowAllFields,
Expand Down Expand Up @@ -100,6 +103,7 @@ const CompareDocuments = ({
const { comparisonFields, totalFields } = useComparisonFields({
dataView,
selectedFieldNames,
additionalFieldGroups,
selectedDocs,
showAllFields: Boolean(forceShowAllFields || showAllFields),
showMatchingValues: Boolean(showMatchingValues),
Expand Down Expand Up @@ -184,6 +188,7 @@ const CompareDocuments = ({
diffMode: showDiff ? diffMode : undefined,
fieldFormats,
getDocById: memoizedGetDocById,
additionalFieldGroups,
});
const comparisonCss = useComparisonCss({
diffMode: showDiff ? diffMode : undefined,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import { EuiDataGridCellValueElementProps, EuiDataGridSetCellProps } from '@elastic/eui';
import { buildDataTableRecord } from '@kbn/discover-utils';
import { generateEsHits } from '@kbn/discover-utils/src/__mocks__';
import { generateEsHits, additionalFieldGroups } from '@kbn/discover-utils/src/__mocks__';
import { fieldFormatsMock } from '@kbn/field-formats-plugin/common/mocks';
import { render, screen } from '@testing-library/react';
import { renderHook } from '@testing-library/react-hooks';
Expand Down Expand Up @@ -443,4 +443,19 @@ describe('useComparisonCellValue', () => {
renderComparisonCell(cellProps6);
expect(calculateDiff).toHaveBeenCalledTimes(6);
});
it('should render a tooltip when the field is derived from a Smart Field', async () => {
const { renderCellValue } = renderComparisonCellValue({
comparisonFields: ['message'],
additionalFieldGroups,
});
const baseCell = renderComparisonCell({
columnId: fieldColumnId,
colIndex: 0,
rowIndex: 0,
renderCellValue,
});

expect(await screen.findByTestId('smartFieldFallbackTooltipIcon')).toBeInTheDocument();
expect(baseCell.getCell()).toMatchSnapshot();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ import type { DataTableRecord } from '@kbn/discover-utils/types';
import type { FieldFormatsStart } from '@kbn/field-formats-plugin/public';
import { getFieldIconProps } from '@kbn/field-utils';
import { FieldIcon } from '@kbn/react-field';
import {
AdditionalFieldGroups,
getAllFallbackFields,
getAssociatedSmartFieldsAsString,
SmartFieldFallbackTooltip,
} from '@kbn/unified-field-list';
import classNames from 'classnames';
import { isEqual, memoize } from 'lodash';
import React, { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react';
Expand All @@ -37,6 +43,7 @@ export interface UseComparisonCellValueProps {
diffMode: DocumentDiffMode | undefined;
fieldFormats: FieldFormatsStart;
getDocById: (id: string) => DataTableRecord | undefined;
additionalFieldGroups?: AdditionalFieldGroups;
}

export const useComparisonCellValue = ({
Expand All @@ -47,6 +54,7 @@ export const useComparisonCellValue = ({
diffMode,
fieldFormats,
getDocById,
additionalFieldGroups,
}: UseComparisonCellValueProps) => {
const baseDocId = selectedDocs[0];
const baseDoc = useMemo(() => getDocById(baseDocId)?.flattened, [baseDocId, getDocById]);
Expand All @@ -64,11 +72,13 @@ export const useComparisonCellValue = ({
diffMode={diffMode}
fieldFormats={fieldFormats}
getDocById={getDocById}
additionalFieldGroups={additionalFieldGroups}
{...props}
/>
</DiffProvider>
),
[
additionalFieldGroups,
baseDoc,
baseDocId,
calculateDiffMemoized,
Expand All @@ -86,18 +96,37 @@ type CellValueProps = Omit<UseComparisonCellValueProps, 'selectedDocs'> &
EuiDataGridCellValueElementProps & {
baseDocId: string;
baseDoc: DataTableRecord['flattened'] | undefined;
additionalFieldGroups?: AdditionalFieldGroups;
};

const EMPTY_VALUE = '-';

const CellValue = (props: CellValueProps) => {
const { dataView, comparisonFields, fieldColumnId, rowIndex, columnId, getDocById } = props;
const {
dataView,
comparisonFields,
fieldColumnId,
rowIndex,
columnId,
getDocById,
additionalFieldGroups,
} = props;
const fieldName = comparisonFields[rowIndex];
const field = useMemo(() => dataView.fields.getByName(fieldName), [dataView.fields, fieldName]);
const comparisonDoc = useMemo(() => getDocById(columnId), [columnId, getDocById]);

const allFallbackFields = useMemo(
() => getAllFallbackFields(additionalFieldGroups),
[additionalFieldGroups]
);
if (columnId === fieldColumnId) {
return <FieldCellValue field={field} fieldName={fieldName} />;
return (
<FieldCellValue
field={field}
fieldName={fieldName}
additionalFieldGroups={additionalFieldGroups}
allFallbackFields={allFallbackFields}
/>
);
}

if (!comparisonDoc) {
Expand All @@ -114,7 +143,24 @@ interface FieldCellValueProps {
fieldName: string;
}

const FieldCellValue = ({ field, fieldName }: FieldCellValueProps) => {
const FieldCellValue = ({
field,
fieldName,
additionalFieldGroups,
allFallbackFields,
}: FieldCellValueProps & {
additionalFieldGroups?: AdditionalFieldGroups;
allFallbackFields: string[]; // NOTE: Used purely as an optimisation to avoid looking up Smart Field names unless needed.
}) => {
const isDerivedAsPartOfSmartField = allFallbackFields.includes(fieldName);
const associatedSmartFields = useMemo(
() =>
isDerivedAsPartOfSmartField
? getAssociatedSmartFieldsAsString(fieldName, additionalFieldGroups)
: '',
[isDerivedAsPartOfSmartField, fieldName, additionalFieldGroups]
);

return (
<EuiFlexGroup responsive={false} gutterSize="s">
{field && (
Expand All @@ -129,6 +175,12 @@ const FieldCellValue = ({ field, fieldName }: FieldCellValueProps) => {
data-test-subj="unifiedDataTableComparisonFieldName"
>
{field?.displayName ?? fieldName}
{isDerivedAsPartOfSmartField ? (
<>
{' '}
<SmartFieldFallbackTooltip associatedSmartFields={associatedSmartFields} />
</>
) : null}
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import type { DataView } from '@kbn/data-views-plugin/common';
import type { DataTableRecord } from '@kbn/discover-utils/types';
import { AdditionalFieldGroups, convertFieldsToFallbackFields } from '@kbn/unified-field-list';
import { isEqual } from 'lodash';
import { useMemo } from 'react';

Expand All @@ -20,6 +21,7 @@ export interface UseComparisonFieldsProps {
showAllFields: boolean;
showMatchingValues: boolean;
getDocById: (id: string) => DataTableRecord | undefined;
additionalFieldGroups?: AdditionalFieldGroups;
}

export const useComparisonFields = ({
Expand All @@ -29,6 +31,7 @@ export const useComparisonFields = ({
showAllFields,
showMatchingValues,
getDocById,
additionalFieldGroups,
}: UseComparisonFieldsProps) => {
const { baseDoc, comparisonDocs } = useMemo(() => {
const [baseDocId, ...comparisonDocIds] = selectedDocs;
Expand All @@ -42,7 +45,10 @@ export const useComparisonFields = ({
}, [getDocById, selectedDocs]);

return useMemo(() => {
let comparisonFields = selectedFieldNames;
let comparisonFields = convertFieldsToFallbackFields({
fields: selectedFieldNames,
additionalFieldGroups,
});

if (showAllFields) {
const sortedFields = dataView.fields
Expand Down Expand Up @@ -79,5 +85,13 @@ export const useComparisonFields = ({
}

return { comparisonFields, totalFields };
}, [baseDoc, comparisonDocs, dataView, selectedFieldNames, showAllFields, showMatchingValues]);
}, [
additionalFieldGroups,
baseDoc,
comparisonDocs,
dataView,
selectedFieldNames,
showAllFields,
showMatchingValues,
]);
};
7 changes: 7 additions & 0 deletions packages/kbn-unified-data-table/src/components/data_table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import type { FieldFormatsStart } from '@kbn/field-formats-plugin/public';
import type { ThemeServiceStart } from '@kbn/react-kibana-context-common';
import type { DataPublicPluginStart } from '@kbn/data-plugin/public';
import type { DocViewFilterFn } from '@kbn/unified-doc-viewer/types';
import { AdditionalFieldGroups } from '@kbn/unified-field-list';
import {
UnifiedDataTableSettings,
ValueToStringConverter,
Expand Down Expand Up @@ -338,6 +339,10 @@ export interface UnifiedDataTableProps {
* An optional settings for a specified fields rendering like links. Applied only for the listed fields rendering.
*/
externalCustomRenderers?: CustomCellRenderer;
/**
* An optional prop to provide awareness of additional field groups when paired with the Unified Field List.
*/
additionalFieldGroups?: AdditionalFieldGroups;
/**
* An optional settings for customising the column
*/
Expand Down Expand Up @@ -441,6 +446,7 @@ export const UnifiedDataTable = ({
rowsPerPageOptions,
visibleCellActions,
externalCustomRenderers,
additionalFieldGroups,
consumer = 'discover',
componentsTourSteps,
gridStyleOverride,
Expand Down Expand Up @@ -1043,6 +1049,7 @@ export const UnifiedDataTable = ({
dataView={dataView}
isPlainRecord={isPlainRecord}
selectedFieldNames={visibleColumns}
additionalFieldGroups={additionalFieldGroups}
selectedDocs={selectedDocs}
schemaDetectors={schemaDetectors}
forceShowAllFields={defaultColumns}
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-unified-data-table/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@
"@kbn/field-utils",
"@kbn/react-field",
"@kbn/shared-ux-utility",
"@kbn/unified-field-list",
]
}
5 changes: 5 additions & 0 deletions packages/kbn-unified-field-list/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ export type {
BucketedAggregation,
NumberSummary,
AddFieldFilterHandler,
FieldsGroup,
FieldListGroups,
FieldsGroupDetails,
FieldListItem,
GetCustomFieldType,
RenderFieldItemParams,
SearchMode,
AdditionalFieldGroups,
} from './src/types';
export { ExistenceFetchStatus, FieldsGroupNames } from './src/types';

Expand Down Expand Up @@ -89,3 +91,6 @@ export {
type UnifiedFieldListSidebarContainerApi,
type UnifiedFieldListSidebarContainerProps,
} from './src/containers/unified_field_list_sidebar';

export * from './src/utils/fallback_fields';
export { SmartFieldFallbackTooltip } from './src/components/fallback_fields';
Loading

0 comments on commit 105ba94

Please sign in to comment.