From b5f05d6fec23c67f455a845d94af5b074884d675 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Mon, 22 Feb 2021 10:08:58 +0000 Subject: [PATCH] [Discover] Always show unmapped fields (#91735) (#92071) * [Discover] Always show unmapped fields * Updating the functional test * Remove unmapped switch toggle * Some more code cleanup Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../public/application/angular/discover.js | 11 +-- .../sidebar/discover_field_search.test.tsx | 18 ----- .../sidebar/discover_field_search.tsx | 77 +++---------------- .../components/sidebar/discover_sidebar.tsx | 2 - .../discover_sidebar_responsive.test.tsx | 2 - .../sidebar/discover_sidebar_responsive.tsx | 13 +--- .../public/application/components/types.ts | 13 +--- .../embeddable/search_embeddable.ts | 7 +- .../public/saved_searches/_saved_search.ts | 3 - .../discover/public/saved_searches/types.ts | 1 - .../discover/server/saved_objects/search.ts | 1 - .../saved_objects/search_migrations.test.ts | 37 --------- .../server/saved_objects/search_migrations.ts | 19 ----- .../_indexpattern_with_unmapped_fields.ts | 29 +------ 14 files changed, 17 insertions(+), 216 deletions(-) diff --git a/src/plugins/discover/public/application/angular/discover.js b/src/plugins/discover/public/application/angular/discover.js index 78ad40e48fd965..88747cf9e84d8c 100644 --- a/src/plugins/discover/public/application/angular/discover.js +++ b/src/plugins/discover/public/application/angular/discover.js @@ -675,19 +675,10 @@ function discoverController($route, $scope, Promise) { history.push('/'); }; - const showUnmappedFieldsDefaultValue = $scope.useNewFieldsApi && !!$scope.opts.savedSearch.pre712; - let showUnmappedFields = showUnmappedFieldsDefaultValue; - - const onChangeUnmappedFields = (value) => { - showUnmappedFields = value; - $scope.unmappedFieldsConfig.showUnmappedFields = value; - $scope.fetch(); - }; + const showUnmappedFields = $scope.useNewFieldsApi; $scope.unmappedFieldsConfig = { - showUnmappedFieldsDefaultValue, showUnmappedFields, - onChangeUnmappedFields, }; $scope.updateDataSource = () => { diff --git a/src/plugins/discover/public/application/components/sidebar/discover_field_search.test.tsx b/src/plugins/discover/public/application/components/sidebar/discover_field_search.test.tsx index 797a6c9697c351..04562cbd26520b 100644 --- a/src/plugins/discover/public/application/components/sidebar/discover_field_search.test.tsx +++ b/src/plugins/discover/public/application/components/sidebar/discover_field_search.test.tsx @@ -136,22 +136,4 @@ describe('DiscoverFieldSearch', () => { popover = component.find(EuiPopover); expect(popover.prop('isOpen')).toBe(false); }); - - test('unmapped fields', () => { - const onChangeUnmappedFields = jest.fn(); - const componentProps = { - ...defaultProps, - showUnmappedFields: true, - useNewFieldsApi: false, - onChangeUnmappedFields, - }; - const component = mountComponent(componentProps); - const btn = findTestSubject(component, 'toggleFieldFilterButton'); - btn.simulate('click'); - const unmappedFieldsSwitch = findTestSubject(component, 'unmappedFieldsSwitch'); - act(() => { - unmappedFieldsSwitch.simulate('click'); - }); - expect(onChangeUnmappedFields).toHaveBeenCalledWith(false); - }); }); diff --git a/src/plugins/discover/public/application/components/sidebar/discover_field_search.tsx b/src/plugins/discover/public/application/components/sidebar/discover_field_search.tsx index 8fb90bfea3a950..1e99959d77134f 100644 --- a/src/plugins/discover/public/application/components/sidebar/discover_field_search.tsx +++ b/src/plugins/discover/public/application/components/sidebar/discover_field_search.tsx @@ -27,8 +27,6 @@ import { EuiOutsideClickDetector, EuiFilterButton, EuiSpacer, - EuiIcon, - EuiToolTip, } from '@elastic/eui'; import { FormattedMessage } from '@kbn/i18n/react'; @@ -37,7 +35,6 @@ export interface State { aggregatable: string; type: string; missing: boolean; - unmappedFields: boolean; [index: string]: string | boolean; } @@ -61,31 +58,13 @@ export interface Props { * use new fields api */ useNewFieldsApi?: boolean; - - /** - * callback funtion to change the value of unmapped fields switch - * @param value new value to set - */ - onChangeUnmappedFields?: (value: boolean) => void; - - /** - * should unmapped fields switch be rendered - */ - showUnmappedFields?: boolean; } /** * Component is Discover's side bar to search of available fields * Additionally there's a button displayed that allows the user to show/hide more filter fields */ -export function DiscoverFieldSearch({ - onChange, - value, - types, - useNewFieldsApi, - showUnmappedFields, - onChangeUnmappedFields, -}: Props) { +export function DiscoverFieldSearch({ onChange, value, types, useNewFieldsApi }: Props) { const searchPlaceholder = i18n.translate('discover.fieldChooser.searchPlaceHolder', { defaultMessage: 'Search field names', }); @@ -111,7 +90,6 @@ export function DiscoverFieldSearch({ aggregatable: 'any', type: 'any', missing: true, - unmappedFields: !!showUnmappedFields, }); if (typeof value !== 'string') { @@ -181,14 +159,6 @@ export function DiscoverFieldSearch({ handleValueChange('missing', missingValue); }; - const handleUnmappedFieldsChange = (e: EuiSwitchEvent) => { - const unmappedFieldsValue = e.target.checked; - handleValueChange('unmappedFields', unmappedFieldsValue); - if (onChangeUnmappedFields) { - onChangeUnmappedFields(unmappedFieldsValue); - } - }; - const buttonContent = ( { - if (!showUnmappedFields && useNewFieldsApi) { + if (useNewFieldsApi) { return null; } return ( - {showUnmappedFields ? ( - - - - - - - - - - - ) : null} - {useNewFieldsApi ? null : ( - - )} + ); }; diff --git a/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx b/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx index f0303553dfac0a..c0a192550e6c4a 100644 --- a/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx +++ b/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx @@ -205,8 +205,6 @@ export function DiscoverSidebar({ value={fieldFilter.name} types={fieldTypes} useNewFieldsApi={useNewFieldsApi} - onChangeUnmappedFields={unmappedFieldsConfig?.onChangeUnmappedFields} - showUnmappedFields={unmappedFieldsConfig?.showUnmappedFieldsDefaultValue} /> diff --git a/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.test.tsx b/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.test.tsx index 7b12ab5f9bcd9e..79e8caabd49307 100644 --- a/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.test.tsx +++ b/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.test.tsx @@ -137,9 +137,7 @@ describe('discover responsive sidebar', function () { }); it('renders sidebar with unmapped fields config', function () { const unmappedFieldsConfig = { - onChangeUnmappedFields: jest.fn(), showUnmappedFields: false, - showUnmappedFieldsDefaultValue: false, }; const componentProps = { ...props, unmappedFieldsConfig }; const component = mountWithIntl(); diff --git a/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx b/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx index b689db12969222..f0e7c71f9c970d 100644 --- a/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx +++ b/src/plugins/discover/public/application/components/sidebar/discover_sidebar_responsive.tsx @@ -113,24 +113,13 @@ export interface DiscoverSidebarResponsiveProps { useNewFieldsApi?: boolean; /** - * an object containing properties for proper handling of unmapped fields in the UI + * an object containing properties for proper handling of unmapped fields */ unmappedFieldsConfig?: { - /** - * callback function to change the value of `showUnmappedFields` flag - * @param value new value to set - */ - onChangeUnmappedFields: (value: boolean) => void; /** * determines whether to display unmapped fields - * configurable through the switch in the UI */ showUnmappedFields: boolean; - /** - * determines if we should display an option to toggle showUnmappedFields value in the first place - * this value is not configurable through the UI - */ - showUnmappedFieldsDefaultValue: boolean; }; } diff --git a/src/plugins/discover/public/application/components/types.ts b/src/plugins/discover/public/application/components/types.ts index e276795f9ed7fd..e488f596cece85 100644 --- a/src/plugins/discover/public/application/components/types.ts +++ b/src/plugins/discover/public/application/components/types.ts @@ -159,23 +159,12 @@ export interface DiscoverProps { */ timeRange?: { from: string; to: string }; /** - * An object containing properties for proper handling of unmapped fields in the UI + * An object containing properties for unmapped fields behavior */ unmappedFieldsConfig?: { /** * determines whether to display unmapped fields - * configurable through the switch in the UI */ showUnmappedFields: boolean; - /** - * determines if we should display an option to toggle showUnmappedFields value in the first place - * this value is not configurable through the UI - */ - showUnmappedFieldsDefaultValue: boolean; - /** - * callback function to change the value of `showUnmappedFields` flag - * @param value new value to set - */ - onChangeUnmappedFields: (value: boolean) => void; }; } diff --git a/src/plugins/discover/public/application/embeddable/search_embeddable.ts b/src/plugins/discover/public/application/embeddable/search_embeddable.ts index 658734aa46cb02..2bafa239075027 100644 --- a/src/plugins/discover/public/application/embeddable/search_embeddable.ts +++ b/src/plugins/discover/public/application/embeddable/search_embeddable.ts @@ -291,7 +291,7 @@ export class SearchEmbeddable const useNewFieldsApi = !this.services.uiSettings.get(SEARCH_FIELDS_FROM_SOURCE, false); if (!this.searchScope) return; - const { searchSource, pre712 } = this.savedSearch; + const { searchSource } = this.savedSearch; // Abort any in-progress requests if (this.abortController) this.abortController.abort(); @@ -308,10 +308,7 @@ export class SearchEmbeddable ); if (useNewFieldsApi) { searchSource.removeField('fieldsFromSource'); - const fields: Record = { field: '*' }; - if (pre712) { - fields.include_unmapped = 'true'; - } + const fields: Record = { field: '*', include_unmapped: 'true' }; searchSource.setField('fields', [fields]); } else { searchSource.removeField('fields'); diff --git a/src/plugins/discover/public/saved_searches/_saved_search.ts b/src/plugins/discover/public/saved_searches/_saved_search.ts index a7b6ef49cacd21..320332ca4ace54 100644 --- a/src/plugins/discover/public/saved_searches/_saved_search.ts +++ b/src/plugins/discover/public/saved_searches/_saved_search.ts @@ -20,7 +20,6 @@ export function createSavedSearchClass(savedObjects: SavedObjectsStart) { grid: 'object', sort: 'keyword', version: 'integer', - pre712: 'boolean', }; // Order these fields to the top, the rest are alphabetical public static fieldOrder = ['title', 'description']; @@ -42,7 +41,6 @@ export function createSavedSearchClass(savedObjects: SavedObjectsStart) { grid: 'object', sort: 'keyword', version: 'integer', - pre712: 'boolean', }, searchSource: true, defaults: { @@ -52,7 +50,6 @@ export function createSavedSearchClass(savedObjects: SavedObjectsStart) { hits: 0, sort: [], version: 1, - pre712: false, }, }); this.showInRecentlyAccessed = true; diff --git a/src/plugins/discover/public/saved_searches/types.ts b/src/plugins/discover/public/saved_searches/types.ts index 4646744ee0ef3c..b1c7b48d696b34 100644 --- a/src/plugins/discover/public/saved_searches/types.ts +++ b/src/plugins/discover/public/saved_searches/types.ts @@ -23,7 +23,6 @@ export interface SavedSearch { save: (saveOptions: SavedObjectSaveOpts) => Promise; lastSavedTitle?: string; copyOnSave?: boolean; - pre712?: boolean; hideChart?: boolean; } export interface SavedSearchLoader { diff --git a/src/plugins/discover/server/saved_objects/search.ts b/src/plugins/discover/server/saved_objects/search.ts index de3a2197fe0acc..b66c06db3e1200 100644 --- a/src/plugins/discover/server/saved_objects/search.ts +++ b/src/plugins/discover/server/saved_objects/search.ts @@ -45,7 +45,6 @@ export const searchSavedObjectType: SavedObjectsType = { title: { type: 'text' }, grid: { type: 'object', enabled: false }, version: { type: 'integer' }, - pre712: { type: 'boolean' }, }, }, migrations: searchMigrations as any, diff --git a/src/plugins/discover/server/saved_objects/search_migrations.test.ts b/src/plugins/discover/server/saved_objects/search_migrations.test.ts index f1dc228a9ac082..fb608c0b6f3e81 100644 --- a/src/plugins/discover/server/saved_objects/search_migrations.test.ts +++ b/src/plugins/discover/server/saved_objects/search_migrations.test.ts @@ -350,41 +350,4 @@ Object { testMigrateMatchAllQuery(migrationFn); }); }); - - describe('7.12.0', () => { - const migrationFn = searchMigrations['7.12.0']; - - describe('migrateExistingSavedSearch', () => { - it('should add a new flag to existing saved searches', () => { - const migratedDoc = migrationFn( - { - type: 'search', - attributes: { - kibanaSavedObjectMeta: {}, - }, - }, - savedObjectMigrationContext - ); - const migratedPre712Flag = migratedDoc.attributes.pre712; - - expect(migratedPre712Flag).toEqual(true); - }); - - it('should not modify a flag if it already exists', () => { - const migratedDoc = migrationFn( - { - type: 'search', - attributes: { - kibanaSavedObjectMeta: {}, - pre712: false, - }, - }, - savedObjectMigrationContext - ); - const migratedPre712Flag = migratedDoc.attributes.pre712; - - expect(migratedPre712Flag).toEqual(false); - }); - }); - }); }); diff --git a/src/plugins/discover/server/saved_objects/search_migrations.ts b/src/plugins/discover/server/saved_objects/search_migrations.ts index 72749bfd2e9cdd..feaf91409797a2 100644 --- a/src/plugins/discover/server/saved_objects/search_migrations.ts +++ b/src/plugins/discover/server/saved_objects/search_migrations.ts @@ -117,28 +117,9 @@ const migrateSearchSortToNestedArray: SavedObjectMigrationFn = (doc) = }; }; -const migrateExistingSavedSearch: SavedObjectMigrationFn = (doc) => { - if (!doc.attributes) { - return doc; - } - const pre712 = doc.attributes.pre712; - // pre712 already has a value - if (pre712 !== undefined) { - return doc; - } - return { - ...doc, - attributes: { - ...doc.attributes, - pre712: true, - }, - }; -}; - export const searchMigrations = { '6.7.2': flow(migrateMatchAllQuery), '7.0.0': flow(setNewReferences), '7.4.0': flow(migrateSearchSortToNestedArray), '7.9.3': flow(migrateMatchAllQuery), - '7.12.0': flow(migrateExistingSavedSearch), }; diff --git a/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts b/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts index 0990b3fa29f708..06933e828db7e1 100644 --- a/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts +++ b/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts @@ -12,14 +12,11 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ getService, getPageObjects }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const kibanaServer = getService('kibanaServer'); - const testSubjects = getService('testSubjects'); const log = getService('log'); const retry = getService('retry'); const PageObjects = getPageObjects(['common', 'timePicker', 'discover']); describe('index pattern with unmapped fields', () => { - const unmappedFieldsSwitchSelector = 'unmappedFieldsSwitch'; - before(async () => { await esArchiver.loadIfNeeded('unmapped_fields'); await kibanaServer.uiSettings.replace({ @@ -37,7 +34,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await esArchiver.unload('unmapped_fields'); }); - it('unmapped fields do not exist on a new saved search', async () => { + it('unmapped fields exist on a new saved search', async () => { const expectedHitCount = '4'; await retry.try(async function () { expect(await PageObjects.discover.getHitCount()).to.be(expectedHitCount); @@ -46,13 +43,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { // message is a mapped field expect(allFields.includes('message')).to.be(true); // sender is not a mapped field - expect(allFields.includes('sender')).to.be(false); - }); - - it('unmapped fields toggle does not exist on a new saved search', async () => { - await PageObjects.discover.openSidebarFieldFilter(); - await testSubjects.existOrFail('filterSelectionPanel'); - await testSubjects.missingOrFail('unmappedFieldsSwitch'); + expect(allFields.includes('sender')).to.be(true); }); it('unmapped fields exist on an existing saved search', async () => { @@ -66,21 +57,5 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { expect(allFields.includes('sender')).to.be(true); expect(allFields.includes('receiver')).to.be(true); }); - - it('unmapped fields toggle exists on an existing saved search', async () => { - await PageObjects.discover.openSidebarFieldFilter(); - await testSubjects.existOrFail('filterSelectionPanel'); - await testSubjects.existOrFail(unmappedFieldsSwitchSelector); - expect(await testSubjects.isEuiSwitchChecked(unmappedFieldsSwitchSelector)).to.be(true); - }); - - it('switching unmapped fields toggle off hides unmapped fields', async () => { - await testSubjects.setEuiSwitch(unmappedFieldsSwitchSelector, 'uncheck'); - await PageObjects.discover.closeSidebarFieldFilter(); - const allFields = await PageObjects.discover.getAllFieldNames(); - expect(allFields.includes('message')).to.be(true); - expect(allFields.includes('sender')).to.be(false); - expect(allFields.includes('receiver')).to.be(false); - }); }); }