Skip to content

Commit

Permalink
[data view editor] Fix data view timestamp validation (elastic#150398)
Browse files Browse the repository at this point in the history
## 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: elastic#150219
  • Loading branch information
mattkime authored Jun 30, 2023
1 parent ef661d1 commit 646539c
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}}
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@ interface Props {
matchedIndices$: Observable<MatchedIndicesSet>;
}

const requireTimestampOptionValidator = (options: TimestampOption[]): ValidationConfig => ({
validator: async ({ value }) => {
export const requireTimestampOptionValidator = (
options: TimestampOption[]
): ValidationConfig<any, string, { value?: any }> => ({
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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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 () {
Expand All @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion test/functional/apps/management/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down

0 comments on commit 646539c

Please sign in to comment.