github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-23 09:19:04 -04:00
Code Issues Projects Releases Packages Wiki Activity

[8.15] [Discover] [ES|QL] Disables sorting for Document view (#187553) (#187731)

Browse source 
# Backport

This will backport the following commits from `main` to `8.15`:
- [[Discover] [ES|QL] Disables sorting for Document view
(#187553)](https://github.com/elastic/kibana/pull/187553)

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

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

<!--BACKPORT [{"author":{"name":"Stratoula
Kalafateli","email":"efstratia.kalafateli@elastic.co"},"sourceCommit":{"committedDate":"2024-07-08T09:14:59Z","message":"[Discover]
[ES|QL] Disables sorting for Document view (#187553)\n\n##
Summary\r\n\r\nDisables the `@timestamp` sorting for ES|QL Document
view. \r\n\r\nThe sorting doesnt work currently. I could enable it but
this causes 2\r\nproblems:\r\n\r\n- The fix is
here\r\nhttps://github.com/elastic/kibana/blob/main/packages/kbn-unified-data-table/src/components/data_table.tsx#L962\r\nThe
timestamp column is a special column for Discover so
the\r\ncolumns.length is 0 here even if the timestamp column is being
rendered.\r\nAs a result the inMemory is false and the client side
sorting doesnt\r\nwork. Removing the columns.length fixes it but it
makes Discover\r\nsignificantly slower.\r\n- As the data are not by
default sorted by timestamp even if we enable\r\nit client side, it
won't be of great help. I think that for the\r\ntimestamp column it
would be better to enable server side sorting but\r\nthis needs
discussion\r\n\r\nI think that hiding this for now it will fix the
confusion and is a good\r\ntemporary decision before we decide what to
do with sorting in
general.","sha":"ab3c76dde0a6a862073e0734c95bed037262b346","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:fix","Team:DataDiscovery","backport:prev-minor","Feature:ES|QL","v8.15.0","v8.16.0"],"title":"[Discover]
[ES|QL] Disables sorting for Document
view","number":187553,"url":"https://github.com/elastic/kibana/pull/187553","mergeCommit":{"message":"[Discover]
[ES|QL] Disables sorting for Document view (#187553)\n\n##
Summary\r\n\r\nDisables the `@timestamp` sorting for ES|QL Document
view. \r\n\r\nThe sorting doesnt work currently. I could enable it but
this causes 2\r\nproblems:\r\n\r\n- The fix is
here\r\nhttps://github.com/elastic/kibana/blob/main/packages/kbn-unified-data-table/src/components/data_table.tsx#L962\r\nThe
timestamp column is a special column for Discover so
the\r\ncolumns.length is 0 here even if the timestamp column is being
rendered.\r\nAs a result the inMemory is false and the client side
sorting doesnt\r\nwork. Removing the columns.length fixes it but it
makes Discover\r\nsignificantly slower.\r\n- As the data are not by
default sorted by timestamp even if we enable\r\nit client side, it
won't be of great help. I think that for the\r\ntimestamp column it
would be better to enable server side sorting but\r\nthis needs
discussion\r\n\r\nI think that hiding this for now it will fix the
confusion and is a good\r\ntemporary decision before we decide what to
do with sorting in
general.","sha":"ab3c76dde0a6a862073e0734c95bed037262b346"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"8.15","label":"v8.15.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/187553","number":187553,"mergeCommit":{"message":"[Discover]
[ES|QL] Disables sorting for Document view (#187553)\n\n##
Summary\r\n\r\nDisables the `@timestamp` sorting for ES|QL Document
view. \r\n\r\nThe sorting doesnt work currently. I could enable it but
this causes 2\r\nproblems:\r\n\r\n- The fix is
here\r\nhttps://github.com/elastic/kibana/blob/main/packages/kbn-unified-data-table/src/components/data_table.tsx#L962\r\nThe
timestamp column is a special column for Discover so
the\r\ncolumns.length is 0 here even if the timestamp column is being
rendered.\r\nAs a result the inMemory is false and the client side
sorting doesnt\r\nwork. Removing the columns.length fixes it but it
makes Discover\r\nsignificantly slower.\r\n- As the data are not by
default sorted by timestamp even if we enable\r\nit client side, it
won't be of great help. I think that for the\r\ntimestamp column it
would be better to enable server side sorting but\r\nthis needs
discussion\r\n\r\nI think that hiding this for now it will fix the
confusion and is a good\r\ntemporary decision before we decide what to
do with sorting in
general.","sha":"ab3c76dde0a6a862073e0734c95bed037262b346"}}]}]
BACKPORT-->

Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
This commit is contained in:
Kibana Machine 2024-07-08 12:46:01 +02:00 • committed by GitHub
parent 63232f2a60
commit 828c2cde74
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 10 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

8
packages/kbn-unified-data-table/src/components/data_table.tsx
View file

@ -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,
@ -837,7 +843,7 @@ export const UnifiedDataTable = ({
columns: sortingColumns,
onSort: () => {},
};
}, [isSortEnabled, sortingColumns, onTableSort]);
}, [isSortEnabled, sortingColumns, isPlainRecord, columns.length, onTableSort]);
const canSetExpandedDoc = Boolean(setExpandedDoc && !!renderDocumentView);

3
test/functional/apps/discover/esql/_esql_view.ts
View file

@ -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');

2
x-pack/test_serverless/functional/test_suites/common/discover/esql/_esql_view.ts
View file

@ -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');