Skip to content

Commit

Permalink
[ES|QL] Removes last duration column (elastic#192253)
Browse files Browse the repository at this point in the history
## Summary

This PR removes the last duration column as discussed with the team as
we decided to get this information from ES. Until then the column gives
very wrong results so we decided to remove it

### Checklist

- [ ] [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
  • Loading branch information
stratoula authored Sep 10, 2024
1 parent 62119fb commit 4e8b050
Show file tree
Hide file tree
Showing 11 changed files with 16 additions and 152 deletions.
3 changes: 0 additions & 3 deletions packages/kbn-text-based-editor/src/editor_footer/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ interface EditorFooterProps {
isSpaceReduced?: boolean;
hideTimeFilterInfo?: boolean;
hideQueryHistory?: boolean;
refetchHistoryItems?: boolean;
isInCompactMode?: boolean;
}

Expand All @@ -71,7 +70,6 @@ export const EditorFooter = memo(function EditorFooter({
isHistoryOpen,
setIsHistoryOpen,
hideQueryHistory,
refetchHistoryItems,
isInCompactMode,
measuredContainerWidth,
code,
Expand Down Expand Up @@ -360,7 +358,6 @@ export const EditorFooter = memo(function EditorFooter({
containerCSS={styles.historyContainer}
onUpdateAndSubmit={onUpdateAndSubmit}
containerWidth={measuredContainerWidth}
refetchHistoryItems={refetchHistoryItems}
isInCompactMode={isInCompactMode}
/>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

import React from 'react';
import { QueryHistoryAction, getTableColumns, QueryHistory, QueryColumn } from './query_history';
import { QueryHistoryAction, getTableColumns, QueryColumn } from './query_history';
import { render, screen } from '@testing-library/react';

jest.mock('../history_local_storage', () => {
Expand All @@ -21,7 +21,6 @@ jest.mock('../history_local_storage', () => {
timeZone: 'Browser',
timeRan: 'Mar. 25, 24 08:45:27',
queryRunning: false,
duration: '2ms',
status: 'success',
},
],
Expand Down Expand Up @@ -78,16 +77,6 @@ describe('QueryHistory', () => {
sortable: true,
width: '240px',
},
{
'data-test-subj': 'lastDuration',
field: 'duration',
name: 'Last duration',
sortable: false,
width: '120px',
css: {
justifyContent: 'flex-end',
},
},
{
actions: [],
'data-test-subj': 'actions',
Expand Down Expand Up @@ -126,16 +115,6 @@ describe('QueryHistory', () => {
name: 'Recent queries',
render: expect.anything(),
},
{
'data-test-subj': 'lastDuration',
field: 'duration',
name: 'Last duration',
sortable: false,
width: 'auto',
css: {
justifyContent: 'flex-end',
},
},
{
actions: [],
'data-test-subj': 'actions',
Expand All @@ -145,27 +124,6 @@ describe('QueryHistory', () => {
]);
});

describe('QueryHistory component', () => {
it('should not fetch the query items if refetchHistoryItems is not given', async () => {
render(<QueryHistory onUpdateAndSubmit={jest.fn()} containerWidth={400} containerCSS={''} />);
expect(screen.getByRole('table')).toHaveTextContent('No items found');
});

it('should fetch the query items if refetchHistoryItems is given ', async () => {
render(
<QueryHistory
onUpdateAndSubmit={jest.fn()}
containerWidth={400}
containerCSS={''}
refetchHistoryItems
/>
);
expect(screen.getByRole('table')).toHaveTextContent(
'Time ranRecent queriesLast durationMar. 25, 24 08:45:27from kibana_sample_data_flights | limit 102ms'
);
});
});

describe('Querystring column', () => {
it('should not render the expanded button for large viewports', async () => {
render(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,19 +211,6 @@ export const getTableColumns = (
render: (timeRan: QueryHistoryItem['timeRan']) => timeRan,
width: isOnReducedSpaceLayout ? 'auto' : '240px',
},
{
field: 'duration',
'data-test-subj': 'lastDuration',
name: i18n.translate(
'textBasedEditor.query.textBasedLanguagesEditor.lastDurationColumnLabel',
{
defaultMessage: 'Last duration',
}
),
sortable: false,
width: isOnReducedSpaceLayout ? 'auto' : '120px',
css: { justifyContent: 'flex-end' as const }, // right alignment
},
{
name: '',
actions,
Expand All @@ -239,27 +226,18 @@ export const getTableColumns = (
export function QueryHistory({
containerCSS,
containerWidth,
refetchHistoryItems,
onUpdateAndSubmit,
isInCompactMode,
}: {
containerCSS: Interpolation<Theme>;
containerWidth: number;
onUpdateAndSubmit: (qs: string) => void;
refetchHistoryItems?: boolean;
isInCompactMode?: boolean;
}) {
const theme = useEuiTheme();
const scrollBarStyles = euiScrollBarStyles(theme);
const [sortDirection, setSortDirection] = useState<'asc' | 'desc'>('desc');
const [historyItems, setHistoryItems] = useState<QueryHistoryItem[]>([]);

useEffect(() => {
if (refetchHistoryItems) {
// get history items from local storage
setHistoryItems(getHistoryItems(sortDirection));
}
}, [refetchHistoryItems, sortDirection]);
const historyItems: QueryHistoryItem[] = getHistoryItems(sortDirection);

const actions: Array<CustomItemAction<QueryHistoryItem>> = useMemo(() => {
return [
Expand Down Expand Up @@ -362,9 +340,6 @@ export function QueryHistory({
vertical-align: top;
border: none;
}
.euiTable th[data-test-subj='tableHeaderCell_duration_3'] span {
justify-content: flex-end;
}
border-bottom-left-radius: ${euiTheme.border.radius.medium};
border-top-left-radius: ${euiTheme.border.radius.medium};
max-height: ${isInCompactMode ? CONTAINER_MAX_HEIGHT_COMPACT : CONTAINER_MAX_HEIGHT_EXPANDED}px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ export const getReducedSpaceStyling = () => {
}
.euiTable thead tr {
display: grid;
grid-template-columns: 40px 1fr 0 auto 72px;
grid-template-columns: 40px 1fr 0 auto;
}
.euiTable tbody tr {
display: grid;
grid-template-columns: 40px 1fr auto 72px;
grid-template-columns: 40px 1fr auto;
grid-template-areas:
'status timeRan lastDuration actions'
'. queryString queryString queryString';
Expand Down
19 changes: 3 additions & 16 deletions packages/kbn-text-based-editor/src/history_local_storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
* 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 { addQueriesToCache, getCachedQueries, updateCachedQueries } from './history_local_storage';
import { addQueriesToCache, getCachedQueries } from './history_local_storage';

describe('history local storage', function () {
const mockGetItem = jest.fn();
Expand All @@ -25,27 +24,20 @@ describe('history local storage', function () {
});
const historyItems = getCachedQueries();
expect(historyItems.length).toBe(1);
expect(historyItems[0].queryRunning).toBe(true);
expect(historyItems[0].timeRan).toBeDefined();
expect(historyItems[0].duration).toBeUndefined();
expect(historyItems[0].status).toBeUndefined();
});

it('should update queries to cache correctly ', function () {
addQueriesToCache({
queryString: 'from kibana_sample_data_flights \n | limit 10 \n | stats meow = avg(woof)',
timeZone: 'Browser',
});
updateCachedQueries({
queryString: 'from kibana_sample_data_flights \n | limit 10 \n | stats meow = avg(woof)',
status: 'success',
});

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

expect(mockSetItem).toHaveBeenCalledWith(
Expand All @@ -55,19 +47,14 @@ describe('history local storage', function () {
});

it('should allow maximum x queries ', function () {
addQueriesToCache({
queryString: 'row 1',
timeZone: 'Browser',
});
// allow maximum 2 queries
updateCachedQueries(
addQueriesToCache(
{
queryString: 'row 1',
timeZone: 'Browser',
status: 'success',
},
2
);

const historyItems = getCachedQueries();
expect(historyItems.length).toBe(2);
});
Expand Down
29 changes: 6 additions & 23 deletions packages/kbn-text-based-editor/src/history_local_storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ export interface QueryHistoryItem {
startDateMilliseconds?: number;
timeRan?: string;
timeZone?: string;
duration?: string;
queryRunning?: boolean;
}

const MAX_QUERIES_NUMBER = 20;
Expand Down Expand Up @@ -57,7 +55,11 @@ export const getCachedQueries = (): QueryHistoryItem[] => {
return Array.from(cachedQueries, ([name, value]) => ({ ...value }));
};

export const addQueriesToCache = (item: QueryHistoryItem) => {
// Adding the maxQueriesAllowed here for testing purposes
export const addQueriesToCache = (
item: QueryHistoryItem,
maxQueriesAllowed = MAX_QUERIES_NUMBER
) => {
const queries = getHistoryItems('desc');
queries.forEach((queryItem) => {
const trimmedQueryString = getKey(queryItem.queryString);
Expand All @@ -71,29 +73,10 @@ export const addQueriesToCache = (item: QueryHistoryItem) => {
...item,
timeRan: moment().tz(tz).format(dateFormat),
startDateMilliseconds: moment().valueOf(),
queryRunning: true,
});
}
};
// Adding the maxQueriesAllowed here for testing purposes
export const updateCachedQueries = (
item: QueryHistoryItem,
maxQueriesAllowed = MAX_QUERIES_NUMBER
) => {
const trimmedQueryString = getKey(item.queryString);
const query = cachedQueries.get(trimmedQueryString);

if (query) {
const now = moment().valueOf();
const duration = moment(now).diff(moment(query?.startDateMilliseconds));
cachedQueries.set(trimmedQueryString, {
...query,
timeRan: query.queryRunning ? query.timeRan : moment().format('MMM. D, YY HH:mm:ss'),
duration: query.queryRunning ? `${duration}ms` : query.duration,
status: item.status,
queryRunning: false,
});
}

const queriesToStore = getCachedQueries();
const localStorageQueries = getHistoryItems('desc');
// if the user is working on multiple tabs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,26 +112,6 @@ describe('TextBasedLanguagesEditor', () => {
).toStrictEqual('LIMIT 1000 rows');
});

it('should render the query history action if isLoading is defined', async () => {
const newProps = {
...props,
isLoading: true,
};
const component = mount(renderTextBasedLanguagesEditorComponent({ ...newProps }));
expect(
component.find('[data-test-subj="TextBasedLangEditor-toggle-query-history-button-container"]')
.length
).not.toBe(0);
});

it('should not render the query history action if isLoading is undefined', async () => {
const component = mount(renderTextBasedLanguagesEditorComponent({ ...props }));
expect(
component.find('[data-test-subj="TextBasedLangEditor-toggle-query-history-button-container"]')
.length
).toBe(0);
});

it('should not render the query history action if hideQueryHistory is set to true', async () => {
const newProps = {
...props,
Expand Down
19 changes: 3 additions & 16 deletions packages/kbn-text-based-editor/src/text_based_languages_editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import {
useDebounceWithOptions,
type MonacoMessage,
} from './helpers';
import { addQueriesToCache, updateCachedQueries } from './history_local_storage';
import { addQueriesToCache } from './history_local_storage';
import { ResizableButton } from './resizable_button';
import {
EDITOR_INITIAL_HEIGHT,
Expand Down Expand Up @@ -128,12 +128,7 @@ export const TextBasedLanguagesEditor = memo(function TextBasedLanguagesEditor({
errors: [],
warnings: [],
});
const [refetchHistoryItems, setRefetchHistoryItems] = useState(false);

// as the duration on the history component is being calculated from
// the isLoading property, if this property is not defined we want
// to hide the history component
const hideHistoryComponent = hideQueryHistory || isLoading == null;
const hideHistoryComponent = hideQueryHistory;

const onQueryUpdate = useCallback(
(value: string) => {
Expand Down Expand Up @@ -450,19 +445,12 @@ export const TextBasedLanguagesEditor = memo(function TextBasedLanguagesEditor({
}
};
if (isQueryLoading || isLoading) {
validateQuery();
addQueriesToCache({
queryString: code,
timeZone,
});
validateQuery();
setRefetchHistoryItems(false);
} else {
updateCachedQueries({
queryString: code,
status: clientParserStatus,
});

setRefetchHistoryItems(true);
}
}, [clientParserStatus, isLoading, isQueryLoading, parseMessages, code, timeZone]);

Expand Down Expand Up @@ -744,7 +732,6 @@ export const TextBasedLanguagesEditor = memo(function TextBasedLanguagesEditor({
setIsHistoryOpen={toggleHistory}
measuredContainerWidth={measuredEditorWidth}
hideQueryHistory={hideHistoryComponent}
refetchHistoryItems={refetchHistoryItems}
isHelpMenuOpen={isLanguagePopoverOpen}
setIsHelpMenuOpen={setIsLanguagePopoverOpen}
/>
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -7462,7 +7462,6 @@
"textBasedEditor.query.textBasedLanguagesEditor.groupingFunctions": "Fonctions de groupage",
"textBasedEditor.query.textBasedLanguagesEditor.groupingFunctionsDocumentationESQLDescription": "Ces fonctions de regroupement peuvent être utilisées avec `STATS...BY` :",
"textBasedEditor.query.textBasedLanguagesEditor.hideQueriesLabel": "Masquer les recherches récentes",
"textBasedEditor.query.textBasedLanguagesEditor.lastDurationColumnLabel": "Dernière durée",
"textBasedEditor.query.textBasedLanguagesEditor.lineCount": "{count} {count, plural, one {ligne} other {lignes}}",
"textBasedEditor.query.textBasedLanguagesEditor.lineNumber": "Ligne {lineNumber}",
"textBasedEditor.query.textBasedLanguagesEditor.operators": "Opérateurs",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -7456,7 +7456,6 @@
"textBasedEditor.query.textBasedLanguagesEditor.groupingFunctions": "グループ関数",
"textBasedEditor.query.textBasedLanguagesEditor.groupingFunctionsDocumentationESQLDescription": "これらのグループ関数はSTATS...BYで使用できます。",
"textBasedEditor.query.textBasedLanguagesEditor.hideQueriesLabel": "最近のクエリーを非表示",
"textBasedEditor.query.textBasedLanguagesEditor.lastDurationColumnLabel": "前回の期間",
"textBasedEditor.query.textBasedLanguagesEditor.lineCount": "{count} {count, plural, other {行}}",
"textBasedEditor.query.textBasedLanguagesEditor.lineNumber": "行{lineNumber}",
"textBasedEditor.query.textBasedLanguagesEditor.operators": "演算子",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -7469,7 +7469,6 @@
"textBasedEditor.query.textBasedLanguagesEditor.groupingFunctions": "分组函数",
"textBasedEditor.query.textBasedLanguagesEditor.groupingFunctionsDocumentationESQLDescription": "这些分组函数可以与 `STATS...BY` 搭配使用:",
"textBasedEditor.query.textBasedLanguagesEditor.hideQueriesLabel": "隐藏最近查询",
"textBasedEditor.query.textBasedLanguagesEditor.lastDurationColumnLabel": "上次持续时间",
"textBasedEditor.query.textBasedLanguagesEditor.lineCount": "{count} {count, plural, other {行}}",
"textBasedEditor.query.textBasedLanguagesEditor.lineNumber": "第 {lineNumber} 行",
"textBasedEditor.query.textBasedLanguagesEditor.operators": "运算符",
Expand Down

0 comments on commit 4e8b050

Please sign in to comment.