From 91822c9010ea2c06e0e060c3bed7a507b6365f86 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 30 Aug 2021 17:56:53 -0400 Subject: [PATCH] [Security Solution][Endpoint] Fix activity log load empty state flicker (#110233) (#110526) * account for API errors and uninitialized state before fetching data fixes elastic/kibana/issues/107129 * better name refs elastic/kibana/pull/102261 * don't show date picker when loading data initially fixes elastic/kibana/issues/107129 * use a readable selector instead review changes * remove redundant data fetch using paging action on tab switch. refs elastic/kibana/pull/102261 * remove redundant validation review comments Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Ashokaditya --- .../endpoint_hosts/store/middleware.test.ts | 28 +++++++---- .../pages/endpoint_hosts/store/middleware.ts | 6 ++- .../pages/endpoint_hosts/store/reducer.ts | 4 +- .../pages/endpoint_hosts/store/selectors.ts | 19 ++++++++ .../components/endpoint_details_tabs.tsx | 48 +++---------------- .../view/details/endpoint_activity_log.tsx | 6 ++- 6 files changed, 57 insertions(+), 54 deletions(-) diff --git a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.test.ts b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.test.ts index 17d836695bcf7..e51fe15e7130f 100644 --- a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.test.ts +++ b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.test.ts @@ -286,22 +286,15 @@ describe('endpoint list middleware', () => { it('should set ActivityLog state to loading', async () => { dispatchUserChangedUrl(); - dispatchGetActivityLogLoading(); const loadingDispatched = waitForAction('endpointDetailsActivityLogChanged', { validate(action) { return isLoadingResourceState(action.payload); }, }); + dispatchGetActivityLogLoading(); const loadingDispatchedResponse = await loadingDispatched; - expect(mockedApis.responseProvider.activityLogResponse).toHaveBeenCalledWith({ - path: expect.any(String), - query: { - page: 1, - page_size: 50, - }, - }); expect(loadingDispatchedResponse.payload.type).toEqual('LoadingResourceState'); }); @@ -343,6 +336,25 @@ describe('endpoint list middleware', () => { expect(failedAction.error).toBe(apiError); }); + it('should not call API again if it fails', async () => { + dispatchUserChangedUrl(); + + const apiError = new Error('oh oh'); + const failedDispatched = waitForAction('endpointDetailsActivityLogChanged', { + validate(action) { + return isFailedResourceState(action.payload); + }, + }); + + mockedApis.responseProvider.activityLogResponse.mockImplementation(() => { + throw apiError; + }); + + await failedDispatched; + + expect(mockedApis.responseProvider.activityLogResponse).toHaveBeenCalledTimes(1); + }); + it('should not fetch Activity Log with invalid date ranges', async () => { dispatchUserChangedUrl(); diff --git a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.ts b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.ts index 1be9ff5be55ef..df4361a6048a8 100644 --- a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.ts +++ b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.ts @@ -33,11 +33,13 @@ import { getActivityLogData, getActivityLogDataPaging, getLastLoadedActivityLogData, + getActivityLogError, detailsData, getIsEndpointPackageInfoUninitialized, getIsOnEndpointDetailsActivityLog, getMetadataTransformStats, isMetadataTransformStatsLoading, + getActivityLogIsUninitializedOrHasSubsequentAPIError, } from './selectors'; import { AgentIdsPendingActions, @@ -124,7 +126,8 @@ export const endpointMiddlewareFactory: ImmutableMiddlewareFactory { - const pagingOptions = + const updatedActivityLog = action.payload.type === 'LoadedResourceState' ? { ...state.endpointDetails.activityLog, @@ -53,7 +53,7 @@ const handleEndpointDetailsActivityLogChanged: CaseReducer +) => boolean = createSelector(getActivityLogData, (activityLog) => + isUninitialisedResourceState(activityLog) +); + export const getActivityLogRequestLoading: ( state: Immutable ) => boolean = createSelector(getActivityLogData, (activityLog) => @@ -413,6 +419,19 @@ export const getActivityLogError: ( } }); +// returns a true if either lgo is uninitialised +// or if it has failed an api call after having fetched a non empty log list earlier +export const getActivityLogIsUninitializedOrHasSubsequentAPIError: ( + state: Immutable +) => boolean = createSelector( + getActivityLogUninitialized, + getLastLoadedActivityLogData, + getActivityLogError, + (isUninitialized, lastLoadedLogData, isAPIError) => { + return isUninitialized || (!isAPIError && !!lastLoadedLogData?.data.length); + } +); + export const getIsEndpointHostIsolated = createSelector(detailsData, (details) => { return (details && isEndpointHostIsolated(details)) || false; }); diff --git a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/view/details/components/endpoint_details_tabs.tsx b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/view/details/components/endpoint_details_tabs.tsx index a6e571dbd7df9..8f044959f4c90 100644 --- a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/view/details/components/endpoint_details_tabs.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/view/details/components/endpoint_details_tabs.tsx @@ -5,13 +5,10 @@ * 2.0. */ -import { useDispatch } from 'react-redux'; -import React, { memo, useCallback, useMemo } from 'react'; -import { EuiTab, EuiTabs, EuiFlyoutBody, EuiTabbedContentTab, EuiSpacer } from '@elastic/eui'; +import React, { memo, useMemo } from 'react'; +import { EuiTab, EuiTabs, EuiFlyoutBody, EuiSpacer } from '@elastic/eui'; import { EndpointIndexUIQueryParams } from '../../../types'; -import { EndpointAction } from '../../../store/action'; -import { useEndpointSelector } from '../../hooks'; -import { getActivityLogDataPaging } from '../../../store/selectors'; + import { EndpointDetailsFlyoutHeader } from './flyout_header'; import { useNavigateByRouterEventHandler } from '../../../../../../common/hooks/endpoint/use_navigate_by_router_event_handler'; import { useAppUrl } from '../../../../../../common/lib/kibana'; @@ -33,17 +30,9 @@ interface EndpointDetailsTabs { } const EndpointDetailsTab = memo( - ({ - tab, - isSelected, - handleTabClick, - }: { - tab: EndpointDetailsTabs; - isSelected: boolean; - handleTabClick: () => void; - }) => { + ({ tab, isSelected }: { tab: EndpointDetailsTabs; isSelected: boolean }) => { const { getAppUrl } = useAppUrl(); - const onClick = useNavigateByRouterEventHandler(tab.route, handleTabClick); + const onClick = useNavigateByRouterEventHandler(tab.route); return ( { - const dispatch = useDispatch<(action: EndpointAction) => void>(); - const { pageSize } = useEndpointSelector(getActivityLogDataPaging); - - const handleTabClick = useCallback( - (tab: EuiTabbedContentTab) => { - if (tab.id === EndpointDetailsTabsTypes.activityLog) { - dispatch({ - type: 'endpointDetailsActivityLogUpdatePaging', - payload: { - disabled: false, - page: 1, - pageSize, - startDate: undefined, - endDate: undefined, - }, - }); - } - }, - [dispatch, pageSize] - ); - const selectedTab = useMemo(() => tabs.find((tab) => tab.id === show), [tabs, show]); const renderTabs = tabs.map((tab) => ( - handleTabClick(tab)} - isSelected={tab.id === selectedTab?.id} - /> + )); return ( diff --git a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/view/details/endpoint_activity_log.tsx b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/view/details/endpoint_activity_log.tsx index 121f23fdb3a9e..5172b59450e03 100644 --- a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/view/details/endpoint_activity_log.tsx +++ b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/view/details/endpoint_activity_log.tsx @@ -30,6 +30,7 @@ import { getActivityLogRequestLoaded, getLastLoadedActivityLogData, getActivityLogRequestLoading, + getActivityLogUninitialized, } from '../../store/selectors'; const StyledEuiFlexGroup = styled(EuiFlexGroup)<{ isShorter: boolean }>` @@ -42,6 +43,7 @@ const LoadMoreTrigger = styled.div` export const EndpointActivityLog = memo( ({ activityLog }: { activityLog: AsyncResourceState> }) => { + const activityLogUninitialized = useEndpointSelector(getActivityLogUninitialized); const activityLogLoading = useEndpointSelector(getActivityLogRequestLoading); const activityLogLoaded = useEndpointSelector(getActivityLogRequestLoaded); const activityLastLogData = useEndpointSelector(getLastLoadedActivityLogData); @@ -96,7 +98,9 @@ export const EndpointActivityLog = memo( return ( <> - {showEmptyState ? ( + {(activityLogLoading && !activityLastLogData?.data.length) || activityLogUninitialized ? ( + + ) : showEmptyState ? (