[Lens] Fixes broken charts with terms agg and no dataview (elastic#16…

…9503) ## Summary In Lens when a dataview is missing, the editing should be allowed. There are cases which it doesn't. The easiest to reproduce is to create a visualization with terms and remove the dataview. Then you will see an error (cannot getFieldByName of undefined) and editing and changing the dataview is not allowed. This PR fixes this and all other occurences. ### Checklist - [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
e40pud · Oct 25, 2023 · 33c409b · 33c409b
1 parent 6f22eef
commit 33c409b
Show file tree

Hide file tree

Showing 26 changed files with 73 additions and 51 deletions.
diff --git a/x-pack/plugins/lens/public/datasources/form_based/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/datasources/form_based/dimension_panel/dimension_editor.tsx
@@ -1087,8 +1087,8 @@ export function DimensionEditor(props: DimensionEditorProps) {
         selectedColumn &&
           operationDefinitionMap[selectedColumn.operationType].getDefaultLabel(
             selectedColumn,
-            props.indexPatterns[state.layers[layerId].indexPatternId],
-            state.layers[layerId].columns
+            state.layers[layerId].columns,
+            props.indexPatterns[state.layers[layerId].indexPatternId]
           )
       ),
     [layerId, selectedColumn, props.indexPatterns, state.layers]
@@ -1263,8 +1263,8 @@ export function DimensionEditor(props: DimensionEditorProps) {
                         customLabel:
                           operationDefinitionMap[selectedColumn.operationType].getDefaultLabel(
                             selectedColumn,
-                            props.indexPatterns[state.layers[layerId].indexPatternId],
-                            state.layers[layerId].columns
+                            state.layers[layerId].columns,
+                            props.indexPatterns[state.layers[layerId].indexPatternId]
                           ) !== value,
                       },
                     },

diff --git a/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx b/x-pack/plugins/lens/public/datasources/form_based/form_based.tsx
@@ -497,8 +497,8 @@ export function getFormBasedDatasource({
               ? column.label
               : operationDefinitionMap[column.operationType].getDefaultLabel(
                   column,
-                  indexPatternsMap[layer.indexPatternId],
-                  layer.columns
+                  layer.columns,
+                  indexPatternsMap[layer.indexPatternId]
                 )
           );
         });

diff --git a/...s/lens/public/datasources/form_based/operations/definitions/calculations/counter_rate.tsx b/...s/lens/public/datasources/form_based/operations/definitions/calculations/counter_rate.tsx
@@ -70,11 +70,11 @@ export const counterRateOperation: OperationDefinition<
       };
     }
   },
-  getDefaultLabel: (column, indexPattern, columns) => {
+  getDefaultLabel: (column, columns, indexPattern) => {
     const ref = columns[column.references[0]];
     return ofName(
       ref && 'sourceField' in ref
-        ? indexPattern.getFieldByName(ref.sourceField)?.displayName
+        ? indexPattern?.getFieldByName(ref.sourceField)?.displayName
         : undefined,
       column.timeScale,
       column.timeShift

diff --git a/...lens/public/datasources/form_based/operations/definitions/calculations/cumulative_sum.tsx b/...lens/public/datasources/form_based/operations/definitions/calculations/cumulative_sum.tsx
@@ -69,11 +69,11 @@ export const cumulativeSumOperation: OperationDefinition<
       };
     }
   },
-  getDefaultLabel: (column, indexPattern, columns) => {
+  getDefaultLabel: (column, columns, indexPattern) => {
     const ref = columns[column.references[0]];
     return ofName(
       ref && 'sourceField' in ref
-        ? indexPattern.getFieldByName(ref.sourceField)?.displayName
+        ? indexPattern?.getFieldByName(ref.sourceField)?.displayName
         : undefined,
       undefined,
       column.timeShift

diff --git a/...ns/lens/public/datasources/form_based/operations/definitions/calculations/differences.tsx b/...ns/lens/public/datasources/form_based/operations/definitions/calculations/differences.tsx
@@ -65,7 +65,7 @@ export const derivativeOperation: OperationDefinition<
       };
     }
   },
-  getDefaultLabel: (column, indexPattern, columns) => {
+  getDefaultLabel: (column, columns, indexPattern) => {
     return ofName(columns[column.references[0]]?.label, column.timeScale, column.timeShift);
   },
   toExpression: (layer, columnId) => {

diff --git a/...lens/public/datasources/form_based/operations/definitions/calculations/moving_average.tsx b/...lens/public/datasources/form_based/operations/definitions/calculations/moving_average.tsx
@@ -76,7 +76,7 @@ export const movingAverageOperation: OperationDefinition<
       };
     }
   },
