Skip to content

Commit

Permalink
Don't save the current timezone in visualizations (#34795) (#34924)
Browse files Browse the repository at this point in the history 
* Don't save the current timezone in visualizations

* Add additional test

* Add test and switch migration number

* Don't clone object according to review feedback

* Better documentation

* Update src/legacy/core_plugins/kibana/migrations.js

Co-Authored-By: timroes <[email protected]>
  • Loading branch information
@timroes
timroes authored Apr 11, 2019
1 parent 7836db5 commit e353b3f
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 0 deletions.
39 changes: 39 additions & 0 deletions src/legacy/core_plugins/kibana/migrations.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,33 @@ function migrateIndexPattern(doc) {
doc.attributes.kibanaSavedObjectMeta.searchSourceJSON = JSON.stringify(searchSource);
}

function removeDateHistogramTimeZones(doc) {
const visStateJSON = get(doc, 'attributes.visState');
if (visStateJSON) {
let visState;
try {
visState = JSON.parse(visStateJSON);
} catch (e) {
// Let it go, the data is invalid and we'll leave it as is
}
if (visState && visState.aggs) {
visState.aggs.forEach(agg => {
// We're checking always for the existance of agg.params here. This should always exist, but better
// be safe then sorry during migrations.
if (agg.type === 'date_histogram' && agg.params) {
delete agg.params.time_zone;
}

if (get(agg, 'params.customBucket.type', null) === 'date_histogram' && agg.params.customBucket.params) {
delete agg.params.customBucket.params.time_zone;
}
});
doc.attributes.visState = JSON.stringify(visState);
}
}
return doc;
}

export const migrations = {
'index-pattern': {
'6.5.0': (doc) => {
Expand All @@ -66,6 +93,17 @@ export const migrations = {
}
},
visualization: {
/**
* We need to have this migration twice, once with a version prior to 7.0.0 once with a version
* after it. The reason for that is, that this migration has been introduced once 7.0.0 was already
* released. Thus a user who already had 7.0.0 installed already got the 7.0.0 migrations below running,
* so we need a version higher than that. But this fix was backported to the 6.7 release, meaning if we
* would only have the 7.0.1 migration in here a user on the 6.7 release will migrate their saved objects
* to the 7.0.1 state, and thus when updating their Kibana to 7.0, will never run the 7.0.0 migrations introduced
* in that version. So we apply this twice, once with 6.7.2 and once with 7.0.1 while the backport to 6.7
* only contained the 6.7.2 migration and not the 7.0.1 migration.
*/
'6.7.2': removeDateHistogramTimeZones,
'7.0.0': (doc) => {
// Set new "references" attribute
doc.references = doc.references || [];
Expand Down Expand Up @@ -146,6 +184,7 @@ export const migrations = {
throw new Error(`Failure attempting to migrate saved object '${doc.attributes.title}' - ${e}`);
}
},
'7.0.1': removeDateHistogramTimeZones,
'7.1.0': doc => {
// [TSVB] Migrate percentile-rank aggregation (value -> values)
const migratePercentileRankAggregation = doc => {
Expand Down
118 changes: 118 additions & 0 deletions src/legacy/core_plugins/kibana/migrations.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,124 @@ Object {
});

describe('visualization', () => {
describe('date histogram time zone removal', () => {
const migrate = doc => migrations.visualization['6.7.2'](doc);
let doc;
beforeEach(() => {
doc = {
attributes: {
visState: JSON.stringify({
aggs: [
{
'enabled': true,
'id': '1',
'params': {
// Doesn't make much sense but we want to test it's not removing it from anything else
time_zone: 'Europe/Berlin',
},
'schema': 'metric',
'type': 'count'
},
{
'enabled': true,
'id': '2',
'params': {
'customInterval': '2h',
'drop_partials': false,
'extended_bounds': {},
'field': 'timestamp',
'time_zone': 'Europe/Berlin',
'interval': 'auto',
'min_doc_count': 1,
'useNormalizedEsInterval': true
},
'schema': 'segment',
'type': 'date_histogram'
},
{
'enabled': true,
'id': '4',
'params': {
'customInterval': '2h',
'drop_partials': false,
'extended_bounds': {},
'field': 'timestamp',
'interval': 'auto',
'min_doc_count': 1,
'useNormalizedEsInterval': true
},
'schema': 'segment',
'type': 'date_histogram'
},
{
'enabled': true,
'id': '3',
'params': {
'customBucket': {
'enabled': true,
'id': '1-bucket',
'params': {
'customInterval': '2h',
'drop_partials': false,
'extended_bounds': {},
'field': 'timestamp',
'interval': 'auto',
'min_doc_count': 1,
'time_zone': 'Europe/Berlin',
'useNormalizedEsInterval': true
},
'type': 'date_histogram'
},
'customMetric': {
'enabled': true,
'id': '1-metric',
'params': {},
'type': 'count'
}
},
'schema': 'metric',
'type': 'max_bucket'
},
]
}),
}
};
});

it('should remove time_zone from date_histogram aggregations', () => {
const migratedDoc = migrate(doc);
const aggs = JSON.parse(migratedDoc.attributes.visState).aggs;
expect(aggs[1]).not.toHaveProperty('params.time_zone');
});

it('should not remove time_zone from non date_histogram aggregations', () => {
const migratedDoc = migrate(doc);
const aggs = JSON.parse(migratedDoc.attributes.visState).aggs;
expect(aggs[0]).toHaveProperty('params.time_zone');
});

it('should remove time_zone from nested aggregations', () => {
const migratedDoc = migrate(doc);
const aggs = JSON.parse(migratedDoc.attributes.visState).aggs;
expect(aggs[3]).not.toHaveProperty('params.customBucket.params.time_zone');
});

it('should not fail on date histograms without a time_zone', () => {
const migratedDoc = migrate(doc);
const aggs = JSON.parse(migratedDoc.attributes.visState).aggs;
expect(aggs[2]).not.toHaveProperty('params.time_zone');
});

it('should be able to apply the migration twice, since we need it for 6.7.2 and 7.0.1', () => {
const migratedDoc = migrate(migrate(doc));
const aggs = JSON.parse(migratedDoc.attributes.visState).aggs;
expect(aggs[1]).not.toHaveProperty('params.time_zone');
expect(aggs[0]).toHaveProperty('params.time_zone');
expect(aggs[3]).not.toHaveProperty('params.customBucket.params.time_zone');
expect(aggs[2]).not.toHaveProperty('params.time_zone');
});
});

describe('7.0.0', () => {
const migrate = doc => migrations.visualization['7.0.0'](doc);
const generateDoc = ({ type, aggs }) => ({
Expand Down
7 changes: 7 additions & 0 deletions src/legacy/ui/public/agg_types/buckets/date_histogram.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,13 @@ export const dateHistogramBucketAgg = new BucketAggType({
const isDefaultTimezone = config.isDefault('dateFormat:tz');
return isDefaultTimezone ? detectedTimezone || tzOffset : config.get('dateFormat:tz');
},
serialize() {
// We don't want to store the `time_zone` parameter ever in the saved object for the visualization.
// If we would store this changing the time zone in Kibana would not affect any already saved visualizations
// anymore, which is not the desired behavior. So always returning undefined here, makes sure we're never
// saving that parameter and just keep it "transient".
return undefined;
},
},
{
name: 'drop_partials',
Expand Down

0 comments on commit e353b3f

Please sign in to comment.