From 8b2cb75a575a69d17d122f7e971190f8c10b10c8 Mon Sep 17 00:00:00 2001 From: Jiawei Wu <74562234+JiaweiWu@users.noreply.github.com> Date: Tue, 3 Sep 2024 18:31:31 -0700 Subject: [PATCH] [Response Ops] Fix flaky maintenance window E2E tests (#188487) ## Summary Issue: https://github.com/elastic/kibana/issues/187818 Fix flaky test where it's not waiting for the page to load before searching maintenance window. Also slightly improve the maintenance window table to not flicker when loading. Instead it should just show the spinner. ### Checklist - [x] [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 --- .../hooks/use_find_maintenance_windows.ts | 2 + .../maintenance_windows_list.test.tsx | 6 +- .../components/maintenance_windows_list.tsx | 56 ++++++++++++------- .../components/table_actions_popover.test.tsx | 5 ++ .../components/table_actions_popover.tsx | 6 +- .../pages/maintenance_windows/index.tsx | 6 +- .../maintenance_windows_table.ts | 3 +- .../triggers_actions_ui/stack_alerts_page.ts | 12 ++++ 8 files changed, 67 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/alerting/public/hooks/use_find_maintenance_windows.ts b/x-pack/plugins/alerting/public/hooks/use_find_maintenance_windows.ts index ab36720e893a5..7f8293dbd5dd0 100644 --- a/x-pack/plugins/alerting/public/hooks/use_find_maintenance_windows.ts +++ b/x-pack/plugins/alerting/public/hooks/use_find_maintenance_windows.ts @@ -39,6 +39,7 @@ export const useFindMaintenanceWindows = (props?: UseFindMaintenanceWindowsProps const { isLoading, isFetching, + isInitialLoading, data = [], refetch, } = useQuery({ @@ -54,6 +55,7 @@ export const useFindMaintenanceWindows = (props?: UseFindMaintenanceWindowsProps return { maintenanceWindows: data, isLoading: enabled && (isLoading || isFetching), + isInitialLoading, refetch, }; }; diff --git a/x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.test.tsx b/x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.test.tsx index 6efbe04e99b5f..5f9c5b23409c0 100644 --- a/x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.test.tsx +++ b/x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.test.tsx @@ -92,7 +92,7 @@ describe('MaintenanceWindowsList', () => { const result = appMockRenderer.render( {}} - loading={false} + isLoading={false} items={items} readOnly={false} /> @@ -125,7 +125,7 @@ describe('MaintenanceWindowsList', () => { const result = appMockRenderer.render( {}} - loading={false} + isLoading={false} items={items} readOnly={true} /> @@ -142,7 +142,7 @@ describe('MaintenanceWindowsList', () => { const result = appMockRenderer.render( diff --git a/x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.tsx b/x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.tsx index 7e474859981ef..f91856dc8aec9 100644 --- a/x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.tsx +++ b/x-pack/plugins/alerting/public/pages/maintenance_windows/components/maintenance_windows_list.tsx @@ -5,17 +5,17 @@ * 2.0. */ -import React, { ReactElement, useCallback, useMemo } from 'react'; +import React, { useCallback, useMemo } from 'react'; import { formatDate, EuiInMemoryTable, EuiBasicTableColumn, EuiFlexGroup, EuiFlexItem, - SearchFilterConfig, EuiBadge, useEuiTheme, EuiButton, + EuiSearchBarProps, } from '@elastic/eui'; import { css } from '@emotion/react'; import { SortDirection } from '../types'; @@ -35,7 +35,7 @@ import { useArchiveMaintenanceWindow } from '../../../hooks/use_archive_maintena import { useFinishAndArchiveMaintenanceWindow } from '../../../hooks/use_finish_and_archive_maintenance_window'; interface MaintenanceWindowsListProps { - loading: boolean; + isLoading: boolean; items: MaintenanceWindow[]; readOnly: boolean; refreshData: () => void; @@ -99,21 +99,9 @@ const rowProps = (item: MaintenanceWindow) => ({ }); export const MaintenanceWindowsList = React.memo( - ({ loading, items, readOnly, refreshData }) => { - const search: { filters: SearchFilterConfig[]; toolsRight: ReactElement } = { - filters: [ - { - type: 'custom_component', - component: StatusFilter, - }, - ], - toolsRight: ( - - {i18n.REFRESH} - - ), - }; + ({ isLoading, items, readOnly, refreshData }) => { const { euiTheme } = useEuiTheme(); + const { navigateToEditMaintenanceWindows } = useEditMaintenanceWindowsNavigation(); const onEdit = useCallback( (id) => navigateToEditMaintenanceWindows(id), @@ -127,6 +115,7 @@ export const MaintenanceWindowsList = React.memo( ); const { mutate: archiveMaintenanceWindow, isLoading: isLoadingArchive } = useArchiveMaintenanceWindow(); + const onArchive = useCallback( (id: string, archive: boolean) => archiveMaintenanceWindow( @@ -137,11 +126,16 @@ export const MaintenanceWindowsList = React.memo( ); const { mutate: finishAndArchiveMaintenanceWindow, isLoading: isLoadingFinishAndArchive } = useFinishAndArchiveMaintenanceWindow(); + const onCancelAndArchive = useCallback( (id: string) => finishAndArchiveMaintenanceWindow(id, { onSuccess: () => refreshData() }), [finishAndArchiveMaintenanceWindow, refreshData] ); + const isMutatingOrLoading = useMemo(() => { + return isLoadingFinish || isLoadingArchive || isLoadingFinishAndArchive || isLoading; + }, [isLoadingFinish, isLoadingArchive, isLoadingFinishAndArchive, isLoading]); + const tableCss = useMemo(() => { return css` .euiTableRow { @@ -160,6 +154,7 @@ export const MaintenanceWindowsList = React.memo( return ( ( }, }, ], - [onArchive, onCancel, onCancelAndArchive, onEdit] + [isMutatingOrLoading, onArchive, onCancel, onCancelAndArchive, onEdit] ); const columns = useMemo( @@ -178,12 +173,35 @@ export const MaintenanceWindowsList = React.memo( [actions, readOnly] ); + const search: EuiSearchBarProps = useMemo( + () => ({ + filters: [ + { + type: 'custom_component', + component: StatusFilter, + }, + ], + toolsRight: ( + + {i18n.REFRESH} + + ), + }), + [isMutatingOrLoading, refreshData] + ); + return ( { const result = appMockRenderer.render( {}} onCancel={() => {}} @@ -39,6 +40,7 @@ describe('TableActionsPopover', () => { const result = appMockRenderer.render( {}} onCancel={() => {}} @@ -56,6 +58,7 @@ describe('TableActionsPopover', () => { const result = appMockRenderer.render( {}} onCancel={() => {}} @@ -72,6 +75,7 @@ describe('TableActionsPopover', () => { const result = appMockRenderer.render( {}} onCancel={() => {}} @@ -88,6 +92,7 @@ describe('TableActionsPopover', () => { const result = appMockRenderer.render( {}} onCancel={() => {}} diff --git a/x-pack/plugins/alerting/public/pages/maintenance_windows/components/table_actions_popover.tsx b/x-pack/plugins/alerting/public/pages/maintenance_windows/components/table_actions_popover.tsx index c91f518f6976f..b50351eb1188e 100644 --- a/x-pack/plugins/alerting/public/pages/maintenance_windows/components/table_actions_popover.tsx +++ b/x-pack/plugins/alerting/public/pages/maintenance_windows/components/table_actions_popover.tsx @@ -21,6 +21,7 @@ import { MaintenanceWindowStatus } from '../../../../common'; export interface TableActionsPopoverProps { id: string; status: MaintenanceWindowStatus; + isLoading: boolean; onEdit: (id: string) => void; onCancel: (id: string) => void; onArchive: (id: string, archive: boolean) => void; @@ -30,7 +31,7 @@ type ModalType = 'cancel' | 'cancelAndArchive' | 'archive' | 'unarchive'; type ActionType = ModalType | 'edit'; export const TableActionsPopover: React.FC = React.memo( - ({ id, status, onEdit, onCancel, onArchive, onCancelAndArchive }) => { + ({ id, status, isLoading, onEdit, onCancel, onArchive, onCancelAndArchive }) => { const [isPopoverOpen, setIsPopoverOpen] = useState(false); const [isModalVisible, setIsModalVisible] = useState(false); const [modalType, setModalType] = useState(); @@ -197,6 +198,7 @@ export const TableActionsPopover: React.FC = React.mem const button = useMemo( () => ( = React.mem onClick={onButtonClick} /> ), - [onButtonClick] + [isLoading, onButtonClick] ); return ( diff --git a/x-pack/plugins/alerting/public/pages/maintenance_windows/index.tsx b/x-pack/plugins/alerting/public/pages/maintenance_windows/index.tsx index e517625c7212d..f88cba50d8a39 100644 --- a/x-pack/plugins/alerting/public/pages/maintenance_windows/index.tsx +++ b/x-pack/plugins/alerting/public/pages/maintenance_windows/index.tsx @@ -40,7 +40,7 @@ export const MaintenanceWindowsPage = React.memo(() => { const { navigateToCreateMaintenanceWindow } = useCreateMaintenanceWindowNavigation(); - const { isLoading, maintenanceWindows, refetch } = useFindMaintenanceWindows({ + const { isLoading, isInitialLoading, maintenanceWindows, refetch } = useFindMaintenanceWindows({ enabled: hasLicense, }); @@ -81,7 +81,7 @@ export const MaintenanceWindowsPage = React.memo(() => { }; }, [setBadge, chrome]); - if (isLoading) { + if (isInitialLoading) { return ; } @@ -127,7 +127,7 @@ export const MaintenanceWindowsPage = React.memo(() => { diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/maintenance_windows/maintenance_windows_table.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/maintenance_windows/maintenance_windows_table.ts index 3b01650a9c6e8..0b228be252ca3 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/maintenance_windows/maintenance_windows_table.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/maintenance_windows/maintenance_windows_table.ts @@ -20,8 +20,7 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { let objectRemover: ObjectRemover; const browser = getService('browser'); - // Failing: See https://github.com/elastic/kibana/issues/187818 - describe.skip('Maintenance windows table', function () { + describe('Maintenance windows table', function () { before(async () => { objectRemover = await createObjectRemover({ getService }); }); diff --git a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/stack_alerts_page.ts b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/stack_alerts_page.ts index d5ce657fd5981..5fa3a6cc0d9a3 100644 --- a/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/stack_alerts_page.ts +++ b/x-pack/test/functional_with_es_ssl/apps/triggers_actions_ui/stack_alerts_page.ts @@ -34,6 +34,10 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await security.testUser.setRoles(['alerts_and_actions_role']); }); + after(async () => { + await security.testUser.restoreDefaults(); + }); + it('Loads the page', async () => { await pageObjects.common.navigateToUrl( 'management', @@ -77,6 +81,10 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { await security.testUser.setRoles(['only_actions_role']); }); + after(async () => { + await security.testUser.restoreDefaults(); + }); + it('Loads the page but shows missing permission prompt', async () => { await pageObjects.common.navigateToUrl( 'management', @@ -103,6 +111,10 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { ); }); + after(async () => { + await security.testUser.restoreDefaults(); + }); + it('Loads the page', async () => { log.debug('Checking for section heading to say Alerts.');