[9.0] [Lens] Do not crash when editing a Lens chart with a by reference annotation layer (#213090) (#215654)

# Backport This will backport the following commits from `main` to `9.0`: - [[Lens] Do not crash when editing a Lens chart with a by reference annotation layer (#213090)](https://github.com/elastic/kibana/pull/213090)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport)
2025-04-24 09:48:58 -04:00 · 2025-03-25 16:26:54 +01:00 · 2025-03-25 16:26:54 +01:00 · e163ff3c49
commit e163ff3c49
parent 673c43bf3e
2 changed files with 107 additions and 50 deletions
--- a/x-pack/platform/plugins/shared/lens/public/visualizations/xy/persistence.ts
+++ b/x-pack/platform/plugins/shared/lens/public/visualizations/xy/persistence.ts
@ -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 } };
 }
--- a/x-pack/test/functional/apps/lens/group3/dashboard_inline_editing.ts
+++ b/x-pack/test/functional/apps/lens/group3/dashboard_inline_editing.ts
@ -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');