From 5d0d6f91cb6b5d4c80c99b6dfeb6970e8058eeaf Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Wed, 17 Feb 2021 19:41:47 +0000 Subject: [PATCH 1/4] [Discover] Always show unmapped fields --- .../public/application/angular/discover.js | 2 +- .../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 ---------- 7 files changed, 3 insertions(+), 67 deletions(-) diff --git a/src/plugins/discover/public/application/angular/discover.js b/src/plugins/discover/public/application/angular/discover.js index c2bbbf2e57a9a..7d7e8bf8bb486 100644 --- a/src/plugins/discover/public/application/angular/discover.js +++ b/src/plugins/discover/public/application/angular/discover.js @@ -674,7 +674,7 @@ function discoverController($route, $scope, Promise) { history.push('/'); }; - const showUnmappedFieldsDefaultValue = $scope.useNewFieldsApi && !!$scope.opts.savedSearch.pre712; + const showUnmappedFieldsDefaultValue = $scope.useNewFieldsApi; let showUnmappedFields = showUnmappedFieldsDefaultValue; const onChangeUnmappedFields = (value) => { diff --git a/src/plugins/discover/public/application/embeddable/search_embeddable.ts b/src/plugins/discover/public/application/embeddable/search_embeddable.ts index 658734aa46cb0..2bafa23907502 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 a7b6ef49cacd2..320332ca4ace5 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 4646744ee0ef3..b1c7b48d696b3 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 de3a2197fe0ac..b66c06db3e120 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 f1dc228a9ac08..fb608c0b6f3e8 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 72749bfd2e9cd..feaf91409797a 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), }; From e05f479f58db27802478adf91ff3720e9eb87756 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Thu, 18 Feb 2021 07:44:12 +0000 Subject: [PATCH 2/4] Updating the functional test --- .../discover/_indexpattern_with_unmapped_fields.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts b/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts index 0990b3fa29f70..b2eec32e5a366 100644 --- a/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts +++ b/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts @@ -37,7 +37,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 +46,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 () => { From 2df09d1dcfae84a39bb3ebfb024c38fca60fe065 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Thu, 18 Feb 2021 17:29:27 +0000 Subject: [PATCH 3/4] Remove unmapped switch toggle --- .../public/application/angular/discover.js | 10 +-- .../sidebar/discover_field_search.test.tsx | 18 ----- .../sidebar/discover_field_search.tsx | 77 +++---------------- .../components/sidebar/discover_sidebar.tsx | 2 - .../_indexpattern_with_unmapped_fields.ts | 19 ----- 5 files changed, 12 insertions(+), 114 deletions(-) diff --git a/src/plugins/discover/public/application/angular/discover.js b/src/plugins/discover/public/application/angular/discover.js index 7d7e8bf8bb486..0af35da56efa2 100644 --- a/src/plugins/discover/public/application/angular/discover.js +++ b/src/plugins/discover/public/application/angular/discover.js @@ -675,18 +675,12 @@ function discoverController($route, $scope, Promise) { }; const showUnmappedFieldsDefaultValue = $scope.useNewFieldsApi; - let showUnmappedFields = showUnmappedFieldsDefaultValue; - - const onChangeUnmappedFields = (value) => { - showUnmappedFields = value; - $scope.unmappedFieldsConfig.showUnmappedFields = value; - $scope.fetch(); - }; + const showUnmappedFields = true; $scope.unmappedFieldsConfig = { showUnmappedFieldsDefaultValue, showUnmappedFields, - onChangeUnmappedFields, + 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 797a6c9697c35..04562cbd26520 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 8fb90bfea3a95..1e99959d77134 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 f0303553dfac0..c0a192550e6c4 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/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts b/test/functional/apps/discover/_indexpattern_with_unmapped_fields.ts index b2eec32e5a366..06933e828db7e 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({ @@ -60,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); - }); }); } From e264ce356108afa4f6a4b2c1d581f275e1097905 Mon Sep 17 00:00:00 2001 From: Maja Grubic Date: Thu, 18 Feb 2021 17:48:43 +0000 Subject: [PATCH 4/4] Some more code cleanup --- .../discover/public/application/angular/discover.js | 5 +---- .../sidebar/discover_sidebar_responsive.test.tsx | 2 -- .../sidebar/discover_sidebar_responsive.tsx | 13 +------------ .../discover/public/application/components/types.ts | 13 +------------ 4 files changed, 3 insertions(+), 30 deletions(-) diff --git a/src/plugins/discover/public/application/angular/discover.js b/src/plugins/discover/public/application/angular/discover.js index 0af35da56efa2..b7bcad694dceb 100644 --- a/src/plugins/discover/public/application/angular/discover.js +++ b/src/plugins/discover/public/application/angular/discover.js @@ -674,13 +674,10 @@ function discoverController($route, $scope, Promise) { history.push('/'); }; - const showUnmappedFieldsDefaultValue = $scope.useNewFieldsApi; - const showUnmappedFields = true; + const showUnmappedFields = $scope.useNewFieldsApi; $scope.unmappedFieldsConfig = { - showUnmappedFieldsDefaultValue, showUnmappedFields, - onChangeUnmappedFields: () => {}, }; $scope.updateDataSource = () => { 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 7b12ab5f9bcd9..79e8caabd4930 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 b689db1296922..f0e7c71f9c970 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 e276795f9ed7f..e488f596cece8 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; }; }