Skip to content

Commit

Permalink
[Discover] [ES|QL] Disables sorting for Document view (elastic#187553)
Browse files Browse the repository at this point in the history
## Summary

Disables the `@timestamp` sorting for ES|QL Document view. 

The sorting doesnt work currently. I could enable it but this causes 2
problems:

- The fix is here
https://github.com/elastic/kibana/blob/main/packages/kbn-unified-data-table/src/components/data_table.tsx#L962
The timestamp column is a special column for Discover so the
columns.length is 0 here even if the timestamp column is being rendered.
As a result the inMemory is false and the client side sorting doesnt
work. Removing the columns.length fixes it but it makes Discover
significantly slower.
- As the data are not by default sorted by timestamp even if we enable
it client side, it won't be of great help. I think that for the
timestamp column it would be better to enable server side sorting but
this needs discussion

I think that hiding this for now it will fix the confusion and is a good
temporary decision before we decide what to do with sorting in general.
  • Loading branch information
stratoula authored Jul 8, 2024
1 parent e320935 commit ab3c76d
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,12 @@ export const UnifiedDataTable = ({

const sorting = useMemo(() => {
if (isSortEnabled) {
// in ES|QL mode, sorting is disabled when in Document view
// ideally we want the @timestamp column to be sortable server side
// but it needs discussion before moving forward like this
if (isPlainRecord && !columns.length) {
return undefined;
}
return {
columns: sortingColumns,
onSort: onTableSort,
Expand All @@ -837,7 +843,7 @@ export const UnifiedDataTable = ({
columns: sortingColumns,
onSort: () => {},
};
}, [isSortEnabled, sortingColumns, onTableSort]);
}, [isSortEnabled, sortingColumns, isPlainRecord, columns.length, onTableSort]);

const canSetExpandedDoc = Boolean(setExpandedDoc && !!renderDocumentView);

Expand Down
3 changes: 2 additions & 1 deletion test/functional/apps/discover/esql/_esql_view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(await testSubjects.exists('discoverQueryHits')).to.be(true);
expect(await testSubjects.exists('discoverAlertsButton')).to.be(true);
expect(await testSubjects.exists('shareTopNavButton')).to.be(true);
expect(await testSubjects.exists('dataGridColumnSortingButton')).to.be(true);
// we don't sort for the Document view
expect(await testSubjects.exists('dataGridColumnSortingButton')).to.be(false);
expect(await testSubjects.exists('docTableExpandToggleColumn')).to.be(true);
expect(await testSubjects.exists('fieldListFiltersFieldTypeFilterToggle')).to.be(true);
await testSubjects.click('field-@message-showDetails');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await testSubjects.existOrFail('discoverQueryHits');
await testSubjects.existOrFail('discoverAlertsButton');
await testSubjects.existOrFail('shareTopNavButton');
await testSubjects.existOrFail('dataGridColumnSortingButton');
await testSubjects.missingOrFail('dataGridColumnSortingButton');
await testSubjects.existOrFail('docTableExpandToggleColumn');
await testSubjects.existOrFail('fieldListFiltersFieldTypeFilterToggle');
await testSubjects.click('field-@message-showDetails');
Expand Down

0 comments on commit ab3c76d

Please sign in to comment.