Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] Don't save the current timezone in visualizations (#34795) #34924

Merged
merged 1 commit into from
Apr 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions src/legacy/core_plugins/kibana/migrations.js
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
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
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