From 646539c45b454b48c9fa985b7ac5cb69a63c70f8 Mon Sep 17 00:00:00 2001 From: Matthew Kime Date: Fri, 30 Jun 2023 07:29:44 -0500 Subject: [PATCH] [data view editor] Fix data view timestamp validation (#150398) ## Summary Previously - If you changed a data view's index pattern AND the new pattern didn't contain the timestamp field, you'd see a blank timestamp field and it would let you save. The data view would have been saved with the previous timestamp field which doesn't exist. Now - The timestamp validator checks to make sure the selected timestamp field is in the list of available options. This is helpful because it keeps the previous timestamp value in case you do select an index pattern that contains it. Closes: https://github.com/elastic/kibana/issues/150219 --- .../data_view_editor_flyout_content.tsx | 2 + .../form_fields/timestamp_field.test.ts | 47 +++++++++++++++++++ .../form_fields/timestamp_field.tsx | 9 ++-- ..._delete.ts => _data_view_create_delete.ts} | 22 ++++++++- test/functional/apps/management/index.ts | 2 +- 5 files changed, 77 insertions(+), 5 deletions(-) create mode 100644 src/plugins/data_view_editor/public/components/form_fields/timestamp_field.test.ts rename test/functional/apps/management/{_index_pattern_create_delete.ts => _data_view_create_delete.ts} (88%) diff --git a/src/plugins/data_view_editor/public/components/data_view_editor_flyout_content.tsx b/src/plugins/data_view_editor/public/components/data_view_editor_flyout_content.tsx index e6987c11a920b..42614d33b660a 100644 --- a/src/plugins/data_view_editor/public/components/data_view_editor_flyout_content.tsx +++ b/src/plugins/data_view_editor/public/components/data_view_editor_flyout_content.tsx @@ -299,6 +299,8 @@ const IndexPatternEditorFlyoutContentComponent = ({ form.updateFieldValues({ name: formData.title }); await form.getFields().name.validate(); } + // Ensures timestamp field is validated against current set of options + form.validateFields(['timestampField']); form.setFieldValue('isAdHoc', adhoc || false); form.submit(); }} diff --git a/src/plugins/data_view_editor/public/components/form_fields/timestamp_field.test.ts b/src/plugins/data_view_editor/public/components/form_fields/timestamp_field.test.ts new file mode 100644 index 0000000000000..59663b2a083b4 --- /dev/null +++ b/src/plugins/data_view_editor/public/components/form_fields/timestamp_field.test.ts @@ -0,0 +1,47 @@ +/* + * 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 { requireTimestampOptionValidator } from './timestamp_field'; +import { TimestampOption } from '../../types'; + +const noOptions: TimestampOption[] = []; +const options: TimestampOption[] = [ + { display: 'a', fieldName: 'a' }, + { display: 'a', fieldName: 'b' }, +]; + +describe('timestamp field validator', () => { + test('no timestamp options - pass without value', async () => { + const result = await requireTimestampOptionValidator(noOptions).validator({ + value: undefined, + } as any); + // no error + expect(result).toBeUndefined(); + }); + test('timestamp options - fail without value', async () => { + const result = await requireTimestampOptionValidator(options).validator({ + value: undefined, + } as any); + // returns error + expect(result).toBeDefined(); + }); + test('timestamp options - pass with value', async () => { + const result = await requireTimestampOptionValidator(options).validator({ + value: { label: 'a', value: 'a' }, + } as any); + // no error + expect(result).toBeUndefined(); + }); + test('timestamp options - fail, value not in list', async () => { + const result = await requireTimestampOptionValidator(options).validator({ + value: { label: 'c', value: 'c' }, + } as any); + // returns error + expect(result).toBeDefined(); + }); +}); diff --git a/src/plugins/data_view_editor/public/components/form_fields/timestamp_field.tsx b/src/plugins/data_view_editor/public/components/form_fields/timestamp_field.tsx index 310cb9a3e5835..0fcb948d83c63 100644 --- a/src/plugins/data_view_editor/public/components/form_fields/timestamp_field.tsx +++ b/src/plugins/data_view_editor/public/components/form_fields/timestamp_field.tsx @@ -29,10 +29,13 @@ interface Props { matchedIndices$: Observable; } -const requireTimestampOptionValidator = (options: TimestampOption[]): ValidationConfig => ({ - validator: async ({ value }) => { +export const requireTimestampOptionValidator = ( + options: TimestampOption[] +): ValidationConfig => ({ + validator: async ({ value: selectedOption }) => { const isValueRequired = !!options.length; - if (isValueRequired && !value) { + const valueSelected = options.find((item) => item.fieldName === selectedOption?.value); + if (isValueRequired && (!selectedOption || !valueSelected)) { return { message: i18n.translate( 'indexPatternEditor.requireTimestampOption.ValidationErrorMessage', diff --git a/test/functional/apps/management/_index_pattern_create_delete.ts b/test/functional/apps/management/_data_view_create_delete.ts similarity index 88% rename from test/functional/apps/management/_index_pattern_create_delete.ts rename to test/functional/apps/management/_data_view_create_delete.ts index 7a44cc2f63592..cf33acf414932 100644 --- a/test/functional/apps/management/_index_pattern_create_delete.ts +++ b/test/functional/apps/management/_data_view_create_delete.ts @@ -19,12 +19,13 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { const find = getService('find'); const PageObjects = getPageObjects(['settings', 'common', 'header']); - describe('creating and deleting default index', function describeIndexTests() { + describe('creating and deleting default data view', function describeIndexTests() { before(async function () { await esArchiver.emptyKibanaIndex(); await esArchiver.loadIfNeeded( 'test/functional/fixtures/es_archiver/kibana_sample_data_flights_index_pattern' ); + await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional'); await kibanaServer.uiSettings.replace({}); await PageObjects.settings.navigateTo(); await PageObjects.settings.clickKibanaIndexPatterns(); @@ -34,6 +35,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await esArchiver.unload( 'test/functional/fixtures/es_archiver/kibana_sample_data_flights_index_pattern' ); + + await esArchiver.unload('test/functional/fixtures/es_archiver/logstash_functional'); }); describe('can open and close editor', function () { @@ -59,6 +62,23 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await (await PageObjects.settings.getSaveDataViewButtonActive()).click(); await PageObjects.settings.removeIndexPattern(); }); + + it('correctly validates timestamp after index pattern changes', async function () { + await PageObjects.settings.clickKibanaIndexPatterns(); + await PageObjects.settings.clickAddNewIndexPatternButton(); + // setting the index pattern also sets the time field + await PageObjects.settings.setIndexPatternField('log*'); + // wait for timestamp fields to load + await new Promise((e) => setTimeout(e, 1000)); + // this won't have 'timestamp' field + await PageObjects.settings.setIndexPatternField('kibana*'); + // wait for timestamp fields to load + await new Promise((e) => setTimeout(e, 1000)); + await (await PageObjects.settings.getSaveIndexPatternButton()).click(); + // verify an error is displayed + await find.byClassName('euiFormErrorText'); + await testSubjects.click('closeFlyoutButton'); + }); }); describe('special character handling', () => { diff --git a/test/functional/apps/management/index.ts b/test/functional/apps/management/index.ts index fb5e4145d356b..1119fc118eada 100644 --- a/test/functional/apps/management/index.ts +++ b/test/functional/apps/management/index.ts @@ -22,7 +22,7 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) { }); loadTestFile(require.resolve('./_create_index_pattern_wizard')); - loadTestFile(require.resolve('./_index_pattern_create_delete')); + loadTestFile(require.resolve('./_data_view_create_delete')); loadTestFile(require.resolve('./_index_pattern_results_sort')); loadTestFile(require.resolve('./_index_pattern_popularity')); loadTestFile(require.resolve('./_kibana_settings'));