From 74b759d3c0069c4d98958318b306ddfc7310b161 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Tue, 21 Mar 2023 18:44:24 +0100 Subject: [PATCH 1/9] support custom sorting --- .../notifications_service_provider.ts | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/ml/server/models/notifications_service/notifications_service_provider.ts b/x-pack/plugins/ml/server/models/notifications_service/notifications_service_provider.ts index 5505687525db1..8b9c0263f765b 100644 --- a/x-pack/plugins/ml/server/models/notifications_service/notifications_service_provider.ts +++ b/x-pack/plugins/ml/server/models/notifications_service/notifications_service_provider.ts @@ -154,18 +154,35 @@ export class NotificationsService { sortField: keyof NotificationItem, sortDirection: 'asc' | 'desc' ): (a: NotificationItem, b: NotificationItem) => number { - if (sortField === 'timestamp') { - if (sortDirection === 'asc') { - return (a, b) => a.timestamp - b.timestamp; - } else { - return (a, b) => b.timestamp - a.timestamp; - } - } else { - if (sortDirection === 'asc') { - return (a, b) => (a[sortField] ?? '').localeCompare(b[sortField]); - } else { - return (a, b) => (b[sortField] ?? '').localeCompare(a[sortField]); - } + switch (sortField) { + case 'timestamp': + if (sortDirection === 'asc') { + return (a, b) => a.timestamp - b.timestamp; + } else { + return (a, b) => b.timestamp - a.timestamp; + } + case 'level': + if (sortDirection === 'asc') { + const levelOrder: Record = { + error: 0, + warning: 1, + info: 2, + }; + return (a, b) => levelOrder[a.level] - levelOrder[b.level]; + } else { + const levelOrder: Record = { + error: 2, + warning: 1, + info: 0, + }; + return (a, b) => levelOrder[b.level] - levelOrder[a.level]; + } + default: + if (sortDirection === 'asc') { + return (a, b) => (a[sortField] ?? '').localeCompare(b[sortField]); + } else { + return (a, b) => (b[sortField] ?? '').localeCompare(a[sortField]); + } } } From 753b27e81356671444c0c0078e654b8c64133ad9 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Tue, 21 Mar 2023 18:44:56 +0100 Subject: [PATCH 2/9] functional tests --- .../notifications/notification_list.ts | 5 +++ .../services/ml/common_table_service.ts | 41 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts b/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts index 68a6090bd99e2..e9574734a1f0e 100644 --- a/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts +++ b/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts @@ -58,6 +58,11 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await ml.notifications.table.waitForTableToLoad(); await ml.notifications.table.assertRowsNumberPerPage(25); + await ml.notifications.table.assertTableSorting('timestamp', 0, 'desc'); + }); + + it('support custom sorting for notifications level', async () => { + await ml.notifications.table.sortByField('level', 1, 'desc'); }); it('does not show notifications from another space', async () => { diff --git a/x-pack/test/functional/services/ml/common_table_service.ts b/x-pack/test/functional/services/ml/common_table_service.ts index 50a40ab43f35a..a0b6f651e39bf 100644 --- a/x-pack/test/functional/services/ml/common_table_service.ts +++ b/x-pack/test/functional/services/ml/common_table_service.ts @@ -101,6 +101,47 @@ export function MlTableServiceProvider({ getPageObject, getService }: FtrProvide `Filtered table should have ${expectedRowCount} row(s) for filter '${queryString}' (got ${rows.length} matching items)` ); } + + public async assertTableSorting( + columnName: string, + columnIndex: number, + expectedDirection: 'desc' | 'asc' + ) { + const actualDirection = await this.getCurrentSorting(); + expect(actualDirection?.direction).to.eql(expectedDirection); + expect(actualDirection?.columnName).to.eql(columnName); + } + + public async getCurrentSorting(): Promise< + { columnName: string; direction: string } | undefined + > { + const table = await testSubjects.find(`~${this.tableTestSubj}`); + const headers = await table.findAllByClassName('euiTableHeaderCell'); + for (const header of headers) { + const ariaSort = await header.getAttribute('aria-sort'); + if (ariaSort !== 'none') { + // TODO Add support for column names with _ + const columnName = (await header.getAttribute('data-test-subj')).split('_')[1]; + return { columnName, direction: ariaSort.replace('ending', '') }; + } + } + } + + public async sortByField(columnName: string, columnIndex: number, direction: 'desc' | 'asc') { + const currentSorting = await this.getCurrentSorting(); + + const testSubjString = `tableHeaderCell_${columnName}_${columnIndex}`; + + if (currentSorting && currentSorting.direction !== direction) { + // if current sorting order is different, need to click twice + await testSubjects.click(testSubjString); + await this.waitForTableToLoad(); + } + + await testSubjects.click(testSubjString); + await this.waitForTableToLoad(); + await this.assertTableSorting(columnName, columnIndex, direction); + } }; return { From 30f62157b623c53c631dfcf267aefa47847e1e46 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Thu, 23 Mar 2023 18:59:13 +0100 Subject: [PATCH 3/9] use retry --- .../functional/services/ml/common_table_service.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/x-pack/test/functional/services/ml/common_table_service.ts b/x-pack/test/functional/services/ml/common_table_service.ts index a0b6f651e39bf..ef57e0e9d5b84 100644 --- a/x-pack/test/functional/services/ml/common_table_service.ts +++ b/x-pack/test/functional/services/ml/common_table_service.ts @@ -14,6 +14,7 @@ export type MlTableService = ReturnType; export function MlTableServiceProvider({ getPageObject, getService }: FtrProviderContext) { const testSubjects = getService('testSubjects'); const commonPage = getPageObject('common'); + const retry = getService('retry'); const TableService = class { constructor( @@ -128,19 +129,13 @@ export function MlTableServiceProvider({ getPageObject, getService }: FtrProvide } public async sortByField(columnName: string, columnIndex: number, direction: 'desc' | 'asc') { - const currentSorting = await this.getCurrentSorting(); - const testSubjString = `tableHeaderCell_${columnName}_${columnIndex}`; - if (currentSorting && currentSorting.direction !== direction) { - // if current sorting order is different, need to click twice + await retry.tryForTime(5000, async () => { await testSubjects.click(testSubjString); await this.waitForTableToLoad(); - } - - await testSubjects.click(testSubjString); - await this.waitForTableToLoad(); - await this.assertTableSorting(columnName, columnIndex, direction); + await this.assertTableSorting(columnName, columnIndex, direction); + }); } }; From 60ad884779247a7f4c5af68c94cb85083fecca88 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Thu, 23 Mar 2023 19:22:22 +0100 Subject: [PATCH 4/9] assert level --- .../notifications/notification_list.ts | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts b/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts index e9574734a1f0e..89e640d6543cf 100644 --- a/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts +++ b/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts @@ -5,10 +5,13 @@ * 2.0. */ +import expect from '@kbn/expect'; +import moment from 'moment'; import { FtrProviderContext } from '../../../../ftr_provider_context'; +const timepickerFormat = 'MMM D, YYYY @ HH:mm:ss.SSS'; export default function ({ getService, getPageObjects }: FtrProviderContext) { - const PageObjects = getPageObjects(['common']); + const PageObjects = getPageObjects(['common', 'timePicker']); const esArchiver = getService('esArchiver'); const ml = getService('ml'); const browser = getService('browser'); @@ -61,10 +64,6 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await ml.notifications.table.assertTableSorting('timestamp', 0, 'desc'); }); - it('support custom sorting for notifications level', async () => { - await ml.notifications.table.sortByField('level', 1, 'desc'); - }); - it('does not show notifications from another space', async () => { await ml.notifications.table.filterWithSearchString('Job created', 1); }); @@ -97,5 +96,22 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await browser.refresh(); await ml.notifications.assertNotificationErrorsCount(0); }); + + it('support custom sorting for notifications level', async () => { + await ml.navigation.navigateToNotifications(); + await ml.notifications.table.waitForTableToLoad(); + + await PageObjects.timePicker.pauseAutoRefresh(); + const fromTime = moment().subtract(1, 'week').format(timepickerFormat); + const toTime = moment().format(timepickerFormat); + await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime); + + await ml.notifications.table.waitForTableToLoad(); + + await ml.notifications.table.sortByField('level', 1, 'desc'); + + const rows = await ml.notifications.table.parseTable(); + expect(rows[0].level).to.eql('error'); + }); }); } From d394a9d7e0b51cd1a2093b78de82d07a963f12d2 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Fri, 24 Mar 2023 17:06:23 +0100 Subject: [PATCH 5/9] use basic table --- .../components/notifications_list.tsx | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/ml/public/application/notifications/components/notifications_list.tsx b/x-pack/plugins/ml/public/application/notifications/components/notifications_list.tsx index a2a677670e2ea..7b08bcec0331f 100644 --- a/x-pack/plugins/ml/public/application/notifications/components/notifications_list.tsx +++ b/x-pack/plugins/ml/public/application/notifications/components/notifications_list.tsx @@ -12,7 +12,7 @@ import { FormattedMessage } from '@kbn/i18n-react'; import { EuiBadge, EuiCallOut, - EuiInMemoryTable, + EuiBasicTable, EuiSearchBar, EuiSpacer, IconColor, @@ -294,6 +294,12 @@ export const NotificationsList: FC = () => { const newNotificationsCount = Object.values(notificationsCounts).reduce((a, b) => a + b); + const itemsPerPage = useMemo(() => { + const fromIndex = pagination.pageIndex * pagination.pageSize; + const toIndex = fromIndex + pagination.pageSize; + return items.slice(fromIndex, toIndex); + }, [items, pagination]); + return ( <> @@ -382,12 +388,12 @@ export const NotificationsList: FC = () => { ) : null} - + columns={columns} hasActions={false} isExpandable={false} isSelectable={false} - items={items} + items={itemsPerPage} itemId={'id'} loading={isLoading} rowProps={(item) => ({ @@ -397,7 +403,7 @@ export const NotificationsList: FC = () => { onChange={onTableChange} sorting={sorting} data-test-subj={isLoading ? 'mlNotificationsTable loading' : 'mlNotificationsTable loaded'} - message={ + noItemsMessage={ Date: Fri, 24 Mar 2023 17:26:46 +0100 Subject: [PATCH 6/9] fix, update assertion --- .../notifications_service/notifications_service_provider.ts | 2 +- x-pack/test/functional/services/ml/common_table_service.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/ml/server/models/notifications_service/notifications_service_provider.ts b/x-pack/plugins/ml/server/models/notifications_service/notifications_service_provider.ts index 8b9c0263f765b..57f117561463c 100644 --- a/x-pack/plugins/ml/server/models/notifications_service/notifications_service_provider.ts +++ b/x-pack/plugins/ml/server/models/notifications_service/notifications_service_provider.ts @@ -168,7 +168,7 @@ export class NotificationsService { warning: 1, info: 2, }; - return (a, b) => levelOrder[a.level] - levelOrder[b.level]; + return (a, b) => levelOrder[b.level] - levelOrder[a.level]; } else { const levelOrder: Record = { error: 2, diff --git a/x-pack/test/functional/services/ml/common_table_service.ts b/x-pack/test/functional/services/ml/common_table_service.ts index ef57e0e9d5b84..68ddf703dbd46 100644 --- a/x-pack/test/functional/services/ml/common_table_service.ts +++ b/x-pack/test/functional/services/ml/common_table_service.ts @@ -133,6 +133,7 @@ export function MlTableServiceProvider({ getPageObject, getService }: FtrProvide await retry.tryForTime(5000, async () => { await testSubjects.click(testSubjString); + await this.waitForTableToStartLoading(); await this.waitForTableToLoad(); await this.assertTableSorting(columnName, columnIndex, direction); }); From 6b6f375965b3047a6a6e30f20287736932a2e084 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Mon, 27 Mar 2023 10:11:17 +0200 Subject: [PATCH 7/9] assert for asc sorting --- .../apps/ml/short_tests/notifications/notification_list.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts b/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts index 89e640d6543cf..f0efb5187d22d 100644 --- a/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts +++ b/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts @@ -109,9 +109,12 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await ml.notifications.table.waitForTableToLoad(); await ml.notifications.table.sortByField('level', 1, 'desc'); + const rowsDesc = await ml.notifications.table.parseTable(); + expect(rowsDesc[0].level).to.eql('error'); - const rows = await ml.notifications.table.parseTable(); - expect(rows[0].level).to.eql('error'); + await ml.notifications.table.sortByField('level', 1, 'asc'); + const rowsAsc = await ml.notifications.table.parseTable(); + expect(rowsAsc[0].level).to.eql('info'); }); }); } From ceb2bc5c3366097bacfba3283cc5a063b06a08bb Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Mon, 27 Mar 2023 10:15:37 +0200 Subject: [PATCH 8/9] support for column names with _ --- x-pack/test/functional/services/ml/common_table_service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/functional/services/ml/common_table_service.ts b/x-pack/test/functional/services/ml/common_table_service.ts index 68ddf703dbd46..063c6095c85b5 100644 --- a/x-pack/test/functional/services/ml/common_table_service.ts +++ b/x-pack/test/functional/services/ml/common_table_service.ts @@ -121,8 +121,8 @@ export function MlTableServiceProvider({ getPageObject, getService }: FtrProvide for (const header of headers) { const ariaSort = await header.getAttribute('aria-sort'); if (ariaSort !== 'none') { - // TODO Add support for column names with _ - const columnName = (await header.getAttribute('data-test-subj')).split('_')[1]; + const columnNameFragments = (await header.getAttribute('data-test-subj')).split('_'); + const columnName = columnNameFragments.slice(1, columnNameFragments.length - 1).join('_'); return { columnName, direction: ariaSort.replace('ending', '') }; } } From 757f797ada55aaaf0e820289c9b68bcc64fd5fb2 Mon Sep 17 00:00:00 2001 From: Dima Arnautov Date: Mon, 27 Mar 2023 10:16:07 +0200 Subject: [PATCH 9/9] fix typo --- .../apps/ml/short_tests/notifications/notification_list.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts b/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts index f0efb5187d22d..eb0f99c97e3c0 100644 --- a/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts +++ b/x-pack/test/functional/apps/ml/short_tests/notifications/notification_list.ts @@ -97,7 +97,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) { await ml.notifications.assertNotificationErrorsCount(0); }); - it('support custom sorting for notifications level', async () => { + it('supports custom sorting for notifications level', async () => { await ml.navigation.navigateToNotifications(); await ml.notifications.table.waitForTableToLoad();