[Lens] Do not persist time zone (#102735) (#103620)

Co-authored-by: Joe Reuter <johannes.reuter@elastic.co>

x-pack/plugins/lens/public/indexpattern_datasource/loader.test.ts

@@ -770,7 +770,7 @@ describe('loader', () => {
                 label: 'My hist',
                 operationType: 'date_histogram',
                 params: {
-                  interval: '1d',
+                  interval: 'm',
                 },
                 sourceField: 'timestamp',
               },

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.test.tsx

@@ -176,30 +176,6 @@ describe('date_histogram', () => {
       });
       expect(column.params.interval).toEqual('auto');
     });
-
-    it('should create column object with restrictions', () => {
-      const column = dateHistogramOperation.buildColumn({
-        layer: { columns: {}, columnOrder: [], indexPatternId: '' },
-        indexPattern: createMockedIndexPattern(),
-        field: {
-          name: 'timestamp',
-          displayName: 'timestampLabel',
-          type: 'date',
-          esTypes: ['date'],
-          aggregatable: true,
-          searchable: true,
-          aggregationRestrictions: {
-            date_histogram: {
-              agg: 'date_histogram',
-              time_zone: 'UTC',
-              calendar_interval: '1y',
-            },
-          },
-        },
-      });
-      expect(column.params.interval).toEqual('1y');
-      expect(column.params.timeZone).toEqual('UTC');
-    });
   });
 
   describe('toEsAggsFn', () => {
@@ -223,7 +199,7 @@ describe('date_histogram', () => {
       );
     });
 
-    it('should not use normalized es interval for rollups', () => {
+    it('should use restricted time zone and omit use normalized es interval for rollups', () => {
       const esAggsFn = dateHistogramOperation.toEsAggsFn(
         layer.columns.col1 as DateHistogramIndexPatternColumn,
         'col1',
@@ -271,6 +247,7 @@ describe('date_histogram', () => {
           arguments: expect.objectContaining({
             interval: ['42w'],
             field: ['timestamp'],
+            time_zone: ['UTC'],
             useNormalizedEsInterval: [false],
           }),
         })
@@ -320,114 +297,6 @@ describe('date_histogram', () => {
     });
   });
 
-  describe('transfer', () => {
-    it('should adjust interval and time zone params if that is necessary due to restrictions', () => {
-      const transferedColumn = dateHistogramOperation.transfer!(
-        {
-          dataType: 'date',
-          isBucketed: true,
-          label: '',
-          operationType: 'date_histogram',
-          sourceField: 'dateField',
-          params: {
-            interval: 'd',
-          },
-        },
-        {
-          title: '',
-          id: '',
-          hasRestrictions: true,
-          fields: [
-            {
-              name: 'dateField',
-              displayName: 'dateField',
-              type: 'date',
-              aggregatable: true,
-              searchable: true,
-              aggregationRestrictions: {
-                date_histogram: {
-                  agg: 'date_histogram',
-                  time_zone: 'CET',
-                  calendar_interval: 'w',
-                },
-              },
-            },
-          ],
-          getFieldByName: getFieldByNameFactory([
-            {
-              name: 'dateField',
-              displayName: 'dateField',
-              type: 'date',
-              aggregatable: true,
-              searchable: true,
-              aggregationRestrictions: {
-                date_histogram: {
-                  agg: 'date_histogram',
-                  time_zone: 'CET',
-                  calendar_interval: 'w',
-                },
-              },
-            },
-          ]),
-        }
-      );
-      expect(transferedColumn).toEqual(
-        expect.objectContaining({
-          params: {
-            interval: 'w',
-            timeZone: 'CET',
-          },
-        })
-      );
-    });
-
-    it('should retain interval', () => {
-      const transferedColumn = dateHistogramOperation.transfer!(
-        {
-          dataType: 'date',
-          isBucketed: true,
-          label: '',
-          operationType: 'date_histogram',
-          sourceField: 'dateField',
-          params: {
-            interval: '20s',
-          },
-        },
-        {
-          title: '',
-          id: '',
-          hasRestrictions: false,
-          fields: [
-            {
-              name: 'dateField',
-              displayName: 'dateField',
-              type: 'date',
-              aggregatable: true,
-              searchable: true,
-            },
-          ],
-          getFieldByName: getFieldByNameFactory([
-            {
-              name: 'dateField',
-              displayName: 'dateField',
-              type: 'date',
-              aggregatable: true,
-              searchable: true,
-            },
-          ]),
-        }
-      );
-      expect(transferedColumn).toEqual(
-        expect.objectContaining({
-          params: {
-            interval: '20s',
-            timeZone: undefined,
-          },
-        })
-      );
-    });
-  });
-
   describe('param editor', () => {
     it('should render current value', () => {
       const updateLayerSpy = jest.fn();

x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx

@@ -45,7 +45,6 @@ export interface DateHistogramIndexPatternColumn extends FieldBasedIndexPatternC
   operationType: 'date_histogram';
   params: {
     interval: string;
-    timeZone?: string;
   };
 }
 
@@ -106,12 +105,6 @@ export const dateHistogramOperation: OperationDefinition<
   },
   getDefaultLabel: (column, indexPattern) => getSafeName(column.sourceField, indexPattern),
   buildColumn({ field }, columnParams) {
-    let interval = columnParams?.interval ?? autoInterval;
-    let timeZone: string | undefined;
-    if (field.aggregationRestrictions && field.aggregationRestrictions.date_histogram) {
-      interval = restrictedInterval(field.aggregationRestrictions) as string;
-      timeZone = field.aggregationRestrictions.date_histogram.time_zone;
-    }
     return {
       label: field.displayName,
       dataType: 'date',
@@ -120,8 +113,7 @@ export const dateHistogramOperation: OperationDefinition<
       isBucketed: true,
       scale: 'interval',
       params: {
-        interval,
-        timeZone,
+        interval: columnParams?.interval ?? autoInterval,
       },
     };
   },
@@ -135,28 +127,6 @@ export const dateHistogramOperation: OperationDefinition<
         (!newField.aggregationRestrictions || newField.aggregationRestrictions.date_histogram)
     );
   },
-  transfer: (column, newIndexPattern) => {
-    const newField = newIndexPattern.getFieldByName(column.sourceField);
-
-    if (newField?.aggregationRestrictions?.date_histogram) {
-      const restrictions = newField.aggregationRestrictions.date_histogram;
-
-      return {
-        ...column,
-        params: {
-          ...column.params,
-          timeZone: restrictions.time_zone,
-          // TODO this rewrite logic is simplified - if the current interval is a multiple of
-          // the restricted interval, we could carry it over directly. However as the current
-          // UI does not allow to select multiples of an interval anyway, this is not included yet.
-          // If the UI allows to pick more complicated intervals, this should be re-visited.
-          interval: restrictedInterval(newField.aggregationRestrictions) as string,
-        },
-      };
-    }
-
-    return column;
-  },
   onFieldChange: (oldColumn, field) => {
     return {
       ...oldColumn,
@@ -166,14 +136,24 @@ export const dateHistogramOperation: OperationDefinition<
   },
   toEsAggsFn: (column, columnId, indexPattern) => {
     const usedField = indexPattern.getFieldByName(column.sourceField);
+    let timeZone: string | undefined;
+    let interval = column.params?.interval ?? autoInterval;
+    if (
+      usedField &&
+      usedField.aggregationRestrictions &&
+      usedField.aggregationRestrictions.date_histogram
+    ) {
+      interval = restrictedInterval(usedField.aggregationRestrictions) as string;
+      timeZone = usedField.aggregationRestrictions.date_histogram.time_zone;
+    }
     return buildExpressionFunction<AggFunctionsMapping['aggDateHistogram']>('aggDateHistogram', {
       id: columnId,
       enabled: true,
       schema: 'segment',
       field: column.sourceField,
-      time_zone: column.params.timeZone,
+      time_zone: timeZone,
       useNormalizedEsInterval: !usedField?.aggregationRestrictions?.date_histogram,
-      interval: column.params.interval,
+      interval,
       drop_partials: false,
       min_doc_count: 0,
       extended_bounds: JSON.stringify({}),
@@ -244,7 +224,7 @@ export const dateHistogramOperation: OperationDefinition<
                 id="xpack.lens.indexPattern.dateHistogram.restrictedInterval"
                 defaultMessage="Interval fixed to {intervalValue} due to aggregation restrictions."
                 values={{
-                  intervalValue: currentColumn.params.interval,
+                  intervalValue: restrictedInterval(field!.aggregationRestrictions),
                 }}
               />
             ) : (

x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts

@@ -25,6 +25,7 @@ import { documentField } from '../document_field';
 import { getFieldByNameFactory } from '../pure_helpers';
 import { generateId } from '../../id_generator';
 import { createMockedFullReference, createMockedManagedReference } from './mocks';
+import { IndexPatternColumn, OperationDefinition } from './definitions';
 import { TinymathAST } from 'packages/kbn-tinymath';
 
 jest.mock('../operations');
@@ -2642,7 +2643,15 @@ describe('state_helpers', () => {
       });
     });
 
+    // no operation currently requires this check, faking a transfer function to check whether the generic logic works
     it('should rewrite column params if that is necessary due to restrictions', () => {
+      operationDefinitionMap.date_histogram.transfer = ((oldColumn) => ({
+        ...oldColumn,
+        params: {
+          ...oldColumn.params,
+          interval: 'w',
+        },
+      })) as OperationDefinition<IndexPatternColumn, 'field'>['transfer'];
       const layer: IndexPatternLayer = {
         columnOrder: ['col1', 'col2'],
         columns: {
@@ -2666,10 +2675,10 @@ describe('state_helpers', () => {
           ...layer.columns.col1,
           params: {
             interval: 'w',
-            timeZone: 'CET',
           },
         },
       });
+      delete operationDefinitionMap.date_histogram.transfer;
     });
 
     it('should remove operations referencing fields with wrong field types', () => {

x-pack/plugins/lens/server/embeddable/lens_embeddable_factory.ts

@@ -8,8 +8,11 @@
 import { EmbeddableRegistryDefinition } from 'src/plugins/embeddable/server';
 import { SerializableState } from '../../../../../src/plugins/kibana_utils/common';
 import { DOC_TYPE } from '../../common';
-import { commonRenameOperationsForFormula } from '../migrations/common_migrations';
-import { LensDocShapePre712 } from '../migrations/types';
+import {
+  commonRemoveTimezoneDateHistogramParam,
+  commonRenameOperationsForFormula,
+} from '../migrations/common_migrations';
+import { LensDocShape713, LensDocShapePre712 } from '../migrations/types';
 
 export const lensEmbeddableFactory = (): EmbeddableRegistryDefinition => {
   return {
@@ -24,6 +27,14 @@ export const lensEmbeddableFactory = (): EmbeddableRegistryDefinition => {
           attributes: migratedLensState,
         } as unknown) as SerializableState;
       },
+      '7.14.0': (state) => {
+        const lensState = (state as unknown) as { attributes: LensDocShape713 };
+        const migratedLensState = commonRemoveTimezoneDateHistogramParam(lensState.attributes);
+        return ({
+          ...lensState,
+          attributes: migratedLensState,
+        } as unknown) as SerializableState;
+      },
     },
   };
 };

x-pack/plugins/lens/server/migrations/common_migrations.ts

@@ -6,7 +6,13 @@
  */
 
 import { cloneDeep } from 'lodash';
-import { LensDocShapePre712, OperationTypePre712, LensDocShapePost712 } from './types';
+import {
+  LensDocShapePre712,
+  OperationTypePre712,
+  LensDocShapePost712,
+  LensDocShape713,
+  LensDocShape714,
+} from './types';
 
 export const commonRenameOperationsForFormula = (
   attributes: LensDocShapePre712
@@ -44,3 +50,31 @@ export const commonRenameOperationsForFormula = (
   );
   return newAttributes as LensDocShapePost712;
 };
+
+export const commonRemoveTimezoneDateHistogramParam = (
+  attributes: LensDocShape713
+): LensDocShape714 => {
+  const newAttributes = cloneDeep(attributes);
+  const datasourceLayers = newAttributes.state.datasourceStates.indexpattern.layers || {};
+  (newAttributes as LensDocShapePost712).state.datasourceStates.indexpattern.layers = Object.fromEntries(
+    Object.entries(datasourceLayers).map(([layerId, layer]) => {
+      return [
+        layerId,
+        {
+          ...layer,
+          columns: Object.fromEntries(
+            Object.entries(layer.columns).map(([columnId, column]) => {
+              if (column.operationType === 'date_histogram' && 'params' in column) {
+                const copy = { ...column, params: { ...column.params } };
+                delete copy.params.timeZone;
+                return [columnId, copy];
+              }
+              return [columnId, column];
+            })
+          ),
+        },
+      ];
+    })
+  );
+  return newAttributes as LensDocShapePost712;
+};