Skip to content

Commit

Permalink
[Saved Queries] Improve saved query management (#170599)
Browse files Browse the repository at this point in the history
## Summary

This PR introduces a number of changes and improvements to saved query
management:
- Add server side pagination (5 queries per page) and search
functionality to the "Load query" list, which improves UX and
performance by no longer requesting all queries at once.
- Redesign the "Load query" list to improve the UX and a11y, making it
possible for keyboard users to effectively navigate the list and
load/delete queries.
- Add an "Active" badge to the "Load query" list to indicate which list
entry represents the currently loaded query, and hoist the entry to the
top of the first page for better visibility when no search term exists.
- Deprecate the saved query `/_all` endpoint and update it to return
only the first 100 queries instead of loading them all into memory at
once.
- Add a new `titleKeyword` field to the saved query SO, which allows
sorting queries alphabetically by title when displaying them in the
"Load query" list.
- Improve the performance of the "has saved queries" check when Unified
Search is mounted to no longer request actual queries, and instead just
request the count.
- Update the saved query duplicate title check to no longer rely on
fetching all queries at once, and instead asynchronously check for
duplicates by title on save.
- Add server side duplicate title validation to the create and update
saved query endpoints.
- Various small fixes and cleanups throughout saved query management.


https://github.com/elastic/kibana/assets/25592674/43328aea-0f7b-4b7a-a5fb-e33ed822f317

Resolves #172044.
Resolves #176427.

## Testing

To generate saved queries for testing, run the script below and replace
`{KIBANA_REQUEST_COOKIE}` with the cookie header value from an API
request of a Kibana user with an active session:
```shell
for i in {1..100}; do curl 'http://localhost:5601/internal/saved_query/_create' \
  -H 'Accept: */*' \
  -H 'Accept-Language: en-US,en;q=0.9,az;q=0.8,es;q=0.7' \
  -H 'Cache-Control: no-cache' \
  -H 'Connection: keep-alive' \
  -H 'Content-Type: application/json' \
  -H 'Cookie: {KIBANA_REQUEST_COOKIE}' \
  -H 'elastic-api-version: 1' \
  -H 'kbn-build-number: 9007199254740991' \
  -H 'kbn-version: 8.13.0' \
  -H 'x-elastic-internal-origin: Kibana' \
  --data-raw '{"title":"query '"$(echo $(($i - 1)) | tr '[0-9]' '[a-j]')"'","description":"","query":{"query":"bytes > 500","language":"kuery"},"filters":[]}' \
  --compressed; done
```

### Checklist

- [x] 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
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] 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)
- [x] 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))
- [x] 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 <[email protected]>
Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
3 people authored Feb 12, 2024
1 parent 385ddf8 commit 509248b
Show file tree
Hide file tree
Showing 42 changed files with 2,204 additions and 859 deletions.
3 changes: 2 additions & 1 deletion packages/kbn-check-mappings-update-cli/current_fields.json
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,8 @@
],
"query": [
"description",
"title"
"title",
"titleKeyword"
],
"risk-engine-configuration": [
"dataViewId",
Expand Down
3 changes: 3 additions & 0 deletions packages/kbn-check-mappings-update-cli/current_mappings.json
Original file line number Diff line number Diff line change
Expand Up @@ -2403,6 +2403,9 @@
},
"title": {
"type": "text"
},
"titleKeyword": {
"type": "keyword"
}
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
"osquery-pack-asset": "cd140bc2e4b092e93692b587bf6e38051ef94c75",
"osquery-saved-query": "6095e288750aa3164dfe186c74bc5195c2bf2bd4",
"policy-settings-protection-updates-note": "33924bb246f9e5bcb876109cc83e3c7a28308352",
"query": "21cbbaa09abb679078145ce90087b1e88b7eae95",
"query": "501bece68f26fe561286a488eabb1a8ab12f1137",
"risk-engine-configuration": "b105d4a3c6adce40708d729d12e5ef3c8fbd9508",
"rules-settings": "892a2918ebaeba809a612b8d97cec0b07c800b5f",
"sample-data-telemetry": "37441b12f5b0159c2d6d5138a494c9f440e950b5",
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/public/query/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const createStartContractMock = () => {
addToQueryLog: jest.fn(),
filterManager: createFilterManagerMock(),
queryString: queryStringManagerMock.createStartContract(),
savedQueries: { getSavedQuery: jest.fn() } as any,
savedQueries: { getSavedQuery: jest.fn(), getSavedQueryCount: jest.fn() } as any,
state$: new Observable(),
getState: jest.fn(),
timefilter: timefilterServiceMock.createStartContract(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import { SAVED_QUERY_BASE_URL } from '../../../common/constants';
const http = httpServiceMock.createStartContract();

const {
isDuplicateTitle,
deleteSavedQuery,
getSavedQuery,
findSavedQueries,
createQuery,
updateQuery,
getAllSavedQueries,
getSavedQueryCount,
} = createSavedQueryService(http);

Expand All @@ -42,6 +42,18 @@ describe('saved query service', () => {
http.delete.mockReset();
});

describe('isDuplicateTitle', function () {
it('should post the title and ID', async () => {
http.post.mockResolvedValue({ isDuplicate: true });
await isDuplicateTitle('foo', 'bar');
expect(http.post).toBeCalled();
expect(http.post).toHaveBeenCalledWith(`${SAVED_QUERY_BASE_URL}/_is_duplicate_title`, {
body: '{"title":"foo","id":"bar"}',
version,
});
});
});

describe('createQuery', function () {
it('should post the stringified given attributes', async () => {
await createQuery(savedQueryAttributes);
Expand All @@ -64,19 +76,6 @@ describe('saved query service', () => {
});
});

describe('getAllSavedQueries', function () {
it('should post and extract the saved queries from the response', async () => {
http.post.mockResolvedValue({
total: 0,
savedQueries: [{ attributes: savedQueryAttributes }],
});
const result = await getAllSavedQueries();
expect(http.post).toBeCalled();
expect(http.post).toHaveBeenCalledWith(`${SAVED_QUERY_BASE_URL}/_all`, { version });
expect(result).toEqual([{ attributes: savedQueryAttributes }]);
});
});

describe('findSavedQueries', function () {
it('should post and return the total & saved queries', async () => {
http.post.mockResolvedValue({
Expand Down
22 changes: 12 additions & 10 deletions src/plugins/data/public/query/saved_query/saved_query_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@ import { SAVED_QUERY_BASE_URL } from '../../../common/constants';
const version = '1';

export const createSavedQueryService = (http: HttpStart) => {
const isDuplicateTitle = async (title: string, id?: string) => {
const response = await http.post<{ isDuplicate: boolean }>(
`${SAVED_QUERY_BASE_URL}/_is_duplicate_title`,
{
body: JSON.stringify({ title, id }),
version,
}
);
return response.isDuplicate;
};

const createQuery = async (attributes: SavedQueryAttributes, { overwrite = false } = {}) => {
const savedQuery = await http.post<SavedQuery>(`${SAVED_QUERY_BASE_URL}/_create`, {
body: JSON.stringify(attributes),
Expand All @@ -30,15 +41,6 @@ export const createSavedQueryService = (http: HttpStart) => {
return savedQuery;
};

// we have to tell the saved objects client how many to fetch, otherwise it defaults to fetching 20 per page
const getAllSavedQueries = async (): Promise<SavedQuery[]> => {
const { savedQueries } = await http.post<{ savedQueries: SavedQuery[] }>(
`${SAVED_QUERY_BASE_URL}/_all`,
{ version }
);
return savedQueries;
};

// findSavedQueries will do a 'match_all' if no search string is passed in
const findSavedQueries = async (
search: string = '',
Expand Down Expand Up @@ -69,9 +71,9 @@ export const createSavedQueryService = (http: HttpStart) => {
};

return {
isDuplicateTitle,
createQuery,
updateQuery,
getAllSavedQueries,
findSavedQueries,
getSavedQuery,
deleteSavedQuery,
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/data/public/query/saved_query/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ export type SavedQueryTimeFilter = TimeRange & {
export type { SavedQuery, SavedQueryAttributes };

export interface SavedQueryService {
isDuplicateTitle: (title: string, id?: string) => Promise<boolean>;
createQuery: (attributes: SavedQueryAttributes) => Promise<SavedQuery>;
updateQuery: (id: string, attributes: SavedQueryAttributes) => Promise<SavedQuery>;
getAllSavedQueries: () => Promise<SavedQuery[]>;
findSavedQueries: (
searchText?: string,
perPage?: number,
Expand Down
Loading

0 comments on commit 509248b

Please sign in to comment.