Skip to content

Commit

Permalink
[8.x] [ES|QL] Don't store duplicate queries even of different fo…
Browse files Browse the repository at this point in the history
…rmatting (elastic#194267) (elastic#194316)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ES|QL] Don't store duplicate queries even of different
formatting (elastic#194267)](elastic#194267)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Stratoula
Kalafateli","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-27T15:25:55Z","message":"[ES|QL]
Don't store duplicate queries even of different formatting
(elastic#194267)\n\n## Summary\r\n\r\nOne of the things that bother me in the
history component is that you\r\ncan have different spaces in the
queries but they query is actually the\r\nsame. These appear 2 times
which I find very annoying.\r\n\r\nThis PR identifies if the query is
the same, regardless of the\r\nformatting, and if yes it will display it
only once (it actually updates\r\nthe cache old item with the new
one)\r\n\r\n### Checklist\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"21a8bf8c20e4324312ce80d599245435d299d958","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","Feature:ES|QL","Team:ESQL","v8.16.0"],"title":"[ES|QL]
Don't store duplicate queries even of different
formatting","number":194267,"url":"https://github.com/elastic/kibana/pull/194267","mergeCommit":{"message":"[ES|QL]
Don't store duplicate queries even of different formatting
(elastic#194267)\n\n## Summary\r\n\r\nOne of the things that bother me in the
history component is that you\r\ncan have different spaces in the
queries but they query is actually the\r\nsame. These appear 2 times
which I find very annoying.\r\n\r\nThis PR identifies if the query is
the same, regardless of the\r\nformatting, and if yes it will display it
only once (it actually updates\r\nthe cache old item with the new
one)\r\n\r\n### Checklist\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"21a8bf8c20e4324312ce80d599245435d299d958"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194267","number":194267,"mergeCommit":{"message":"[ES|QL]
Don't store duplicate queries even of different formatting
(elastic#194267)\n\n## Summary\r\n\r\nOne of the things that bother me in the
history component is that you\r\ncan have different spaces in the
queries but they query is actually the\r\nsame. These appear 2 times
which I find very annoying.\r\n\r\nThis PR identifies if the query is
the same, regardless of the\r\nformatting, and if yes it will display it
only once (it actually updates\r\nthe cache old item with the new
one)\r\n\r\n### Checklist\r\n- [ ] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"21a8bf8c20e4324312ce80d599245435d299d958"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Stratoula Kalafateli <[email protected]>
  • Loading branch information
kibanamachine and stratoula authored Sep 27, 2024
1 parent 9b406b4 commit 1eb88db
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 10 deletions.
18 changes: 18 additions & 0 deletions packages/kbn-esql-editor/src/history_local_storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,24 @@ describe('history local storage', function () {
);
});

it('should update queries to cache correctly if they are the same with different format', function () {
addQueriesToCache({
queryString: 'from kibana_sample_data_flights | limit 10 | stats meow = avg(woof) ',
timeZone: 'Browser',
status: 'success',
});

const historyItems = getCachedQueries();
expect(historyItems.length).toBe(2);
expect(historyItems[1].timeRan).toBeDefined();
expect(historyItems[1].status).toBe('success');

expect(mockSetItem).toHaveBeenCalledWith(
'QUERY_HISTORY_ITEM_KEY',
JSON.stringify(historyItems)
);
});

it('should allow maximum x queries ', function () {
addQueriesToCache(
{
Expand Down
15 changes: 5 additions & 10 deletions packages/kbn-esql-editor/src/history_local_storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export interface QueryHistoryItem {
const MAX_QUERIES_NUMBER = 20;

const getKey = (queryString: string) => {
return queryString.replaceAll('\n', '').trim();
return queryString.replaceAll('\n', '').trim().replace(/\s\s+/g, ' ');
};

const getMomentTimeZone = (timeZone?: string) => {
Expand Down Expand Up @@ -60,6 +60,9 @@ export const addQueriesToCache = (
item: QueryHistoryItem,
maxQueriesAllowed = MAX_QUERIES_NUMBER
) => {
// if the user is working on multiple tabs
// the cachedQueries Map might not contain all
// the localStorage queries
const queries = getHistoryItems('desc');
queries.forEach((queryItem) => {
const trimmedQueryString = getKey(queryItem.queryString);
Expand All @@ -77,15 +80,7 @@ export const addQueriesToCache = (
});
}

const queriesToStore = getCachedQueries();
const localStorageQueries = getHistoryItems('desc');
// if the user is working on multiple tabs
// the cachedQueries Map might not contain all
// the localStorage queries
const newQueries = localStorageQueries.filter(
(ls) => !queriesToStore.find((cachedQuery) => cachedQuery.queryString === ls.queryString)
);
let allQueries = [...queriesToStore, ...newQueries];
let allQueries = [...getCachedQueries()];

if (allQueries.length >= maxQueriesAllowed + 1) {
const sortedByDate = allQueries.sort((a, b) =>
Expand Down

0 comments on commit 1eb88db

Please sign in to comment.