Skip to content

Commit

Permalink
[Saved Searches] Add support for saved searches by value (elastic#146849
Browse files Browse the repository at this point in the history
)

## Summary

This PR adds support for saved searches by value. Functionality-wise
this means that similar to other Dashboard panels, saved search panels
now allow unlinking from the library as well as cloning by value instead
of by reference.

Testing guide:
- Test saved searches using both persisted and temporary data views.
- Ensure your saved searches include a query, filters, time range,
columns, sorting, breakdown field, etc.
- Test both the "Unlink from library" and "Save to library"
functionality.
- Test "Clone panel" functionality using both by reference and by value
saved searches (both should clone to a by value saved search).
- Test the "Edit search" button functionality in Dashboard edit mode:
- All saved search configurations should be included when navigating
(search params + persisted & temporary data views).
- Test navigation within the same tab (which will use in-app navigation
to pass state) and opening the link in a new tab (which will use query
params to pass state).
- By reference saved searches should use the
`/app/discover#/view/{savedSearchId}` route.
- By value saved searches using persisted data views should pass all
saved search configurations through the app state (`_a`) query param.
- By value saved searches using temporary data views should use a
locator redirect URL (`/app/r?l=DISCOVER_APP_LOCATOR...`) in order to
support encoding their temporary data view in the URL state.
- Test the "Open in Discover" button functionality in Dashboard view
mode:
- By reference saved searches should open the actual saved search in
Discover.
- By value saved searches should pass all saved search configurations to
Discover.

The following features are not included in this PR and comprise the
remaining work for implementing Time to Visualize for saved searches (to
be done at a later date; issue here: elastic#141629):
- Save and return / state transfer service.
- Save modal with the ability to save directly to a dashboard.

Resolves elastic#148995.
Unblocks elastic#158632.

### Checklist

- [ ] ~Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)~
- [ ]
~[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials~
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] ~Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard
accessibility](https://webaim.org/techniques/keyboard/))~
- [ ] ~Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))~
- [ ] ~If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)~
- [ ] ~This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))~
- [ ] ~This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)~

### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
  • Loading branch information
3 people authored Aug 2, 2023
1 parent 59a4e56 commit 716fb14
Show file tree
Hide file tree
Showing 44 changed files with 1,514 additions and 413 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const dashboardAddToLibraryActionStrings = {
}),
getSuccessMessage: (panelTitle: string) =>
i18n.translate('dashboard.panel.addToLibrary.successMessage', {
defaultMessage: `Panel {panelTitle} was added to the visualize library`,
defaultMessage: `Panel {panelTitle} was added to the library`,
values: { panelTitle },
}),
};
Expand Down Expand Up @@ -91,15 +91,15 @@ export const dashboardUnlinkFromLibraryActionStrings = {
}),
getSuccessMessage: (panelTitle: string) =>
i18n.translate('dashboard.panel.unlinkFromLibrary.successMessage', {
defaultMessage: `Panel {panelTitle} is no longer connected to the visualize library`,
defaultMessage: `Panel {panelTitle} is no longer connected to the library`,
values: { panelTitle },
}),
};

export const dashboardLibraryNotificationStrings = {
getDisplayName: () =>
i18n.translate('dashboard.panel.LibraryNotification', {
defaultMessage: 'Visualize Library Notification',
defaultMessage: 'Library Notification',
}),
getTooltip: () =>
i18n.translate('dashboard.panel.libraryNotification.toolTip', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const emptyScreenStrings = {
}),
getEditModeSubtitle: () =>
i18n.translate('dashboard.emptyScreen.editModeSubtitle', {
defaultMessage: 'Create a visualization of your data, or add one from the Visualize Library.',
defaultMessage: 'Create a visualization of your data, or add one from the library.',
}),
getAddFromLibraryButtonTitle: () =>
i18n.translate('dashboard.emptyScreen.addFromLibrary', {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions src/plugins/discover/common/embeddable/index.ts
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';
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 src/plugins/discover/common/embeddable/search_inject_extract.ts
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;
1 change: 1 addition & 0 deletions src/plugins/discover/public/__mocks__/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ export function createDiscoverServicesMock(): DiscoverServices {
useUrl: jest.fn(() => ''),
navigate: jest.fn(),
getUrl: jest.fn(() => Promise.resolve('')),
getRedirectUrl: jest.fn(() => ''),
},
contextLocator: { getRedirectUrl: jest.fn(() => '') },
singleDocLocator: { getRedirectUrl: jest.fn(() => '') },
Expand Down
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,
});
});
});
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;
};
Loading

0 comments on commit 716fb14

Please sign in to comment.