-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Saved Searches] Add support for saved searches by value #146849
Merged
davismcphee
merged 33 commits into
elastic:main
from
davismcphee:enhancement-saved-searches-by-value
Aug 2, 2023
Merged
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
dc0c67f
[Discover] Initial implementation of saved searches by value
davismcphee 0340368
Finish initial implementation of saved searches by value
davismcphee ac280fa
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine 3da5211
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine 09f942c
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine 33aaa6a
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine 9a66a71
Change savedSearch.embeddable to savedSearch.byValue
davismcphee 9e4b4ed
Add support for description and cleanup embeddable and embeeddable fa…
davismcphee 7c1479c
More saved search by value cleanup
davismcphee 7dc1e4c
Resolving merge conflicts
davismcphee 32b4445
Fix broken TS types
davismcphee 9f454df
Fixing Jest tests
davismcphee c18662f
Finish fixing Jest tests
davismcphee 5a4cefc
Add support for Discover edit/open actions for by value saved search …
davismcphee 5c65c1f
Add support for inject/extract
davismcphee bece30b
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine 5154bca
Add tests for new by value functionality
davismcphee b9b455a
Remove unused services from functional test file
davismcphee 1bf6fe3
Resolve merge conflicts
davismcphee 7a7edeb
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine a55c7ca
Fix type in src/plugins/discover/server/embeddable/search_embeddable_…
davismcphee 37242bb
Merge branch 'main' into enhancement-saved-searches-by-value
stratoula 87ffe57
Export SavedSearchUnwrapMetaInfo
davismcphee 584257a
Merge branch 'main' into enhancement-saved-searches-by-value
davismcphee 2464c63
Change from toSpec to toMinimalSpec when creating saved search embedd…
davismcphee 9c78dc2
Merge branch 'main' into enhancement-saved-searches-by-value
davismcphee f9ee121
Remove isDestroyed from saved search embeddable and replaced with des…
davismcphee 52160a6
Update Dashboard copy to change 'Visualization Library' to 'library'
davismcphee e087529
Merge branch 'main' into enhancement-saved-searches-by-value
davismcphee 361d410
Update Dashboard Jest test snapshot
davismcphee 44f3621
Merge branch 'main' into enhancement-saved-searches-by-value
davismcphee 86dc808
Merge branch 'main' into enhancement-saved-searches-by-value
davismcphee 13cf1fa
Fix saved search embeddable edit link base path
davismcphee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 changes: 1 addition & 1 deletion
2
...board_container/component/empty_screen/__snapshots__/dashboard_empty_screen.test.tsx.snap
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/* | ||
* 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 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 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
export { inject, extract } from './search_inject_extract'; |
76 changes: 76 additions & 0 deletions
76
src/plugins/discover/common/embeddable/search_inject_extract.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
/* | ||
* 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 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 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { extract, inject } from './search_inject_extract'; | ||
|
||
describe('search inject extract', () => { | ||
describe('inject', () => { | ||
it('should not inject references if state does not have attributes', () => { | ||
const state = { type: 'type', id: 'id' }; | ||
const injectedReferences = [{ name: 'name', type: 'type', id: 'id' }]; | ||
expect(inject(state, injectedReferences)).toEqual(state); | ||
}); | ||
|
||
it('should inject references if state has references with the same name', () => { | ||
const state = { | ||
type: 'type', | ||
id: 'id', | ||
attributes: { | ||
references: [{ name: 'name', type: 'type', id: '1' }], | ||
}, | ||
}; | ||
const injectedReferences = [{ name: 'name', type: 'type', id: '2' }]; | ||
expect(inject(state, injectedReferences)).toEqual({ | ||
...state, | ||
attributes: { | ||
...state.attributes, | ||
references: injectedReferences, | ||
}, | ||
}); | ||
}); | ||
|
||
it('should clear references if state has no references with the same name', () => { | ||
const state = { | ||
type: 'type', | ||
id: 'id', | ||
attributes: { | ||
references: [{ name: 'name', type: 'type', id: '1' }], | ||
}, | ||
}; | ||
const injectedReferences = [{ name: 'other', type: 'type', id: '2' }]; | ||
expect(inject(state, injectedReferences)).toEqual({ | ||
...state, | ||
attributes: { | ||
...state.attributes, | ||
references: [], | ||
}, | ||
}); | ||
}); | ||
}); | ||
|
||
describe('extract', () => { | ||
it('should not extract references if state does not have attributes', () => { | ||
const state = { type: 'type', id: 'id' }; | ||
expect(extract(state)).toEqual({ state, references: [] }); | ||
}); | ||
|
||
it('should extract references if state has references', () => { | ||
const state = { | ||
type: 'type', | ||
id: 'id', | ||
attributes: { | ||
references: [{ name: 'name', type: 'type', id: '1' }], | ||
}, | ||
}; | ||
expect(extract(state)).toEqual({ | ||
state, | ||
references: [{ name: 'name', type: 'type', id: '1' }], | ||
}); | ||
}); | ||
}); | ||
}); |
52 changes: 52 additions & 0 deletions
52
src/plugins/discover/common/embeddable/search_inject_extract.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
/* | ||
* 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 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 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import type { SavedObjectReference } from '@kbn/core-saved-objects-server'; | ||
import type { EmbeddableStateWithType } from '@kbn/embeddable-plugin/common'; | ||
import type { SearchByValueInput } from '@kbn/saved-search-plugin/public'; | ||
|
||
export const inject = ( | ||
state: EmbeddableStateWithType, | ||
injectedReferences: SavedObjectReference[] | ||
): EmbeddableStateWithType => { | ||
if (hasAttributes(state)) { | ||
// Filter out references that are not in the state | ||
// https://github.com/elastic/kibana/pull/119079 | ||
const references = state.attributes.references | ||
.map((stateRef) => | ||
injectedReferences.find((injectedRef) => injectedRef.name === stateRef.name) | ||
) | ||
.filter(Boolean); | ||
|
||
state = { | ||
...state, | ||
attributes: { | ||
...state.attributes, | ||
references, | ||
}, | ||
} as EmbeddableStateWithType; | ||
} | ||
|
||
return state; | ||
}; | ||
|
||
export const extract = ( | ||
state: EmbeddableStateWithType | ||
): { state: EmbeddableStateWithType; references: SavedObjectReference[] } => { | ||
let references: SavedObjectReference[] = []; | ||
|
||
if (hasAttributes(state)) { | ||
references = state.attributes.references; | ||
} | ||
|
||
return { state, references }; | ||
}; | ||
|
||
const hasAttributes = ( | ||
state: EmbeddableStateWithType | ||
): state is EmbeddableStateWithType & SearchByValueInput => 'attributes' in state; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
37 changes: 37 additions & 0 deletions
37
src/plugins/discover/public/embeddable/get_discover_locator_params.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* 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 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 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { savedSearchMock } from '../__mocks__/saved_search'; | ||
import { getDiscoverLocatorParams } from './get_discover_locator_params'; | ||
import type { SearchInput } from './types'; | ||
|
||
describe('getDiscoverLocatorParams', () => { | ||
it('should return saved search id if input has savedObjectId', () => { | ||
const input = { savedObjectId: 'savedObjectId' } as SearchInput; | ||
expect(getDiscoverLocatorParams({ input, savedSearch: savedSearchMock })).toEqual({ | ||
savedSearchId: 'savedObjectId', | ||
}); | ||
}); | ||
|
||
it('should return Discover params if input has no savedObjectId', () => { | ||
const input = {} as SearchInput; | ||
expect(getDiscoverLocatorParams({ input, savedSearch: savedSearchMock })).toEqual({ | ||
dataViewId: savedSearchMock.searchSource.getField('index')?.id, | ||
dataViewSpec: savedSearchMock.searchSource.getField('index')?.toMinimalSpec(), | ||
timeRange: savedSearchMock.timeRange, | ||
refreshInterval: savedSearchMock.refreshInterval, | ||
filters: savedSearchMock.searchSource.getField('filter'), | ||
query: savedSearchMock.searchSource.getField('query'), | ||
columns: savedSearchMock.columns, | ||
sort: savedSearchMock.sort, | ||
viewMode: savedSearchMock.viewMode, | ||
hideAggregatedPreview: savedSearchMock.hideAggregatedPreview, | ||
breakdownField: savedSearchMock.breakdownField, | ||
}); | ||
}); | ||
}); |
41 changes: 41 additions & 0 deletions
41
src/plugins/discover/public/embeddable/get_discover_locator_params.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
/* | ||
* 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 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 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import type { Filter } from '@kbn/es-query'; | ||
import type { SavedSearch } from '@kbn/saved-search-plugin/common'; | ||
import type { SearchByReferenceInput } from '@kbn/saved-search-plugin/public'; | ||
import type { DiscoverAppLocatorParams } from '../../common'; | ||
import type { SearchInput } from './types'; | ||
|
||
export const getDiscoverLocatorParams = ({ | ||
input, | ||
savedSearch, | ||
}: { | ||
input: SearchInput; | ||
savedSearch: SavedSearch; | ||
}) => { | ||
const dataView = savedSearch.searchSource.getField('index'); | ||
const savedObjectId = (input as SearchByReferenceInput).savedObjectId; | ||
const locatorParams: DiscoverAppLocatorParams = savedObjectId | ||
? { savedSearchId: savedObjectId } | ||
: { | ||
dataViewId: dataView?.id, | ||
dataViewSpec: dataView?.toMinimalSpec(), | ||
timeRange: savedSearch.timeRange, | ||
refreshInterval: savedSearch.refreshInterval, | ||
filters: savedSearch.searchSource.getField('filter') as Filter[], | ||
query: savedSearch.searchSource.getField('query'), | ||
columns: savedSearch.columns, | ||
sort: savedSearch.sort, | ||
viewMode: savedSearch.viewMode, | ||
hideAggregatedPreview: savedSearch.hideAggregatedPreview, | ||
breakdownField: savedSearch.breakdownField, | ||
}; | ||
|
||
return locatorParams; | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@ThomThomson It seems like we have to use the same approach to filtering references as the Lens
inject
function due to the same issue outlined in #119079. When we have a by value saved search using a temporary data view, the saved search has no references which seems to result in the Dashboardinject
code passing in all dashboard references instead:kibana/src/plugins/dashboard/common/dashboard_container/persistable_state/dashboard_container_references.ts
Lines 41 to 45 in 0d8a2da
Let me know if this makes sense to you or if I'm missing something here.
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.
Ah of course, that makes a lot of sense!