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

Migrate old style queries stored in filters array #38945

21 changes: 21 additions & 0 deletions src/legacy/core_plugins/kibana/migrations/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

// @ts-ignore
export { migrations } from './migrations';
31 changes: 31 additions & 0 deletions src/legacy/core_plugins/kibana/migrations/is_doc.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { Doc } from './types';

export function isDoc(doc: { [key: string]: unknown } | Doc): doc is Doc {
return (
typeof doc.id === 'string' &&
typeof doc.type === 'string' &&
doc.attributes !== null &&
typeof doc.attributes === 'object' &&
stacey-gammon marked this conversation as resolved.
Show resolved Hide resolved
doc.references !== null &&
typeof doc.references === 'object'
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/

import { cloneDeep, get, omit, has, flow } from 'lodash';
import { migrations730 as dashboardMigrations730 } from '../public/dashboard/migrations';

function migrateIndexPattern(doc) {
const searchSourceJSON = get(doc, 'attributes.kibanaSavedObjectMeta.searchSourceJSON');
Expand Down Expand Up @@ -422,6 +423,7 @@ export const migrations = {
'7.0.0': (doc) => {
// Set new "references" attribute
doc.references = doc.references || [];

// Migrate index pattern
migrateIndexPattern(doc);
// Migrate panels
Expand Down Expand Up @@ -455,6 +457,7 @@ export const migrations = {
doc.attributes.panelsJSON = JSON.stringify(panels);
return doc;
},
'7.3.0': dashboardMigrations730
},
search: {
'7.0.0': (doc) => {
Expand Down
39 changes: 39 additions & 0 deletions src/legacy/core_plugins/kibana/migrations/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { SavedObjectReference } from '../../../../legacy/server/saved_objects/routes/types';

export interface SavedObjectAttributes {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these not just be the existing saved object types? (Same question for the other types.ts file under the dashboard directory)

/**
*
* @public
*/
export interface SavedObjectAttributes {
[key: string]: SavedObjectAttributes | string | number | boolean | null;
}
/**
*
* @public
*/
export interface SavedObject<T extends SavedObjectAttributes = any> {
id: string;
type: string;
version?: string;
updated_at?: string;
error?: {
message: string;
statusCode: number;
};
attributes: T;
references: SavedObjectReference[];
migrationVersion?: SavedObjectsMigrationVersion;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you these types could all be refactored/improved. It's just not super straightforward. I started going down many typescript rabbit holes with this endeavor (started trying to type all of migrations.js), but they quickly grew out of scope.

For instance, we have interface SavedObjectDashboard extends SavedObject. That's actually not the SavedObject here though, that's actually the SavedObjectAttributes (but also with id... so maybe it's actually the saved object after a post processing step). This extends SavedObject type is also different than the raw SavedObject.

Another type that gets in the mix is the one before and after the extractReferences call. It actually changes the raw saved object type by injecting the panel ids into the panelsJSON and removing the outer references.

So I think I'd prefer for this PR and just focus on the migration plus minimal typing support. Ideally we should typescript all of migrations and start a pattern of adding types for each iteration of saved object.

kibanaSavedObjectMeta: {
searchSourceJSON: string;
};
}

export interface Doc<Attributes extends SavedObjectAttributes = SavedObjectAttributes> {
references: SavedObjectReference[];
attributes: Attributes;
id: string;
type: string;
}

export interface DocPre700<Attributes extends SavedObjectAttributes = SavedObjectAttributes> {
attributes: Attributes;
id: string;
type: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ export function getSavedDashboardMock(
save: () => {
return Promise.resolve('123');
},
getQuery: () => ({ query: '', language: 'kuery' }),
getFilters: () => [],
...config,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import { stateMonitorFactory, StateMonitor } from 'ui/state_management/state_mon
import { StaticIndexPattern } from 'ui/index_patterns';
import { AppStateClass as TAppStateClass } from 'ui/state_management/app_state';
import { Timefilter } from 'ui/timefilter';
import { RefreshInterval } from 'ui/timefilter/timefilter';
import { Filter } from '@kbn/es-query';
import moment from 'moment';
import { RefreshInterval } from 'ui/timefilter/timefilter';
import { Query } from 'src/legacy/core_plugins/data/public';
import { TimeRange } from 'ui/timefilter/time_history';
import { DashboardViewMode } from './dashboard_view_mode';
Expand Down Expand Up @@ -278,7 +278,7 @@ export class DashboardStateManager {

_pushFiltersToStore() {
const state = store.getState();
const dashboardFilters = this.getDashboardFilterBars();
const dashboardFilters = this.savedDashboard.getFilters();
if (
!_.isEqual(
FilterUtils.cleanFiltersForComparison(dashboardFilters),
Expand Down Expand Up @@ -386,8 +386,8 @@ export class DashboardStateManager {
return {
timeTo: this.savedDashboard.timeTo,
timeFrom: this.savedDashboard.timeFrom,
filterBars: this.getDashboardFilterBars(),
query: this.getDashboardQuery(),
filterBars: this.savedDashboard.getFilters(),
query: this.savedDashboard.getQuery(),
};
}

Expand Down Expand Up @@ -455,14 +455,6 @@ export class DashboardStateManager {
return this.savedDashboard.timeRestore;
}

public getDashboardFilterBars() {
return FilterUtils.getFilterBarsForDashboard(this.savedDashboard);
}

public getDashboardQuery() {
return FilterUtils.getQueryFilterForDashboard(this.savedDashboard);
}

public getLastSavedFilterBars(): Filter[] {
return this.lastSavedDashboardFilters.filterBars;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@

import _ from 'lodash';
import moment, { Moment } from 'moment';
import { QueryFilter } from 'ui/filter_manager/query_filter';
import { Filter } from '@kbn/es-query';
import { SavedObjectDashboard } from '../saved_dashboard/saved_dashboard';

/**
* @typedef {Object} QueryFilter
Expand All @@ -30,51 +28,6 @@ import { SavedObjectDashboard } from '../saved_dashboard/saved_dashboard';
*/

export class FilterUtils {
/**
*
* @param filter
* @returns {Boolean} True if the filter is of the special query type
* (e.g. goes in the query input bar), false otherwise (e.g. is in the filter bar).
*/
public static isQueryFilter(filter: Filter) {
return filter.query && !filter.meta;
}

/**
*
* @param {SavedDashboard} dashboard
* @returns {Array.<Object>} An array of filters stored with the dashboard. Includes
* both query filters and filter bar filters.
*/
public static getDashboardFilters(dashboard: SavedObjectDashboard): Filter[] {
return dashboard.searchSource.getOwnField('filter');
}

/**
* Grabs a saved query to use from the dashboard, or if none exists, creates a default one.
* @param {SavedDashboard} dashboard
* @returns {QueryFilter}
*/
public static getQueryFilterForDashboard(dashboard: SavedObjectDashboard): QueryFilter | string {
if (dashboard.searchSource.getOwnField('query')) {
return dashboard.searchSource.getOwnField('query');
}

const dashboardFilters = this.getDashboardFilters(dashboard);
const dashboardQueryFilter = _.find(dashboardFilters, this.isQueryFilter);
return dashboardQueryFilter ? dashboardQueryFilter.query : '';
}

/**
* Returns the filters for the dashboard that should appear in the filter bar area.
* @param {SavedDashboard} dashboard
* @return {Array.<Object>} Array of filters that should appear in the filter bar for the
* given dashboard
*/
public static getFilterBarsForDashboard(dashboard: SavedObjectDashboard) {
return _.reject(this.getDashboardFilters(dashboard), this.isQueryFilter);
}

/**
* Converts the time to a utc formatted string. If the time is not valid (e.g. it might be in a relative format like
* 'now-15m', then it just returns what it was passed).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/

import { DashboardViewMode } from '../dashboard_view_mode';
import { FilterUtils } from './filter_utils';
import { SavedObjectDashboard } from '../saved_dashboard/saved_dashboard';
import {
Pre61SavedDashboardPanel,
Expand All @@ -37,8 +36,8 @@ export function getAppStateDefaults(
timeRestore: savedDashboard.timeRestore,
panels: savedDashboard.panelsJSON ? JSON.parse(savedDashboard.panelsJSON) : [],
options: savedDashboard.optionsJSON ? JSON.parse(savedDashboard.optionsJSON) : {},
query: FilterUtils.getQueryFilterForDashboard(savedDashboard),
filters: FilterUtils.getFilterBarsForDashboard(savedDashboard),
query: savedDashboard.getQuery(),
filters: savedDashboard.getFilters(),
viewMode:
savedDashboard.id || hideWriteControls ? DashboardViewMode.VIEW : DashboardViewMode.EDIT,
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export { migrations730 } from './migrations_730';
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { DashboardDoc } from './types';
import { isDoc } from '../../../migrations/is_doc';

export function isDashboardDoc(
doc: { [key: string]: unknown } | DashboardDoc
): doc is DashboardDoc {
if (!isDoc(doc)) {
return false;
}

if (typeof (doc as DashboardDoc).attributes.panelsJSON !== 'string') {
return false;
}

return true;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { migrations730 } from './migrations_730';
import { DashboardDoc } from './types';

test('dashboard migration 7.3.0 migrates filters to query on search source', () => {
const doc: DashboardDoc = {
id: '1',
type: 'dashboard',
references: [],
attributes: {
description: '',
uiStateJSON: '{}',
version: 1,
timeRestore: false,
kibanaSavedObjectMeta: {
searchSourceJSON:
'{"filter":[{"query":{"query_string":{"query":"n: 6","analyze_wildcard":true}}}],"highlightAll":true,"version":true}',
},
panelsJSON:
'[{"id":"1","type":"visualization","foo":true},{"id":"2","type":"visualization","bar":true}]',
},
};
const newDoc = migrations730(doc);

expect(newDoc).toMatchInlineSnapshot(`
Object {
"attributes": Object {
"description": "",
"kibanaSavedObjectMeta": Object {
"searchSourceJSON": "{\\"filter\\":[],\\"highlightAll\\":true,\\"version\\":true,\\"query\\":{\\"query\\":\\"n: 6\\",\\"language\\":\\"lucene\\"}}",
},
"panelsJSON": "[{\\"id\\":\\"1\\",\\"type\\":\\"visualization\\",\\"foo\\":true},{\\"id\\":\\"2\\",\\"type\\":\\"visualization\\",\\"bar\\":true}]",
"timeRestore": false,
"uiStateJSON": "{}",
"version": 1,
},
"id": "1",
"references": Array [],
"type": "dashboard",
}
`);
});
Loading