-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[Discover] Save and load Discover session tabs #227159
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
Changes from all commits
d614e19
2bbae78
95c95ab
4572b4f
d9dfe92
5485392
fdaec72
9105bec
cccce03
091e2b3
087a611
291b559
e88ff54
8de9ca2
36d87f7
5e55211
beaed7f
3792dfd
8785f5c
cd93173
d91f6d0
4d49fa9
a2981d0
55c25c2
2c1ebef
5d7bf9e
86da074
ddbbd83
0e7db64
150fd77
bb1d4b9
00954fe
0d5cd94
4520b22
1d50347
4521be0
878e7ff
fddf270
fa94632
fb17d24
cff4066
365eeff
8cb9448
fd47969
1dd7dd9
c6479a8
ad480b6
bd92ba9
e75af6e
f349df9
88abaff
b353b4a
b3dccbe
c929222
9ad5209
b4cb687
72d5492
6e0b7b5
1b9a3f0
07b73fd
c780be8
af676bf
aa16c9e
66bb3e9
91809d5
56b949d
1028670
91aef31
db13229
d9bb6ca
6f5e1b7
ce242d8
0493bc4
61233d2
9b9c80d
f88be2e
e85e31b
9dba4d4
57f8d93
5c9bf55
b710df3
73a99d7
4b3aa8d
00d5923
3f78abd
0e90f1f
ae1d6a8
4b41136
0e888b8
c9ddf88
a1c0fda
cf148ca
b494036
e8dd2d3
816ef88
0168ca8
61840c6
0b19730
4eceea9
945bd85
6b59e4d
926460d
ffbb16e
093348a
0ef6c01
b9835af
435d5c0
460014c
aab9428
8837251
49badc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| import { extractReferences } from './extract_references'; | ||
| import type { SerializedSearchSourceFields } from './types'; | ||
|
|
||
| describe('extractReferences', () => { | ||
| it('should extract reference for data view ID', () => { | ||
| const searchSource: SerializedSearchSourceFields = { | ||
| index: 'test-index', | ||
| }; | ||
| const result = extractReferences(searchSource); | ||
| expect(result).toEqual([ | ||
| { | ||
| indexRefName: 'kibanaSavedObjectMeta.searchSourceJSON.index', | ||
| }, | ||
| [ | ||
| { | ||
| id: 'test-index', | ||
| name: 'kibanaSavedObjectMeta.searchSourceJSON.index', | ||
| type: 'index-pattern', | ||
| }, | ||
| ], | ||
| ]); | ||
| }); | ||
|
|
||
| it('should not extract reference for data view spec', () => { | ||
| const searchSource: SerializedSearchSourceFields = { | ||
| index: { | ||
| id: 'test-id', | ||
| title: 'test-title', | ||
| }, | ||
| }; | ||
| const result = extractReferences(searchSource); | ||
| expect(result).toEqual([ | ||
| { | ||
| index: { | ||
| id: 'test-id', | ||
| title: 'test-title', | ||
| }, | ||
| }, | ||
| [], | ||
| ]); | ||
| }); | ||
|
|
||
| it('should extract reference for filter', () => { | ||
| const searchSource: SerializedSearchSourceFields = { | ||
| filter: [ | ||
| { | ||
| meta: { | ||
| index: 'test-index', | ||
| }, | ||
| }, | ||
| ], | ||
| }; | ||
| const result = extractReferences(searchSource); | ||
| expect(result).toEqual([ | ||
| { | ||
| filter: [ | ||
| { | ||
| meta: { | ||
| indexRefName: 'kibanaSavedObjectMeta.searchSourceJSON.filter[0].meta.index', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| [ | ||
| { | ||
| id: 'test-index', | ||
| name: 'kibanaSavedObjectMeta.searchSourceJSON.filter[0].meta.index', | ||
| type: 'index-pattern', | ||
| }, | ||
| ], | ||
| ]); | ||
| }); | ||
|
|
||
| it('should apply prefix to references', () => { | ||
| const searchSource: SerializedSearchSourceFields = { | ||
| index: 'test-index', | ||
| filter: [ | ||
| { | ||
| meta: { | ||
| index: 'test-index', | ||
| }, | ||
| }, | ||
| ], | ||
| }; | ||
| const result = extractReferences(searchSource, { refNamePrefix: 'testPrefix' }); | ||
| expect(result).toEqual([ | ||
| { | ||
| indexRefName: 'testPrefix.kibanaSavedObjectMeta.searchSourceJSON.index', | ||
| filter: [ | ||
| { | ||
| meta: { | ||
| indexRefName: | ||
| 'testPrefix.kibanaSavedObjectMeta.searchSourceJSON.filter[0].meta.index', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| [ | ||
| { | ||
| id: 'test-index', | ||
| name: 'testPrefix.kibanaSavedObjectMeta.searchSourceJSON.index', | ||
| type: 'index-pattern', | ||
| }, | ||
| { | ||
| id: 'test-index', | ||
| name: 'testPrefix.kibanaSavedObjectMeta.searchSourceJSON.filter[0].meta.index', | ||
| type: 'index-pattern', | ||
| }, | ||
| ], | ||
| ]); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,14 +14,16 @@ import type { SerializedSearchSourceFields } from './types'; | |
| import { DATA_VIEW_SAVED_OBJECT_TYPE } from '../..'; | ||
|
|
||
| export const extractReferences = ( | ||
| state: SerializedSearchSourceFields | ||
| state: SerializedSearchSourceFields, | ||
| options?: { refNamePrefix?: string } | ||
| ): [SerializedSearchSourceFields, SavedObjectReference[]] => { | ||
| const refNamePrefix = options?.refNamePrefix ? `${options.refNamePrefix}.` : ''; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This approach seems to work reliably for generating unique ref names when saving multiple search sources within a multi-tab Discover session. The existing |
||
| let searchSourceFields: SerializedSearchSourceFields & { indexRefName?: string } = { ...state }; | ||
| const references: SavedObjectReference[] = []; | ||
| if (searchSourceFields.index) { | ||
| if (typeof searchSourceFields.index === 'string') { | ||
| const indexId = searchSourceFields.index; | ||
| const refName = 'kibanaSavedObjectMeta.searchSourceJSON.index'; | ||
| const refName = `${refNamePrefix}kibanaSavedObjectMeta.searchSourceJSON.index`; | ||
| references.push({ | ||
| name: refName, | ||
| type: DATA_VIEW_SAVED_OBJECT_TYPE, | ||
|
|
@@ -47,7 +49,7 @@ export const extractReferences = ( | |
| if (!filterRow.meta || !filterRow.meta.index) { | ||
| return filterRow; | ||
| } | ||
| const refName = `kibanaSavedObjectMeta.searchSourceJSON.filter[${i}].meta.index`; | ||
| const refName = `${refNamePrefix}kibanaSavedObjectMeta.searchSourceJSON.filter[${i}].meta.index`; | ||
| references.push({ | ||
| name: refName, | ||
| type: DATA_VIEW_SAVED_OBJECT_TYPE, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,12 +55,7 @@ const createFetchFieldErrorTitle = ({ id, title }: { id?: string; title?: string | |
| */ | ||
| export type DataViewSavedObjectAttrs = Pick< | ||
| DataViewAttributes, | ||
| 'title' | 'type' | 'typeMeta' | 'name' | ||
| >; | ||
|
|
||
| export type IndexPatternListSavedObjectAttrs = Pick< | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dead code. |
||
| DataViewAttributes, | ||
| 'title' | 'type' | 'typeMeta' | 'name' | ||
| 'title' | 'type' | 'typeMeta' | 'name' | 'timeFieldName' | ||
| >; | ||
|
|
||
| /** | ||
|
|
@@ -87,7 +82,14 @@ export interface DataViewListItem { | |
| * Data view type meta | ||
| */ | ||
| typeMeta?: TypeMeta; | ||
| /** | ||
| * Human-readable name | ||
| */ | ||
| name?: string; | ||
| /** | ||
| * Time field name if applicable | ||
| */ | ||
| timeFieldName?: string; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I needed a way to know the |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -404,7 +406,7 @@ export class DataViewsService { | |
| */ | ||
| private async refreshSavedObjectsCache() { | ||
| const so = await this.savedObjectsClient.find({ | ||
| fields: ['title', 'type', 'typeMeta', 'name'], | ||
| fields: ['title', 'type', 'typeMeta', 'name', 'timeFieldName'], | ||
| perPage: 10000, | ||
| }); | ||
| this.savedObjectsCache = so; | ||
|
|
@@ -494,6 +496,7 @@ export class DataViewsService { | |
| type: obj?.attributes?.type, | ||
| typeMeta: obj?.attributes?.typeMeta && JSON.parse(obj?.attributes?.typeMeta), | ||
| name: obj?.attributes?.name, | ||
| timeFieldName: obj?.attributes?.timeFieldName, | ||
| })); | ||
| }; | ||
|
|
||
|
|
@@ -1269,7 +1272,7 @@ export class DataViewsService { | |
| })) as SavedObject<DataViewAttributes>; | ||
|
|
||
| if (this.savedObjectsCache) { | ||
| this.savedObjectsCache.push(response as SavedObject<IndexPatternListSavedObjectAttrs>); | ||
| this.savedObjectsCache.push(response); | ||
| } | ||
| dataView.version = response.version; | ||
| dataView.namespaces = response.namespaces || []; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,7 +58,12 @@ export class ContentMagementWrapper implements PersistenceAPI { | |
| }); | ||
| } catch (e) { | ||
| if (e.body?.statusCode === 404) { | ||
| throw new SavedObjectNotFound('data view', id, 'management/kibana/dataViews'); | ||
| throw new SavedObjectNotFound({ | ||
| type: DataViewSOType, | ||
| typeDisplayName: 'data view', | ||
|
Comment on lines
+62
to
+63
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was the issue with data view redirects. At some point we changed the type to
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean this issue?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't aware we already had an issue for it, but yes these changes fix that!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jughosta this seems to be a different one, about missing Discover session, but it should also be improved 👍
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems this PR resolves #203405 🥳
|
||
| id, | ||
| link: '/app/management/kibana/dataViews', | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Plus the link itself was outdated and didn't go anywhere... |
||
| }); | ||
| } else { | ||
| throw e; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ | |
| */ | ||
|
|
||
| import { isOfAggregateQueryType, type AggregateQuery, type Query } from '@kbn/es-query'; | ||
| import type { DataView } from '@kbn/data-views-plugin/common'; | ||
| import type { DataView, DataViewSpec } from '@kbn/data-views-plugin/common'; | ||
| import { | ||
| DataSourceType, | ||
| type DataViewDataSource, | ||
|
|
@@ -33,14 +33,19 @@ export const createDataSource = ({ | |
| dataView, | ||
| query, | ||
| }: { | ||
| dataView: DataView | undefined; | ||
| dataView: DataView | DataViewSpec | string | undefined; | ||
| query: Query | AggregateQuery | undefined; | ||
| }) => { | ||
| return isOfAggregateQueryType(query) | ||
| ? createEsqlDataSource() | ||
| : dataView?.id | ||
| ? createDataViewDataSource({ dataViewId: dataView.id }) | ||
| : undefined; | ||
| if (isOfAggregateQueryType(query)) { | ||
| return createEsqlDataSource(); | ||
| } | ||
| if (typeof dataView === 'string') { | ||
| return createDataViewDataSource({ dataViewId: dataView }); | ||
| } | ||
| if (dataView?.id) { | ||
| return createDataViewDataSource({ dataViewId: dataView.id }); | ||
| } | ||
| return undefined; | ||
|
Comment on lines
+39
to
+48
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for simplifying it instead of extending the chaining. This is much more readable |
||
| }; | ||
|
|
||
| export const isDataSourceType = <T extends DataSourceType>( | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small change to
SavedObjectNotFoundconstructor to fix an issue with SO not found redirects.