-  getDefaultLabel: (column, indexPattern, columns) => {
+  getDefaultLabel: (column, columns, indexPattern) => {
     return ofName(columns[column.references[0]]?.label, column.timeScale, column.timeShift);
   },
   toExpression: (layer, columnId) => {

diff --git a/...lens/public/datasources/form_based/operations/definitions/calculations/overall_metric.tsx b/...lens/public/datasources/form_based/operations/definitions/calculations/overall_metric.tsx
@@ -56,11 +56,11 @@ function buildOverallMetricOperation<T extends OverallMetricIndexPatternColumn<s
         scale: 'ratio',
       };
     },
-    getDefaultLabel: (column, indexPattern, columns) => {
+    getDefaultLabel: (column, columns, indexPattern) => {
       const ref = columns[column.references[0]];
       return ofName(
         ref && 'sourceField' in ref
-          ? indexPattern.getFieldByName(ref.sourceField)?.displayName
+          ? indexPattern?.getFieldByName(ref.sourceField)?.displayName
           : undefined
       );
     },

diff --git a/...ins/lens/public/datasources/form_based/operations/definitions/calculations/time_scale.tsx b/...ins/lens/public/datasources/form_based/operations/definitions/calculations/time_scale.tsx
@@ -56,7 +56,7 @@ export const timeScaleOperation: OperationDefinition<TimeScaleIndexPatternColumn
         scale: 'ratio',
       };
     },
-    getDefaultLabel: (column, indexPattern, columns) => {
+    getDefaultLabel: (column, columns, indexPattern) => {
       return 'normalize_by_unit';
     },
     toExpression: (layer, columnId) => {

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/cardinality.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/cardinality.tsx
@@ -113,7 +113,7 @@ export const cardinalityOperation: OperationDefinition<
   filterable: true,
   shiftable: true,
   canReduceTimeRange: true,
-  getDefaultLabel: (column, indexPattern) =>
+  getDefaultLabel: (column, columns, indexPattern) =>
     ofName(
       getSafeName(column.sourceField, indexPattern),
       column.timeShift,

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/count.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/count.tsx
@@ -117,7 +117,7 @@ export const countOperation: OperationDefinition<CountIndexPatternColumn, 'field
       return { dataType: 'number', isBucketed: IS_BUCKETED, scale: SCALE };
     }
   },
-  getDefaultLabel: (column, indexPattern) => {
+  getDefaultLabel: (column, columns, indexPattern) => {
     const field = indexPattern?.getFieldByName(column.sourceField);
     return ofName(field, column.timeShift, column.timeScale, column.reducedTimeRange);
   },

diff --git a/...plugins/lens/public/datasources/form_based/operations/definitions/date_histogram.test.tsx b/...plugins/lens/public/datasources/form_based/operations/definitions/date_histogram.test.tsx
@@ -776,7 +776,6 @@ describe('date_histogram', () => {
             sourceField: 'missing',
             params: { interval: 'auto' },
           },
-          indexPattern1,
           {
             col1: {
               label: '',
@@ -786,7 +785,8 @@ describe('date_histogram', () => {
               sourceField: 'missing',
               params: { interval: 'auto' },
             } as DateHistogramIndexPatternColumn,
-          }
+          },
+          indexPattern1
         )
       ).toEqual('Missing field');
     });

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/date_histogram.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/date_histogram.tsx
@@ -102,7 +102,7 @@ export const dateHistogramOperation: OperationDefinition<
       };
     }
   },
