Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Visualize] Allows editing broken visualizations caused by runtime fields changes #94798

Merged
merged 22 commits into from
Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/plugins/data/common/search/aggs/agg_configs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ describe('AggConfigs', () => {
describe('#toDsl', () => {
beforeEach(() => {
indexPattern = stubIndexPattern as IndexPattern;
indexPattern.fields.getByName = (name) => (name as unknown) as IndexPatternField;
indexPattern.fields.getByName = (name) => (({ name } as unknown) as IndexPatternField);
});

it('uses the sorted aggs', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,33 @@ import { AggConfigs, CreateAggConfigParams } from '../agg_configs';
import { BUCKET_TYPES } from './bucket_agg_types';
import { IBucketAggConfig } from './bucket_agg_type';
import { mockAggTypesRegistry } from '../test_helpers';
import type { IndexPatternField } from '../../../index_patterns';
import { IndexPattern } from '../../../index_patterns/index_patterns/index_pattern';

const indexPattern = {
id: '1234',
title: 'logstash-*',
fields: [
{
name: 'field',
name: 'machine.os.raw',
type: 'string',
esTypes: ['string'],
aggregatable: true,
filterable: true,
searchable: true,
},
{
name: 'geo.src',
type: 'string',
esTypes: ['string'],
aggregatable: true,
filterable: true,
searchable: true,
},
],
} as any;
} as IndexPattern;

indexPattern.fields.getByName = (name) => (({ name } as unknown) as IndexPatternField);

const singleTerm = {
aggs: [
Expand Down
78 changes: 60 additions & 18 deletions src/plugins/data/common/search/aggs/buckets/terms.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,53 @@ import { AggConfigs } from '../agg_configs';
import { METRIC_TYPES } from '../metrics';
import { mockAggTypesRegistry } from '../test_helpers';
import { BUCKET_TYPES } from './bucket_agg_types';
import type { IndexPatternField } from '../../../index_patterns';
import { IndexPattern } from '../../../index_patterns/index_patterns/index_pattern';

describe('Terms Agg', () => {
describe('order agg editor UI', () => {
const getAggConfigs = (params: Record<string, any> = {}) => {
const indexPattern = {
id: '1234',
title: 'logstash-*',
fields: {
getByName: () => field,
filter: () => [field],
},
} as any;
fields: [
{
name: 'field',
type: 'string',
esTypes: ['string'],
aggregatable: true,
filterable: true,
searchable: true,
},
{
name: 'string_field',
type: 'string',
esTypes: ['string'],
aggregatable: true,
filterable: true,
searchable: true,
},
{
name: 'empty_number_field',
type: 'number',
esTypes: ['number'],
aggregatable: true,
filterable: true,
searchable: true,
},
{
name: 'number_field',
type: 'number',
esTypes: ['number'],
aggregatable: true,
filterable: true,
searchable: true,
},
],
} as IndexPattern;

const field = {
name: 'field',
indexPattern,
};
indexPattern.fields.getByName = (name) => (({ name } as unknown) as IndexPatternField);
indexPattern.fields.filter = () => indexPattern.fields;

return new AggConfigs(
indexPattern,
Expand Down Expand Up @@ -207,16 +237,28 @@ describe('Terms Agg', () => {
const indexPattern = {
id: '1234',
title: 'logstash-*',
fields: {
getByName: () => field,
filter: () => [field],
},
} as any;
fields: [
{
name: 'string_field',
type: 'string',
esTypes: ['string'],
aggregatable: true,
filterable: true,
searchable: true,
},
{
name: 'number_field',
type: 'number',
esTypes: ['number'],
aggregatable: true,
filterable: true,
searchable: true,
},
],
} as IndexPattern;

const field = {
name: 'field',
indexPattern,
};
indexPattern.fields.getByName = (name) => (({ name } as unknown) as IndexPatternField);
indexPattern.fields.filter = () => indexPattern.fields;

const aggConfigs = new AggConfigs(
indexPattern,
Expand Down
74 changes: 50 additions & 24 deletions src/plugins/data/common/search/aggs/param_types/field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,52 @@ export class FieldParamType extends BaseParamType {
);
}

if (field.scripted) {
if (field.type === 'missing') {
throw new SavedObjectNotFound(
'index-pattern-field',
field.name,
undefined,
i18n.translate(
'data.search.aggs.paramTypes.field.notFoundSavedFieldParameterErrorMessage',
{
defaultMessage:
'The field "{fieldParameter}" associated with this object no longer exists in the index pattern. Please use another field.',
values: {
fieldParameter: field.name,
},
}
)
);
}

const validField = this.getAvailableFields(aggConfig).find(
(f: any) => f.name === field.name
);

if (!validField) {
throw new Error(
i18n.translate(
'data.search.aggs.paramTypes.field.invalidSavedFieldParameterErrorMessage',
{
defaultMessage:
'Saved field "{fieldParameter}" of index pattern "{indexPatternTitle}" is invalid for use with the "{aggType}" aggregation. Please select a new field.',
values: {
fieldParameter: field.name,
aggType: aggConfig?.type?.title,
indexPatternTitle: aggConfig.getIndexPattern().title,
},
}
)
);
}

if (validField.scripted) {
output.params.script = {
source: field.script,
lang: field.lang,
source: validField.script,
lang: validField.lang,
};
} else {
output.params.field = field.name;
output.params.field = validField.name;
}
};
}
Expand All @@ -69,28 +108,15 @@ export class FieldParamType extends BaseParamType {
const field = aggConfig.getIndexPattern().fields.getByName(fieldName);

if (!field) {
throw new SavedObjectNotFound('index-pattern-field', fieldName);
}

const validField = this.getAvailableFields(aggConfig).find((f: any) => f.name === fieldName);
if (!validField) {
throw new Error(
i18n.translate(
'data.search.aggs.paramTypes.field.invalidSavedFieldParameterErrorMessage',
{
defaultMessage:
'Saved field "{fieldParameter}" of index pattern "{indexPatternTitle}" is invalid for use with the "{aggType}" aggregation. Please select a new field.',
values: {
fieldParameter: fieldName,
aggType: aggConfig?.type?.title,
indexPatternTitle: aggConfig.getIndexPattern().title,
},
}
)
);
return new IndexPatternField({
type: 'missing',
name: fieldName,
searchable: false,
aggregatable: false,
});
}

return validField;
return field;
};
}

Expand Down
4 changes: 2 additions & 2 deletions src/plugins/kibana_utils/common/errors/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,15 @@ export class DuplicateField extends KbnError {
export class SavedObjectNotFound extends KbnError {
public savedObjectType: string;
public savedObjectId?: string;
constructor(type: string, id?: string, link?: string) {
constructor(type: string, id?: string, link?: string, customMessage?: string) {
const idMsg = id ? ` (id: ${id})` : '';
let message = `Could not locate that ${type}${idMsg}`;

if (link) {
message += `, [click here to re-create it](${link})`;
}

super(message);
super(customMessage || message);

this.savedObjectType = type;
this.savedObjectId = id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { act } from 'react-dom/test-utils';
import { mount, shallow, ReactWrapper } from 'enzyme';
import { EuiComboBoxProps, EuiComboBox } from '@elastic/eui';

import { IAggConfig, IndexPatternField } from 'src/plugins/data/public';
import { IAggConfig, IndexPatternField, AggParam } from 'src/plugins/data/public';
import { ComboBoxGroupedOptions } from '../../utils';
import { FieldParamEditor, FieldParamEditorProps } from './field';
import { EditorVisState } from '../sidebar/state/reducers';
Expand Down Expand Up @@ -42,7 +42,7 @@ describe('FieldParamEditor component', () => {
setTouched = jest.fn();
onChange = jest.fn();

field = { displayName: 'bytes' } as IndexPatternField;
field = { displayName: 'bytes', type: 'bytes' } as IndexPatternField;
option = { label: 'bytes', target: field };
indexedFields = [
{
Expand All @@ -52,7 +52,16 @@ describe('FieldParamEditor component', () => {
];

defaultProps = {
agg: {} as IAggConfig,
agg: {
type: {
params: [
({
name: 'field',
filterFieldTypes: ['bytes'],
} as unknown) as AggParam,
],
},
} as IAggConfig,
aggParam: {
name: 'field',
type: 'field',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,32 @@ function FieldParamEditor({
);
}

if (value && value.type === 'missing') {
VladLasitsa marked this conversation as resolved.
Show resolved Hide resolved
errors.push(
i18n.translate('visDefaultEditor.controls.field.fieldIsNotExists', {
defaultMessage:
'The field "{fieldParameter}" associated with this object no longer exists in the index pattern. Please use another field.',
values: {
fieldParameter: value.name,
},
})
);
} else if (
value &&
!getFieldTypes(agg).find((type: string) => type === value.type || type === '*')
) {
errors.push(
i18n.translate('visDefaultEditor.controls.field.invalidFieldForAggregation', {
defaultMessage:
'Saved field "{fieldParameter}" of index pattern "{indexPatternTitle}" is invalid for use with this aggregation. Please select a new field.',
values: {
fieldParameter: value?.name,
indexPatternTitle: agg.getIndexPattern && agg.getIndexPattern().title,
},
})
);
}

const isValid = !!value && !errors.length && !isDirty;
// we show an error message right away if there is no compatible fields
const showErrorMessage = (showValidation || !indexedFields.length) && !isValid;
Expand Down Expand Up @@ -122,10 +148,14 @@ function FieldParamEditor({
}

function getFieldTypesString(agg: IAggConfig) {
return formatListAsProse(getFieldTypes(agg), { inclusive: false });
}

function getFieldTypes(agg: IAggConfig) {
const param =
get(agg, 'type.params', []).find((p: AggParam) => p.name === 'field') ||
({} as IFieldParamType);
return formatListAsProse(parseCommaSeparatedList(param.filterFieldTypes), { inclusive: false });
return parseCommaSeparatedList(param.filterFieldTypes || []);
}

export { FieldParamEditor };
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,12 @@ import { EventEmitter } from 'events';
import { parse } from 'query-string';
import { i18n } from '@kbn/i18n';

import { redirectWhenMissing } from '../../../../../kibana_utils/public';

import { getVisualizationInstance } from '../get_visualization_instance';
import { getEditBreadcrumbs, getCreateBreadcrumbs } from '../breadcrumbs';
import { SavedVisInstance, VisualizeServices, IEditorController } from '../../types';
import { VisualizeConstants } from '../../visualize_constants';
import { getVisEditorsRegistry } from '../../../services';
import { redirectToSavedObjectPage } from '../utils';

/**
* This effect is responsible for instantiating a saved vis or creating a new one
Expand All @@ -43,9 +42,7 @@ export const useSavedVisInstance = (
chrome,
history,
dashboard,
setActiveUrl,
toastNotifications,
http: { basePath },
stateTransferService,
application: { navigateToApp },
} = services;
Expand Down Expand Up @@ -131,27 +128,8 @@ export const useSavedVisInstance = (
visEditorController,
});
} catch (error) {
const managementRedirectTarget = {
app: 'management',
path: `kibana/objects/savedVisualizations/${visualizationIdFromUrl}`,
};

try {
redirectWhenMissing({
history,
navigateToApp,
toastNotifications,
basePath,
mapping: {
visualization: VisualizeConstants.LANDING_PAGE_PATH,
search: managementRedirectTarget,
'index-pattern': managementRedirectTarget,
'index-pattern-field': managementRedirectTarget,
},
onBeforeRedirect() {
setActiveUrl(VisualizeConstants.LANDING_PAGE_PATH);
},
})(error);
redirectToSavedObjectPage(services, error, visualizationIdFromUrl);
} catch (e) {
toastNotifications.addWarning({
title: i18n.translate('visualize.createVisualization.failedToLoadErrorMessage', {
Expand Down
Loading