From 9765492042ea3bc32ceba8b86294b6247891a639 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 11 Oct 2021 17:35:24 +0200 Subject: [PATCH 01/43] WIP replacing indexPattern.flattenHit by tabify --- .../__snapshots__/tabify_docs.test.ts.snap | 176 ------------------ .../data/common/search/tabify/index.ts | 2 +- .../common/search/tabify/tabify_docs.test.ts | 2 +- .../data/common/search/tabify/tabify_docs.ts | 69 ++++++- .../data_views/common/data_views/data_view.ts | 3 + .../doc_table/components/table_row.tsx | 9 +- .../discover_grid/discover_grid.tsx | 9 +- .../discover_grid_cell_actions.tsx | 6 +- .../discover_grid/discover_grid_context.tsx | 2 + .../discover_grid_document_selection.test.tsx | 9 + .../components/table/table.test.tsx | 4 +- .../application/components/table/table.tsx | 9 +- .../generate_csv/generate_csv.ts | 6 +- 13 files changed, 110 insertions(+), 196 deletions(-) diff --git a/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap b/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap index 22276335a0599..a670dcfaff4e3 100644 --- a/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap +++ b/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap @@ -155,53 +155,9 @@ Object { }, "name": "sourceTest", }, - Object { - "id": "_id", - "meta": Object { - "field": "_id", - "index": "test-index", - "params": undefined, - "type": "string", - }, - "name": "_id", - }, - Object { - "id": "_index", - "meta": Object { - "field": "_index", - "index": "test-index", - "params": undefined, - "type": "string", - }, - "name": "_index", - }, - Object { - "id": "_score", - "meta": Object { - "field": "_score", - "index": "test-index", - "params": undefined, - "type": "number", - }, - "name": "_score", - }, - Object { - "id": "_type", - "meta": Object { - "field": "_type", - "index": "test-index", - "params": undefined, - "type": "string", - }, - "name": "_type", - }, ], "rows": Array [ Object { - "_id": "hit-id-value", - "_index": "hit-index-value", - "_score": 77, - "_type": "hit-type-value", "fieldTest": 123, "invalidMapping": 345, "nested": Array [ @@ -263,53 +219,9 @@ Object { }, "name": "sourceTest", }, - Object { - "id": "_id", - "meta": Object { - "field": "_id", - "index": "test-index", - "params": undefined, - "type": "string", - }, - "name": "_id", - }, - Object { - "id": "_index", - "meta": Object { - "field": "_index", - "index": "test-index", - "params": undefined, - "type": "string", - }, - "name": "_index", - }, - Object { - "id": "_score", - "meta": Object { - "field": "_score", - "index": "test-index", - "params": undefined, - "type": "number", - }, - "name": "_score", - }, - Object { - "id": "_type", - "meta": Object { - "field": "_type", - "index": "test-index", - "params": undefined, - "type": "string", - }, - "name": "_type", - }, ], "rows": Array [ Object { - "_id": "hit-id-value", - "_index": "hit-index-value", - "_score": 77, - "_type": "hit-type-value", "fieldTest": 123, "invalidMapping": 345, "nested": Array [ @@ -371,53 +283,9 @@ Object { }, "name": "sourceTest", }, - Object { - "id": "_id", - "meta": Object { - "field": "_id", - "index": "test-index", - "params": undefined, - "type": "string", - }, - "name": "_id", - }, - Object { - "id": "_index", - "meta": Object { - "field": "_index", - "index": "test-index", - "params": undefined, - "type": "string", - }, - "name": "_index", - }, - Object { - "id": "_score", - "meta": Object { - "field": "_score", - "index": "test-index", - "params": undefined, - "type": "number", - }, - "name": "_score", - }, - Object { - "id": "_type", - "meta": Object { - "field": "_type", - "index": "test-index", - "params": undefined, - "type": "string", - }, - "name": "_type", - }, ], "rows": Array [ Object { - "_id": "hit-id-value", - "_index": "hit-index-value", - "_score": 77, - "_type": "hit-type-value", "fieldTest": 123, "invalidMapping": 345, "nested": Array [ @@ -475,53 +343,9 @@ Object { }, "name": "sourceTest", }, - Object { - "id": "_id", - "meta": Object { - "field": "_id", - "index": undefined, - "params": undefined, - "type": "string", - }, - "name": "_id", - }, - Object { - "id": "_index", - "meta": Object { - "field": "_index", - "index": undefined, - "params": undefined, - "type": "string", - }, - "name": "_index", - }, - Object { - "id": "_score", - "meta": Object { - "field": "_score", - "index": undefined, - "params": undefined, - "type": "number", - }, - "name": "_score", - }, - Object { - "id": "_type", - "meta": Object { - "field": "_type", - "index": undefined, - "params": undefined, - "type": "string", - }, - "name": "_type", - }, ], "rows": Array [ Object { - "_id": "hit-id-value", - "_index": "hit-index-value", - "_score": 77, - "_type": "hit-type-value", "fieldTest": 123, "invalidMapping": 345, "nested": Array [ diff --git a/src/plugins/data/common/search/tabify/index.ts b/src/plugins/data/common/search/tabify/index.ts index 74fbc7ba4cfa4..279ff705f231c 100644 --- a/src/plugins/data/common/search/tabify/index.ts +++ b/src/plugins/data/common/search/tabify/index.ts @@ -6,6 +6,6 @@ * Side Public License, v 1. */ -export { tabifyDocs } from './tabify_docs'; +export { tabifyDocs, flattenHit } from './tabify_docs'; export { tabifyAggResponse } from './tabify'; export { tabifyGetColumns } from './get_columns'; diff --git a/src/plugins/data/common/search/tabify/tabify_docs.test.ts b/src/plugins/data/common/search/tabify/tabify_docs.test.ts index 1a3f7195b722e..dfe5baf10dcbc 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.test.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.test.ts @@ -65,7 +65,7 @@ describe('tabifyDocs', () => { }); it('combines meta fields if meta option is set', () => { - const table = tabifyDocs(response, index, { meta: true }); + const table = tabifyDocs(response, index, { meta: ['_id', '_index', '_score', '_type'] }); expect(table).toMatchSnapshot(); }); diff --git a/src/plugins/data/common/search/tabify/tabify_docs.ts b/src/plugins/data/common/search/tabify/tabify_docs.ts index 8e628e7741df5..3e0a3c1d22eda 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.ts @@ -11,18 +11,54 @@ import { isPlainObject } from 'lodash'; import { IndexPattern } from '../..'; import { Datatable, DatatableColumn, DatatableColumnType } from '../../../../expressions/common'; +// TODO: complete this list +type ValidMetaFieldNames = keyof Pick< + estypes.SearchHit, + '_id' | '_index' | '_source' | '_type' | '_score' +>; +const VALID_META_FIELD_NAMES: ValidMetaFieldNames[] = [ + '_id', + '_index', + '_source', + '_type', + '_score', +]; + +function isValidMetaFieldName(field: string): field is ValidMetaFieldNames { + // TODO: Leave comment why to cast here + return (VALID_META_FIELD_NAMES as string[]).includes(field); +} + export interface TabifyDocsOptions { shallow?: boolean; + /** + * If set to `false` the _source of the document, if requested, won't be + * merged into the flattened document. + */ source?: boolean; - meta?: boolean; + /** + * Specify a list of meta field names that should be merged from the top + * level of the hit (i.e. not nested under _source or fields) into the flattened version. + * If not specified no meta fields (like _id, _index, etc.) will be merged. + */ + meta?: string[]; } +/** + * Flattens an individual hit (from an ES response) into an object. This will + * create flattened field names, like `user.name`. + * + * @param hit The hit from an ES reponse's hits.hits[] + * @param indexPattern The index pattern for the requested index if available. + * @param params Parameters how to flatten the hit + */ export function flattenHit( hit: estypes.SearchHit, indexPattern?: IndexPattern, params?: TabifyDocsOptions ) { const flat = {} as Record; + const metaFields = params?.meta ?? []; function flatten(obj: Record, keyPrefix: string = '') { for (const [k, val] of Object.entries(obj)) { @@ -62,13 +98,32 @@ export function flattenHit( if (params?.source !== false && hit._source) { flatten(hit._source as Record); } - if (params?.meta !== false) { - // combine the fields that Discover allows to add as columns - const { _id, _index, _type, _score } = hit; - flatten({ _id, _index, _score, _type }); - } - return flat; + metaFields.forEach((metaFieldName) => { + if (!isValidMetaFieldName(metaFieldName) || metaFieldName === '_source') { + return; + } + flat[metaFieldName] = hit[metaFieldName]; + }); + + return new Proxy(flat, { + ownKeys: (target) => { + return Reflect.ownKeys(target).sort((a, b) => { + const aIsMeta = metaFields.includes(String(a)); + const bIsMeta = metaFields.includes(String(b)); + if (aIsMeta && bIsMeta) { + return String(a).localeCompare(String(b)); + } + if (aIsMeta) { + return 1; + } + if (bIsMeta) { + return -1; + } + return String(a).localeCompare(String(b)); + }); + }, + }); } export const tabifyDocs = ( diff --git a/src/plugins/data_views/common/data_views/data_view.ts b/src/plugins/data_views/common/data_views/data_view.ts index 00b96cda32ad7..2122eac151b3d 100644 --- a/src/plugins/data_views/common/data_views/data_view.ts +++ b/src/plugins/data_views/common/data_views/data_view.ts @@ -69,6 +69,9 @@ export class DataView implements IIndexPattern { formatField: FormatFieldFn; }; public formatField: FormatFieldFn; + /** + * @deprecated Use `flattenHit` utility method exported from data plugin instead. + */ public flattenHit: (hit: Record, deep?: boolean) => Record; public metaFields: string[]; /** diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx index 8d56f2adeaf65..d4a29f46fdb22 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx @@ -10,6 +10,7 @@ import React, { Fragment, useCallback, useMemo, useState } from 'react'; import classNames from 'classnames'; import { i18n } from '@kbn/i18n'; import { EuiButtonEmpty, EuiIcon } from '@elastic/eui'; +import { flattenHit, META_FIELDS } from '../../../../../../../../data/common'; import { DocViewer } from '../../../../../components/doc_viewer/doc_viewer'; import { FilterManager, IndexPattern } from '../../../../../../../../data/public'; import { TableCell } from './table_row/table_cell'; @@ -18,6 +19,7 @@ import { getContextUrl } from '../../../../../helpers/get_context_url'; import { getSingleDocUrl } from '../../../../../helpers/get_single_doc_url'; import { TableRowDetails } from './table_row_details'; import { formatRow, formatTopLevelObject } from '../lib/row_formatter'; +import { getServices } from '../../../../../../kibana_services'; export type DocTableRow = ElasticSearchHit & { isAnchor?: boolean; @@ -50,6 +52,8 @@ export const TableRow = ({ filterManager, addBasePath, }: TableRowProps) => { + const { uiSettings } = getServices(); + const [open, setOpen] = useState(false); const docTableRowClassName = classNames('kbnDocTable__row', { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -57,7 +61,10 @@ export const TableRow = ({ }); const anchorDocTableRowSubj = row.isAnchor ? ' docTableAnchorRow' : ''; - const flattenedRow = useMemo(() => indexPattern.flattenHit(row), [indexPattern, row]); + const flattenedRow = useMemo( + () => flattenHit(row, indexPattern, { meta: uiSettings.get(META_FIELDS) }), + [indexPattern, row, uiSettings] + ); const mapping = useMemo(() => indexPattern.fields.getByName, [indexPattern]); // toggle display of the rows details, a full list of the fields from each row diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx index 0fe506b3b8537..3eccb9f5eda71 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx @@ -21,7 +21,7 @@ import { EuiLoadingSpinner, EuiIcon, } from '@elastic/eui'; -import type { IndexPattern } from 'src/plugins/data/common'; +import { flattenHit, IndexPattern, META_FIELDS } from '../../../../../data/common'; import { DocViewFilterFn, ElasticSearchHit } from '../../doc_views/doc_views_types'; import { getSchemaDetectors } from './discover_grid_schema'; import { DiscoverGridFlyout } from './discover_grid_flyout'; @@ -271,7 +271,11 @@ export const DiscoverGrid = ({ getRenderCellValueFn( indexPattern, displayedRows, - displayedRows ? displayedRows.map((hit) => indexPattern.flattenHit(hit)) : [], + displayedRows + ? displayedRows.map((hit) => + flattenHit(hit, indexPattern, { meta: services.uiSettings.get(META_FIELDS) }) + ) + : [], useNewFieldsApi, fieldsToShow, services.uiSettings.get(MAX_DOC_FIELDS_DISPLAYED) @@ -373,6 +377,7 @@ export const DiscoverGrid = ({ setIsFilterActive(false); } }, + uiSettings: services.uiSettings, }} > { const row = context.rows[rowIndex]; - const flattened = context.indexPattern.flattenHit(row); + const flattened = flattenHit(row, context.indexPattern, { + meta: context.uiSettings.get(META_FIELDS), + }); if (flattened) { context.onFilter(columnId, flattened[columnId], '+'); diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid_context.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid_context.tsx index 8d0fbec9d7933..1451cfea197a6 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid_context.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid_context.tsx @@ -7,6 +7,7 @@ */ import React from 'react'; +import { IUiSettingsClient } from 'src/core/public'; import type { IndexPattern } from 'src/plugins/data/common'; import { DocViewFilterFn, ElasticSearchHit } from '../../doc_views/doc_views_types'; @@ -19,6 +20,7 @@ export interface GridContext { isDarkMode: boolean; selectedDocs: string[]; setSelectedDocs: (selected: string[]) => void; + uiSettings: IUiSettingsClient; } const defaultContext = {} as unknown as GridContext; diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx index 41cf3f5a68edb..7e6c4725d63f9 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx @@ -16,6 +16,8 @@ import { import { esHits } from '../../../__mocks__/es_hits'; import { indexPatternMock } from '../../../__mocks__/index_pattern'; import { DiscoverGridContext } from './discover_grid_context'; +import { META_FIELDS } from 'src/plugins/data/common'; +import { IUiSettingsClient } from 'kibana/public'; describe('document selection', () => { describe('getDocId', () => { @@ -47,6 +49,13 @@ describe('document selection', () => { isDarkMode: false, selectedDocs: [], setSelectedDocs: jest.fn(), + uiSettings: { + get: jest.fn((key) => { + if (key === META_FIELDS) { + return ['_id', '_index']; + } + }), + } as unknown as IUiSettingsClient, }; const component = mountWithIntl( diff --git a/src/plugins/discover/public/application/components/table/table.test.tsx b/src/plugins/discover/public/application/components/table/table.test.tsx index 3f010d9d07737..2f779f8d55566 100644 --- a/src/plugins/discover/public/application/components/table/table.test.tsx +++ b/src/plugins/discover/public/application/components/table/table.test.tsx @@ -65,7 +65,7 @@ const indexPattern = { ], }, metaFields: ['_index', '_score'], - flattenHit: undefined, + flattenHit: jest.fn(), formatHit: jest.fn((hit) => hit._source), } as unknown as IndexPattern; @@ -73,8 +73,6 @@ indexPattern.fields.getByName = (name: string) => { return indexPattern.fields.getAll().find((field) => field.name === name); }; -indexPattern.flattenHit = indexPatterns.flattenHitWrapper(indexPattern, indexPattern.metaFields); - const mountComponent = (props: DocViewerTableProps) => { return mountWithIntl(); }; diff --git a/src/plugins/discover/public/application/components/table/table.tsx b/src/plugins/discover/public/application/components/table/table.tsx index eab3ba6e3d29a..501a38b97cfaf 100644 --- a/src/plugins/discover/public/application/components/table/table.tsx +++ b/src/plugins/discover/public/application/components/table/table.tsx @@ -9,6 +9,7 @@ import React, { useCallback, useMemo } from 'react'; import { EuiInMemoryTable } from '@elastic/eui'; import { IndexPattern, IndexPatternField } from '../../../../../data/public'; +import { flattenHit, META_FIELDS } from '../../../../../data/common'; import { SHOW_MULTIFIELDS } from '../../../../common'; import { getServices } from '../../../kibana_services'; import { isNestedFieldParent } from '../../apps/main/utils/nested_fields'; @@ -56,7 +57,8 @@ export const DocViewerTable = ({ onAddColumn, onRemoveColumn, }: DocViewRenderProps) => { - const showMultiFields = getServices().uiSettings.get(SHOW_MULTIFIELDS); + const { uiSettings } = getServices(); + const showMultiFields = uiSettings.get(SHOW_MULTIFIELDS); const mapping = useCallback( (name: string) => indexPattern?.fields.getByName(name), @@ -95,7 +97,10 @@ export const DocViewerTable = ({ return null; } - const flattened = indexPattern?.flattenHit(hit); + const flattened = flattenHit(hit, indexPattern, { + meta: uiSettings.get(META_FIELDS), + source: true, + }); const fieldsToShow = getFieldsToShow(Object.keys(flattened), indexPattern, showMultiFields); const items: FieldRecord[] = Object.keys(flattened) diff --git a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts index c269677ae930d..ce814713fc705 100644 --- a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts +++ b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts @@ -18,6 +18,7 @@ import { IndexPattern, ISearchSource, ISearchStartSearchSource, + META_FIELDS, SearchFieldValue, SearchSourceFields, tabifyDocs, @@ -356,7 +357,10 @@ export class CsvGenerator { let table: Datatable | undefined; try { - table = tabifyDocs(results, index, { shallow: true, meta: true }); + table = tabifyDocs(results, index, { + shallow: true, + meta: await this.clients.uiSettings.get(META_FIELDS), + }); } catch (err) { this.logger.error(err); } From 76e44ae9d0e648987a9d4f81f184b9df7ed6debf Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 12 Oct 2021 12:46:38 +0200 Subject: [PATCH 02/43] Fix jest tests --- .../discover_grid_cell_actions.test.tsx | 7 +++ .../discover_grid_document_selection.test.tsx | 59 +++++++------------ .../discover_grid_expand_button.test.tsx | 43 +++++++------- .../components/table/table.test.tsx | 11 +++- 4 files changed, 59 insertions(+), 61 deletions(-) diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid_cell_actions.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid_cell_actions.test.tsx index de3c55ad7a869..623c59e19e799 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid_cell_actions.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid_cell_actions.test.tsx @@ -16,6 +16,7 @@ import { indexPatternMock } from '../../../__mocks__/index_pattern'; import { esHits } from '../../../__mocks__/es_hits'; import { EuiButton } from '@elastic/eui'; import { IndexPatternField } from 'src/plugins/data/common'; +import { IUiSettingsClient } from 'kibana/public'; describe('Discover cell actions ', function () { it('should not show cell actions for unfilterable fields', async () => { @@ -34,6 +35,9 @@ describe('Discover cell actions ', function () { isDarkMode: false, selectedDocs: [], setSelectedDocs: jest.fn(), + uiSettings: { + get: jest.fn(), + } as unknown as IUiSettingsClient, }; const component = mountWithIntl( @@ -62,6 +66,9 @@ describe('Discover cell actions ', function () { isDarkMode: false, selectedDocs: [], setSelectedDocs: jest.fn(), + uiSettings: { + get: jest.fn(), + } as unknown as IUiSettingsClient, }; const component = mountWithIntl( diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx index 7e6c4725d63f9..403bbd504ad1f 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx @@ -19,6 +19,24 @@ import { DiscoverGridContext } from './discover_grid_context'; import { META_FIELDS } from 'src/plugins/data/common'; import { IUiSettingsClient } from 'kibana/public'; +const baseContextMock = { + expanded: undefined, + setExpanded: jest.fn(), + rows: esHits, + onFilter: jest.fn(), + indexPattern: indexPatternMock, + isDarkMode: false, + selectedDocs: [], + setSelectedDocs: jest.fn(), + uiSettings: { + get: jest.fn((key) => { + if (key === META_FIELDS) { + return ['_id', '_index']; + } + }), + } as unknown as IUiSettingsClient, +}; + describe('document selection', () => { describe('getDocId', () => { test('doc with custom routing', () => { @@ -41,21 +59,7 @@ describe('document selection', () => { describe('SelectButton', () => { test('is not checked', () => { const contextMock = { - expanded: undefined, - setExpanded: jest.fn(), - rows: esHits, - onFilter: jest.fn(), - indexPattern: indexPatternMock, - isDarkMode: false, - selectedDocs: [], - setSelectedDocs: jest.fn(), - uiSettings: { - get: jest.fn((key) => { - if (key === META_FIELDS) { - return ['_id', '_index']; - } - }), - } as unknown as IUiSettingsClient, + ...baseContextMock, }; const component = mountWithIntl( @@ -77,14 +81,8 @@ describe('document selection', () => { test('is checked', () => { const contextMock = { - expanded: undefined, - setExpanded: jest.fn(), - rows: esHits, - onFilter: jest.fn(), - indexPattern: indexPatternMock, - isDarkMode: false, + ...baseContextMock, selectedDocs: ['i::1::'], - setSelectedDocs: jest.fn(), }; const component = mountWithIntl( @@ -106,14 +104,7 @@ describe('document selection', () => { test('adding a selection', () => { const contextMock = { - expanded: undefined, - setExpanded: jest.fn(), - rows: esHits, - onFilter: jest.fn(), - indexPattern: indexPatternMock, - isDarkMode: false, - selectedDocs: [], - setSelectedDocs: jest.fn(), + ...baseContextMock, }; const component = mountWithIntl( @@ -135,14 +126,8 @@ describe('document selection', () => { }); test('removing a selection', () => { const contextMock = { - expanded: undefined, - setExpanded: jest.fn(), - rows: esHits, - onFilter: jest.fn(), - indexPattern: indexPatternMock, - isDarkMode: false, + ...baseContextMock, selectedDocs: ['i::1::'], - setSelectedDocs: jest.fn(), }; const component = mountWithIntl( diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid_expand_button.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid_expand_button.test.tsx index d1299b39a25b2..4d0382ebaa6de 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid_expand_button.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid_expand_button.test.tsx @@ -14,17 +14,28 @@ import { DiscoverGridContext } from './discover_grid_context'; import { indexPatternMock } from '../../../__mocks__/index_pattern'; import { esHits } from '../../../__mocks__/es_hits'; +const baseContextMock = { + expanded: undefined, + setExpanded: jest.fn(), + rows: esHits, + onFilter: jest.fn(), + indexPattern: indexPatternMock, + isDarkMode: false, + selectedDocs: [], + setSelectedDocs: jest.fn(), + uiSettings: { + get: jest.fn((key) => { + if (key === META_FIELDS) { + return ['_id', '_index']; + } + }), + } as unknown as IUiSettingsClient, +}; + describe('Discover grid view button ', function () { it('when no document is expanded, setExpanded is called with current document', async () => { const contextMock = { - expanded: undefined, - setExpanded: jest.fn(), - rows: esHits, - onFilter: jest.fn(), - indexPattern: indexPatternMock, - isDarkMode: false, - selectedDocs: [], - setSelectedDocs: jest.fn(), + ...baseContextMock, }; const component = mountWithIntl( @@ -45,14 +56,8 @@ describe('Discover grid view button ', function () { }); it('when the current document is expanded, setExpanded is called with undefined', async () => { const contextMock = { + ...baseContextMock, expanded: esHits[0], - setExpanded: jest.fn(), - rows: esHits, - onFilter: jest.fn(), - indexPattern: indexPatternMock, - isDarkMode: false, - selectedDocs: [], - setSelectedDocs: jest.fn(), }; const component = mountWithIntl( @@ -73,14 +78,8 @@ describe('Discover grid view button ', function () { }); it('when another document is expanded, setExpanded is called with the current document', async () => { const contextMock = { + ...baseContextMock, expanded: esHits[0], - setExpanded: jest.fn(), - rows: esHits, - onFilter: jest.fn(), - indexPattern: indexPatternMock, - isDarkMode: false, - selectedDocs: [], - setSelectedDocs: jest.fn(), }; const component = mountWithIntl( diff --git a/src/plugins/discover/public/application/components/table/table.test.tsx b/src/plugins/discover/public/application/components/table/table.test.tsx index 2f779f8d55566..a493925966969 100644 --- a/src/plugins/discover/public/application/components/table/table.test.tsx +++ b/src/plugins/discover/public/application/components/table/table.test.tsx @@ -10,7 +10,7 @@ import React from 'react'; import { mountWithIntl } from '@kbn/test/jest'; import { findTestSubject } from '@elastic/eui/lib/test'; import { DocViewerTable, DocViewerTableProps } from './table'; -import { indexPatterns, IndexPattern } from '../../../../../data/public'; +import { IndexPattern } from '../../../../../data/public'; import { ElasticSearchHit } from '../../doc_views/doc_views_types'; jest.mock('../../../kibana_services', () => ({ @@ -24,6 +24,8 @@ import { getServices } from '../../../kibana_services'; get: (key: string) => { if (key === 'discover:showMultiFields') { return true; + } else if (key === 'metaFields') { + return ['_id', '_index']; } }, }, @@ -448,7 +450,12 @@ describe('DocViewTable at Discover Doc with Fields API', () => { (getServices as jest.Mock).mockImplementationOnce(() => ({ uiSettings: { get: (key: string) => { - return key === 'discover:showMultiFields' && false; + if (key === 'discover:showMultiFields') { + return false; + } + if (key === 'metaFields') { + return ['_id', '_index']; + } }, }, })); From 4304f55445565b0b3b09939cea03f11b2a280de8 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 12 Oct 2021 13:57:13 +0200 Subject: [PATCH 03/43] Read metaFields from index pattern --- .../__snapshots__/tabify_docs.test.ts.snap | 112 +++++++++++------- .../common/search/tabify/tabify_docs.test.ts | 9 +- .../data/common/search/tabify/tabify_docs.ts | 13 +- .../public/__mocks__/index_pattern.ts | 9 +- .../__mocks__/index_pattern_with_timefield.ts | 9 +- .../doc_table/components/table_row.tsx | 10 +- .../sidebar/discover_sidebar.test.tsx | 4 +- .../discover_sidebar_responsive.test.tsx | 6 +- .../sidebar/lib/field_calculator.js | 4 +- .../sidebar/lib/field_calculator.test.ts | 3 +- .../apps/main/utils/calc_field_counts.ts | 4 +- .../discover_grid/discover_grid.tsx | 7 +- .../discover_grid_cell_actions.tsx | 8 +- .../discover_grid/discover_grid_context.tsx | 2 - .../discover_grid_expand_button.test.tsx | 7 -- .../get_render_cell_value.test.tsx | 33 ++++-- .../application/components/table/table.tsx | 5 +- .../generate_csv/generate_csv.ts | 5 +- 18 files changed, 126 insertions(+), 124 deletions(-) diff --git a/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap b/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap index a670dcfaff4e3..9d616bfb575d9 100644 --- a/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap +++ b/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`tabifyDocs combines meta fields if meta option is set 1`] = ` +exports[`tabifyDocs converts fields by default 1`] = ` Object { "columns": Array [ Object { @@ -108,7 +108,7 @@ Object { } `; -exports[`tabifyDocs converts fields by default 1`] = ` +exports[`tabifyDocs converts source if option is set 1`] = ` Object { "columns": Array [ Object { @@ -155,73 +155,53 @@ Object { }, "name": "sourceTest", }, - ], - "rows": Array [ - Object { - "fieldTest": 123, - "invalidMapping": 345, - "nested": Array [ - Object { - "field": 123, - }, - ], - "sourceTest": 123, - }, - ], - "type": "datatable", -} -`; - -exports[`tabifyDocs converts source if option is set 1`] = ` -Object { - "columns": Array [ Object { - "id": "fieldTest", + "id": "_id", "meta": Object { - "field": "fieldTest", + "field": "_id", "index": "test-index", - "params": Object { - "id": "number", - }, - "type": "number", + "params": undefined, + "type": "string", }, - "name": "fieldTest", + "name": "_id", }, Object { - "id": "invalidMapping", + "id": "_index", "meta": Object { - "field": "invalidMapping", + "field": "_index", "index": "test-index", "params": undefined, - "type": "number", + "type": "string", }, - "name": "invalidMapping", + "name": "_index", }, Object { - "id": "nested", + "id": "_score", "meta": Object { - "field": "nested", + "field": "_score", "index": "test-index", "params": undefined, - "type": "object", + "type": "number", }, - "name": "nested", + "name": "_score", }, Object { - "id": "sourceTest", + "id": "_type", "meta": Object { - "field": "sourceTest", + "field": "_type", "index": "test-index", - "params": Object { - "id": "number", - }, - "type": "number", + "params": undefined, + "type": "string", }, - "name": "sourceTest", + "name": "_type", }, ], "rows": Array [ Object { + "_id": "hit-id-value", + "_index": "hit-index-value", + "_score": 77, + "_type": "hit-type-value", "fieldTest": 123, "invalidMapping": 345, "nested": Array [ @@ -283,9 +263,53 @@ Object { }, "name": "sourceTest", }, + Object { + "id": "_id", + "meta": Object { + "field": "_id", + "index": "test-index", + "params": undefined, + "type": "string", + }, + "name": "_id", + }, + Object { + "id": "_index", + "meta": Object { + "field": "_index", + "index": "test-index", + "params": undefined, + "type": "string", + }, + "name": "_index", + }, + Object { + "id": "_score", + "meta": Object { + "field": "_score", + "index": "test-index", + "params": undefined, + "type": "number", + }, + "name": "_score", + }, + Object { + "id": "_type", + "meta": Object { + "field": "_type", + "index": "test-index", + "params": undefined, + "type": "string", + }, + "name": "_type", + }, ], "rows": Array [ Object { + "_id": "hit-id-value", + "_index": "hit-index-value", + "_score": 77, + "_type": "hit-type-value", "fieldTest": 123, "invalidMapping": 345, "nested": Array [ diff --git a/src/plugins/data/common/search/tabify/tabify_docs.test.ts b/src/plugins/data/common/search/tabify/tabify_docs.test.ts index dfe5baf10dcbc..03ecd1be6b5d6 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.test.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.test.ts @@ -30,6 +30,7 @@ describe('tabifyDocs', () => { }, }, }, + metaFields: ['_id', '_index', '_score', '_type'], fieldFormats: fieldFormats as any, }); @@ -64,9 +65,11 @@ describe('tabifyDocs', () => { expect(table).toMatchSnapshot(); }); - it('combines meta fields if meta option is set', () => { - const table = tabifyDocs(response, index, { meta: ['_id', '_index', '_score', '_type'] }); - expect(table).toMatchSnapshot(); + it('combines meta fields from index pattern', () => { + const table = tabifyDocs(response, index); + expect(table.columns.map((col) => col.id)).toEqual( + expect.arrayContaining(['_id', '_index', '_score', '_type']) + ); }); it('works without provided index pattern', () => { diff --git a/src/plugins/data/common/search/tabify/tabify_docs.ts b/src/plugins/data/common/search/tabify/tabify_docs.ts index 3e0a3c1d22eda..a7168bd4155c0 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.ts @@ -36,12 +36,6 @@ export interface TabifyDocsOptions { * merged into the flattened document. */ source?: boolean; - /** - * Specify a list of meta field names that should be merged from the top - * level of the hit (i.e. not nested under _source or fields) into the flattened version. - * If not specified no meta fields (like _id, _index, etc.) will be merged. - */ - meta?: string[]; } /** @@ -58,7 +52,6 @@ export function flattenHit( params?: TabifyDocsOptions ) { const flat = {} as Record; - const metaFields = params?.meta ?? []; function flatten(obj: Record, keyPrefix: string = '') { for (const [k, val] of Object.entries(obj)) { @@ -99,7 +92,7 @@ export function flattenHit( flatten(hit._source as Record); } - metaFields.forEach((metaFieldName) => { + indexPattern?.metaFields.forEach((metaFieldName) => { if (!isValidMetaFieldName(metaFieldName) || metaFieldName === '_source') { return; } @@ -109,8 +102,8 @@ export function flattenHit( return new Proxy(flat, { ownKeys: (target) => { return Reflect.ownKeys(target).sort((a, b) => { - const aIsMeta = metaFields.includes(String(a)); - const bIsMeta = metaFields.includes(String(b)); + const aIsMeta = indexPattern?.metaFields.includes(String(a)); + const bIsMeta = indexPattern?.metaFields.includes(String(b)); if (aIsMeta && bIsMeta) { return String(a).localeCompare(String(b)); } diff --git a/src/plugins/discover/public/__mocks__/index_pattern.ts b/src/plugins/discover/public/__mocks__/index_pattern.ts index f9cc202f9063e..2acb512617a6b 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern.ts @@ -6,9 +6,9 @@ * Side Public License, v 1. */ -import { IIndexPatternFieldList } from '../../../data/common'; +import type { estypes } from '@elastic/elasticsearch'; +import { flattenHit, IIndexPatternFieldList } from '../../../data/common'; import { IndexPattern } from '../../../data/common'; -import { indexPatterns } from '../../../data/public'; const fields = [ { @@ -85,10 +85,11 @@ const indexPattern = { getFormatterForField: () => ({ convert: () => 'formatted' }), } as unknown as IndexPattern; -indexPattern.flattenHit = indexPatterns.flattenHitWrapper(indexPattern, indexPattern.metaFields); indexPattern.isTimeBased = () => !!indexPattern.timeFieldName; indexPattern.formatField = (hit: Record, fieldName: string) => { - return fieldName === '_source' ? hit._source : indexPattern.flattenHit(hit)[fieldName]; + return fieldName === '_source' + ? hit._source + : flattenHit(hit as unknown as estypes.SearchHit, indexPattern)[fieldName]; }; export const indexPatternMock = indexPattern; diff --git a/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts b/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts index f1f1d74f3af3a..ac30c24ef6f08 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts @@ -6,9 +6,9 @@ * Side Public License, v 1. */ -import { IIndexPatternFieldList } from '../../../data/common'; +import { flattenHit, IIndexPatternFieldList } from '../../../data/common'; import { IndexPattern } from '../../../data/common'; -import { indexPatterns } from '../../../data/public'; +import type { estypes } from '@elastic/elasticsearch'; const fields = [ { @@ -74,10 +74,11 @@ const indexPattern = { getFormatterForField: () => ({ convert: () => 'formatted' }), } as unknown as IndexPattern; -indexPattern.flattenHit = indexPatterns.flattenHitWrapper(indexPattern, indexPattern.metaFields); indexPattern.isTimeBased = () => !!indexPattern.timeFieldName; indexPattern.formatField = (hit: Record, fieldName: string) => { - return fieldName === '_source' ? hit._source : indexPattern.flattenHit(hit)[fieldName]; + return fieldName === '_source' + ? hit._source + : flattenHit(hit as unknown as estypes.SearchHit, indexPattern)[fieldName]; }; export const indexPatternWithTimefieldMock = indexPattern; diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx index d4a29f46fdb22..d91735460af08 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx @@ -10,7 +10,7 @@ import React, { Fragment, useCallback, useMemo, useState } from 'react'; import classNames from 'classnames'; import { i18n } from '@kbn/i18n'; import { EuiButtonEmpty, EuiIcon } from '@elastic/eui'; -import { flattenHit, META_FIELDS } from '../../../../../../../../data/common'; +import { flattenHit } from '../../../../../../../../data/common'; import { DocViewer } from '../../../../../components/doc_viewer/doc_viewer'; import { FilterManager, IndexPattern } from '../../../../../../../../data/public'; import { TableCell } from './table_row/table_cell'; @@ -19,7 +19,6 @@ import { getContextUrl } from '../../../../../helpers/get_context_url'; import { getSingleDocUrl } from '../../../../../helpers/get_single_doc_url'; import { TableRowDetails } from './table_row_details'; import { formatRow, formatTopLevelObject } from '../lib/row_formatter'; -import { getServices } from '../../../../../../kibana_services'; export type DocTableRow = ElasticSearchHit & { isAnchor?: boolean; @@ -52,8 +51,6 @@ export const TableRow = ({ filterManager, addBasePath, }: TableRowProps) => { - const { uiSettings } = getServices(); - const [open, setOpen] = useState(false); const docTableRowClassName = classNames('kbnDocTable__row', { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -61,10 +58,7 @@ export const TableRow = ({ }); const anchorDocTableRowSubj = row.isAnchor ? ' docTableAnchorRow' : ''; - const flattenedRow = useMemo( - () => flattenHit(row, indexPattern, { meta: uiSettings.get(META_FIELDS) }), - [indexPattern, row, uiSettings] - ); + const flattenedRow = useMemo(() => flattenHit(row, indexPattern), [indexPattern, row]); const mapping = useMemo(() => indexPattern.fields.getByName, [indexPattern]); // toggle display of the rows details, a full list of the fields from each row diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.test.tsx b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.test.tsx index e53bf006e2b4e..a550dbd59b9fa 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.test.tsx +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar.test.tsx @@ -15,7 +15,7 @@ import realHits from '../../../../../__fixtures__/real_hits.js'; import { mountWithIntl } from '@kbn/test/jest'; import React from 'react'; import { DiscoverSidebarProps } from './discover_sidebar'; -import { IndexPatternAttributes } from '../../../../../../../data/common'; +import { flattenHit, IndexPatternAttributes } from '../../../../../../../data/common'; import { SavedObject } from '../../../../../../../../core/types'; import { getDefaultFieldFilter } from './lib/field_filter'; import { DiscoverSidebarComponent as DiscoverSidebar } from './discover_sidebar'; @@ -44,7 +44,7 @@ function getCompProps(): DiscoverSidebarProps { const fieldCounts: Record = {}; for (const hit of hits) { - for (const key of Object.keys(indexPattern.flattenHit(hit))) { + for (const key of Object.keys(flattenHit(hit, indexPattern))) { fieldCounts[key] = (fieldCounts[key] || 0) + 1; } } diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.test.tsx b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.test.tsx index 9d73f885c988d..ded7897d2a9e5 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.test.tsx +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/discover_sidebar_responsive.test.tsx @@ -15,7 +15,7 @@ import realHits from '../../../../../__fixtures__/real_hits.js'; import { act } from 'react-dom/test-utils'; import { mountWithIntl } from '@kbn/test/jest'; import React from 'react'; -import { IndexPatternAttributes } from '../../../../../../../data/common'; +import { flattenHit, IndexPatternAttributes } from '../../../../../../../data/common'; import { SavedObject } from '../../../../../../../../core/types'; import { DiscoverSidebarResponsive, @@ -72,7 +72,7 @@ function getCompProps(): DiscoverSidebarResponsiveProps { const indexPattern = stubLogstashIndexPattern; // @ts-expect-error _.each() is passing additional args to flattenHit - const hits = each(cloneDeep(realHits), indexPattern.flattenHit) as Array< + const hits = each(cloneDeep(realHits), (hit) => flattenHit(hit, indexPattern)) as Array< Record > as ElasticSearchHit[]; @@ -83,7 +83,7 @@ function getCompProps(): DiscoverSidebarResponsiveProps { ]; for (const hit of hits) { - for (const key of Object.keys(indexPattern.flattenHit(hit))) { + for (const key of Object.keys(flattenHit(hit, indexPattern))) { mockfieldCounts[key] = (mockfieldCounts[key] || 0) + 1; } } diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.js b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.js index 8f86cdad82cf7..be7e9c616273d 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.js +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.js @@ -8,12 +8,12 @@ import { map, sortBy, without, each, defaults, isObject } from 'lodash'; import { i18n } from '@kbn/i18n'; +import { flattenHit } from '../../../../../../../../data/common'; function getFieldValues(hits, field, indexPattern) { const name = field.name; - const flattenHit = indexPattern.flattenHit; return map(hits, function (hit) { - return flattenHit(hit)[name]; + return flattenHit(hit, indexPattern)[name]; }); } diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.test.ts b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.test.ts index c3ff7970c5aac..d4bc41f36b2d4 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.test.ts +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.test.ts @@ -13,6 +13,7 @@ import { keys, each, cloneDeep, clone, uniq, filter, map } from 'lodash'; import realHits from '../../../../../../__fixtures__/real_hits.js'; import { IndexPattern } from '../../../../../../../../data/public'; +import { flattenHit } from '../../../../../../../../data/common'; // @ts-expect-error import { fieldCalculator } from './field_calculator'; @@ -120,7 +121,7 @@ describe('fieldCalculator', function () { let hits: any; beforeEach(function () { - hits = each(cloneDeep(realHits), (hit) => indexPattern.flattenHit(hit)); + hits = each(cloneDeep(realHits), (hit) => flattenHit(hit, indexPattern)); }); it('Should return an array of values for _source fields', function () { diff --git a/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts b/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts index 1ce7023539be4..d28b167f18767 100644 --- a/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts +++ b/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import type { IndexPattern } from 'src/plugins/data/common'; +import { flattenHit, IndexPattern } from 'src/plugins/data/common'; import { ElasticSearchHit } from '../../../doc_views/doc_views_types'; /** @@ -22,7 +22,7 @@ export function calcFieldCounts( return {}; } for (const hit of rows) { - const fields = Object.keys(indexPattern.flattenHit(hit)); + const fields = Object.keys(flattenHit(hit, indexPattern)); for (const fieldName of fields) { counts[fieldName] = (counts[fieldName] || 0) + 1; } diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx index 3eccb9f5eda71..f1d87b75997e9 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx @@ -271,11 +271,7 @@ export const DiscoverGrid = ({ getRenderCellValueFn( indexPattern, displayedRows, - displayedRows - ? displayedRows.map((hit) => - flattenHit(hit, indexPattern, { meta: services.uiSettings.get(META_FIELDS) }) - ) - : [], + displayedRows ? displayedRows.map((hit) => flattenHit(hit, indexPattern)) : [], useNewFieldsApi, fieldsToShow, services.uiSettings.get(MAX_DOC_FIELDS_DISPLAYED) @@ -377,7 +373,6 @@ export const DiscoverGrid = ({ setIsFilterActive(false); } }, - uiSettings: services.uiSettings, }} > { const row = context.rows[rowIndex]; - const flattened = flattenHit(row, context.indexPattern, { - meta: context.uiSettings.get(META_FIELDS), - }); + const flattened = flattenHit(row, context.indexPattern); if (flattened) { context.onFilter(columnId, flattened[columnId], '+'); @@ -62,7 +60,7 @@ export const FilterOutBtn = ({ { const row = context.rows[rowIndex]; - const flattened = context.indexPattern.flattenHit(row); + const flattened = flattenHit(row, context.indexPattern); if (flattened) { context.onFilter(columnId, flattened[columnId], '-'); diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid_context.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid_context.tsx index 1451cfea197a6..8d0fbec9d7933 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid_context.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid_context.tsx @@ -7,7 +7,6 @@ */ import React from 'react'; -import { IUiSettingsClient } from 'src/core/public'; import type { IndexPattern } from 'src/plugins/data/common'; import { DocViewFilterFn, ElasticSearchHit } from '../../doc_views/doc_views_types'; @@ -20,7 +19,6 @@ export interface GridContext { isDarkMode: boolean; selectedDocs: string[]; setSelectedDocs: (selected: string[]) => void; - uiSettings: IUiSettingsClient; } const defaultContext = {} as unknown as GridContext; diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid_expand_button.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid_expand_button.test.tsx index 4d0382ebaa6de..3f7cb70091cfa 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid_expand_button.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid_expand_button.test.tsx @@ -23,13 +23,6 @@ const baseContextMock = { isDarkMode: false, selectedDocs: [], setSelectedDocs: jest.fn(), - uiSettings: { - get: jest.fn((key) => { - if (key === META_FIELDS) { - return ['_id', '_index']; - } - }), - } as unknown as IUiSettingsClient, }; describe('Discover grid view button ', function () { diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx index 5aca237d46581..6556876217953 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx @@ -11,6 +11,7 @@ import { ReactWrapper, shallow } from 'enzyme'; import { getRenderCellValueFn } from './get_render_cell_value'; import { indexPatternMock } from '../../../__mocks__/index_pattern'; import { ElasticSearchHit } from '../../doc_views/doc_views_types'; +import { flattenHit } from 'src/plugins/data/common'; jest.mock('../../../../../kibana_react/public', () => ({ useUiSetting: () => true, @@ -68,12 +69,16 @@ const rowsFieldsWithTopLevelObject: ElasticSearchHit[] = [ }, ]; +const flatten = (hit: ElasticSearchHit): Record => { + return flattenHit(hit, indexPatternMock); +}; + describe('Discover grid cell rendering', function () { it('renders bytes column correctly', () => { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsSource, - rowsSource.map((row) => indexPatternMock.flattenHit(row)), + rowsSource.map(flatten), false, [], 100 @@ -95,7 +100,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsSource, - rowsSource.map((row) => indexPatternMock.flattenHit(row)), + rowsSource.map(flatten), false, [], 100 @@ -146,7 +151,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsSource, - rowsSource.map((row) => indexPatternMock.flattenHit(row)), + rowsSource.map(flatten), false, [], 100 @@ -189,7 +194,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsFields, - rowsFields.map((row) => indexPatternMock.flattenHit(row)), + rowsFields.map(flatten), true, [], 100 @@ -244,7 +249,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsFields, - rowsFields.map((row) => indexPatternMock.flattenHit(row)), + rowsFields.map(flatten), true, [], // this is the number of rendered items @@ -287,7 +292,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsFields, - rowsFields.map((row) => indexPatternMock.flattenHit(row)), + rowsFields.map(flatten), true, [], 100 @@ -335,7 +340,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsFieldsWithTopLevelObject, - rowsFieldsWithTopLevelObject.map((row) => indexPatternMock.flattenHit(row)), + rowsFieldsWithTopLevelObject.map(flatten), true, [], 100 @@ -376,7 +381,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsFieldsWithTopLevelObject, - rowsFieldsWithTopLevelObject.map((row) => indexPatternMock.flattenHit(row)), + rowsFieldsWithTopLevelObject.map(flatten), true, [], 100 @@ -416,7 +421,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsFieldsWithTopLevelObject, - rowsFieldsWithTopLevelObject.map((row) => indexPatternMock.flattenHit(row)), + rowsFieldsWithTopLevelObject.map(flatten), true, [], 100 @@ -447,7 +452,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsFieldsWithTopLevelObject, - rowsFieldsWithTopLevelObject.map((row) => indexPatternMock.flattenHit(row)), + rowsFieldsWithTopLevelObject.map(flatten), true, [], 100 @@ -466,7 +471,9 @@ describe('Discover grid cell rendering', function () { @@ -477,7 +484,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsSource, - rowsSource.map((row) => indexPatternMock.flattenHit(row)), + rowsSource.map(flatten), false, [], 100 @@ -499,7 +506,7 @@ describe('Discover grid cell rendering', function () { const DiscoverGridCellValue = getRenderCellValueFn( indexPatternMock, rowsSource, - rowsSource.map((row) => indexPatternMock.flattenHit(row)), + rowsSource.map(flatten), false, [], 100 diff --git a/src/plugins/discover/public/application/components/table/table.tsx b/src/plugins/discover/public/application/components/table/table.tsx index 501a38b97cfaf..ab0889d52d3fa 100644 --- a/src/plugins/discover/public/application/components/table/table.tsx +++ b/src/plugins/discover/public/application/components/table/table.tsx @@ -97,10 +97,7 @@ export const DocViewerTable = ({ return null; } - const flattened = flattenHit(hit, indexPattern, { - meta: uiSettings.get(META_FIELDS), - source: true, - }); + const flattened = flattenHit(hit, indexPattern, { source: true }); const fieldsToShow = getFieldsToShow(Object.keys(flattened), indexPattern, showMultiFields); const items: FieldRecord[] = Object.keys(flattened) diff --git a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts index ce814713fc705..41998b95a2f84 100644 --- a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts +++ b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts @@ -357,10 +357,7 @@ export class CsvGenerator { let table: Datatable | undefined; try { - table = tabifyDocs(results, index, { - shallow: true, - meta: await this.clients.uiSettings.get(META_FIELDS), - }); + table = tabifyDocs(results, index, { shallow: true }); } catch (err) { this.logger.error(err); } From 13bb111570905a94a5d3186223e71695f26ffa9f Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 12 Oct 2021 14:00:57 +0200 Subject: [PATCH 04/43] Remove old test code --- .../discover_grid/discover_grid_cell_actions.test.tsx | 7 ------- .../discover_grid_document_selection.test.tsx | 9 --------- .../public/application/components/table/table.test.tsx | 9 +-------- 3 files changed, 1 insertion(+), 24 deletions(-) diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid_cell_actions.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid_cell_actions.test.tsx index 623c59e19e799..de3c55ad7a869 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid_cell_actions.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid_cell_actions.test.tsx @@ -16,7 +16,6 @@ import { indexPatternMock } from '../../../__mocks__/index_pattern'; import { esHits } from '../../../__mocks__/es_hits'; import { EuiButton } from '@elastic/eui'; import { IndexPatternField } from 'src/plugins/data/common'; -import { IUiSettingsClient } from 'kibana/public'; describe('Discover cell actions ', function () { it('should not show cell actions for unfilterable fields', async () => { @@ -35,9 +34,6 @@ describe('Discover cell actions ', function () { isDarkMode: false, selectedDocs: [], setSelectedDocs: jest.fn(), - uiSettings: { - get: jest.fn(), - } as unknown as IUiSettingsClient, }; const component = mountWithIntl( @@ -66,9 +62,6 @@ describe('Discover cell actions ', function () { isDarkMode: false, selectedDocs: [], setSelectedDocs: jest.fn(), - uiSettings: { - get: jest.fn(), - } as unknown as IUiSettingsClient, }; const component = mountWithIntl( diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx index 403bbd504ad1f..e9b93e21553a2 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid_document_selection.test.tsx @@ -16,8 +16,6 @@ import { import { esHits } from '../../../__mocks__/es_hits'; import { indexPatternMock } from '../../../__mocks__/index_pattern'; import { DiscoverGridContext } from './discover_grid_context'; -import { META_FIELDS } from 'src/plugins/data/common'; -import { IUiSettingsClient } from 'kibana/public'; const baseContextMock = { expanded: undefined, @@ -28,13 +26,6 @@ const baseContextMock = { isDarkMode: false, selectedDocs: [], setSelectedDocs: jest.fn(), - uiSettings: { - get: jest.fn((key) => { - if (key === META_FIELDS) { - return ['_id', '_index']; - } - }), - } as unknown as IUiSettingsClient, }; describe('document selection', () => { diff --git a/src/plugins/discover/public/application/components/table/table.test.tsx b/src/plugins/discover/public/application/components/table/table.test.tsx index a493925966969..ce914edcec703 100644 --- a/src/plugins/discover/public/application/components/table/table.test.tsx +++ b/src/plugins/discover/public/application/components/table/table.test.tsx @@ -24,8 +24,6 @@ import { getServices } from '../../../kibana_services'; get: (key: string) => { if (key === 'discover:showMultiFields') { return true; - } else if (key === 'metaFields') { - return ['_id', '_index']; } }, }, @@ -450,12 +448,7 @@ describe('DocViewTable at Discover Doc with Fields API', () => { (getServices as jest.Mock).mockImplementationOnce(() => ({ uiSettings: { get: (key: string) => { - if (key === 'discover:showMultiFields') { - return false; - } - if (key === 'metaFields') { - return ['_id', '_index']; - } + return key === 'discover:showMultiFields' && false; }, }, })); From 07c2231fdc2b0c79dcf2dbac3e9f0c97f4993b6c Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 12 Oct 2021 14:02:55 +0200 Subject: [PATCH 05/43] remove unnecessary changes --- .../discover/public/application/components/table/table.tsx | 5 ++--- .../csv_searchsource/generate_csv/generate_csv.ts | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/plugins/discover/public/application/components/table/table.tsx b/src/plugins/discover/public/application/components/table/table.tsx index ab0889d52d3fa..7f597d846f88f 100644 --- a/src/plugins/discover/public/application/components/table/table.tsx +++ b/src/plugins/discover/public/application/components/table/table.tsx @@ -9,7 +9,7 @@ import React, { useCallback, useMemo } from 'react'; import { EuiInMemoryTable } from '@elastic/eui'; import { IndexPattern, IndexPatternField } from '../../../../../data/public'; -import { flattenHit, META_FIELDS } from '../../../../../data/common'; +import { flattenHit } from '../../../../../data/common'; import { SHOW_MULTIFIELDS } from '../../../../common'; import { getServices } from '../../../kibana_services'; import { isNestedFieldParent } from '../../apps/main/utils/nested_fields'; @@ -57,8 +57,7 @@ export const DocViewerTable = ({ onAddColumn, onRemoveColumn, }: DocViewRenderProps) => { - const { uiSettings } = getServices(); - const showMultiFields = uiSettings.get(SHOW_MULTIFIELDS); + const showMultiFields = getServices().uiSettings.get(SHOW_MULTIFIELDS); const mapping = useCallback( (name: string) => indexPattern?.fields.getByName(name), diff --git a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts index 41998b95a2f84..6c2989d54309d 100644 --- a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts +++ b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts @@ -18,7 +18,6 @@ import { IndexPattern, ISearchSource, ISearchStartSearchSource, - META_FIELDS, SearchFieldValue, SearchSourceFields, tabifyDocs, From 983f830f53654c66c067593ab567c80d36f3b523 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 12 Oct 2021 15:50:37 +0200 Subject: [PATCH 06/43] Remove flattenHitWrapper APIs --- src/plugins/data/public/index.ts | 2 -- .../data_views/common/data_views/flatten_hit.ts | 12 ------------ src/plugins/data_views/public/index.ts | 2 +- 3 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/plugins/data/public/index.ts b/src/plugins/data/public/index.ts index 3ae98c083976e..4d51a7ae0ad77 100644 --- a/src/plugins/data/public/index.ts +++ b/src/plugins/data/public/index.ts @@ -52,7 +52,6 @@ import { ILLEGAL_CHARACTERS_VISIBLE, ILLEGAL_CHARACTERS, validateDataView, - flattenHitWrapper, } from './data_views'; export type { IndexPatternsService } from './data_views'; @@ -69,7 +68,6 @@ export const indexPatterns = { getFieldSubtypeMulti, getFieldSubtypeNested, validate: validateDataView, - flattenHitWrapper, }; export { diff --git a/src/plugins/data_views/common/data_views/flatten_hit.ts b/src/plugins/data_views/common/data_views/flatten_hit.ts index ddf484affa298..1cadf204e5411 100644 --- a/src/plugins/data_views/common/data_views/flatten_hit.ts +++ b/src/plugins/data_views/common/data_views/flatten_hit.ts @@ -114,15 +114,3 @@ export function flattenHitWrapper(dataView: DataView, metaFields = {}, cache = n return decorateFlattened(flattened); }; } - -/** - * This wraps `flattenHitWrapper` so one single cache can be provided for all uses of that - * function. The returned value of this function is what is included in the index patterns - * setup contract. - * - * @public - */ -export function createFlattenHitWrapper() { - const cache = new WeakMap(); - return _.partial(flattenHitWrapper, _, _, cache); -} diff --git a/src/plugins/data_views/public/index.ts b/src/plugins/data_views/public/index.ts index 572806df11fa3..5c810ec1fd4c8 100644 --- a/src/plugins/data_views/public/index.ts +++ b/src/plugins/data_views/public/index.ts @@ -13,7 +13,7 @@ export { ILLEGAL_CHARACTERS, validateDataView, } from '../common/lib'; -export { flattenHitWrapper, formatHitProvider, onRedirectNoIndexPattern } from './data_views'; +export { formatHitProvider, onRedirectNoIndexPattern } from './data_views'; export { IndexPatternField, IIndexPatternFieldList, TypeMeta } from '../common'; From 8be25253655c9092a610a6b1f41043825d218545 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 12 Oct 2021 15:50:51 +0200 Subject: [PATCH 07/43] Fix imports --- .../public/application/apps/main/utils/calc_field_counts.ts | 2 +- .../application/components/discover_grid/discover_grid.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts b/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts index d28b167f18767..211c4e5c8b069 100644 --- a/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts +++ b/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts @@ -6,7 +6,7 @@ * Side Public License, v 1. */ -import { flattenHit, IndexPattern } from 'src/plugins/data/common'; +import { flattenHit, IndexPattern } from '../../../../../../data/common'; import { ElasticSearchHit } from '../../../doc_views/doc_views_types'; /** diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx index f1d87b75997e9..11323080274a9 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx @@ -21,7 +21,7 @@ import { EuiLoadingSpinner, EuiIcon, } from '@elastic/eui'; -import { flattenHit, IndexPattern, META_FIELDS } from '../../../../../data/common'; +import { flattenHit, IndexPattern } from '../../../../../data/common'; import { DocViewFilterFn, ElasticSearchHit } from '../../doc_views/doc_views_types'; import { getSchemaDetectors } from './discover_grid_schema'; import { DiscoverGridFlyout } from './discover_grid_flyout'; From 5c47379f998e5a9dcd29f6418d23c0cb3c1b693d Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 12 Oct 2021 15:56:11 +0200 Subject: [PATCH 08/43] Fix missing metaFields --- src/plugins/data/common/search/tabify/tabify_docs.ts | 6 +++--- .../csv_searchsource/generate_csv/generate_csv.test.ts | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/plugins/data/common/search/tabify/tabify_docs.ts b/src/plugins/data/common/search/tabify/tabify_docs.ts index a7168bd4155c0..e4b44b16ea0c0 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.ts @@ -92,7 +92,7 @@ export function flattenHit( flatten(hit._source as Record); } - indexPattern?.metaFields.forEach((metaFieldName) => { + indexPattern?.metaFields?.forEach((metaFieldName) => { if (!isValidMetaFieldName(metaFieldName) || metaFieldName === '_source') { return; } @@ -102,8 +102,8 @@ export function flattenHit( return new Proxy(flat, { ownKeys: (target) => { return Reflect.ownKeys(target).sort((a, b) => { - const aIsMeta = indexPattern?.metaFields.includes(String(a)); - const bIsMeta = indexPattern?.metaFields.includes(String(b)); + const aIsMeta = indexPattern?.metaFields?.includes(String(a)); + const bIsMeta = indexPattern?.metaFields?.includes(String(b)); if (aIsMeta && bIsMeta) { return String(a).localeCompare(String(b)); } diff --git a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.test.ts b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.test.ts index f393661e4c490..1902c4ed0272e 100644 --- a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.test.ts +++ b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.test.ts @@ -75,6 +75,7 @@ const mockSearchSourceGetFieldDefault = jest.fn().mockImplementation((key: strin getByName: jest.fn().mockImplementation(() => []), getByType: jest.fn().mockImplementation(() => []), }, + metaFields: ['_id', '_index', '_type', '_score'], getFormatterForField: jest.fn(), }; } From 6ebbf26a6bfff93a60c1cec7e578251e13f12adc Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 12 Oct 2021 17:34:14 +0200 Subject: [PATCH 09/43] Add all meta fields to allowlist --- .../data/common/search/tabify/tabify_docs.ts | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/plugins/data/common/search/tabify/tabify_docs.ts b/src/plugins/data/common/search/tabify/tabify_docs.ts index e4b44b16ea0c0..915b24beb85f3 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.ts @@ -14,18 +14,38 @@ import { Datatable, DatatableColumn, DatatableColumnType } from '../../../../exp // TODO: complete this list type ValidMetaFieldNames = keyof Pick< estypes.SearchHit, - '_id' | '_index' | '_source' | '_type' | '_score' + | '_id' + | '_ignored' + | '_index' + | '_node' + | '_primary_term' + | '_routing' + | '_score' + | '_seq_no' + | '_shard' + | '_source' + | '_type' + | '_version' >; const VALID_META_FIELD_NAMES: ValidMetaFieldNames[] = [ '_id', + '_ignored', '_index', + '_node', + '_primary_term', + '_routing', + '_score', + '_seq_no', + '_shard', '_source', '_type', - '_score', + '_version', ]; function isValidMetaFieldName(field: string): field is ValidMetaFieldNames { - // TODO: Leave comment why to cast here + // Since the array above is more narrowly typed than string[], we cannot use + // string to find a value in here. We manually cast it to wider string[] type + // so we're able to use `includes` on it. return (VALID_META_FIELD_NAMES as string[]).includes(field); } From 5349298906713599121cd97d2107ac4393ab9d9b Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Wed, 13 Oct 2021 10:13:44 +0200 Subject: [PATCH 10/43] Improve inline comments --- src/plugins/data/common/search/tabify/tabify_docs.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/plugins/data/common/search/tabify/tabify_docs.ts b/src/plugins/data/common/search/tabify/tabify_docs.ts index 915b24beb85f3..4259488771761 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.ts @@ -11,7 +11,6 @@ import { isPlainObject } from 'lodash'; import { IndexPattern } from '../..'; import { Datatable, DatatableColumn, DatatableColumnType } from '../../../../expressions/common'; -// TODO: complete this list type ValidMetaFieldNames = keyof Pick< estypes.SearchHit, | '_id' @@ -112,6 +111,8 @@ export function flattenHit( flatten(hit._source as Record); } + // Merge all valid meta fields into the flattened object + // expect for _source (in case that was specified as a meta field) indexPattern?.metaFields?.forEach((metaFieldName) => { if (!isValidMetaFieldName(metaFieldName) || metaFieldName === '_source') { return; @@ -119,6 +120,8 @@ export function flattenHit( flat[metaFieldName] = hit[metaFieldName]; }); + // Use a proxy to make sure that keys are always returned in a specific order, + // so we have a guarantee on the flattened order of keys. return new Proxy(flat, { ownKeys: (target) => { return Reflect.ownKeys(target).sort((a, b) => { From 911cf9eabdd89e13cf611d8c564ee5f54eec614d Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Wed, 13 Oct 2021 10:36:30 +0200 Subject: [PATCH 11/43] Move flattenHit test to new implementation --- .../__snapshots__/tabify_docs.test.ts.snap | 8 +- .../common/search/tabify/tabify_docs.test.ts | 173 ++++++++++++------ 2 files changed, 122 insertions(+), 59 deletions(-) diff --git a/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap b/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap index 9d616bfb575d9..9a85ba57ce5ef 100644 --- a/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap +++ b/src/plugins/data/common/search/tabify/__snapshots__/tabify_docs.test.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`tabifyDocs converts fields by default 1`] = ` +exports[`tabify_docs tabifyDocs converts fields by default 1`] = ` Object { "columns": Array [ Object { @@ -108,7 +108,7 @@ Object { } `; -exports[`tabifyDocs converts source if option is set 1`] = ` +exports[`tabify_docs tabifyDocs converts source if option is set 1`] = ` Object { "columns": Array [ Object { @@ -216,7 +216,7 @@ Object { } `; -exports[`tabifyDocs skips nested fields if option is set 1`] = ` +exports[`tabify_docs tabifyDocs skips nested fields if option is set 1`] = ` Object { "columns": Array [ Object { @@ -324,7 +324,7 @@ Object { } `; -exports[`tabifyDocs works without provided index pattern 1`] = ` +exports[`tabify_docs tabifyDocs works without provided index pattern 1`] = ` Object { "columns": Array [ Object { diff --git a/src/plugins/data/common/search/tabify/tabify_docs.test.ts b/src/plugins/data/common/search/tabify/tabify_docs.test.ts index 03ecd1be6b5d6..a2910a1be4a9a 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.test.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.test.ts @@ -6,74 +6,137 @@ * Side Public License, v 1. */ -import { tabifyDocs } from './tabify_docs'; -import { IndexPattern } from '../..'; +import { tabifyDocs, flattenHit } from './tabify_docs'; +import { IndexPattern, DataView } from '../..'; import type { estypes } from '@elastic/elasticsearch'; -describe('tabifyDocs', () => { - const fieldFormats = { - getInstance: (id: string) => ({ toJSON: () => ({ id }) }), - getDefaultInstance: (id: string) => ({ toJSON: () => ({ id }) }), - }; +import { fieldFormatsMock } from '../../../../field_formats/common/mocks'; +import { stubbedSavedObjectIndexPattern } from '../../../../data_views/common/data_view.stub'; - const index = new IndexPattern({ +class MockFieldFormatter {} + +fieldFormatsMock.getInstance = jest.fn().mockImplementation(() => new MockFieldFormatter()) as any; + +// helper function to create index patterns +function create(id: string) { + const { + type, + version, + attributes: { timeFieldName, fields, title }, + } = stubbedSavedObjectIndexPattern(id); + + return new DataView({ spec: { - id: 'test-index', - fields: { - sourceTest: { name: 'sourceTest', type: 'number', searchable: true, aggregatable: true }, - fieldTest: { name: 'fieldTest', type: 'number', searchable: true, aggregatable: true }, - 'nested.field': { - name: 'nested.field', - type: 'number', - searchable: true, - aggregatable: true, - }, - }, + id, + type, + version, + timeFieldName, + fields: JSON.parse(fields), + title, + runtimeFieldMap: {}, }, - metaFields: ['_id', '_index', '_score', '_type'], - fieldFormats: fieldFormats as any, + fieldFormats: fieldFormatsMock, + shortDotsEnable: false, + metaFields: ['_id', '_type', '_score', '_routing'], }); +} - // @ts-expect-error not full inteface - const response = { - hits: { - hits: [ +describe('tabify_docs', () => { + describe('flattenHit', () => { + let indexPattern: DataView; + + // create an indexPattern instance for each test + beforeEach(() => { + indexPattern = create('test-pattern'); + }); + + it('returns sorted object keys that combine _source, fields and metaFields in a defined order', () => { + const response = flattenHit( { - _id: 'hit-id-value', - _index: 'hit-index-value', - _type: 'hit-type-value', - _score: 77, - _source: { sourceTest: 123 }, - fields: { fieldTest: 123, invalidMapping: 345, nested: [{ field: 123 }] }, + _index: 'foobar', + _id: 'a', + _source: { + name: 'first', + }, + fields: { + date: ['1'], + zzz: ['z'], + _abc: ['a'], + }, }, - ], - }, - } as estypes.SearchResponse; - - it('converts fields by default', () => { - const table = tabifyDocs(response, index); - expect(table).toMatchSnapshot(); + indexPattern + ); + const expectedOrder = ['_abc', 'date', 'name', 'zzz', '_id', '_routing', '_score', '_type']; + expect(Object.keys(response)).toEqual(expectedOrder); + expect(Object.entries(response).map(([key]) => key)).toEqual(expectedOrder); + }); }); - it('converts source if option is set', () => { - const table = tabifyDocs(response, index, { source: true }); - expect(table).toMatchSnapshot(); - }); + describe('tabifyDocs', () => { + const fieldFormats = { + getInstance: (id: string) => ({ toJSON: () => ({ id }) }), + getDefaultInstance: (id: string) => ({ toJSON: () => ({ id }) }), + }; - it('skips nested fields if option is set', () => { - const table = tabifyDocs(response, index, { shallow: true }); - expect(table).toMatchSnapshot(); - }); + const index = new IndexPattern({ + spec: { + id: 'test-index', + fields: { + sourceTest: { name: 'sourceTest', type: 'number', searchable: true, aggregatable: true }, + fieldTest: { name: 'fieldTest', type: 'number', searchable: true, aggregatable: true }, + 'nested.field': { + name: 'nested.field', + type: 'number', + searchable: true, + aggregatable: true, + }, + }, + }, + metaFields: ['_id', '_index', '_score', '_type'], + fieldFormats: fieldFormats as any, + }); - it('combines meta fields from index pattern', () => { - const table = tabifyDocs(response, index); - expect(table.columns.map((col) => col.id)).toEqual( - expect.arrayContaining(['_id', '_index', '_score', '_type']) - ); - }); + // @ts-expect-error not full inteface + const response = { + hits: { + hits: [ + { + _id: 'hit-id-value', + _index: 'hit-index-value', + _type: 'hit-type-value', + _score: 77, + _source: { sourceTest: 123 }, + fields: { fieldTest: 123, invalidMapping: 345, nested: [{ field: 123 }] }, + }, + ], + }, + } as estypes.SearchResponse; + + it('converts fields by default', () => { + const table = tabifyDocs(response, index); + expect(table).toMatchSnapshot(); + }); + + it('converts source if option is set', () => { + const table = tabifyDocs(response, index, { source: true }); + expect(table).toMatchSnapshot(); + }); + + it('skips nested fields if option is set', () => { + const table = tabifyDocs(response, index, { shallow: true }); + expect(table).toMatchSnapshot(); + }); + + it('combines meta fields from index pattern', () => { + const table = tabifyDocs(response, index); + expect(table.columns.map((col) => col.id)).toEqual( + expect.arrayContaining(['_id', '_index', '_score', '_type']) + ); + }); - it('works without provided index pattern', () => { - const table = tabifyDocs(response); - expect(table).toMatchSnapshot(); + it('works without provided index pattern', () => { + const table = tabifyDocs(response); + expect(table).toMatchSnapshot(); + }); }); }); From f890df218b4fe343b61a8124089a3bf63705feab Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Wed, 13 Oct 2021 10:37:01 +0200 Subject: [PATCH 12/43] Add deprecation comment to implementation --- src/plugins/data_views/common/data_views/flatten_hit.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/plugins/data_views/common/data_views/flatten_hit.ts b/src/plugins/data_views/common/data_views/flatten_hit.ts index 1cadf204e5411..0a6388f0914b1 100644 --- a/src/plugins/data_views/common/data_views/flatten_hit.ts +++ b/src/plugins/data_views/common/data_views/flatten_hit.ts @@ -6,6 +6,11 @@ * Side Public License, v 1. */ +// --------- DEPRECATED --------- +// This implementation of flattenHit is deprecated and should no longer be used. +// If you consider adding features to this, please don't but use the `flattenHit` +// implementation from the data plugin. + import _ from 'lodash'; import { DataView } from './data_view'; From 4f02850168c4cb9aab7e723e66795d9f8dba1663 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 14 Oct 2021 17:49:01 +0200 Subject: [PATCH 13/43] WIP - Show ignored field values --- .../data/common/search/tabify/tabify_docs.ts | 31 ++++++++++--- src/plugins/discover/kibana.json | 3 +- .../public/__mocks__/index_pattern.ts | 8 +--- .../__mocks__/index_pattern_with_timefield.ts | 8 +--- .../doc_table/components/table_row.tsx | 17 +++++-- .../sidebar/lib/field_calculator.js | 2 +- .../apps/main/utils/calc_field_counts.ts | 2 +- .../discover_grid/discover_grid.tsx | 6 ++- .../discover_grid/get_render_cell_value.tsx | 5 ++- .../application/components/table/table.tsx | 12 +++-- .../components/table/table_cell_value.tsx | 21 ++++++++- .../components/table/table_columns.tsx | 14 +++++- .../application/helpers/format_value.ts | 42 +++++++++++++++++ .../application/helpers/get_ignored_reason.ts | 45 +++++++++++++++++++ src/plugins/discover/public/build_services.ts | 3 ++ src/plugins/discover/public/plugin.tsx | 2 + 16 files changed, 184 insertions(+), 37 deletions(-) create mode 100644 src/plugins/discover/public/application/helpers/format_value.ts create mode 100644 src/plugins/discover/public/application/helpers/get_ignored_reason.ts diff --git a/src/plugins/data/common/search/tabify/tabify_docs.ts b/src/plugins/data/common/search/tabify/tabify_docs.ts index 4259488771761..4b8dca6913b08 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.ts @@ -55,8 +55,17 @@ export interface TabifyDocsOptions { * merged into the flattened document. */ source?: boolean; + /** + * If set to `true` values that have been ignored in ES (ignored_field_values) + * will be merged into the flattened document. + */ + includeIgnoredValues?: boolean; } +// This is an overwrite of the SearchHit type to add the ignored_field_values. +// Can be removed once the estypes.SearchHit knows about ignored_field_values +type Hit = estypes.SearchHit & { ignored_field_values?: Record }; + /** * Flattens an individual hit (from an ES response) into an object. This will * create flattened field names, like `user.name`. @@ -65,11 +74,7 @@ export interface TabifyDocsOptions { * @param indexPattern The index pattern for the requested index if available. * @param params Parameters how to flatten the hit */ -export function flattenHit( - hit: estypes.SearchHit, - indexPattern?: IndexPattern, - params?: TabifyDocsOptions -) { +export function flattenHit(hit: Hit, indexPattern?: IndexPattern, params?: TabifyDocsOptions) { const flat = {} as Record; function flatten(obj: Record, keyPrefix: string = '') { @@ -111,6 +116,22 @@ export function flattenHit( flatten(hit._source as Record); } + // TODO: pack behind a method parameter + // The values in `ignored_field_values` are guaranteed to always be wrapped in an array + if (params?.includeIgnoredValues && hit.ignored_field_values) { + Object.entries(hit.ignored_field_values).forEach(([fieldName, fieldValue]) => { + if (flat[fieldName]) { + if (Array.isArray(flat[fieldName])) { + flat[fieldName] = [...flat[fieldName], ...fieldValue]; + } else { + flat[fieldName] = [flat[fieldName], ...fieldValue]; + } + } else { + flat[fieldName] = fieldValue; + } + }); + } + // Merge all valid meta fields into the flattened object // expect for _source (in case that was specified as a meta field) indexPattern?.metaFields?.forEach((metaFieldName) => { diff --git a/src/plugins/discover/kibana.json b/src/plugins/discover/kibana.json index 3d5fdefd276d3..791ce54a0cb1b 100644 --- a/src/plugins/discover/kibana.json +++ b/src/plugins/discover/kibana.json @@ -8,6 +8,7 @@ "data", "embeddable", "inspector", + "fieldFormats", "kibanaLegacy", "urlForwarding", "navigation", @@ -16,7 +17,7 @@ "indexPatternFieldEditor" ], "optionalPlugins": ["home", "share", "usageCollection", "spaces"], - "requiredBundles": ["kibanaUtils", "home", "kibanaReact", "fieldFormats", "dataViews"], + "requiredBundles": ["kibanaUtils", "home", "kibanaReact", "dataViews"], "extraPublicDirs": ["common"], "owner": { "name": "Data Discovery", diff --git a/src/plugins/discover/public/__mocks__/index_pattern.ts b/src/plugins/discover/public/__mocks__/index_pattern.ts index 2acb512617a6b..b1776395877c4 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern.ts @@ -6,8 +6,7 @@ * Side Public License, v 1. */ -import type { estypes } from '@elastic/elasticsearch'; -import { flattenHit, IIndexPatternFieldList } from '../../../data/common'; +import { IIndexPatternFieldList } from '../../../data/common'; import { IndexPattern } from '../../../data/common'; const fields = [ @@ -86,10 +85,5 @@ const indexPattern = { } as unknown as IndexPattern; indexPattern.isTimeBased = () => !!indexPattern.timeFieldName; -indexPattern.formatField = (hit: Record, fieldName: string) => { - return fieldName === '_source' - ? hit._source - : flattenHit(hit as unknown as estypes.SearchHit, indexPattern)[fieldName]; -}; export const indexPatternMock = indexPattern; diff --git a/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts b/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts index 6cf8e8b3485ff..1b86a4809869a 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts @@ -6,9 +6,8 @@ * Side Public License, v 1. */ -import { flattenHit, IIndexPatternFieldList } from '../../../data/common'; +import { IIndexPatternFieldList } from '../../../data/common'; import { IndexPattern } from '../../../data/common'; -import type { estypes } from '@elastic/elasticsearch'; const fields = [ { @@ -77,10 +76,5 @@ const indexPattern = { } as unknown as IndexPattern; indexPattern.isTimeBased = () => !!indexPattern.timeFieldName; -indexPattern.formatField = (hit: Record, fieldName: string) => { - return fieldName === '_source' - ? hit._source - : flattenHit(hit as unknown as estypes.SearchHit, indexPattern)[fieldName]; -}; export const indexPatternWithTimefieldMock = indexPattern; diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx index d91735460af08..6371b6265adb7 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx @@ -10,6 +10,7 @@ import React, { Fragment, useCallback, useMemo, useState } from 'react'; import classNames from 'classnames'; import { i18n } from '@kbn/i18n'; import { EuiButtonEmpty, EuiIcon } from '@elastic/eui'; +import { formatFieldValue } from '../../../../../helpers/format_value'; import { flattenHit } from '../../../../../../../../data/common'; import { DocViewer } from '../../../../../components/doc_viewer/doc_viewer'; import { FilterManager, IndexPattern } from '../../../../../../../../data/public'; @@ -58,7 +59,10 @@ export const TableRow = ({ }); const anchorDocTableRowSubj = row.isAnchor ? ' docTableAnchorRow' : ''; - const flattenedRow = useMemo(() => flattenHit(row, indexPattern), [indexPattern, row]); + const flattenedRow = useMemo( + () => flattenHit(row, indexPattern, { includeIgnoredValues: true }), + [indexPattern, row] + ); const mapping = useMemo(() => indexPattern.fields.getByName, [indexPattern]); // toggle display of the rows details, a full list of the fields from each row @@ -68,7 +72,12 @@ export const TableRow = ({ * Fill an element with the value of a field */ const displayField = (fieldName: string) => { - const formattedField = indexPattern.formatField(row, fieldName); + const formattedField = formatFieldValue( + flattenedRow[fieldName], + row, + indexPattern, + mapping(fieldName) + ); // field formatters take care of escaping // eslint-disable-next-line react/no-danger @@ -142,9 +151,9 @@ export const TableRow = ({ } else { columns.forEach(function (column: string) { // when useNewFieldsApi is true, addressing to the fields property is safe - if (useNewFieldsApi && !mapping(column) && !row.fields![column]) { + if (useNewFieldsApi && !mapping(column) && row.fields && !row.fields[column]) { const innerColumns = Object.fromEntries( - Object.entries(row.fields!).filter(([key]) => { + Object.entries(row.fields).filter(([key]) => { return key.indexOf(`${column}.`) === 0; }) ); diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.js b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.js index be7e9c616273d..c709f3311105d 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.js +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/lib/field_calculator.js @@ -13,7 +13,7 @@ import { flattenHit } from '../../../../../../../../data/common'; function getFieldValues(hits, field, indexPattern) { const name = field.name; return map(hits, function (hit) { - return flattenHit(hit, indexPattern)[name]; + return flattenHit(hit, indexPattern, { includeIgnoredValues: true })[name]; }); } diff --git a/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts b/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts index 211c4e5c8b069..2198d2f66b6b4 100644 --- a/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts +++ b/src/plugins/discover/public/application/apps/main/utils/calc_field_counts.ts @@ -22,7 +22,7 @@ export function calcFieldCounts( return {}; } for (const hit of rows) { - const fields = Object.keys(flattenHit(hit, indexPattern)); + const fields = Object.keys(flattenHit(hit, indexPattern, { includeIgnoredValues: true })); for (const fieldName of fields) { counts[fieldName] = (counts[fieldName] || 0) + 1; } diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx index 11323080274a9..ca403c813010b 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid.tsx @@ -271,7 +271,11 @@ export const DiscoverGrid = ({ getRenderCellValueFn( indexPattern, displayedRows, - displayedRows ? displayedRows.map((hit) => flattenHit(hit, indexPattern)) : [], + displayedRows + ? displayedRows.map((hit) => + flattenHit(hit, indexPattern, { includeIgnoredValues: true }) + ) + : [], useNewFieldsApi, fieldsToShow, services.uiSettings.get(MAX_DOC_FIELDS_DISPLAYED) diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx index a052971580666..bed75e30751f8 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx @@ -22,6 +22,7 @@ import { DiscoverGridContext } from './discover_grid_context'; import { JsonCodeEditor } from '../json_code_editor/json_code_editor'; import { defaultMonacoEditorWidth } from './constants'; import { EsHitRecord } from '../../types'; +import { formatFieldValue } from '../../helpers/format_value'; export const getRenderCellValueFn = ( @@ -191,12 +192,12 @@ export const getRenderCellValueFn = return {JSON.stringify(rowFlattened[columnId])}; } - const valueFormatted = indexPattern.formatField(row, columnId); + const valueFormatted = formatFieldValue(rowFlattened[columnId], row, indexPattern, field); if (typeof valueFormatted === 'undefined') { return -; } return ( // eslint-disable-next-line react/no-danger - + ); }; diff --git a/src/plugins/discover/public/application/components/table/table.tsx b/src/plugins/discover/public/application/components/table/table.tsx index 7f597d846f88f..8ab60412eac42 100644 --- a/src/plugins/discover/public/application/components/table/table.tsx +++ b/src/plugins/discover/public/application/components/table/table.tsx @@ -20,6 +20,8 @@ import { } from '../../doc_views/doc_views_types'; import { ACTIONS_COLUMN, MAIN_COLUMNS } from './table_columns'; import { getFieldsToShow } from '../../helpers/get_fields_to_show'; +import { getIgnoredReason, IgnoredReason } from '../../helpers/get_ignored_reason'; +import { formatFieldValue } from '../../helpers/format_value'; export interface DocViewerTableProps { columns?: string[]; @@ -46,6 +48,7 @@ export interface FieldRecord { }; value: { formattedValue: string; + ignored?: IgnoredReason; }; } @@ -64,8 +67,6 @@ export const DocViewerTable = ({ [indexPattern?.fields] ); - const formattedHit = useMemo(() => indexPattern?.formatHit(hit, 'html'), [hit, indexPattern]); - const tableColumns = useMemo(() => { return filter ? [ACTIONS_COLUMN, ...MAIN_COLUMNS] : MAIN_COLUMNS; }, [filter]); @@ -96,7 +97,7 @@ export const DocViewerTable = ({ return null; } - const flattened = flattenHit(hit, indexPattern, { source: true }); + const flattened = flattenHit(hit, indexPattern, { source: true, includeIgnoredValues: true }); const fieldsToShow = getFieldsToShow(Object.keys(flattened), indexPattern, showMultiFields); const items: FieldRecord[] = Object.keys(flattened) @@ -115,6 +116,8 @@ export const DocViewerTable = ({ const displayName = fieldMapping?.displayName ?? field; const fieldType = isNestedFieldParent(field, indexPattern) ? 'nested' : fieldMapping?.type; + const ignored = getIgnoredReason(fieldMapping ?? field, hit._ignored); + return { action: { onToggleColumn, @@ -130,7 +133,8 @@ export const DocViewerTable = ({ scripted: Boolean(fieldMapping?.scripted), }, value: { - formattedValue: formattedHit[field], + formattedValue: formatFieldValue(flattened[field], hit, indexPattern, fieldMapping), + ignored, }, }; }); diff --git a/src/plugins/discover/public/application/components/table/table_cell_value.tsx b/src/plugins/discover/public/application/components/table/table_cell_value.tsx index 22c84b23949e1..af19d52c99644 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_value.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_value.tsx @@ -8,14 +8,24 @@ import classNames from 'classnames'; import React, { Fragment, useState } from 'react'; +import { IgnoredReason } from '../../helpers/get_ignored_reason'; import { FieldRecord } from './table'; import { DocViewTableRowBtnCollapse } from './table_row_btn_collapse'; const COLLAPSE_LINE_LENGTH = 350; -type TableFieldValueProps = FieldRecord['value'] & Pick; +type TableFieldValueProps = Pick & { + formattedValue: FieldRecord['value']['formattedValue']; + rawValue: unknown; + ignoreReason?: IgnoredReason; +}; -export const TableFieldValue = ({ formattedValue, field }: TableFieldValueProps) => { +export const TableFieldValue = ({ + formattedValue, + field, + rawValue, + ignoreReason, +}: TableFieldValueProps) => { const [fieldOpen, setFieldOpen] = useState(false); const value = String(formattedValue); @@ -30,11 +40,18 @@ export const TableFieldValue = ({ formattedValue, field }: TableFieldValueProps) const onToggleCollapse = () => setFieldOpen((fieldOpenPrev) => !fieldOpenPrev); + const multiValue = Array.isArray(rawValue) && rawValue.length > 1; + return ( {isCollapsible && ( )} + {ignoreReason && ( + + {ignoreReason.toString()} (multiValue: {String(multiValue)}) + + )}
> = [ ), - render: ({ formattedValue }: FieldRecord['value'], { field: { field } }: FieldRecord) => { - return ; + render: ( + { formattedValue, ignored }: FieldRecord['value'], + { field: { field }, action: { flattenedField } }: FieldRecord + ) => { + return ( + + ); }, }, ]; diff --git a/src/plugins/discover/public/application/helpers/format_value.ts b/src/plugins/discover/public/application/helpers/format_value.ts new file mode 100644 index 0000000000000..ee6e63baee839 --- /dev/null +++ b/src/plugins/discover/public/application/helpers/format_value.ts @@ -0,0 +1,42 @@ +/* + * 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 { estypes } from '@elastic/elasticsearch'; +import { DataView, DataViewField, KBN_FIELD_TYPES } from '../../../../data/common'; +import { getServices } from '../../kibana_services'; + +// TODO: need more test coverage + +/** + * Formats the value of a specific field using the appropriate field formatter if available + * or the default string field formatter otherwise. + * + * @param value The value to format + * @param hit The actual search hit (required to get highlight information from) + * @param dataView The data view if available + * @param field The field that value was from if available + */ +export function formatFieldValue( + value: unknown, + hit: estypes.SearchHit, + dataView?: DataView, + field?: DataViewField +): string { + if (!dataView || !field) { + // If either no field is available or no data view, we'll use the default + // string formatter to format that field. + return getServices() + .fieldFormats.getDefaultInstance(KBN_FIELD_TYPES.STRING) + .convert(value, 'html', { hit, field, indexPattern: dataView }); + } + + // If we have a data view and field we use that fields field formatter + return dataView + .getFormatterForField(field) + .convert(value, 'html', { hit, field, indexPattern: dataView }); +} diff --git a/src/plugins/discover/public/application/helpers/get_ignored_reason.ts b/src/plugins/discover/public/application/helpers/get_ignored_reason.ts new file mode 100644 index 0000000000000..47c6bd04c51d0 --- /dev/null +++ b/src/plugins/discover/public/application/helpers/get_ignored_reason.ts @@ -0,0 +1,45 @@ +/* + * 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 { estypes } from '@elastic/elasticsearch'; +import { DataViewField, KBN_FIELD_TYPES } from '../../../../data/common'; + +export enum IgnoredReason { + IGNORE_ABOVE = 'ignore_above', + MALFORMED = 'malformed', + UNKNOWN = 'unknown', +} + +// TODO: Writing unit tests + +export function getIgnoredReason( + field: DataViewField | string, + ignoredFields: estypes.SearchHit['_ignored'] +): IgnoredReason | undefined { + const fieldName = typeof field === 'string' ? field : field.name; + if (!ignoredFields?.includes(fieldName)) { + return undefined; + } + + if (typeof field === 'string') { + return IgnoredReason.UNKNOWN; + } + + switch (field.type) { + case KBN_FIELD_TYPES.STRING: + return IgnoredReason.IGNORE_ABOVE; + case KBN_FIELD_TYPES.NUMBER: + case KBN_FIELD_TYPES.DATE: + case KBN_FIELD_TYPES.GEO_POINT: + case KBN_FIELD_TYPES.GEO_SHAPE: + case KBN_FIELD_TYPES.IP: + return IgnoredReason.MALFORMED; + default: + return IgnoredReason.UNKNOWN; + } +} diff --git a/src/plugins/discover/public/build_services.ts b/src/plugins/discover/public/build_services.ts index ab2484abee892..ac16b6b3cc2ba 100644 --- a/src/plugins/discover/public/build_services.ts +++ b/src/plugins/discover/public/build_services.ts @@ -36,6 +36,7 @@ import { KibanaLegacyStart } from '../../kibana_legacy/public'; import { UrlForwardingStart } from '../../url_forwarding/public'; import { NavigationPublicPluginStart } from '../../navigation/public'; import { IndexPatternFieldEditorStart } from '../../index_pattern_field_editor/public'; +import { FieldFormatsStart } from '../../field_formats/public'; import type { SpacesApi } from '../../../../x-pack/plugins/spaces/public'; @@ -49,6 +50,7 @@ export interface DiscoverServices { history: () => History; theme: ChartsPluginStart['theme']; filterManager: FilterManager; + fieldFormats: FieldFormatsStart; indexPatterns: IndexPatternsContract; inspector: InspectorPublicPluginStart; metadata: { branch: string }; @@ -82,6 +84,7 @@ export function buildServices( data: plugins.data, docLinks: core.docLinks, theme: plugins.charts.theme, + fieldFormats: plugins.fieldFormats, filterManager: plugins.data.query.filterManager, history: getHistory, indexPatterns: plugins.data.indexPatterns, diff --git a/src/plugins/discover/public/plugin.tsx b/src/plugins/discover/public/plugin.tsx index e34e7644caa25..086e93363827a 100644 --- a/src/plugins/discover/public/plugin.tsx +++ b/src/plugins/discover/public/plugin.tsx @@ -62,6 +62,7 @@ import { IndexPatternFieldEditorStart } from '../../../plugins/index_pattern_fie import { DeferredSpinner } from './shared'; import { ViewSavedSearchAction } from './application/embeddable/view_saved_search_action'; import type { SpacesPluginStart } from '../../../../x-pack/plugins/spaces/public'; +import { FieldFormatsStart } from '../../field_formats/public'; declare module '../../share/public' { export interface UrlGeneratorStateMapping { @@ -185,6 +186,7 @@ export interface DiscoverStartPlugins { navigation: NavigationStart; charts: ChartsPluginStart; data: DataPublicPluginStart; + fieldFormats: FieldFormatsStart; share?: SharePluginStart; kibanaLegacy: KibanaLegacyStart; urlForwarding: UrlForwardingStart; From afad0732897ae8a85aae8fb45e8369c052a5b44a Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 14 Oct 2021 17:55:12 +0200 Subject: [PATCH 14/43] Disable filters in doc_table --- .../main/components/doc_table/components/table_row.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx index 6371b6265adb7..24e6ba58409c9 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx @@ -170,7 +170,13 @@ export const TableRow = ({ /> ); } else { - const isFilterable = Boolean(mapping(column)?.filterable && filter); + // Check whether the field is defined as filterable in the mapping and does + // NOT have ignored values in it to determine whether we want to allow filtering. + // We should improve this and show a helpful tooltip why the filter buttons are not + // there/disabled when there are ignored values. + const isFilterable = Boolean( + mapping(column)?.filterable && filter && !row._ignored?.includes(column) + ); rowCells.push( Date: Thu, 14 Oct 2021 17:59:49 +0200 Subject: [PATCH 15/43] remove redundant comments --- src/plugins/data/common/search/tabify/tabify_docs.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/plugins/data/common/search/tabify/tabify_docs.ts b/src/plugins/data/common/search/tabify/tabify_docs.ts index 4b8dca6913b08..98a9c83224f19 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.ts @@ -116,8 +116,6 @@ export function flattenHit(hit: Hit, indexPattern?: IndexPattern, params?: Tabif flatten(hit._source as Record); } - // TODO: pack behind a method parameter - // The values in `ignored_field_values` are guaranteed to always be wrapped in an array if (params?.includeIgnoredValues && hit.ignored_field_values) { Object.entries(hit.ignored_field_values).forEach(([fieldName, fieldValue]) => { if (flat[fieldName]) { From 71ac26c297baf1ec0bf76ead8fd47f47ad856531 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 14 Oct 2021 18:09:56 +0200 Subject: [PATCH 16/43] No, it wasn't --- .../apps/main/components/doc_table/components/table_row.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx index 24e6ba58409c9..5f820e51dfc9a 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx @@ -150,7 +150,6 @@ export const TableRow = ({ ); } else { columns.forEach(function (column: string) { - // when useNewFieldsApi is true, addressing to the fields property is safe if (useNewFieldsApi && !mapping(column) && row.fields && !row.fields[column]) { const innerColumns = Object.fromEntries( Object.entries(row.fields).filter(([key]) => { From 3dc6746be8aa0c17fac8124e7eb5675fac67b27e Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Thu, 14 Oct 2021 18:41:19 +0200 Subject: [PATCH 17/43] start warning message --- .../components/table/table_cell_value.tsx | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/plugins/discover/public/application/components/table/table_cell_value.tsx b/src/plugins/discover/public/application/components/table/table_cell_value.tsx index af19d52c99644..76ea08d0a1bfb 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_value.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_value.tsx @@ -6,6 +6,7 @@ * Side Public License, v 1. */ +import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiTextColor } from '@elastic/eui'; import classNames from 'classnames'; import React, { Fragment, useState } from 'react'; import { IgnoredReason } from '../../helpers/get_ignored_reason'; @@ -14,6 +15,28 @@ import { DocViewTableRowBtnCollapse } from './table_row_btn_collapse'; const COLLAPSE_LINE_LENGTH = 350; +interface IgnoreWarningProps { + reason: IgnoredReason; + rawValue: unknown; +} + +const IgnoreWarning: React.FC = ({ rawValue, reason }) => { + const multiValue = Array.isArray(rawValue) && rawValue.length > 1; + + // TODO: Add tooltip with a bit more information + // TODO: Differentiate between single value and multi value in wording + return ( + + + + + + Ignored value + + + ); +}; + type TableFieldValueProps = Pick & { formattedValue: FieldRecord['value']['formattedValue']; rawValue: unknown; @@ -40,18 +63,13 @@ export const TableFieldValue = ({ const onToggleCollapse = () => setFieldOpen((fieldOpenPrev) => !fieldOpenPrev); - const multiValue = Array.isArray(rawValue) && rawValue.length > 1; - return ( + {/* TODO: collapse those two into one line if both are showing */} {isCollapsible && ( )} - {ignoreReason && ( - - {ignoreReason.toString()} (multiValue: {String(multiValue)}) - - )} + {ignoreReason && }
Date: Thu, 14 Oct 2021 18:46:37 +0200 Subject: [PATCH 18/43] Enable ignored values in CSV reports --- .../export_types/csv_searchsource/generate_csv/generate_csv.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts index 6c2989d54309d..77ad4fba1ab60 100644 --- a/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts +++ b/x-pack/plugins/reporting/server/export_types/csv_searchsource/generate_csv/generate_csv.ts @@ -356,7 +356,7 @@ export class CsvGenerator { let table: Datatable | undefined; try { - table = tabifyDocs(results, index, { shallow: true }); + table = tabifyDocs(results, index, { shallow: true, includeIgnoredValues: true }); } catch (err) { this.logger.error(err); } From a74679909ee8bde5d21ee3fed9136c424a631e5c Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Fri, 15 Oct 2021 10:08:55 +0200 Subject: [PATCH 19/43] Add help tooltip --- .../components/table/table_cell_value.tsx | 75 +++++++++++++++---- 1 file changed, 62 insertions(+), 13 deletions(-) diff --git a/src/plugins/discover/public/application/components/table/table_cell_value.tsx b/src/plugins/discover/public/application/components/table/table_cell_value.tsx index 76ea08d0a1bfb..eae5764d0273e 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_value.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_value.tsx @@ -6,9 +6,11 @@ * Side Public License, v 1. */ -import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiTextColor } from '@elastic/eui'; +import { css } from '@emotion/react'; +import { EuiFlexGroup, EuiFlexItem, EuiIcon, EuiTextColor, EuiToolTip } from '@elastic/eui'; import classNames from 'classnames'; import React, { Fragment, useState } from 'react'; +import { i18n } from '@kbn/i18n'; import { IgnoredReason } from '../../helpers/get_ignored_reason'; import { FieldRecord } from './table'; import { DocViewTableRowBtnCollapse } from './table_row_btn_collapse'; @@ -20,22 +22,69 @@ interface IgnoreWarningProps { rawValue: unknown; } -const IgnoreWarning: React.FC = ({ rawValue, reason }) => { +const IgnoreWarning: React.FC = React.memo(({ rawValue, reason }) => { const multiValue = Array.isArray(rawValue) && rawValue.length > 1; - // TODO: Add tooltip with a bit more information - // TODO: Differentiate between single value and multi value in wording + const getToolTipContent = (): string => { + switch (reason) { + case IgnoredReason.IGNORE_ABOVE: + return multiValue + ? i18n.translate('discover.docView.table.ignored.multiAboveTooltip', { + defaultMessage: + 'This field contains one or more values that are too long and are thus not searchable or filterable.', + }) + : i18n.translate('discover.docView.table.ignored.singleAboveTooltip', { + defaultMessage: + 'The value in this field is too long and thus not searchable or filterable.', + }); + case IgnoredReason.MALFORMED: + return multiValue + ? i18n.translate('discover.docView.table.ignored.multiMalformedTooltip', { + defaultMessage: + 'This field contains one or more values that are malformed and are thus not searchable or filterable.', + }) + : i18n.translate('discover.docView.table.ignored.singleMalformedTooltip', { + defaultMessage: `The value in this field is malformed and not parsable. It's not searchable or filterable.`, + }); + case IgnoredReason.UNKNOWN: + return multiValue + ? i18n.translate('discover.docView.table.ignored.multiUnknownTooltip', { + defaultMessage: + 'This field contains one or more values that are ignored by Elasticsearch and are thus not searchable or filterable.', + }) + : i18n.translate('discover.docView.table.ignored.singleUnknownTooltip', { + defaultMessage: `The value in this field is ignored by Elasticsearch. It's not searchable or filterable.`, + }); + } + }; + return ( - - - - - - Ignored value - - + + + + + + + + {multiValue + ? i18n.translate('discover.docViews.table.ignored.multiValueLabel', { + defaultMessage: 'Contains ignored values', + }) + : i18n.translate('discover.docViews.table.ignored.singleValueLabel', { + defaultMessage: 'Ignored value', + })} + + + + ); -}; +}); type TableFieldValueProps = Pick & { formattedValue: FieldRecord['value']['formattedValue']; From ddfaf53ed9a3c82dccd9f730c1e005ceaed889a1 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Fri, 15 Oct 2021 10:13:15 +0200 Subject: [PATCH 20/43] Better styling with warning plus collapsible button --- .../components/table/table_cell_value.tsx | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/plugins/discover/public/application/components/table/table_cell_value.tsx b/src/plugins/discover/public/application/components/table/table_cell_value.tsx index eae5764d0273e..ebdd7c6750241 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_value.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_value.tsx @@ -114,11 +114,20 @@ export const TableFieldValue = ({ return ( - {/* TODO: collapse those two into one line if both are showing */} - {isCollapsible && ( - + {(isCollapsible || ignoreReason) && ( + + {isCollapsible && ( + + + + )} + {ignoreReason && ( + + + + )} + )} - {ignoreReason && }
Date: Fri, 15 Oct 2021 10:23:30 +0200 Subject: [PATCH 21/43] Disable filtering within table for ignored values --- .../application/components/table/table_cell_actions.tsx | 6 ++++-- .../public/application/components/table/table_columns.tsx | 3 ++- .../components/table/table_row_btn_filter_add.tsx | 2 +- .../components/table/table_row_btn_filter_remove.tsx | 2 +- x-pack/plugins/translations/translations/ja-JP.json | 1 - x-pack/plugins/translations/translations/zh-CN.json | 3 +-- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/plugins/discover/public/application/components/table/table_cell_actions.tsx b/src/plugins/discover/public/application/components/table/table_cell_actions.tsx index 7f2f87e7c296c..e43a17448de2e 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_actions.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_actions.tsx @@ -21,6 +21,7 @@ interface TableActionsProps { fieldMapping?: IndexPatternField; onFilter: DocViewFilterFn; onToggleColumn: (field: string) => void; + ignoredValue: boolean; } export const TableActions = ({ @@ -30,15 +31,16 @@ export const TableActions = ({ flattenedField, onToggleColumn, onFilter, + ignoredValue, }: TableActionsProps) => { return (
onFilter(fieldMapping, flattenedField, '+')} /> onFilter(fieldMapping, flattenedField, '-')} /> = { ), render: ( { flattenedField, isActive, onFilter, onToggleColumn }: FieldRecord['action'], - { field: { field, fieldMapping } }: FieldRecord + { field: { field, fieldMapping }, value: { ignored } }: FieldRecord ) => { return ( = { flattenedField={flattenedField} onFilter={onFilter!} onToggleColumn={onToggleColumn} + ignoredValue={!!ignored} /> ); }, diff --git a/src/plugins/discover/public/application/components/table/table_row_btn_filter_add.tsx b/src/plugins/discover/public/application/components/table/table_row_btn_filter_add.tsx index 5fe1b4dc33342..5812e0859bd94 100644 --- a/src/plugins/discover/public/application/components/table/table_row_btn_filter_add.tsx +++ b/src/plugins/discover/public/application/components/table/table_row_btn_filter_add.tsx @@ -20,7 +20,7 @@ export function DocViewTableRowBtnFilterAdd({ onClick, disabled = false }: Props const tooltipContent = disabled ? ( ) : ( ) : ( Date: Fri, 15 Oct 2021 12:05:20 +0200 Subject: [PATCH 22/43] Fix jest tests --- .../public/__mocks__/index_pattern.ts | 3 +-- .../__mocks__/index_pattern_with_timefield.ts | 3 +-- .../layout/discover_layout.test.tsx | 10 ++++++++++ .../get_render_cell_value.test.tsx | 5 ++++- .../components/table/table.test.tsx | 20 ++++++------------- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/plugins/discover/public/__mocks__/index_pattern.ts b/src/plugins/discover/public/__mocks__/index_pattern.ts index b1776395877c4..7a6b75494729a 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern.ts @@ -73,7 +73,6 @@ const indexPattern = { title: 'the-index-pattern-title', metaFields: ['_index', '_score'], formatField: jest.fn(), - flattenHit: undefined, formatHit: jest.fn((hit) => (hit.fields ? hit.fields : hit._source)), fields, getComputedFields: () => ({ docvalueFields: [], scriptFields: {}, storedFields: ['*'] }), @@ -81,7 +80,7 @@ const indexPattern = { getFieldByName: jest.fn(() => ({})), timeFieldName: '', docvalueFields: [], - getFormatterForField: () => ({ convert: () => 'formatted' }), + getFormatterForField: () => ({ convert: (value: unknown) => value }), } as unknown as IndexPattern; indexPattern.isTimeBased = () => !!indexPattern.timeFieldName; diff --git a/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts b/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts index 1b86a4809869a..221784335f10f 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts @@ -63,14 +63,13 @@ const indexPattern = { id: 'index-pattern-with-timefield-id', title: 'index-pattern-with-timefield', metaFields: ['_index', '_score'], - flattenHit: undefined, formatHit: jest.fn((hit) => hit._source), fields, getComputedFields: () => ({}), getSourceFiltering: () => ({}), getFieldByName: (name: string) => fields.getByName(name), timeFieldName: 'timestamp', - getFormatterForField: () => ({ convert: () => 'formatted' }), + getFormatterForField: () => ({ convert: (value: unknown) => value }), isTimeNanosBased: () => false, popularizeField: () => {}, } as unknown as IndexPattern; diff --git a/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx b/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx index 6ebed3185e2f1..ac68986280242 100644 --- a/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx +++ b/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx @@ -33,6 +33,16 @@ import { RequestAdapter } from '../../../../../../../inspector'; import { Chart } from '../chart/point_series'; import { DiscoverSidebar } from '../sidebar/discover_sidebar'; +jest.mock('../../../../../kibana_services', () => ({ + ...jest.requireActual('../../../../../kibana_services'), + getServices: () => ({ + fieldFormats: { + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), + }, + }), +})); + setHeaderActionMenuMounter(jest.fn()); function getProps(indexPattern: IndexPattern, wasSidebarClosed?: boolean): DiscoverLayoutProps { diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx index 6556876217953..b0da80c962b00 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx @@ -25,6 +25,9 @@ jest.mock('../../../kibana_services', () => ({ uiSettings: { get: jest.fn(), }, + fieldFormats: { + getDefaultInstance: jest.fn(() => ({ convert: (value) => value })), + }, }), })); @@ -368,7 +371,7 @@ describe('Discover grid cell rendering', function () { className="dscDiscoverGrid__descriptionListDescription" dangerouslySetInnerHTML={ Object { - "__html": "formatted", + "__html": "100", } } /> diff --git a/src/plugins/discover/public/application/components/table/table.test.tsx b/src/plugins/discover/public/application/components/table/table.test.tsx index ce914edcec703..715de181e7536 100644 --- a/src/plugins/discover/public/application/components/table/table.test.tsx +++ b/src/plugins/discover/public/application/components/table/table.test.tsx @@ -27,6 +27,10 @@ import { getServices } from '../../../kibana_services'; } }, }, + fieldFormats: { + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), + }, })); const indexPattern = { @@ -65,7 +69,7 @@ const indexPattern = { ], }, metaFields: ['_index', '_score'], - flattenHit: jest.fn(), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), formatHit: jest.fn((hit) => hit._source), } as unknown as IndexPattern; @@ -359,19 +363,7 @@ describe('DocViewTable at Discover Doc with Fields API', () => { ], }, metaFields: ['_index', '_type', '_score', '_id'], - flattenHit: jest.fn((hit) => { - const result = {} as Record; - Object.keys(hit).forEach((key) => { - if (key !== 'fields') { - result[key] = hit[key]; - } else { - Object.keys(hit.fields).forEach((field) => { - result[field] = hit.fields[field]; - }); - } - }); - return result; - }), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), formatHit: jest.fn((hit) => { const result = {} as Record; Object.keys(hit).forEach((key) => { From cc07ef4a8cac4cd8ebc1a6d1e95f994c85a3a7e6 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Fri, 15 Oct 2021 12:21:01 +0200 Subject: [PATCH 23/43] Fix types in tests --- .../components/discover_grid/get_render_cell_value.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx index b0da80c962b00..a0f6fc7bcd3e6 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx @@ -26,7 +26,7 @@ jest.mock('../../../kibana_services', () => ({ get: jest.fn(), }, fieldFormats: { - getDefaultInstance: jest.fn(() => ({ convert: (value) => value })), + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), }, }), })); From 0337bafb7afd65db157fae80c6bc4dcd0fa140ca Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Fri, 15 Oct 2021 15:25:16 +0200 Subject: [PATCH 24/43] Add more tests and documentation --- .../common/search/tabify/tabify_docs.test.ts | 49 +++++++++++++++++ .../data/common/search/tabify/tabify_docs.ts | 17 ++++-- .../helpers/get_ignored_reason.test.ts | 54 +++++++++++++++++++ .../application/helpers/get_ignored_reason.ts | 9 ++++ .../translations/translations/zh-CN.json | 2 +- 5 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 src/plugins/discover/public/application/helpers/get_ignored_reason.test.ts diff --git a/src/plugins/data/common/search/tabify/tabify_docs.test.ts b/src/plugins/data/common/search/tabify/tabify_docs.test.ts index a2910a1be4a9a..1964247b09585 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.test.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.test.ts @@ -41,6 +41,11 @@ function create(id: string) { }); } +const meta = { + _index: 'index-name', + _id: '1', +}; + describe('tabify_docs', () => { describe('flattenHit', () => { let indexPattern: DataView; @@ -70,6 +75,50 @@ describe('tabify_docs', () => { expect(Object.keys(response)).toEqual(expectedOrder); expect(Object.entries(response).map(([key]) => key)).toEqual(expectedOrder); }); + + it('does merge values from ignored_field_values and fields correctly', () => { + const flatten = flattenHit( + { + ...meta, + fields: { 'extension.keyword': ['foo'], extension: ['foo', 'ignored'] }, + ignored_field_values: { + 'extension.keyword': ['ignored'], + fully_ignored: ['some', 'value'], + }, + }, + indexPattern, + { includeIgnoredValues: true } + ); + expect(flatten).toHaveProperty(['extension.keyword'], ['foo', 'ignored']); + expect(flatten).toHaveProperty('extension', ['foo', 'ignored']); + expect(flatten).toHaveProperty('fully_ignored', ['some', 'value']); + }); + + it('does not merge values from ignored_field_values into _source', () => { + const flatten = flattenHit( + { + ...meta, + _source: { 'extension.keyword': ['foo', 'ignored'] }, + ignored_field_values: { 'extension.keyword': ['ignored'] }, + }, + indexPattern, + { includeIgnoredValues: true, source: true } + ); + expect(flatten).toHaveProperty(['extension.keyword'], ['foo', 'ignored']); + }); + + it('does merge ignored_field_values when no _source was present, even when parameter was on', () => { + const flatten = flattenHit( + { + ...meta, + fields: { 'extension.keyword': ['foo'] }, + ignored_field_values: { 'extension.keyword': ['ignored'] }, + }, + indexPattern, + { includeIgnoredValues: true, source: true } + ); + expect(flatten).toHaveProperty(['extension.keyword'], ['foo', 'ignored']); + }); }); describe('tabifyDocs', () => { diff --git a/src/plugins/data/common/search/tabify/tabify_docs.ts b/src/plugins/data/common/search/tabify/tabify_docs.ts index 98a9c83224f19..353a0c10ba12a 100644 --- a/src/plugins/data/common/search/tabify/tabify_docs.ts +++ b/src/plugins/data/common/search/tabify/tabify_docs.ts @@ -57,7 +57,8 @@ export interface TabifyDocsOptions { source?: boolean; /** * If set to `true` values that have been ignored in ES (ignored_field_values) - * will be merged into the flattened document. + * will be merged into the flattened document. This will only have an effect if + * the `hit` has been retrieved using the `fields` option. */ includeIgnoredValues?: boolean; } @@ -114,17 +115,25 @@ export function flattenHit(hit: Hit, indexPattern?: IndexPattern, params?: Tabif flatten(hit.fields || {}); if (params?.source !== false && hit._source) { flatten(hit._source as Record); - } - - if (params?.includeIgnoredValues && hit.ignored_field_values) { + } else if (params?.includeIgnoredValues && hit.ignored_field_values) { + // If enabled merge the ignored_field_values into the flattened hit. This will + // merge values that are not actually indexed by ES (i.e. ignored), e.g. because + // they were above the `ignore_above` limit or malformed for specific types. + // This API will only contain the values that were actually ignored, i.e. for the same + // field there might exist another value in the `fields` response, why this logic + // merged them both together. We do not merge this (even if enabled) in case source has been + // merged, since we would otherwise duplicate values, since ignore_field_values and _source + // contain the same values. Object.entries(hit.ignored_field_values).forEach(([fieldName, fieldValue]) => { if (flat[fieldName]) { + // If there was already a value from the fields API, make sure we're merging both together if (Array.isArray(flat[fieldName])) { flat[fieldName] = [...flat[fieldName], ...fieldValue]; } else { flat[fieldName] = [flat[fieldName], ...fieldValue]; } } else { + // If no previous value was assigned we can simply use the value from `ignored_field_values` as it is flat[fieldName] = fieldValue; } }); diff --git a/src/plugins/discover/public/application/helpers/get_ignored_reason.test.ts b/src/plugins/discover/public/application/helpers/get_ignored_reason.test.ts new file mode 100644 index 0000000000000..13632ca5ed901 --- /dev/null +++ b/src/plugins/discover/public/application/helpers/get_ignored_reason.test.ts @@ -0,0 +1,54 @@ +/* + * 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 { getIgnoredReason, IgnoredReason } from './get_ignored_reason'; +import { DataViewField, KBN_FIELD_TYPES } from '../../../../data/common'; + +function field(params: Partial): DataViewField { + return { + name: 'text', + type: 'keyword', + ...params, + } as unknown as DataViewField; +} + +describe('getIgnoredReason', () => { + it('will correctly return undefined when no value was ignored', () => { + expect(getIgnoredReason(field({ name: 'foo' }), undefined)).toBeUndefined(); + expect(getIgnoredReason(field({ name: 'foo' }), ['bar', 'baz'])).toBeUndefined(); + }); + + it('will return UNKNOWN if the field passed in was only a name, and thus no type information is present', () => { + expect(getIgnoredReason('foo', ['foo'])).toBe(IgnoredReason.UNKNOWN); + }); + + it('will return IGNORE_ABOVE for string types', () => { + expect(getIgnoredReason(field({ name: 'foo', type: KBN_FIELD_TYPES.STRING }), ['foo'])).toBe( + IgnoredReason.IGNORE_ABOVE + ); + }); + + // Each type that can have malformed values + [ + KBN_FIELD_TYPES.DATE, + KBN_FIELD_TYPES.IP, + KBN_FIELD_TYPES.GEO_POINT, + KBN_FIELD_TYPES.GEO_SHAPE, + KBN_FIELD_TYPES.NUMBER, + ].forEach((type) => { + it(`will return MALFORMED for ${type} fields`, () => { + expect(getIgnoredReason(field({ name: 'foo', type }), ['foo'])).toBe(IgnoredReason.MALFORMED); + }); + }); + + it('will return unknown reasons if it does not know what the reason was', () => { + expect(getIgnoredReason(field({ name: 'foo', type: 'range' }), ['foo'])).toBe( + IgnoredReason.UNKNOWN + ); + }); +}); diff --git a/src/plugins/discover/public/application/helpers/get_ignored_reason.ts b/src/plugins/discover/public/application/helpers/get_ignored_reason.ts index 47c6bd04c51d0..7550c7b2fc92a 100644 --- a/src/plugins/discover/public/application/helpers/get_ignored_reason.ts +++ b/src/plugins/discover/public/application/helpers/get_ignored_reason.ts @@ -17,6 +17,15 @@ export enum IgnoredReason { // TODO: Writing unit tests +/** + * Returns the reason why a specific field was ignored in the response. + * Will return undefined if the field had no ignored values in it. + * This implementation will make some assumptions based on specific types + * of ignored values can only happen with specific field types in Elasticsearch. + * + * @param field Either the data view field or the strnig name of it. + * @param ignoredFields The hit._ignored value of the hit to validate. + */ export function getIgnoredReason( field: DataViewField | string, ignoredFields: estypes.SearchHit['_ignored'] diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index afac6f15e74f1..9194a49658caf 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -2432,7 +2432,7 @@ "discover.docTable.tableRow.filterOutValueButtonTooltip": "筛除值", "discover.docTable.tableRow.toggleRowDetailsButtonAriaLabel": "切换行详细信息", "discover.docTable.tableRow.viewSingleDocumentLinkText": "查看单个文档", - "discover.docTable.tableRow.viewSurroundingDocumentsLinkText": "查看周围文档", + "discover.docTable.tableRow.viewSurroundingDocumentsLinkText": "查看周围文档", "discover.docTable.totalDocuments": "{totalDocuments} 个文档", "discover.documentsAriaLabel": "文档", "discover.docViews.json.jsonTitle": "JSON", From 1fd926c22449dba80c8757100ad10bc4dd5bc50a Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Fri, 15 Oct 2021 15:26:22 +0200 Subject: [PATCH 25/43] Remove comment --- .../discover/public/application/helpers/get_ignored_reason.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/plugins/discover/public/application/helpers/get_ignored_reason.ts b/src/plugins/discover/public/application/helpers/get_ignored_reason.ts index 7550c7b2fc92a..a64a8f587175d 100644 --- a/src/plugins/discover/public/application/helpers/get_ignored_reason.ts +++ b/src/plugins/discover/public/application/helpers/get_ignored_reason.ts @@ -15,8 +15,6 @@ export enum IgnoredReason { UNKNOWN = 'unknown', } -// TODO: Writing unit tests - /** * Returns the reason why a specific field was ignored in the response. * Will return undefined if the field had no ignored values in it. From 95a31c500028231dcaa7085fad3c7f90219ca835 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 09:23:39 +0200 Subject: [PATCH 26/43] Move dangerouslySetInnerHTML into helper method --- .../doc_table/components/table_row.tsx | 13 +++++----- .../discover_grid/get_render_cell_value.tsx | 9 +------ .../application/components/table/table.tsx | 2 +- .../components/table/table_cell_value.tsx | 15 ++++-------- .../{format_value.ts => format_value.tsx} | 24 +++++++++++++------ 5 files changed, 30 insertions(+), 33 deletions(-) rename src/plugins/discover/public/application/helpers/{format_value.ts => format_value.tsx} (71%) diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx index 5f820e51dfc9a..b8931d887a568 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx @@ -72,6 +72,7 @@ export const TableRow = ({ * Fill an element with the value of a field */ const displayField = (fieldName: string) => { + // TODO: move decision if _source here and call formatRow in case const formattedField = formatFieldValue( flattenedRow[fieldName], row, @@ -79,11 +80,7 @@ export const TableRow = ({ mapping(fieldName) ); - // field formatters take care of escaping - // eslint-disable-next-line react/no-danger - const fieldElement = ; - - return
{fieldElement}
; + return
{formattedField}
; }; const inlineFilter = useCallback( (column: string, type: '+' | '-') => { @@ -181,7 +178,11 @@ export const TableRow = ({ key={column} timefield={false} sourcefield={column === '_source'} - formatted={displayField(column)} + formatted={ + column === '_source' + ? formatRow(row, indexPattern, fieldsToShow) + : displayField(column) + } filterable={isFilterable} column={column} inlineFilter={inlineFilter} diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx index bed75e30751f8..19be58d77c019 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx @@ -192,12 +192,5 @@ export const getRenderCellValueFn = return {JSON.stringify(rowFlattened[columnId])}; } - const valueFormatted = formatFieldValue(rowFlattened[columnId], row, indexPattern, field); - if (typeof valueFormatted === 'undefined') { - return -; - } - return ( - // eslint-disable-next-line react/no-danger - - ); + return formatFieldValue(rowFlattened[columnId], row, indexPattern, field); }; diff --git a/src/plugins/discover/public/application/components/table/table.tsx b/src/plugins/discover/public/application/components/table/table.tsx index 8ab60412eac42..97c31f237ff2f 100644 --- a/src/plugins/discover/public/application/components/table/table.tsx +++ b/src/plugins/discover/public/application/components/table/table.tsx @@ -47,7 +47,7 @@ export interface FieldRecord { fieldMapping?: IndexPatternField; }; value: { - formattedValue: string; + formattedValue: JSX.Element; ignored?: IgnoredReason; }; } diff --git a/src/plugins/discover/public/application/components/table/table_cell_value.tsx b/src/plugins/discover/public/application/components/table/table_cell_value.tsx index ebdd7c6750241..58286f3348d62 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_value.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_value.tsx @@ -100,7 +100,7 @@ export const TableFieldValue = ({ }: TableFieldValueProps) => { const [fieldOpen, setFieldOpen] = useState(false); - const value = String(formattedValue); + const value = String(rawValue); const isCollapsible = value.length > COLLAPSE_LINE_LENGTH; const isCollapsed = isCollapsible && !fieldOpen; @@ -128,16 +128,9 @@ export const TableFieldValue = ({ )} )} -
+
+ {formattedValue} +
); }; diff --git a/src/plugins/discover/public/application/helpers/format_value.ts b/src/plugins/discover/public/application/helpers/format_value.tsx similarity index 71% rename from src/plugins/discover/public/application/helpers/format_value.ts rename to src/plugins/discover/public/application/helpers/format_value.tsx index ee6e63baee839..753104aa29373 100644 --- a/src/plugins/discover/public/application/helpers/format_value.ts +++ b/src/plugins/discover/public/application/helpers/format_value.tsx @@ -6,6 +6,7 @@ * Side Public License, v 1. */ +import React from 'react'; import { estypes } from '@elastic/elasticsearch'; import { DataView, DataViewField, KBN_FIELD_TYPES } from '../../../../data/common'; import { getServices } from '../../kibana_services'; @@ -26,17 +27,26 @@ export function formatFieldValue( hit: estypes.SearchHit, dataView?: DataView, field?: DataViewField -): string { +): React.ReactElement { + // We rely on field formatters to always produce safe and escaped HTML, thus we can set it + // here as HTML + // eslint-disable-next-line react/no-danger + const render = (html: string) => ; + if (!dataView || !field) { // If either no field is available or no data view, we'll use the default // string formatter to format that field. - return getServices() - .fieldFormats.getDefaultInstance(KBN_FIELD_TYPES.STRING) - .convert(value, 'html', { hit, field, indexPattern: dataView }); + return render( + getServices() + .fieldFormats.getDefaultInstance(KBN_FIELD_TYPES.STRING) + .convert(value, 'html', { hit, field, indexPattern: dataView }) + ); } // If we have a data view and field we use that fields field formatter - return dataView - .getFormatterForField(field) - .convert(value, 'html', { hit, field, indexPattern: dataView }); + return render( + dataView + .getFormatterForField(field) + .convert(value, 'html', { hit, field, indexPattern: dataView }) + ); } From 17ed7faa1fb11882f0ad2017ff93a6b1ea123f77 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 10:20:59 +0200 Subject: [PATCH 27/43] Extract document formatting into common utility --- .../doc_table/components/table_row.tsx | 13 +++--- .../doc_table/lib/row_formatter.tsx | 26 +++--------- .../discover_grid/get_render_cell_value.tsx | 41 +++++-------------- .../public/application/helpers/format_hit.ts | 40 ++++++++++++++++++ 4 files changed, 63 insertions(+), 57 deletions(-) create mode 100644 src/plugins/discover/public/application/helpers/format_hit.ts diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx index b8931d887a568..47d5aa59b4836 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx @@ -72,7 +72,12 @@ export const TableRow = ({ * Fill an element with the value of a field */ const displayField = (fieldName: string) => { - // TODO: move decision if _source here and call formatRow in case + // If we're formatting the _source column, don't use the regular field formatter, + // but our Discover mechanism to format a hit in a better human-readable way. + if (fieldName === '_source') { + return formatRow(row, indexPattern, fieldsToShow); + } + const formattedField = formatFieldValue( flattenedRow[fieldName], row, @@ -178,11 +183,7 @@ export const TableRow = ({ key={column} timefield={false} sourcefield={column === '_source'} - formatted={ - column === '_source' - ? formatRow(row, indexPattern, fieldsToShow) - : displayField(column) - } + formatted={displayField(column)} filterable={isFilterable} column={column} inlineFilter={inlineFilter} diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx index 14cf1839107e7..1838119f1c58b 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx @@ -6,10 +6,12 @@ * Side Public License, v 1. */ +import { estypes } from '@elastic/elasticsearch'; import React, { Fragment } from 'react'; import type { IndexPattern } from 'src/plugins/data/common'; import { MAX_DOC_FIELDS_DISPLAYED } from '../../../../../../../common'; import { getServices } from '../../../../../../kibana_services'; +import { formatHit } from '../../../../../helpers/format_hit'; import './row_formatter.scss'; @@ -34,30 +36,12 @@ const TemplateComponent = ({ defPairs }: Props) => { }; export const formatRow = ( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - hit: Record, + hit: estypes.SearchHit, indexPattern: IndexPattern, fieldsToShow: string[] ) => { - const highlights = hit?.highlight ?? {}; - // Keys are sorted in the hits object - const formatted = indexPattern.formatHit(hit); - const fields = indexPattern.fields; - const highlightPairs: Array<[string, unknown]> = []; - const sourcePairs: Array<[string, unknown]> = []; - Object.entries(formatted).forEach(([key, val]) => { - const displayKey = fields.getByName ? fields.getByName(key)?.displayName : undefined; - const pairs = highlights[key] ? highlightPairs : sourcePairs; - if (displayKey) { - if (fieldsToShow.includes(displayKey)) { - pairs.push([displayKey, val]); - } - } else { - pairs.push([key, val]); - } - }); - const maxEntries = getServices().uiSettings.get(MAX_DOC_FIELDS_DISPLAYED); - return ; + const pairs = formatHit(hit, indexPattern, fieldsToShow); + return ; }; export const formatTopLevelObject = ( diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx index 19be58d77c019..8e7fb6ada478b 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx @@ -23,6 +23,7 @@ import { JsonCodeEditor } from '../json_code_editor/json_code_editor'; import { defaultMonacoEditorWidth } from './constants'; import { EsHitRecord } from '../../types'; import { formatFieldValue } from '../../helpers/format_value'; +import { formatHit } from '../../helpers/format_hit'; export const getRenderCellValueFn = ( @@ -146,39 +147,19 @@ export const getRenderCellValueFn = // eslint-disable-next-line @typescript-eslint/no-explicit-any return ; } - const formatted = indexPattern.formatHit(row); - - // Put the most important fields first - const highlights: Record = (row.highlight as Record) ?? {}; - const highlightPairs: Array<[string, string]> = []; - const sourcePairs: Array<[string, string]> = []; - Object.entries(formatted).forEach(([key, val]) => { - const pairs = highlights[key] ? highlightPairs : sourcePairs; - const displayKey = indexPattern.fields.getByName - ? indexPattern.fields.getByName(key)?.displayName - : undefined; - if (displayKey) { - if (fieldsToShow.includes(displayKey)) { - pairs.push([displayKey, val as string]); - } - } else { - pairs.push([key, val as string]); - } - }); + const pairs = formatHit(row, indexPattern, fieldsToShow); return ( - {[...highlightPairs, ...sourcePairs] - .slice(0, maxDocFieldsDisplayed) - .map(([key, value]) => ( - - {key} - - - ))} + {pairs.map(([key, value]) => ( + + {key} + + + ))} ); } diff --git a/src/plugins/discover/public/application/helpers/format_hit.ts b/src/plugins/discover/public/application/helpers/format_hit.ts new file mode 100644 index 0000000000000..b53c219395521 --- /dev/null +++ b/src/plugins/discover/public/application/helpers/format_hit.ts @@ -0,0 +1,40 @@ +/* + * 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 { estypes } from '@elastic/elasticsearch'; +import { DataView } from '../../../../data/common'; +import { MAX_DOC_FIELDS_DISPLAYED } from '../../../common'; +import { getServices } from '../../kibana_services'; + +// TODO: Test coverage +// TODO: documentation + +export function formatHit( + hit: estypes.SearchHit, + dataView: DataView, + fieldsToShow: string[] +): Array<[string, string]> { + const highlights = hit.highlight ?? {}; + // Keys are sorted in the hits object + const formatted = dataView.formatHit(hit); + const highlightPairs: Array<[string, string]> = []; + const sourcePairs: Array<[string, string]> = []; + Object.entries(formatted).forEach(([key, val]) => { + const displayKey = dataView.fields.getByName(key)?.displayName; + const pairs = highlights[key] ? highlightPairs : sourcePairs; + if (displayKey) { + if (fieldsToShow.includes(key)) { + pairs.push([displayKey, val as string]); + } + } else { + pairs.push([key, val as string]); + } + }); + const maxEntries = getServices().uiSettings.get(MAX_DOC_FIELDS_DISPLAYED); + return [...highlightPairs, ...sourcePairs].slice(0, maxEntries); +} From 1d2f19f2332b97285201b20ea5f326f67209c407 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 10:30:25 +0200 Subject: [PATCH 28/43] Remove HTML source field formatter --- .../common/converters/source.test.ts | 21 +------ .../common/converters/source.tsx | 55 +------------------ 2 files changed, 4 insertions(+), 72 deletions(-) diff --git a/src/plugins/field_formats/common/converters/source.test.ts b/src/plugins/field_formats/common/converters/source.test.ts index 298c93dac8c4e..6f9e96a136d0b 100644 --- a/src/plugins/field_formats/common/converters/source.test.ts +++ b/src/plugins/field_formats/common/converters/source.test.ts @@ -19,7 +19,7 @@ describe('Source Format', () => { convertHtml = source.getConverterFor(HTML_CONTEXT_TYPE) as HtmlContextTypeConvert; }); - test('should use the text content type if a field is not passed', () => { + test('should render stringified object', () => { const hit = { foo: 'bar', number: 42, @@ -27,23 +27,8 @@ describe('Source Format', () => { also: 'with "quotes" or \'single quotes\'', }; - expect(convertHtml(hit)).toBe( - '{"foo":"bar","number":42,"hello":"<h1>World</h1>","also":"with \\"quotes\\" or 'single quotes'"}' - ); - }); - - test('should render a description list if a field is passed', () => { - const hit = { - foo: 'bar', - number: 42, - hello: '

World

', - also: 'with "quotes" or \'single quotes\'', - }; - - expect( - convertHtml(hit, { field: 'field', indexPattern: { formatHit: (h: string) => h }, hit }) - ).toMatchInlineSnapshot( - `"
foo:
bar
number:
42
hello:

World

also:
with \\"quotes\\" or 'single quotes'
"` + expect(convertHtml(hit, { field: 'field', hit })).toMatchInlineSnapshot( + `"{"foo":"bar","number":42,"hello":"<h1>World</h1>","also":"with \\\\"quotes\\\\" or 'single quotes'"}"` ); }); }); diff --git a/src/plugins/field_formats/common/converters/source.tsx b/src/plugins/field_formats/common/converters/source.tsx index 1caffb5bfb9a8..f92027ec07451 100644 --- a/src/plugins/field_formats/common/converters/source.tsx +++ b/src/plugins/field_formats/common/converters/source.tsx @@ -7,33 +7,8 @@ */ import { KBN_FIELD_TYPES } from '@kbn/field-types'; -import React, { Fragment } from 'react'; -import ReactDOM from 'react-dom/server'; -import { escape, keys } from 'lodash'; -import { shortenDottedString } from '../utils'; import { FieldFormat } from '../field_format'; -import { TextContextTypeConvert, HtmlContextTypeConvert, FIELD_FORMAT_IDS } from '../types'; -import { FORMATS_UI_SETTINGS } from '../constants/ui_settings'; - -interface Props { - defPairs: Array<[string, string]>; -} -const TemplateComponent = ({ defPairs }: Props) => { - return ( -
- {defPairs.map((pair, idx) => ( - -
-
{' '} - - ))} -
- ); -}; +import { TextContextTypeConvert, FIELD_FORMAT_IDS } from '../types'; /** @public */ export class SourceFormat extends FieldFormat { @@ -42,32 +17,4 @@ export class SourceFormat extends FieldFormat { static fieldType = KBN_FIELD_TYPES._SOURCE; textConvert: TextContextTypeConvert = (value: string) => JSON.stringify(value); - - htmlConvert: HtmlContextTypeConvert = (value: string, options = {}) => { - const { field, hit, indexPattern } = options; - - if (!field) { - const converter = this.getConverterFor('text') as Function; - - return escape(converter(value)); - } - - const highlights: Record = (hit && hit.highlight) || {}; - // TODO: remove index pattern dependency - const formatted = hit ? indexPattern!.formatHit(hit) : {}; - const highlightPairs: Array<[string, string]> = []; - const sourcePairs: Array<[string, string]> = []; - const isShortDots = this.getConfig!(FORMATS_UI_SETTINGS.SHORT_DOTS_ENABLE); - - keys(formatted).forEach((key) => { - const pairs = highlights[key] ? highlightPairs : sourcePairs; - const newField = isShortDots ? shortenDottedString(key) : key; - const val = formatted![key]; - pairs.push([newField as string, val]); - }, []); - - return ReactDOM.renderToStaticMarkup( - - ); - }; } From 6966db14a88e56674860aff70a4ad265cea8352d Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 17:46:40 +0200 Subject: [PATCH 29/43] Move formatHit to Discover --- .../data_views/common/data_views/data_view.ts | 11 -- .../common/data_views/format_hit.ts | 74 -------- .../data_views/common/data_views/index.ts | 1 - src/plugins/data_views/public/index.ts | 2 +- .../public/__mocks__/index_pattern.ts | 2 - .../__mocks__/index_pattern_with_timefield.ts | 1 - .../discover/public/__mocks__/services.ts | 7 + .../apps/context/context_app.test.tsx | 4 + .../doc_table/lib/row_formatter.test.ts | 160 ++++++++++++---- .../doc_table/lib/row_formatter.tsx | 8 +- .../layout/discover_documents.test.tsx | 13 ++ .../layout/discover_layout.test.tsx | 3 + ...ver_index_pattern_management.test.tsx.snap | 39 +--- .../discover_grid/discover_grid.test.tsx | 13 ++ .../get_render_cell_value.test.tsx | 179 ++++++++++++++---- .../discover_grid/get_render_cell_value.tsx | 7 +- .../components/table/table.test.tsx | 14 -- .../public/application/helpers/format_hit.ts | 20 +- .../application/helpers/format_value.tsx | 8 +- src/plugins/field_formats/common/types.ts | 4 - 20 files changed, 330 insertions(+), 240 deletions(-) delete mode 100644 src/plugins/data_views/common/data_views/format_hit.ts diff --git a/src/plugins/data_views/common/data_views/data_view.ts b/src/plugins/data_views/common/data_views/data_view.ts index 57db127208dc3..5bec7a9af6f7a 100644 --- a/src/plugins/data_views/common/data_views/data_view.ts +++ b/src/plugins/data_views/common/data_views/data_view.ts @@ -17,7 +17,6 @@ import { DuplicateField } from '../../../kibana_utils/common'; import { IIndexPattern, IFieldType } from '../../common'; import { DataViewField, IIndexPatternFieldList, fieldList } from '../fields'; -import { formatHitProvider } from './format_hit'; import { flattenHitWrapper } from './flatten_hit'; import { FieldFormatsStartCommon, @@ -67,11 +66,6 @@ export class DataView implements IIndexPattern { * Type is used to identify rollup index patterns */ public type: string | undefined; - public formatHit: { - (hit: Record, type?: string): any; - formatField: FormatFieldFn; - }; - public formatField: FormatFieldFn; /** * @deprecated Use `flattenHit` utility method exported from data plugin instead. */ @@ -103,11 +97,6 @@ export class DataView implements IIndexPattern { this.fields = fieldList([], this.shortDotsEnable); this.flattenHit = flattenHitWrapper(this, metaFields); - this.formatHit = formatHitProvider( - this, - fieldFormats.getDefaultInstance(KBN_FIELD_TYPES.STRING) - ); - this.formatField = this.formatHit.formatField; // set values this.id = spec.id; diff --git a/src/plugins/data_views/common/data_views/format_hit.ts b/src/plugins/data_views/common/data_views/format_hit.ts deleted file mode 100644 index c8e6e8e337155..0000000000000 --- a/src/plugins/data_views/common/data_views/format_hit.ts +++ /dev/null @@ -1,74 +0,0 @@ -/* - * 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 _ from 'lodash'; -import { DataView } from './data_view'; -import { FieldFormatsContentType } from '../../../field_formats/common'; - -const formattedCache = new WeakMap(); -const partialFormattedCache = new WeakMap(); - -// Takes a hit, merges it with any stored/scripted fields, and with the metaFields -// returns a formatted version -export function formatHitProvider(dataView: DataView, defaultFormat: any) { - function convert( - hit: Record, - val: any, - fieldName: string, - type: FieldFormatsContentType = 'html' - ) { - const field = dataView.fields.getByName(fieldName); - const format = field ? dataView.getFormatterForField(field) : defaultFormat; - - return format.convert(val, type, { field, hit, indexPattern: dataView }); - } - - function formatHit(hit: Record, type: string = 'html') { - const cached = formattedCache.get(hit); - if (cached) { - return cached; - } - - // use and update the partial cache, but don't rewrite it. - // _source is stored in partialFormattedCache but not formattedCache - const partials = partialFormattedCache.get(hit) || {}; - partialFormattedCache.set(hit, partials); - - const cache: Record = {}; - formattedCache.set(hit, cache); - - _.forOwn(dataView.flattenHit(hit), function (val: any, fieldName?: string) { - // sync the formatted and partial cache - if (!fieldName) { - return; - } - const formatted = - partials[fieldName] == null ? convert(hit, val, fieldName) : partials[fieldName]; - cache[fieldName] = partials[fieldName] = formatted; - }); - - return cache; - } - - formatHit.formatField = function (hit: Record, fieldName: string) { - let partials = partialFormattedCache.get(hit); - if (partials && partials[fieldName] != null) { - return partials[fieldName]; - } - - if (!partials) { - partials = {}; - partialFormattedCache.set(hit, partials); - } - - const val = fieldName === '_source' ? hit._source : dataView.flattenHit(hit)[fieldName]; - return convert(hit, val, fieldName); - }; - - return formatHit; -} diff --git a/src/plugins/data_views/common/data_views/index.ts b/src/plugins/data_views/common/data_views/index.ts index 7c94dff961c9c..d925d42fbea0d 100644 --- a/src/plugins/data_views/common/data_views/index.ts +++ b/src/plugins/data_views/common/data_views/index.ts @@ -8,6 +8,5 @@ export * from './_pattern_cache'; export * from './flatten_hit'; -export * from './format_hit'; export * from './data_view'; export * from './data_views'; diff --git a/src/plugins/data_views/public/index.ts b/src/plugins/data_views/public/index.ts index 5c810ec1fd4c8..3a6b5ccb237f2 100644 --- a/src/plugins/data_views/public/index.ts +++ b/src/plugins/data_views/public/index.ts @@ -13,7 +13,7 @@ export { ILLEGAL_CHARACTERS, validateDataView, } from '../common/lib'; -export { formatHitProvider, onRedirectNoIndexPattern } from './data_views'; +export { onRedirectNoIndexPattern } from './data_views'; export { IndexPatternField, IIndexPatternFieldList, TypeMeta } from '../common'; diff --git a/src/plugins/discover/public/__mocks__/index_pattern.ts b/src/plugins/discover/public/__mocks__/index_pattern.ts index 7a6b75494729a..a3595b457b3aa 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern.ts @@ -72,8 +72,6 @@ const indexPattern = { id: 'the-index-pattern-id', title: 'the-index-pattern-title', metaFields: ['_index', '_score'], - formatField: jest.fn(), - formatHit: jest.fn((hit) => (hit.fields ? hit.fields : hit._source)), fields, getComputedFields: () => ({ docvalueFields: [], scriptFields: {}, storedFields: ['*'] }), getSourceFiltering: () => ({}), diff --git a/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts b/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts index 221784335f10f..906ebdebdd06a 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern_with_timefield.ts @@ -63,7 +63,6 @@ const indexPattern = { id: 'index-pattern-with-timefield-id', title: 'index-pattern-with-timefield', metaFields: ['_index', '_score'], - formatHit: jest.fn((hit) => hit._source), fields, getComputedFields: () => ({}), getSourceFiltering: () => ({}), diff --git a/src/plugins/discover/public/__mocks__/services.ts b/src/plugins/discover/public/__mocks__/services.ts index 8cc5ccf5aa121..858863396ff47 100644 --- a/src/plugins/discover/public/__mocks__/services.ts +++ b/src/plugins/discover/public/__mocks__/services.ts @@ -13,6 +13,7 @@ import { CONTEXT_STEP_SETTING, DEFAULT_COLUMNS_SETTING, DOC_HIDE_TIME_COLUMN_SETTING, + MAX_DOC_FIELDS_DISPLAYED, SAMPLE_SIZE_SETTING, SORT_DEFAULT_ORDER_SETTING, } from '../../common'; @@ -43,6 +44,10 @@ export const discoverServiceMock = { save: true, }, }, + fieldFormats: { + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), + }, filterManager: dataPlugin.query.filterManager, uiSettings: { get: (key: string) => { @@ -62,6 +67,8 @@ export const discoverServiceMock = { return false; } else if (key === SAMPLE_SIZE_SETTING) { return 250; + } else if (key === MAX_DOC_FIELDS_DISPLAYED) { + return 50; } }, isDefault: (key: string) => { diff --git a/src/plugins/discover/public/application/apps/context/context_app.test.tsx b/src/plugins/discover/public/application/apps/context/context_app.test.tsx index 0e50f8f714a2c..d1c557f2839bc 100644 --- a/src/plugins/discover/public/application/apps/context/context_app.test.tsx +++ b/src/plugins/discover/public/application/apps/context/context_app.test.tsx @@ -62,6 +62,10 @@ describe('ContextApp test', () => { navigation: mockNavigationPlugin, core: { notifications: { toasts: [] } }, history: () => {}, + fieldFormats: { + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), + }, filterManager: mockFilterManager, uiSettings: uiSettingsMock, } as unknown as DiscoverServices); diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts index 9cd2959f5d72a..2f29fce84d105 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts @@ -17,6 +17,7 @@ import { stubbedSavedObjectIndexPattern } from '../../../../../../../../data/com describe('Row formatter', () => { const hit = { _id: 'a', + _index: 'foo', _type: 'doc', _score: 1, _source: { @@ -39,7 +40,7 @@ describe('Row formatter', () => { spec: { id, type, version, timeFieldName, fields: JSON.parse(fields), title }, fieldFormats: fieldFormatsMock, shortDotsEnable: false, - metaFields: [], + metaFields: ['_id', '_type', '_score'], }); }; @@ -47,26 +48,15 @@ describe('Row formatter', () => { const fieldsToShow = indexPattern.fields.getAll().map((fld) => fld.name); - // Realistic response with alphabetical insertion order - const formatHitReturnValue = { - also: 'with \\"quotes\\" or 'single qoutes'', - foo: 'bar', - number: '42', - hello: '<h1>World</h1>', - _id: 'a', - _type: 'doc', - _score: 1, - }; - - const formatHitMock = jest.fn().mockReturnValue(formatHitReturnValue); - beforeEach(() => { - // @ts-expect-error - indexPattern.formatHit = formatHitMock; setServices({ uiSettings: { get: () => 100, }, + fieldFormats: { + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), + }, } as unknown as DiscoverServices); }); @@ -77,31 +67,73 @@ describe('Row formatter', () => { Array [ Array [ "also", - "with \\\\"quotes\\\\" or 'single qoutes'", + , ], Array [ "foo", - "bar", + , ], Array [ - "number", - "42", + "hello", + World", + } + } + />, ], Array [ - "hello", - "<h1>World</h1>", + "number", + , ], Array [ "_id", - "a", + , ], Array [ - "_type", - "doc", + "_score", + , ], Array [ - "_score", - 1, + "_type", + , ], ] } @@ -114,6 +146,10 @@ describe('Row formatter', () => { uiSettings: { get: () => 1, }, + fieldFormats: { + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), + }, } as unknown as DiscoverServices); expect(formatRow(hit, indexPattern, [])).toMatchInlineSnapshot(` { Array [ Array [ "also", - "with \\\\"quotes\\\\" or 'single qoutes'", + , ], ] } @@ -130,38 +172,80 @@ describe('Row formatter', () => { }); it('formats document with highlighted fields first', () => { - expect(formatRow({ ...hit, highlight: { number: '42' } }, indexPattern, fieldsToShow)) + expect(formatRow({ ...hit, highlight: { number: ['42'] } }, indexPattern, fieldsToShow)) .toMatchInlineSnapshot(` , ], Array [ "also", - "with \\\\"quotes\\\\" or 'single qoutes'", + , ], Array [ "foo", - "bar", + , ], Array [ "hello", - "<h1>World</h1>", + World", + } + } + />, ], Array [ "_id", - "a", + , ], Array [ - "_type", - "doc", + "_score", + , ], Array [ - "_score", - 1, + "_type", + , ], ] } diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx index 1838119f1c58b..3e6b6bf60d138 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx @@ -16,7 +16,7 @@ import { formatHit } from '../../../../../helpers/format_hit'; import './row_formatter.scss'; interface Props { - defPairs: Array<[string, unknown]>; + defPairs: Array<[string, JSX.Element]>; } const TemplateComponent = ({ defPairs }: Props) => { return ( @@ -24,11 +24,7 @@ const TemplateComponent = ({ defPairs }: Props) => { {defPairs.map((pair, idx) => (
{pair[0]}:
-
{' '} +
{pair[1]}
{' '}
))} diff --git a/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx b/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx index e5212e877e8ba..c916d556ead0a 100644 --- a/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx +++ b/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx @@ -20,6 +20,19 @@ import { DiscoverDocuments } from './discover_documents'; import { ElasticSearchHit } from '../../../../doc_views/doc_views_types'; import { indexPatternMock } from '../../../../../__mocks__/index_pattern'; +jest.mock('../../../../../kibana_services', () => ({ + ...jest.requireActual('../../../../../kibana_services'), + getServices: () => ({ + fieldFormats: { + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), + }, + uiSettings: { + get: jest.fn((key: string) => key === 'discover:maxDocFieldsDisplayed' && 50), + }, + }), +})); + setHeaderActionMenuMounter(jest.fn()); function getProps(fetchStatus: FetchStatus, hits: ElasticSearchHit[]) { diff --git a/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx b/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx index ac68986280242..7e3252dce1ef5 100644 --- a/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx +++ b/src/plugins/discover/public/application/apps/main/components/layout/discover_layout.test.tsx @@ -40,6 +40,9 @@ jest.mock('../../../../../kibana_services', () => ({ getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), }, + uiSettings: { + get: jest.fn((key: string) => key === 'discover:maxDocFieldsDisplayed' && 50), + }, }), })); diff --git a/src/plugins/discover/public/application/apps/main/components/sidebar/__snapshots__/discover_index_pattern_management.test.tsx.snap b/src/plugins/discover/public/application/apps/main/components/sidebar/__snapshots__/discover_index_pattern_management.test.tsx.snap index ebb06e0b2ecd3..02e2879476a5e 100644 --- a/src/plugins/discover/public/application/apps/main/components/sidebar/__snapshots__/discover_index_pattern_management.test.tsx.snap +++ b/src/plugins/discover/public/application/apps/main/components/sidebar/__snapshots__/discover_index_pattern_management.test.tsx.snap @@ -115,42 +115,7 @@ exports[`Discover IndexPattern Management renders correctly 1`] = ` "deserialize": [MockFunction], "getByFieldType": [MockFunction], "getDefaultConfig": [MockFunction], - "getDefaultInstance": [MockFunction] { - "calls": Array [ - Array [ - "string", - ], - Array [ - "string", - ], - Array [ - "string", - ], - ], - "results": Array [ - Object { - "type": "return", - "value": Object { - "convert": [MockFunction], - "getConverterFor": [MockFunction], - }, - }, - Object { - "type": "return", - "value": Object { - "convert": [MockFunction], - "getConverterFor": [MockFunction], - }, - }, - Object { - "type": "return", - "value": Object { - "convert": [MockFunction], - "getConverterFor": [MockFunction], - }, - }, - ], - }, + "getDefaultInstance": [MockFunction], "getDefaultInstanceCacheResolver": [MockFunction], "getDefaultInstancePlain": [MockFunction], "getDefaultType": [MockFunction], @@ -651,8 +616,6 @@ exports[`Discover IndexPattern Management renders correctly 1`] = ` }, ], "flattenHit": [Function], - "formatField": [Function], - "formatHit": [Function], "getFieldAttrs": [Function], "getOriginalSavedObjectBody": [Function], "id": "logstash-*", diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx index b2be40c008200..435ae9a6b8302 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx @@ -19,6 +19,19 @@ import { DiscoverServices } from '../../../build_services'; import { ElasticSearchHit } from '../../doc_views/doc_views_types'; import { getDocId } from './discover_grid_document_selection'; +jest.mock('../../../kibana_services', () => ({ + ...jest.requireActual('../../../kibana_services'), + getServices: () => ({ + fieldFormats: { + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), + }, + uiSettings: { + get: jest.fn((key: string) => key === 'discover:maxDocFieldsDisplayed' && 50), + }, + }), +})); + function getProps() { const servicesMock = { uiSettings: uiSettingsMock, diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx index a0f6fc7bcd3e6..d3e3dc00e8c8c 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx @@ -129,23 +129,57 @@ describe('Discover grid cell rendering', function () { + + /> + bytes + + /> + + + _index + + + + + + _score + + + + `); }); @@ -223,27 +257,61 @@ describe('Discover grid cell rendering', function () { + + /> + bytes + + /> + + + _index + + + + + + _score + + + + `); }); @@ -279,14 +347,61 @@ describe('Discover grid cell rendering', function () { + + /> + + + bytes + + + + + + _index + + + + + + _score + + + + `); }); @@ -524,6 +639,6 @@ describe('Discover grid cell rendering', function () { setCellProps={jest.fn()} /> ); - expect(component.html()).toMatchInlineSnapshot(`"-"`); + expect(component.html()).toMatchInlineSnapshot(`""`); }); }); diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx index 8e7fb6ada478b..7f5c9aea7ced6 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx @@ -154,10 +154,9 @@ export const getRenderCellValueFn = {pairs.map(([key, value]) => ( {key} - + + {value} + ))} diff --git a/src/plugins/discover/public/application/components/table/table.test.tsx b/src/plugins/discover/public/application/components/table/table.test.tsx index 715de181e7536..e61333cce1166 100644 --- a/src/plugins/discover/public/application/components/table/table.test.tsx +++ b/src/plugins/discover/public/application/components/table/table.test.tsx @@ -70,7 +70,6 @@ const indexPattern = { }, metaFields: ['_index', '_score'], getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), - formatHit: jest.fn((hit) => hit._source), } as unknown as IndexPattern; indexPattern.fields.getByName = (name: string) => { @@ -364,19 +363,6 @@ describe('DocViewTable at Discover Doc with Fields API', () => { }, metaFields: ['_index', '_type', '_score', '_id'], getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), - formatHit: jest.fn((hit) => { - const result = {} as Record; - Object.keys(hit).forEach((key) => { - if (key !== 'fields') { - result[key] = hit[key]; - } else { - Object.keys(hit.fields).forEach((field) => { - result[field] = hit.fields[field]; - }); - } - }); - return result; - }), } as unknown as IndexPattern; indexPatterneCommerce.fields.getByName = (name: string) => { diff --git a/src/plugins/discover/public/application/helpers/format_hit.ts b/src/plugins/discover/public/application/helpers/format_hit.ts index b53c219395521..9cbd8b7ff576c 100644 --- a/src/plugins/discover/public/application/helpers/format_hit.ts +++ b/src/plugins/discover/public/application/helpers/format_hit.ts @@ -7,32 +7,36 @@ */ import { estypes } from '@elastic/elasticsearch'; -import { DataView } from '../../../../data/common'; +import { DataView, flattenHit } from '../../../../data/common'; import { MAX_DOC_FIELDS_DISPLAYED } from '../../../common'; import { getServices } from '../../kibana_services'; +import { formatFieldValue } from './format_value'; // TODO: Test coverage // TODO: documentation +// TODO: Build cache export function formatHit( hit: estypes.SearchHit, dataView: DataView, fieldsToShow: string[] -): Array<[string, string]> { +): Array<[string, JSX.Element]> { const highlights = hit.highlight ?? {}; // Keys are sorted in the hits object - const formatted = dataView.formatHit(hit); - const highlightPairs: Array<[string, string]> = []; - const sourcePairs: Array<[string, string]> = []; - Object.entries(formatted).forEach(([key, val]) => { + const flattened = flattenHit(hit, dataView, { includeIgnoredValues: true, source: true }); + + const highlightPairs: Array<[string, JSX.Element]> = []; + const sourcePairs: Array<[string, JSX.Element]> = []; + Object.entries(flattened).forEach(([key, val]) => { const displayKey = dataView.fields.getByName(key)?.displayName; const pairs = highlights[key] ? highlightPairs : sourcePairs; + const formattedValue = formatFieldValue(val, hit, dataView, dataView.fields.getByName(key)); if (displayKey) { if (fieldsToShow.includes(key)) { - pairs.push([displayKey, val as string]); + pairs.push([displayKey, formattedValue]); } } else { - pairs.push([key, val as string]); + pairs.push([key, formattedValue]); } }); const maxEntries = getServices().uiSettings.get(MAX_DOC_FIELDS_DISPLAYED); diff --git a/src/plugins/discover/public/application/helpers/format_value.tsx b/src/plugins/discover/public/application/helpers/format_value.tsx index 753104aa29373..08027dd426404 100644 --- a/src/plugins/discover/public/application/helpers/format_value.tsx +++ b/src/plugins/discover/public/application/helpers/format_value.tsx @@ -39,14 +39,10 @@ export function formatFieldValue( return render( getServices() .fieldFormats.getDefaultInstance(KBN_FIELD_TYPES.STRING) - .convert(value, 'html', { hit, field, indexPattern: dataView }) + .convert(value, 'html', { hit, field }) ); } // If we have a data view and field we use that fields field formatter - return render( - dataView - .getFormatterForField(field) - .convert(value, 'html', { hit, field, indexPattern: dataView }) - ); + return render(dataView.getFormatterForField(field).convert(value, 'html', { hit, field })); } diff --git a/src/plugins/field_formats/common/types.ts b/src/plugins/field_formats/common/types.ts index 00f9f5d707e89..6f0efebe389a1 100644 --- a/src/plugins/field_formats/common/types.ts +++ b/src/plugins/field_formats/common/types.ts @@ -17,10 +17,6 @@ export type FieldFormatsContentType = 'html' | 'text'; */ export interface HtmlContextTypeOptions { field?: { name: string }; - // TODO: get rid of indexPattern dep completely - indexPattern?: { - formatHit: (hit: { highlight: Record }) => Record; - }; hit?: { highlight: Record }; } From 22be1287c778add15d0a96c19f6df30348f1a376 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 17:50:16 +0200 Subject: [PATCH 30/43] Change wording of ignored warning --- .../components/table/table_cell_value.tsx | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/plugins/discover/public/application/components/table/table_cell_value.tsx b/src/plugins/discover/public/application/components/table/table_cell_value.tsx index 58286f3348d62..ea9bc44f82c39 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_value.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_value.tsx @@ -30,30 +30,26 @@ const IgnoreWarning: React.FC = React.memo(({ rawValue, reas case IgnoredReason.IGNORE_ABOVE: return multiValue ? i18n.translate('discover.docView.table.ignored.multiAboveTooltip', { - defaultMessage: - 'This field contains one or more values that are too long and are thus not searchable or filterable.', + defaultMessage: `One or more values in this field are too long and can't be searched or filtered.`, }) : i18n.translate('discover.docView.table.ignored.singleAboveTooltip', { - defaultMessage: - 'The value in this field is too long and thus not searchable or filterable.', + defaultMessage: `The value in this field is too long and and can't be searched or filtered.`, }); case IgnoredReason.MALFORMED: return multiValue ? i18n.translate('discover.docView.table.ignored.multiMalformedTooltip', { - defaultMessage: - 'This field contains one or more values that are malformed and are thus not searchable or filterable.', + defaultMessage: `This field has one or more malformed values that can't be searched or filtered.`, }) : i18n.translate('discover.docView.table.ignored.singleMalformedTooltip', { - defaultMessage: `The value in this field is malformed and not parsable. It's not searchable or filterable.`, + defaultMessage: `The value in this field is malformed and can't be searched or filtered.`, }); case IgnoredReason.UNKNOWN: return multiValue ? i18n.translate('discover.docView.table.ignored.multiUnknownTooltip', { - defaultMessage: - 'This field contains one or more values that are ignored by Elasticsearch and are thus not searchable or filterable.', + defaultMessage: `One or more values in this field were ignored by Elasticsearch and can't be searched or filtered.`, }) : i18n.translate('discover.docView.table.ignored.singleUnknownTooltip', { - defaultMessage: `The value in this field is ignored by Elasticsearch. It's not searchable or filterable.`, + defaultMessage: `The value in this field was ignored by Elasticsearch and can't be searched or filtered.`, }); } }; From 97c49843cfb5d10a68e49afb4e0343b3bca0bd03 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 18:19:38 +0200 Subject: [PATCH 31/43] Add cache for formatted hits --- .../public/application/helpers/format_hit.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/plugins/discover/public/application/helpers/format_hit.ts b/src/plugins/discover/public/application/helpers/format_hit.ts index 9cbd8b7ff576c..0dff147d28935 100644 --- a/src/plugins/discover/public/application/helpers/format_hit.ts +++ b/src/plugins/discover/public/application/helpers/format_hit.ts @@ -14,13 +14,21 @@ import { formatFieldValue } from './format_value'; // TODO: Test coverage // TODO: documentation -// TODO: Build cache + +const formattedHitCache = new WeakMap(); + +type FormattedHit = Array<[string, JSX.Element]>; export function formatHit( hit: estypes.SearchHit, dataView: DataView, fieldsToShow: string[] -): Array<[string, JSX.Element]> { +): FormattedHit { + const cached = formattedHitCache.get(hit); + if (cached) { + return cached; + } + const highlights = hit.highlight ?? {}; // Keys are sorted in the hits object const flattened = flattenHit(hit, dataView, { includeIgnoredValues: true, source: true }); @@ -40,5 +48,7 @@ export function formatHit( } }); const maxEntries = getServices().uiSettings.get(MAX_DOC_FIELDS_DISPLAYED); - return [...highlightPairs, ...sourcePairs].slice(0, maxEntries); + const formatted = [...highlightPairs, ...sourcePairs].slice(0, maxEntries); + formattedHitCache.set(hit, formatted); + return formatted; } From 0e6ef917d74843701bd7fc7ee16b8445c50a93bc Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 18:20:21 +0200 Subject: [PATCH 32/43] Remove dead type --- src/plugins/data_views/common/data_views/data_view.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/plugins/data_views/common/data_views/data_view.ts b/src/plugins/data_views/common/data_views/data_view.ts index 5bec7a9af6f7a..b7823677b70f9 100644 --- a/src/plugins/data_views/common/data_views/data_view.ts +++ b/src/plugins/data_views/common/data_views/data_view.ts @@ -44,8 +44,6 @@ interface SavedObjectBody { type?: string; } -type FormatFieldFn = (hit: Record, fieldName: string) => any; - export class DataView implements IIndexPattern { public id?: string; public title: string = ''; From 7713e5ce8e7ec96f7c010517b1f29d7e746d2ab3 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 18:42:46 +0200 Subject: [PATCH 33/43] Fix row_formatter for objects --- .../doc_table/lib/row_formatter.tsx | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx index 3e6b6bf60d138..e1daccfb6e07f 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.tsx @@ -31,6 +31,26 @@ const TemplateComponent = ({ defPairs }: Props) => { ); }; +interface RawProps { + defPairs: Array<[string, unknown]>; +} +const TemplateComponentRaw = ({ defPairs }: RawProps) => { + return ( +
+ {defPairs.map((pair, idx) => ( + +
{pair[0]}:
+
{' '} + + ))} +
+ ); +}; + export const formatRow = ( hit: estypes.SearchHit, indexPattern: IndexPattern, @@ -71,5 +91,7 @@ export const formatTopLevelObject = ( pairs.push([displayKey ? displayKey : key, formatted]); }); const maxEntries = getServices().uiSettings.get(MAX_DOC_FIELDS_DISPLAYED); - return ; + return ( + + ); }; From 81f2b8808432f811b8abad0e7f2c805423526c1e Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 18:43:07 +0200 Subject: [PATCH 34/43] Improve mobile layout --- .../public/application/components/table/table_cell_value.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plugins/discover/public/application/components/table/table_cell_value.tsx b/src/plugins/discover/public/application/components/table/table_cell_value.tsx index ea9bc44f82c39..569944ec17b7a 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_value.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_value.tsx @@ -58,6 +58,7 @@ const IgnoreWarning: React.FC = React.memo(({ rawValue, reas Date: Sat, 16 Oct 2021 20:19:16 +0200 Subject: [PATCH 35/43] Fix jest tests --- .../doc_table/lib/row_formatter.test.ts | 66 ++++++++++++++++++- 1 file changed, 63 insertions(+), 3 deletions(-) diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts index 2f29fce84d105..c9a7152c7b21a 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts @@ -165,6 +165,66 @@ describe('Row formatter', () => { } />, ], + Array [ + "foo", + , + ], + Array [ + "hello", + World", + } + } + />, + ], + Array [ + "number", + , + ], + Array [ + "_id", + , + ], + Array [ + "_score", + , + ], + Array [ + "_type", + , + ], ] } /> @@ -274,7 +334,7 @@ describe('Row formatter', () => { indexPattern ) ).toMatchInlineSnapshot(` - { indexPattern ) ).toMatchInlineSnapshot(` - { indexPattern ) ).toMatchInlineSnapshot(` - Date: Sat, 16 Oct 2021 20:19:27 +0200 Subject: [PATCH 36/43] Fix typo --- .../discover/public/application/helpers/get_ignored_reason.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/helpers/get_ignored_reason.ts b/src/plugins/discover/public/application/helpers/get_ignored_reason.ts index a64a8f587175d..4d2fb85bdb2c4 100644 --- a/src/plugins/discover/public/application/helpers/get_ignored_reason.ts +++ b/src/plugins/discover/public/application/helpers/get_ignored_reason.ts @@ -21,7 +21,7 @@ export enum IgnoredReason { * This implementation will make some assumptions based on specific types * of ignored values can only happen with specific field types in Elasticsearch. * - * @param field Either the data view field or the strnig name of it. + * @param field Either the data view field or the string name of it. * @param ignoredFields The hit._ignored value of the hit to validate. */ export function getIgnoredReason( From 79abe371a21e8f6fe8c4cf6e349be6023fed1e81 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Sat, 16 Oct 2021 20:59:06 +0200 Subject: [PATCH 37/43] Remove additional span again --- .../doc_table/components/table_row.tsx | 6 +- .../doc_table/lib/row_formatter.test.ts | 174 +++--------------- .../doc_table/lib/row_formatter.tsx | 30 +-- .../get_render_cell_value.test.tsx | 172 +++++++---------- .../discover_grid/get_render_cell_value.tsx | 17 +- .../application/components/table/table.tsx | 2 +- .../components/table/table_cell_value.tsx | 10 +- .../public/application/helpers/format_hit.ts | 6 +- .../{format_value.tsx => format_value.ts} | 19 +- 9 files changed, 133 insertions(+), 303 deletions(-) rename src/plugins/discover/public/application/helpers/{format_value.tsx => format_value.ts} (70%) diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx index 47d5aa59b4836..0bf4a36555d16 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/components/table_row.tsx @@ -85,7 +85,11 @@ export const TableRow = ({ mapping(fieldName) ); - return
{formattedField}
; + return ( + // formatFieldValue always returns sanitized HTML + // eslint-disable-next-line react/no-danger +
+ ); }; const inlineFilter = useCallback( (column: string, type: '+' | '-') => { diff --git a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts index c9a7152c7b21a..2e777a18ce906 100644 --- a/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts +++ b/src/plugins/discover/public/application/apps/main/components/doc_table/lib/row_formatter.test.ts @@ -67,73 +67,31 @@ describe('Row formatter', () => { Array [ Array [ "also", - , + "with \\"quotes\\" or 'single quotes'", ], Array [ "foo", - , + "bar", ], Array [ "hello", - World", - } - } - />, + "

World

", ], Array [ "number", - , + 42, ], Array [ "_id", - , + "a", ], Array [ "_score", - , + 1, ], Array [ "_type", - , + "doc", ], ] } @@ -157,73 +115,31 @@ describe('Row formatter', () => { Array [ Array [ "also", - , + "with \\"quotes\\" or 'single quotes'", ], Array [ "foo", - , + "bar", ], Array [ "hello", - World", - } - } - />, + "

World

", ], Array [ "number", - , + 42, ], Array [ "_id", - , + "a", ], Array [ "_score", - , + 1, ], Array [ "_type", - , + "doc", ], ] } @@ -239,73 +155,31 @@ describe('Row formatter', () => { Array [ Array [ "number", - , + 42, ], Array [ "also", - , + "with \\"quotes\\" or 'single quotes'", ], Array [ "foo", - , + "bar", ], Array [ "hello", - World", - } - } - />, + "

World

", ], Array [ "_id", - , + "a", ], Array [ "_score", - , + 1, ], Array [ "_type", - , + "doc", ], ] } @@ -334,7 +208,7 @@ describe('Row formatter', () => { indexPattern ) ).toMatchInlineSnapshot(` - { indexPattern ) ).toMatchInlineSnapshot(` - { indexPattern ) ).toMatchInlineSnapshot(` - ; + defPairs: Array<[string, string]>; } const TemplateComponent = ({ defPairs }: Props) => { - return ( -
- {defPairs.map((pair, idx) => ( - -
{pair[0]}:
-
{pair[1]}
{' '} -
- ))} -
- ); -}; - -interface RawProps { - defPairs: Array<[string, unknown]>; -} -const TemplateComponentRaw = ({ defPairs }: RawProps) => { return (
{defPairs.map((pair, idx) => ( @@ -42,8 +26,8 @@ const TemplateComponentRaw = ({ defPairs }: RawProps) => {
{pair[0]}:
{' '} ))} @@ -68,8 +52,8 @@ export const formatTopLevelObject = ( indexPattern: IndexPattern ) => { const highlights = row.highlight ?? {}; - const highlightPairs: Array<[string, unknown]> = []; - const sourcePairs: Array<[string, unknown]> = []; + const highlightPairs: Array<[string, string]> = []; + const sourcePairs: Array<[string, string]> = []; const sorted = Object.entries(fields).sort(([keyA], [keyB]) => keyA.localeCompare(keyB)); sorted.forEach(([key, values]) => { const field = indexPattern.getFieldByName(key); @@ -91,7 +75,5 @@ export const formatTopLevelObject = ( pairs.push([displayKey ? displayKey : key, formatted]); }); const maxEntries = getServices().uiSettings.get(MAX_DOC_FIELDS_DISPLAYED); - return ( - - ); + return ; }; diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx index d3e3dc00e8c8c..63e9906633f64 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx @@ -129,57 +129,45 @@ describe('Discover grid cell rendering', function () { - - + } + /> bytes - - + } + /> _index - - + } + /> _score - - + } + /> `); }); @@ -257,61 +245,49 @@ describe('Discover grid cell rendering', function () { - - + } + /> bytes - - + } + /> _index - - + } + /> _score - - + } + /> `); }); @@ -347,61 +323,49 @@ describe('Discover grid cell rendering', function () { - - + } + /> bytes - - + } + /> _index - - + } + /> _score - - + } + /> `); }); diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx index 7f5c9aea7ced6..4066c13f6391e 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.tsx @@ -154,9 +154,10 @@ export const getRenderCellValueFn = {pairs.map(([key, value]) => ( {key} - - {value} - + ))} @@ -172,5 +173,13 @@ export const getRenderCellValueFn = return {JSON.stringify(rowFlattened[columnId])}; } - return formatFieldValue(rowFlattened[columnId], row, indexPattern, field); + return ( + + ); }; diff --git a/src/plugins/discover/public/application/components/table/table.tsx b/src/plugins/discover/public/application/components/table/table.tsx index 97c31f237ff2f..8ab60412eac42 100644 --- a/src/plugins/discover/public/application/components/table/table.tsx +++ b/src/plugins/discover/public/application/components/table/table.tsx @@ -47,7 +47,7 @@ export interface FieldRecord { fieldMapping?: IndexPatternField; }; value: { - formattedValue: JSX.Element; + formattedValue: string; ignored?: IgnoredReason; }; } diff --git a/src/plugins/discover/public/application/components/table/table_cell_value.tsx b/src/plugins/discover/public/application/components/table/table_cell_value.tsx index 569944ec17b7a..c8bbeda2bfd10 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_value.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_value.tsx @@ -125,9 +125,13 @@ export const TableFieldValue = ({ )} )} -
- {formattedValue} -
+
); }; diff --git a/src/plugins/discover/public/application/helpers/format_hit.ts b/src/plugins/discover/public/application/helpers/format_hit.ts index 0dff147d28935..4ffd3543bc6f4 100644 --- a/src/plugins/discover/public/application/helpers/format_hit.ts +++ b/src/plugins/discover/public/application/helpers/format_hit.ts @@ -17,7 +17,7 @@ import { formatFieldValue } from './format_value'; const formattedHitCache = new WeakMap(); -type FormattedHit = Array<[string, JSX.Element]>; +type FormattedHit = Array<[fieldName: string, formattedValue: string]>; export function formatHit( hit: estypes.SearchHit, @@ -33,8 +33,8 @@ export function formatHit( // Keys are sorted in the hits object const flattened = flattenHit(hit, dataView, { includeIgnoredValues: true, source: true }); - const highlightPairs: Array<[string, JSX.Element]> = []; - const sourcePairs: Array<[string, JSX.Element]> = []; + const highlightPairs: Array<[fieldName: string, formattedValue: string]> = []; + const sourcePairs: Array<[fieldName: string, formattedValue: string]> = []; Object.entries(flattened).forEach(([key, val]) => { const displayKey = dataView.fields.getByName(key)?.displayName; const pairs = highlights[key] ? highlightPairs : sourcePairs; diff --git a/src/plugins/discover/public/application/helpers/format_value.tsx b/src/plugins/discover/public/application/helpers/format_value.ts similarity index 70% rename from src/plugins/discover/public/application/helpers/format_value.tsx rename to src/plugins/discover/public/application/helpers/format_value.ts index 08027dd426404..1ba1072855ad9 100644 --- a/src/plugins/discover/public/application/helpers/format_value.tsx +++ b/src/plugins/discover/public/application/helpers/format_value.ts @@ -6,7 +6,6 @@ * Side Public License, v 1. */ -import React from 'react'; import { estypes } from '@elastic/elasticsearch'; import { DataView, DataViewField, KBN_FIELD_TYPES } from '../../../../data/common'; import { getServices } from '../../kibana_services'; @@ -21,28 +20,22 @@ import { getServices } from '../../kibana_services'; * @param hit The actual search hit (required to get highlight information from) * @param dataView The data view if available * @param field The field that value was from if available + * @returns An sanitized HTML string, that is safe to be applied via dangerouslySetInnerHTML */ export function formatFieldValue( value: unknown, hit: estypes.SearchHit, dataView?: DataView, field?: DataViewField -): React.ReactElement { - // We rely on field formatters to always produce safe and escaped HTML, thus we can set it - // here as HTML - // eslint-disable-next-line react/no-danger - const render = (html: string) => ; - +): string { if (!dataView || !field) { // If either no field is available or no data view, we'll use the default // string formatter to format that field. - return render( - getServices() - .fieldFormats.getDefaultInstance(KBN_FIELD_TYPES.STRING) - .convert(value, 'html', { hit, field }) - ); + return getServices() + .fieldFormats.getDefaultInstance(KBN_FIELD_TYPES.STRING) + .convert(value, 'html', { hit, field }); } // If we have a data view and field we use that fields field formatter - return render(dataView.getFormatterForField(field).convert(value, 'html', { hit, field })); + return dataView.getFormatterForField(field).convert(value, 'html', { hit, field }); } From e24d0dd5a3266077c499e6ff46a05e8fc024b993 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 18 Oct 2021 09:16:38 +0200 Subject: [PATCH 38/43] Change mock to revert test --- .../components/discover_grid/get_render_cell_value.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx index 63e9906633f64..f3fa06952fbfa 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx @@ -26,7 +26,7 @@ jest.mock('../../../kibana_services', () => ({ get: jest.fn(), }, fieldFormats: { - getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), + getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => (value ? value : '-') })), }, }), })); @@ -603,6 +603,6 @@ describe('Discover grid cell rendering', function () { setCellProps={jest.fn()} /> ); - expect(component.html()).toMatchInlineSnapshot(`""`); + expect(component.html()).toMatchInlineSnapshot(`"-"`); }); }); From 5e3974a28228c4ac1c4acde6f43416f4bc56fc04 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 18 Oct 2021 12:20:31 +0200 Subject: [PATCH 39/43] Improve tests --- .../public/__mocks__/index_pattern.ts | 2 +- .../layout/discover_documents.test.tsx | 10 +-- .../discover_grid/discover_grid.test.tsx | 11 +-- .../public/application/helpers/format_hit.ts | 19 ++++- .../application/helpers/format_value.test.ts | 69 +++++++++++++++++++ .../application/helpers/format_value.ts | 2 - 6 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 src/plugins/discover/public/application/helpers/format_value.test.ts diff --git a/src/plugins/discover/public/__mocks__/index_pattern.ts b/src/plugins/discover/public/__mocks__/index_pattern.ts index a3595b457b3aa..62b94c12d61da 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern.ts @@ -78,7 +78,7 @@ const indexPattern = { getFieldByName: jest.fn(() => ({})), timeFieldName: '', docvalueFields: [], - getFormatterForField: () => ({ convert: (value: unknown) => value }), + getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), } as unknown as IndexPattern; indexPattern.isTimeBased = () => !!indexPattern.timeFieldName; diff --git a/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx b/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx index c916d556ead0a..bafcd71e2b373 100644 --- a/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx +++ b/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx @@ -22,15 +22,7 @@ import { indexPatternMock } from '../../../../../__mocks__/index_pattern'; jest.mock('../../../../../kibana_services', () => ({ ...jest.requireActual('../../../../../kibana_services'), - getServices: () => ({ - fieldFormats: { - getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), - getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), - }, - uiSettings: { - get: jest.fn((key: string) => key === 'discover:maxDocFieldsDisplayed' && 50), - }, - }), + getServices: () => discoverServiceMock, })); setHeaderActionMenuMounter(jest.fn()); diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx index 435ae9a6b8302..f8975a1ff0f11 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx @@ -18,18 +18,11 @@ import { uiSettingsMock } from '../../../__mocks__/ui_settings'; import { DiscoverServices } from '../../../build_services'; import { ElasticSearchHit } from '../../doc_views/doc_views_types'; import { getDocId } from './discover_grid_document_selection'; +import { discoverServiceMock } from '../../../__mocks__/services'; jest.mock('../../../kibana_services', () => ({ ...jest.requireActual('../../../kibana_services'), - getServices: () => ({ - fieldFormats: { - getDefaultInstance: jest.fn(() => ({ convert: (value: unknown) => value })), - getFormatterForField: jest.fn(() => ({ convert: (value: unknown) => value })), - }, - uiSettings: { - get: jest.fn((key: string) => key === 'discover:maxDocFieldsDisplayed' && 50), - }, - }), + getServices: () => discoverServiceMock, })); function getProps() { diff --git a/src/plugins/discover/public/application/helpers/format_hit.ts b/src/plugins/discover/public/application/helpers/format_hit.ts index 4ffd3543bc6f4..25f5e870e5eb3 100644 --- a/src/plugins/discover/public/application/helpers/format_hit.ts +++ b/src/plugins/discover/public/application/helpers/format_hit.ts @@ -13,12 +13,19 @@ import { getServices } from '../../kibana_services'; import { formatFieldValue } from './format_value'; // TODO: Test coverage -// TODO: documentation const formattedHitCache = new WeakMap(); type FormattedHit = Array<[fieldName: string, formattedValue: string]>; +/** + * Returns a formatted document in form of key/value pairs of the fields name and a formatted value. + * The value returned in each pair is an HTML string which is safe to be applied to the DOM, since + * it's formatted using field formatters. + * @param hit The hit to format + * @param dataView The corresponding data view + * @param fieldsToShow A list of fields that should be included in the document summary. + */ export function formatHit( hit: estypes.SearchHit, dataView: DataView, @@ -30,15 +37,23 @@ export function formatHit( } const highlights = hit.highlight ?? {}; - // Keys are sorted in the hits object + // Flatten the object using the flattenHit implementation we use across Discover for flattening documents. const flattened = flattenHit(hit, dataView, { includeIgnoredValues: true, source: true }); const highlightPairs: Array<[fieldName: string, formattedValue: string]> = []; const sourcePairs: Array<[fieldName: string, formattedValue: string]> = []; + + // Add each flattened field into the corresponding array for highlighted or other fields, + // depending on whether the original hit had a highlight for it. That way we can later + // put highlighted fields first in the document summary. Object.entries(flattened).forEach(([key, val]) => { + // Retrieve the (display) name of the fields, if it's a mapped field on the data view const displayKey = dataView.fields.getByName(key)?.displayName; const pairs = highlights[key] ? highlightPairs : sourcePairs; + // Format the raw value using the regular field formatters for that field const formattedValue = formatFieldValue(val, hit, dataView, dataView.fields.getByName(key)); + // If the field was a mapped field, we validate it against the fieldsToShow list, if not + // we always include it into the result. if (displayKey) { if (fieldsToShow.includes(key)) { pairs.push([displayKey, formattedValue]); diff --git a/src/plugins/discover/public/application/helpers/format_value.test.ts b/src/plugins/discover/public/application/helpers/format_value.test.ts new file mode 100644 index 0000000000000..76d95c08e4a19 --- /dev/null +++ b/src/plugins/discover/public/application/helpers/format_value.test.ts @@ -0,0 +1,69 @@ +/* + * 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 type { FieldFormat } from '../../../../field_formats/common'; +import { indexPatternMock } from '../../__mocks__/index_pattern'; +import { formatFieldValue } from './format_value'; + +import { getServices } from '../../kibana_services'; + +jest.mock('../../kibana_services', () => { + const services = { + fieldFormats: { + getDefaultInstance: jest.fn( + () => ({ convert: (value: unknown) => value } as FieldFormat) + ), + }, + }; + return { getServices: () => services }; +}); + +const hit = { + _id: '1', + _index: 'index', + fields: { + message: 'foo', + }, +}; + +describe('formatFieldValue', () => { + afterEach(() => { + (indexPatternMock.getFormatterForField as jest.Mock).mockReset(); + (getServices().fieldFormats.getDefaultInstance as jest.Mock).mockReset(); + }); + + it('should call correct fieldFormatter for field', () => { + const formatterForFieldMock = indexPatternMock.getFormatterForField as jest.Mock; + const convertMock = jest.fn((value: unknown) => `formatted:${value}`); + formatterForFieldMock.mockReturnValue({ convert: convertMock }); + const field = indexPatternMock.fields.getByName('message'); + expect(formatFieldValue('foo', hit, indexPatternMock, field)).toBe('formatted:foo'); + expect(indexPatternMock.getFormatterForField).toHaveBeenCalledWith(field); + expect(convertMock).toHaveBeenCalledWith('foo', 'html', { field, hit }); + }); + + it('should call default string formatter if no field specified', () => { + const convertMock = jest.fn((value: unknown) => `formatted:${value}`); + (getServices().fieldFormats.getDefaultInstance as jest.Mock).mockReturnValue({ + convert: convertMock, + }); + expect(formatFieldValue('foo', hit, indexPatternMock)).toBe('formatted:foo'); + expect(getServices().fieldFormats.getDefaultInstance).toHaveBeenCalledWith('string'); + expect(convertMock).toHaveBeenCalledWith('foo', 'html', { field: undefined, hit }); + }); + + it('should call default string formatter if no indexPattern is specified', () => { + const convertMock = jest.fn((value: unknown) => `formatted:${value}`); + (getServices().fieldFormats.getDefaultInstance as jest.Mock).mockReturnValue({ + convert: convertMock, + }); + expect(formatFieldValue('foo', hit)).toBe('formatted:foo'); + expect(getServices().fieldFormats.getDefaultInstance).toHaveBeenCalledWith('string'); + expect(convertMock).toHaveBeenCalledWith('foo', 'html', { field: undefined, hit }); + }); +}); diff --git a/src/plugins/discover/public/application/helpers/format_value.ts b/src/plugins/discover/public/application/helpers/format_value.ts index 1ba1072855ad9..cc33276790372 100644 --- a/src/plugins/discover/public/application/helpers/format_value.ts +++ b/src/plugins/discover/public/application/helpers/format_value.ts @@ -10,8 +10,6 @@ import { estypes } from '@elastic/elasticsearch'; import { DataView, DataViewField, KBN_FIELD_TYPES } from '../../../../data/common'; import { getServices } from '../../kibana_services'; -// TODO: need more test coverage - /** * Formats the value of a specific field using the appropriate field formatter if available * or the default string field formatter otherwise. From 358b77d2fce39d99cafe6a31c3651fef931ec6dd Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 18 Oct 2021 15:56:23 +0200 Subject: [PATCH 40/43] More jest tests --- .../public/__mocks__/index_pattern.ts | 5 + .../discover/public/__mocks__/services.ts | 4 +- .../layout/discover_documents.test.tsx | 2 +- .../discover_grid/discover_grid.test.tsx | 3 +- .../get_render_cell_value.test.tsx | 16 ++-- .../application/helpers/format_hit.test.ts | 96 +++++++++++++++++++ 6 files changed, 113 insertions(+), 13 deletions(-) create mode 100644 src/plugins/discover/public/application/helpers/format_hit.test.ts diff --git a/src/plugins/discover/public/__mocks__/index_pattern.ts b/src/plugins/discover/public/__mocks__/index_pattern.ts index 62b94c12d61da..d33445baa0a2b 100644 --- a/src/plugins/discover/public/__mocks__/index_pattern.ts +++ b/src/plugins/discover/public/__mocks__/index_pattern.ts @@ -27,6 +27,7 @@ const fields = [ { name: 'message', type: 'string', + displayName: 'message', scripted: false, filterable: false, aggregatable: false, @@ -34,6 +35,7 @@ const fields = [ { name: 'extension', type: 'string', + displayName: 'extension', scripted: false, filterable: true, aggregatable: true, @@ -41,6 +43,7 @@ const fields = [ { name: 'bytes', type: 'number', + displayName: 'bytesDisplayName', scripted: false, filterable: true, aggregatable: true, @@ -48,12 +51,14 @@ const fields = [ { name: 'scripted', type: 'number', + displayName: 'scripted', scripted: true, filterable: false, }, { name: 'object.value', type: 'number', + displayName: 'object.value', scripted: false, filterable: true, aggregatable: true, diff --git a/src/plugins/discover/public/__mocks__/services.ts b/src/plugins/discover/public/__mocks__/services.ts index 858863396ff47..6a90ed42417e6 100644 --- a/src/plugins/discover/public/__mocks__/services.ts +++ b/src/plugins/discover/public/__mocks__/services.ts @@ -50,7 +50,7 @@ export const discoverServiceMock = { }, filterManager: dataPlugin.query.filterManager, uiSettings: { - get: (key: string) => { + get: jest.fn((key: string) => { if (key === 'fields:popularLimit') { return 5; } else if (key === DEFAULT_COLUMNS_SETTING) { @@ -70,7 +70,7 @@ export const discoverServiceMock = { } else if (key === MAX_DOC_FIELDS_DISPLAYED) { return 50; } - }, + }), isDefault: (key: string) => { return true; }, diff --git a/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx b/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx index bafcd71e2b373..60540268dcd7f 100644 --- a/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx +++ b/src/plugins/discover/public/application/apps/main/components/layout/discover_documents.test.tsx @@ -22,7 +22,7 @@ import { indexPatternMock } from '../../../../../__mocks__/index_pattern'; jest.mock('../../../../../kibana_services', () => ({ ...jest.requireActual('../../../../../kibana_services'), - getServices: () => discoverServiceMock, + getServices: () => jest.requireActual('../../../../../__mocks__/services').discoverServiceMock, })); setHeaderActionMenuMounter(jest.fn()); diff --git a/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx b/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx index f8975a1ff0f11..22284480afc05 100644 --- a/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/discover_grid.test.tsx @@ -18,11 +18,10 @@ import { uiSettingsMock } from '../../../__mocks__/ui_settings'; import { DiscoverServices } from '../../../build_services'; import { ElasticSearchHit } from '../../doc_views/doc_views_types'; import { getDocId } from './discover_grid_document_selection'; -import { discoverServiceMock } from '../../../__mocks__/services'; jest.mock('../../../kibana_services', () => ({ ...jest.requireActual('../../../kibana_services'), - getServices: () => discoverServiceMock, + getServices: () => jest.requireActual('../../../__mocks__/services').discoverServiceMock, })); function getProps() { diff --git a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx index f3fa06952fbfa..3fb96ba9e9daa 100644 --- a/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx +++ b/src/plugins/discover/public/application/components/discover_grid/get_render_cell_value.test.tsx @@ -105,7 +105,7 @@ describe('Discover grid cell rendering', function () { rowsSource, rowsSource.map(flatten), false, - [], + ['extension', 'bytes'], 100 ); const component = shallow( @@ -136,7 +136,7 @@ describe('Discover grid cell rendering', function () { } /> - bytes + bytesDisplayName - bytes + bytesDisplayName - bytes + bytesDisplayName ({ + getServices: () => jest.requireActual('../../__mocks__/services').discoverServiceMock, +})); + +describe('formatHit', () => { + let hit: estypes.SearchHit; + beforeEach(() => { + hit = { + _id: '1', + _index: 'logs', + fields: { + message: ['foobar'], + extension: ['png'], + 'object.value': [42, 13], + bytes: [123], + }, + }; + (dataViewMock.getFormatterForField as jest.Mock).mockReturnValue({ + convert: (value: unknown) => `formatted:${value}`, + }); + }); + + afterEach(() => { + (discoverServiceMock.uiSettings.get as jest.Mock).mockReset(); + }); + + it('formats a document as expected', () => { + const formatted = formatHit(hit, dataViewMock, ['message', 'extension', 'object.value']); + expect(formatted).toEqual([ + ['extension', 'formatted:png'], + ['message', 'formatted:foobar'], + ['object.value', 'formatted:42,13'], + ['_index', 'formatted:logs'], + ['_score', undefined], + ]); + }); + + it('orders highlighted fields first', () => { + const formatted = formatHit({ ...hit, highlight: { message: ['%%'] } }, dataViewMock, [ + 'message', + 'extension', + 'object.value', + ]); + expect(formatted.map(([fieldName]) => fieldName)).toEqual([ + 'message', + 'extension', + 'object.value', + '_index', + '_score', + ]); + }); + + it('only limits count of pairs based on advanced setting', () => { + (discoverServiceMock.uiSettings.get as jest.Mock).mockImplementation( + (key) => key === MAX_DOC_FIELDS_DISPLAYED && 2 + ); + const formatted = formatHit(hit, dataViewMock, ['message', 'extension', 'object.value']); + expect(formatted).toEqual([ + ['extension', 'formatted:png'], + ['message', 'formatted:foobar'], + ]); + }); + + it('should not include fields not mentioned in fieldsToShow', () => { + const formatted = formatHit(hit, dataViewMock, ['message', 'object.value']); + expect(formatted).toEqual([ + ['message', 'formatted:foobar'], + ['object.value', 'formatted:42,13'], + ['_index', 'formatted:logs'], + ['_score', undefined], + ]); + }); + + it('should filter fields based on their real name not displayName', () => { + const formatted = formatHit(hit, dataViewMock, ['bytes']); + expect(formatted).toEqual([ + ['bytesDisplayName', 'formatted:123'], + ['_index', 'formatted:logs'], + ['_score', undefined], + ]); + }); +}); From 3863d3b131a203773c9f893528737b8f1dcc7d72 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 18 Oct 2021 16:38:52 +0200 Subject: [PATCH 41/43] Fix typo --- .../public/application/components/table/table_cell_value.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/discover/public/application/components/table/table_cell_value.tsx b/src/plugins/discover/public/application/components/table/table_cell_value.tsx index c8bbeda2bfd10..e006de1cd7aeb 100644 --- a/src/plugins/discover/public/application/components/table/table_cell_value.tsx +++ b/src/plugins/discover/public/application/components/table/table_cell_value.tsx @@ -33,7 +33,7 @@ const IgnoreWarning: React.FC = React.memo(({ rawValue, reas defaultMessage: `One or more values in this field are too long and can't be searched or filtered.`, }) : i18n.translate('discover.docView.table.ignored.singleAboveTooltip', { - defaultMessage: `The value in this field is too long and and can't be searched or filtered.`, + defaultMessage: `The value in this field is too long and can't be searched or filtered.`, }); case IgnoredReason.MALFORMED: return multiValue From 54b03ce904ae7aaefc0739ba1a80cb2b994cbbb9 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 19 Oct 2021 09:44:11 +0200 Subject: [PATCH 42/43] Change wording --- .../application/components/table/table_row_btn_filter_add.tsx | 2 +- .../components/table/table_row_btn_filter_remove.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plugins/discover/public/application/components/table/table_row_btn_filter_add.tsx b/src/plugins/discover/public/application/components/table/table_row_btn_filter_add.tsx index 5812e0859bd94..de56d733442d6 100644 --- a/src/plugins/discover/public/application/components/table/table_row_btn_filter_add.tsx +++ b/src/plugins/discover/public/application/components/table/table_row_btn_filter_add.tsx @@ -20,7 +20,7 @@ export function DocViewTableRowBtnFilterAdd({ onClick, disabled = false }: Props const tooltipContent = disabled ? ( ) : ( ) : ( Date: Tue, 19 Oct 2021 11:55:17 +0200 Subject: [PATCH 43/43] Remove dead comment --- src/plugins/discover/public/application/helpers/format_hit.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/plugins/discover/public/application/helpers/format_hit.ts b/src/plugins/discover/public/application/helpers/format_hit.ts index 25f5e870e5eb3..3890973a3f3e4 100644 --- a/src/plugins/discover/public/application/helpers/format_hit.ts +++ b/src/plugins/discover/public/application/helpers/format_hit.ts @@ -12,8 +12,6 @@ import { MAX_DOC_FIELDS_DISPLAYED } from '../../../common'; import { getServices } from '../../kibana_services'; import { formatFieldValue } from './format_value'; -// TODO: Test coverage - const formattedHitCache = new WeakMap(); type FormattedHit = Array<[fieldName: string, formattedValue: string]>;