-  getDefaultLabel: (column, indexPattern) => getSafeName(column.sourceField, indexPattern),
+  getDefaultLabel: (column, columns, indexPattern) => getSafeName(column.sourceField, indexPattern),
   buildColumn({ field }, columnParams) {
     return {
       label: field.displayName,

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/formula.tsx
@@ -52,7 +52,7 @@ export const formulaOperation: OperationDefinition<FormulaIndexPatternColumn, 'm
   {
     type: 'formula',
     displayName: defaultLabel,
-    getDefaultLabel: (column, indexPattern) => column.params.formula ?? defaultLabel,
+    getDefaultLabel: (column) => column.params.formula ?? defaultLabel,
     input: 'managedReference',
     hidden: true,
     filterable: {

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/math.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/formula/math.tsx
@@ -23,7 +23,7 @@ export const mathOperation: OperationDefinition<MathIndexPatternColumn, 'managed
   type: 'math',
   displayName: 'Math',
   hidden: true,
-  getDefaultLabel: (column, indexPattern) => 'Math',
+  getDefaultLabel: (column, columns, indexPattern) => 'Math',
   input: 'managedReference',
   getDisabledStatus(indexPattern: IndexPattern) {
     return undefined;

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/index.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/index.ts
@@ -267,8 +267,8 @@ interface BaseOperationDefinitionProps<
    */
   getDefaultLabel: (
     column: C,
-    indexPattern: IndexPattern,
-    columns: Record<string, GenericIndexPatternColumn>
+    columns: Record<string, GenericIndexPatternColumn>,
+    indexPattern?: IndexPattern
   ) => string;
   /**
    * This function is called if another column in the same layer changed or got added/removed.

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/last_value.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/last_value.tsx
@@ -165,7 +165,7 @@ export const lastValueOperation: OperationDefinition<
   displayName: i18n.translate('xpack.lens.indexPattern.lastValue', {
     defaultMessage: 'Last value',
   }),
-  getDefaultLabel: (column, indexPattern) =>
+  getDefaultLabel: (column, columns, indexPattern) =>
     ofName(
       getSafeName(column.sourceField, indexPattern),
       column.timeShift,

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/metrics.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/metrics.tsx
@@ -129,7 +129,7 @@ function buildMetricOperation<T extends MetricColumn<string>>({
           (!newField.aggregationRestrictions || newField.aggregationRestrictions![type])
       );
     },
-    getDefaultLabel: (column, indexPattern, columns) =>
+    getDefaultLabel: (column, columns, indexPattern) =>
       labelLookup(getSafeName(column.sourceField, indexPattern), column),
     buildColumn: ({ field, previousColumn }, columnParams) => {
       return {

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/percentile.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/percentile.tsx
@@ -151,7 +151,7 @@ export const percentileOperation: OperationDefinition<
         (!newField.aggregationRestrictions || !newField.aggregationRestrictions.percentiles)
     );
   },
-  getDefaultLabel: (column, indexPattern, columns) =>
+  getDefaultLabel: (column, columns, indexPattern) =>
     ofName(
       getSafeName(column.sourceField, indexPattern),
       column.params.percentile,

diff --git a/...ck/plugins/lens/public/datasources/form_based/operations/definitions/percentile_ranks.tsx b/...ck/plugins/lens/public/datasources/form_based/operations/definitions/percentile_ranks.tsx
@@ -109,7 +109,7 @@ export const percentileRanksOperation: OperationDefinition<
         (!newField.aggregationRestrictions || !newField.aggregationRestrictions.percentile_ranks)
     );
   },
-  getDefaultLabel: (column, indexPattern, columns) =>
+  getDefaultLabel: (column, columns, indexPattern) =>
     ofName(
       getSafeName(column.sourceField, indexPattern),
       column.params.value,

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/ranges/ranges.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/ranges/ranges.tsx
@@ -97,8 +97,8 @@ export const rangeOperation: OperationDefinition<
       };
     }
   },
-  getDefaultLabel: (column, indexPattern) =>
-    indexPattern.getFieldByName(column.sourceField)?.displayName ??
+  getDefaultLabel: (column, columns, indexPattern) =>
+    indexPattern?.getFieldByName(column.sourceField)?.displayName ??
     i18n.translate('xpack.lens.indexPattern.missingFieldLabel', {
       defaultMessage: 'Missing field',
     }),

diff --git a/...k/plugins/lens/public/datasources/form_based/operations/definitions/static_value.test.tsx b/...k/plugins/lens/public/datasources/form_based/operations/definitions/static_value.test.tsx
@@ -123,8 +123,8 @@ describe('static_value', () => {
               value: '23',
             },
           },
-          createMockedIndexPattern(),
-          layer.columns
+          layer.columns,
+          createMockedIndexPattern()
         )
       ).toBe('Static value: 23');
     });
@@ -142,8 +142,8 @@ describe('static_value', () => {
               value: '',
             },
           },
-          createMockedIndexPattern(),
-          layer.columns
+          layer.columns,
+          createMockedIndexPattern()
         )
       ).toBe('Static value');
     });

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/index.tsx b/x-pack/plugins/lens/public/datasources/form_based/operations/definitions/terms/index.tsx
@@ -386,9 +386,9 @@ export const termsOperation: OperationDefinition<
       }),
     }).toAst();
   },
-  getDefaultLabel: (column, indexPattern) =>
+  getDefaultLabel: (column, columns, indexPattern) =>
     ofName(
-      indexPattern.getFieldByName(column.sourceField)?.displayName,
+      indexPattern?.getFieldByName(column.sourceField)?.displayName,
       column.params.secondaryFields?.length,
       column.params.orderBy.type === 'rare',
       column.params.orderBy.type === 'significant',

diff --git a/...ck/plugins/lens/public/datasources/form_based/operations/definitions/terms/terms.test.tsx b/...ck/plugins/lens/public/datasources/form_based/operations/definitions/terms/terms.test.tsx
@@ -1177,12 +1177,34 @@ describe('terms', () => {
             },
             sourceField: 'source',
           } as TermsIndexPatternColumn,
-          createMockedIndexPattern(),
-          {}
+          {},
+          createMockedIndexPattern()
         )
       ).toBe('Top 3 values of source');
     });
 
+    it('should not fail if dataview is not given', () => {
+      expect(
+        termsOperation.getDefaultLabel(
+          {
+            dataType: 'string',
+            isBucketed: true,
+
+            // Private
+            operationType: 'terms',
+            params: {
+              orderBy: { type: 'alphabetical', fallback: true },
+              size: 3,
+              orderDirection: 'asc',
+            },
+            sourceField: 'source',
+          } as TermsIndexPatternColumn,
+          {},
+          undefined
+        )
+      ).toBe('Top 3 values of Missing field');
+    });
+
     it('should return main value with single counter for two fields', () => {
       expect(
         termsOperation.getDefaultLabel(
@@ -1200,8 +1222,8 @@ describe('terms', () => {
             },
             sourceField: 'source',
           } as TermsIndexPatternColumn,
-          createMockedIndexPattern(),
-          {}
+          {},
+          createMockedIndexPattern()
         )
       ).toBe('Top values of source + 1 other');
     });
@@ -1223,8 +1245,8 @@ describe('terms', () => {
             },
             sourceField: 'source',
           } as TermsIndexPatternColumn,
-          createMockedIndexPattern(),
-          {}
+          {},
+          createMockedIndexPattern()
         )
       ).toBe('Top values of source + 2 others');
     });

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/layer_helpers.test.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/layer_helpers.test.ts
@@ -2577,7 +2577,6 @@ describe('state_helpers', () => {
           operationType: 'testReference',
           references: ['col1'],
         },
-        indexPattern,
         {
           col2: {
             label: 'Default label',
@@ -2586,7 +2585,8 @@ describe('state_helpers', () => {
             operationType: 'testReference',
             references: ['col1'],
           },
-        }
+        },
+        indexPattern
       );
     });
 

diff --git a/x-pack/plugins/lens/public/datasources/form_based/operations/layer_helpers.ts b/x-pack/plugins/lens/public/datasources/form_based/operations/layer_helpers.ts
@@ -636,7 +636,7 @@ export function replaceColumn({
         previousColumn.customLabel &&
         hypotheticalLayer.columns[columnId] &&
         previousColumn.label !==
-          previousDefinition.getDefaultLabel(previousColumn, indexPattern, tempLayer.columns)
+          previousDefinition.getDefaultLabel(previousColumn, tempLayer.columns, indexPattern)
       ) {
         hypotheticalLayer.columns[columnId].customLabel = true;
         hypotheticalLayer.columns[columnId].label = previousColumn.label;
@@ -1723,8 +1723,8 @@ export function updateDefaultLabels(
         ...col,
         label: operationDefinitionMap[col.operationType].getDefaultLabel(
           col,
-          indexPattern,
-          copiedColumns
+          copiedColumns,
+          indexPattern
         ),
       };
     }

diff --git a/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts b/x-pack/plugins/lens/public/datasources/form_based/to_expression.ts
@@ -242,8 +242,8 @@ function getExpressionForLayer(
               ? col.label
               : operationDefinitionMap[col.operationType].getDefaultLabel(
                   col,
-                  indexPattern,
-                  layer.columns
+                  layer.columns,
+                  indexPattern
                 ),
           },
         ];
@@ -398,8 +398,8 @@ function getExpressionForLayer(
                 ? col.label
                 : operationDefinitionMap[col.operationType].getDefaultLabel(
                     col,
-                    indexPattern,
-                    layer.columns
+                    layer.columns,
+                    indexPattern
                   ),
             ],
             targetUnit: [col.timeScale!],