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

This commit is contained in:
Joe Reuter 2021-06-29 10:19:50 +02:00 committed by GitHub
parent ac17ab1436
commit 4f63abc1e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 248 additions and 174 deletions

View file

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

View file

@ -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();

View file

@ -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),
}}
/>
) : (

View file

@ -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', () => {

View file

@ -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;
},
},
};
};

View file

@ -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;
};

View file

@ -852,4 +852,96 @@ describe('Lens migrations', () => {
validate(result2);
});
});
describe('7.14.0 remove time zone from date histogram', () => {
const context = ({ log: { warning: () => {} } } as unknown) as SavedObjectMigrationContext;
const example = {
type: 'lens',
id: 'mocked-saved-object-id',
attributes: {
savedObjectId: '1',
title: 'MyRenamedOps',
description: '',
visualizationType: 'lnsXY',
state: {
datasourceStates: {
indexpattern: {
layers: {
'2': {
columns: {
'3': {
label: '@timestamp',
dataType: 'date',
operationType: 'date_histogram',
sourceField: '@timestamp',
isBucketed: true,
scale: 'interval',
params: { interval: 'auto', timeZone: 'Europe/Berlin' },
},
'4': {
label: '@timestamp',
dataType: 'date',
operationType: 'date_histogram',
sourceField: '@timestamp',
isBucketed: true,
scale: 'interval',
params: { interval: 'auto' },
},
'5': {
label: '@timestamp',
dataType: 'date',
operationType: 'my_unexpected_operation',
isBucketed: true,
scale: 'interval',
params: { timeZone: 'do not delete' },
},
},
columnOrder: ['3', '4', '5'],
incompleteColumns: {},
},
},
},
},
visualization: {
title: 'Empty XY chart',
legend: { isVisible: true, position: 'right' },
valueLabels: 'hide',
preferredSeriesType: 'bar_stacked',
layers: [
{
layerId: '5ab74ddc-93ca-44e2-9857-ecf85c86b53e',
accessors: [
'5fea2a56-7b73-44b5-9a50-7f0c0c4f8fd0',
'e5efca70-edb5-4d6d-a30a-79384066987e',
'7ffb7bde-4f42-47ab-b74d-1b4fd8393e0f',
],
position: 'top',
seriesType: 'bar_stacked',
showGridlines: false,
xAccessor: '2e57a41e-5a52-42d3-877f-bd211d903ef8',
},
],
},
query: { query: '', language: 'kuery' },
filters: [],
},
},
};
it('should remove time zone param from date histogram', () => {
const result = migrations['7.14.0'](example, context) as ReturnType<
SavedObjectMigrationFn<LensDocShape, LensDocShape>
>;
const layers = Object.values(result.attributes.state.datasourceStates.indexpattern.layers);
expect(layers.length).toBe(1);
const columns = Object.values(layers[0].columns);
expect(columns.length).toBe(3);
expect(columns[0].operationType).toEqual('date_histogram');
expect((columns[0] as { params: {} }).params).toEqual({ interval: 'auto' });
expect(columns[1].operationType).toEqual('date_histogram');
expect((columns[1] as { params: {} }).params).toEqual({ interval: 'auto' });
expect(columns[2].operationType).toEqual('my_unexpected_operation');
expect((columns[2] as { params: {} }).params).toEqual({ timeZone: 'do not delete' });
});
});
});

View file

@ -15,8 +15,11 @@ import {
} from 'src/core/server';
import { Query, Filter } from 'src/plugins/data/public';
import { PersistableFilter } from '../../common';
import { LensDocShapePost712, LensDocShapePre712 } from './types';
import { commonRenameOperationsForFormula } from './common_migrations';
import { LensDocShapePost712, LensDocShapePre712, LensDocShape713, LensDocShape714 } from './types';
import {
commonRenameOperationsForFormula,
commonRemoveTimezoneDateHistogramParam,
} from './common_migrations';
interface LensDocShapePre710<VisualizationState = unknown> {
visualizationType: string | null;
@ -400,6 +403,16 @@ const renameOperationsForFormula: SavedObjectMigrationFn<
};
};
const removeTimezoneDateHistogramParam: SavedObjectMigrationFn<LensDocShape713, LensDocShape714> = (
doc
) => {
const newDoc = cloneDeep(doc);
return {
...newDoc,
attributes: commonRemoveTimezoneDateHistogramParam(newDoc.attributes),
};
};
export const migrations: SavedObjectMigrationMap = {
'7.7.0': removeInvalidAccessors,
// The order of these migrations matter, since the timefield migration relies on the aggConfigs
@ -410,4 +423,5 @@ export const migrations: SavedObjectMigrationMap = {
'7.12.0': transformTableState,
'7.13.0': renameOperationsForFormula,
'7.13.1': renameOperationsForFormula, // duplicate this migration in case a broken by value panel is added to the library
'7.14.0': removeTimezoneDateHistogramParam,
};

View file

@ -87,3 +87,68 @@ export interface LensDocShapePost712<VisualizationState = unknown> {
filters: Filter[];
};
}
export type LensDocShape713 = Omit<LensDocShapePost712, 'state'> & {
state: Omit<LensDocShapePost712['state'], 'datasourceStates'> & {
datasourceStates: {
indexpattern: Omit<
LensDocShapePost712['state']['datasourceStates']['indexpattern'],
'layers'
> & {
layers: Record<
string,
Omit<
LensDocShapePost712['state']['datasourceStates']['indexpattern']['layers'][string],
'columns'
> & {
columns: Record<
string,
| {
operationType: OperationTypePost712;
}
| {
operationType: 'date_histogram';
params: {
interval: string;
timeZone?: string;
};
}
>;
}
>;
};
};
};
};
export type LensDocShape714 = Omit<LensDocShapePost712, 'state'> & {
state: Omit<LensDocShapePost712['state'], 'datasourceStates'> & {
datasourceStates: {
indexpattern: Omit<
LensDocShapePost712['state']['datasourceStates']['indexpattern'],
'layers'
> & {
layers: Record<
string,
Omit<
LensDocShapePost712['state']['datasourceStates']['indexpattern']['layers'][string],
'columns'
> & {
columns: Record<
string,
| {
operationType: OperationTypePost712;
}
| {
operationType: 'date_histogram';
params: {
interval: string;
};
}
>;
}
>;
};
};
};
};