mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 17:28:26 -04:00
[Lens] Do not crash when editing a Lens chart with a by reference annotation layer (#213090)
## Summary Fixes #212917 The root problem is belongs into the annotation layer logic to produce the reference id for the persisted saved object. In the previous logic a new `uuid` was generated all the time leading to a continuous flow of `setState` calls to update the "runtime" state of the Lens object when inline editing: the fix was to produce a stable id in the `extractReferences` logic to avoid the re-renders. The logic has been tweaked a bit now with some extra explanations inline to make it more understandable. New tests have been added to smoke test this scenario. ### Checklist Check the PR satisfies following conditions. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Nick Partridge <nick.ryan.partridge@gmail.com>
This commit is contained in:
parent
2a32ed4755
commit
48926e5173
2 changed files with 107 additions and 50 deletions
|
@ -5,7 +5,6 @@
|
|||
* 2.0.
|
||||
*/
|
||||
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
import type { SavedObjectReference } from '@kbn/core/public';
|
||||
import { EVENT_ANNOTATION_GROUP_TYPE } from '@kbn/event-annotation-common';
|
||||
import { cloneDeep } from 'lodash';
|
||||
|
@ -100,58 +99,68 @@ export function convertToPersistable(state: XYState) {
|
|||
const persistableLayers: XYPersistedLayerConfig[] = [];
|
||||
|
||||
persistableState.layers.forEach((layer) => {
|
||||
// anything but an annotation can just be persisted as is
|
||||
if (!isAnnotationsLayer(layer)) {
|
||||
persistableLayers.push(layer);
|
||||
} else {
|
||||
if (isByReferenceAnnotationsLayer(layer)) {
|
||||
const referenceName = `ref-${uuidv4()}`;
|
||||
savedObjectReferences.push({
|
||||
type: EVENT_ANNOTATION_GROUP_TYPE,
|
||||
id: layer.annotationGroupId,
|
||||
name: referenceName,
|
||||
});
|
||||
|
||||
if (!annotationLayerHasUnsavedChanges(layer)) {
|
||||
const persistableLayer: XYPersistedByReferenceAnnotationLayerConfig = {
|
||||
persistanceType: 'byReference',
|
||||
layerId: layer.layerId,
|
||||
layerType: layer.layerType,
|
||||
annotationGroupRef: referenceName,
|
||||
};
|
||||
|
||||
persistableLayers.push(persistableLayer);
|
||||
} else {
|
||||
const persistableLayer: XYPersistedLinkedByValueAnnotationLayerConfig = {
|
||||
persistanceType: 'linked',
|
||||
cachedMetadata: layer.cachedMetadata || {
|
||||
title: layer.__lastSaved.title,
|
||||
description: layer.__lastSaved.description,
|
||||
tags: layer.__lastSaved.tags,
|
||||
},
|
||||
layerId: layer.layerId,
|
||||
layerType: layer.layerType,
|
||||
annotationGroupRef: referenceName,
|
||||
annotations: layer.annotations,
|
||||
ignoreGlobalFilters: layer.ignoreGlobalFilters,
|
||||
};
|
||||
persistableLayers.push(persistableLayer);
|
||||
|
||||
savedObjectReferences.push({
|
||||
type: 'index-pattern',
|
||||
id: layer.indexPatternId,
|
||||
name: getLayerReferenceName(layer.layerId),
|
||||
});
|
||||
}
|
||||
} else {
|
||||
const { indexPatternId, ...persistableLayer } = layer;
|
||||
savedObjectReferences.push({
|
||||
type: 'index-pattern',
|
||||
id: indexPatternId,
|
||||
name: getLayerReferenceName(layer.layerId),
|
||||
});
|
||||
persistableLayers.push({ ...persistableLayer, persistanceType: 'byValue' });
|
||||
}
|
||||
return;
|
||||
}
|
||||
// a by value annotation layer can be persisted with some config tweak
|
||||
if (!isByReferenceAnnotationsLayer(layer)) {
|
||||
const { indexPatternId, ...persistableLayer } = layer;
|
||||
savedObjectReferences.push({
|
||||
type: 'index-pattern',
|
||||
id: indexPatternId,
|
||||
name: getLayerReferenceName(layer.layerId),
|
||||
});
|
||||
persistableLayers.push({ ...persistableLayer, persistanceType: 'byValue' });
|
||||
return;
|
||||
}
|
||||
/**
|
||||
* by reference annotation layer needs to be handled carefully
|
||||
**/
|
||||
|
||||
// make this id stable so that it won't retrigger all the time a change diff
|
||||
const referenceName = `ref-${layer.layerId}`;
|
||||
savedObjectReferences.push({
|
||||
type: EVENT_ANNOTATION_GROUP_TYPE,
|
||||
id: layer.annotationGroupId,
|
||||
name: referenceName,
|
||||
});
|
||||
|
||||
// if there's no divergence from the library, it can be persisted without much ado
|
||||
if (!annotationLayerHasUnsavedChanges(layer)) {
|
||||
const persistableLayer: XYPersistedByReferenceAnnotationLayerConfig = {
|
||||
persistanceType: 'byReference',
|
||||
layerId: layer.layerId,
|
||||
layerType: layer.layerType,
|
||||
annotationGroupRef: referenceName,
|
||||
};
|
||||
|
||||
persistableLayers.push(persistableLayer);
|
||||
return;
|
||||
}
|
||||
// this is the case where the by reference diverged from library
|
||||
// so it needs to persist some extra metadata
|
||||
const persistableLayer: XYPersistedLinkedByValueAnnotationLayerConfig = {
|
||||
persistanceType: 'linked',
|
||||
cachedMetadata: layer.cachedMetadata || {
|
||||
title: layer.__lastSaved.title,
|
||||
description: layer.__lastSaved.description,
|
||||
tags: layer.__lastSaved.tags,
|
||||
},
|
||||
layerId: layer.layerId,
|
||||
layerType: layer.layerType,
|
||||
annotationGroupRef: referenceName,
|
||||
annotations: layer.annotations,
|
||||
ignoreGlobalFilters: layer.ignoreGlobalFilters,
|
||||
};
|
||||
persistableLayers.push(persistableLayer);
|
||||
|
||||
savedObjectReferences.push({
|
||||
type: 'index-pattern',
|
||||
id: layer.indexPatternId,
|
||||
name: getLayerReferenceName(layer.layerId),
|
||||
});
|
||||
});
|
||||
return { savedObjectReferences, state: { ...persistableState, layers: persistableLayers } };
|
||||
}
|
||||
|
|
|
@ -19,6 +19,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
|
|||
const dashboardPanelActions = getService('dashboardPanelActions');
|
||||
const testSubjects = getService('testSubjects');
|
||||
const elasticChart = getService('elasticChart');
|
||||
const toastsService = getService('toasts');
|
||||
|
||||
const createNewLens = async () => {
|
||||
await visualize.navigateToNewVisualization();
|
||||
|
@ -229,6 +230,53 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
|
|||
await timeToVisualize.resetNewDashboard();
|
||||
});
|
||||
|
||||
it('should allow adding a by reference annotation', async () => {
|
||||
const ANNOTATION_GROUP_TITLE = 'My by reference annotation group';
|
||||
await loadExistingLens();
|
||||
await lens.save('xyVisChart Copy 2', true, false, false, 'new');
|
||||
|
||||
await dashboard.waitForRenderComplete();
|
||||
await elasticChart.setNewChartUiDebugFlag(true);
|
||||
|
||||
await dashboardPanelActions.clickInlineEdit();
|
||||
|
||||
log.debug('Adds by reference annotation');
|
||||
|
||||
await lens.createLayer('annotations');
|
||||
|
||||
await lens.performLayerAction('lnsXY_annotationLayer_saveToLibrary', 1);
|
||||
|
||||
await visualize.setSaveModalValues(ANNOTATION_GROUP_TITLE, {
|
||||
description: 'my description',
|
||||
});
|
||||
|
||||
await testSubjects.click('confirmSaveSavedObjectButton');
|
||||
|
||||
const toastContents = await toastsService.getContentByIndex(1);
|
||||
|
||||
expect(toastContents).to.be(
|
||||
`Saved "${ANNOTATION_GROUP_TITLE}"\nView or manage in the annotation library.`
|
||||
);
|
||||
|
||||
// now close
|
||||
await testSubjects.click('applyFlyoutButton');
|
||||
|
||||
log.debug('Edit the by reference annotation');
|
||||
// and try to edit again the by reference annotation layer event
|
||||
await dashboardPanelActions.clickInlineEdit();
|
||||
|
||||
expect((await find.allByCssSelector(`[data-test-subj^="lns-layerPanel-"]`)).length).to.eql(2);
|
||||
expect(
|
||||
await (
|
||||
await testSubjects.find('lnsXY_xAnnotationsPanel > lns-dimensionTrigger')
|
||||
).getVisibleText()
|
||||
).to.eql('Event');
|
||||
|
||||
await dashboard.waitForRenderComplete();
|
||||
|
||||
await timeToVisualize.resetNewDashboard();
|
||||
});
|
||||
|
||||
it('should allow adding a reference line', async () => {
|
||||
await loadExistingLens();
|
||||
await lens.save('xyVisChart Copy', true, false, false, 'new');
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue