[Security Solution][Endpoint] Fix activity log load empty state flick…

…er (#110233) * account for API errors and uninitialized state before fetching data fixes /issues/107129 * better name refs /pull/102261 * don't show date picker when loading data initially fixes /issues/107129 * use a readable selector instead review changes * remove redundant data fetch using paging action on tab switch. refs /pull/102261 * remove redundant validation review comments Co-authored-by: Kibana Machine <[email protected]>
elastic · Aug 30, 2021 · 22e5565 · 22e5565
1 parent 6e8d86b
commit 22e5565
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 54 deletions.
diff --git a/...plugins/security_solution/public/management/pages/endpoint_hosts/store/middleware.test.ts b/...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
@@ -33,11 +33,13 @@ import {
   getActivityLogData,
   getActivityLogDataPaging,
   getLastLoadedActivityLogData,
+  getActivityLogError,
   detailsData,
   getIsEndpointPackageInfoUninitialized,
   getIsOnEndpointDetailsActivityLog,
   getMetadataTransformStats,
   isMetadataTransformStatsLoading,
+  getActivityLogIsUninitializedOrHasSubsequentAPIError,
 } from './selectors';
 import {
   AgentIdsPendingActions,
@@ -124,14 +126,16 @@ export const endpointMiddlewareFactory: ImmutableMiddlewareFactory<EndpointState
     if (
       action.type === 'userChangedUrl' &&
       hasSelectedEndpoint(getState()) === true &&
-      getIsOnEndpointDetailsActivityLog(getState())
+      getIsOnEndpointDetailsActivityLog(getState()) &&
+      getActivityLogIsUninitializedOrHasSubsequentAPIError(getState())
     ) {
       await endpointDetailsActivityLogChangedMiddleware({ store, coreStart });
     }
 
     // page activity log API
     if (
       action.type === 'endpointDetailsActivityLogUpdatePaging' &&
+      !getActivityLogError(getState()) &&
       hasSelectedEndpoint(getState())
     ) {
       await endpointDetailsActivityLogPagingMiddleware({ store, coreStart });

diff --git a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/reducer.ts b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/reducer.ts
@@ -35,7 +35,7 @@ const handleEndpointDetailsActivityLogChanged: CaseReducer<EndpointDetailsActivi
   state,
   action
 ) => {
-  const pagingOptions =
+  const updatedActivityLog =
     action.payload.type === 'LoadedResourceState'
       ? {
           ...state.endpointDetails.activityLog,
@@ -53,7 +53,7 @@ const handleEndpointDetailsActivityLogChanged: CaseReducer<EndpointDetailsActivi
     endpointDetails: {
       ...state.endpointDetails!,
       activityLog: {
-        ...pagingOptions,
+        ...updatedActivityLog,
         logData: action.payload,
       },
     },

diff --git a/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/selectors.ts b/x-pack/plugins/security_solution/public/management/pages/endpoint_hosts/store/selectors.ts
@@ -386,6 +386,12 @@ export const getLastLoadedActivityLogData: (
   return getLastLoadedResourceState(activityLog)?.data;
 });
 
+export const getActivityLogUninitialized: (
+  state: Immutable<EndpointState>
+) => boolean = createSelector(getActivityLogData, (activityLog) =>
+  isUninitialisedResourceState(activityLog)
+);
+
 export const getActivityLogRequestLoading: (
   state: Immutable<EndpointState>
 ) => 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<EndpointState>
+) => 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/.../public/management/pages/endpoint_hosts/view/details/components/endpoint_details_tabs.tsx b/.../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 (
       <EuiTab
         href={getAppUrl({ path: tab.route })}
@@ -70,35 +59,10 @@ export const EndpointDetailsFlyoutTabs = memo(
     show: EndpointIndexUIQueryParams['show'];
     tabs: EndpointDetailsTabs[];
   }) => {
-    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) => (
-      <EndpointDetailsTab
-        tab={tab}
-        handleTabClick={() => handleTabClick(tab)}
-        isSelected={tab.id === selectedTab?.id}
-      />
+      <EndpointDetailsTab tab={tab} isSelected={tab.id === selectedTab?.id} />
     ));
 
     return (

diff --git a/...ty_solution/public/management/pages/endpoint_hosts/view/details/endpoint_activity_log.tsx b/...ty_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<Immutable<ActivityLog>> }) => {
+    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 (
       <>
         <StyledEuiFlexGroup direction="column" responsive={false} isShorter={isShorter}>
-          {showEmptyState ? (
+          {(activityLogLoading && !activityLastLogData?.data.length) || activityLogUninitialized ? (
+            <EuiLoadingContent lines={3} />
+          ) : showEmptyState ? (
             <EuiFlexItem>
               <EuiEmptyPrompt
                 iconType="editorUnorderedList"