[Uptime] Avoid duplicate requests when loading monitor steps (#121889) (#123441)

(cherry picked from commit e02a1f70e1)

x-pack/plugins/uptime/public/components/synthetics/check_steps/use_check_steps.ts

@@ -5,25 +5,34 @@
  * 2.0.
  */
 
+import { useEffect } from 'react';
 import { useParams } from 'react-router-dom';
-import { FETCH_STATUS, useFetcher } from '../../../../../observability/public';
-import { fetchJourneySteps } from '../../../state/api/journey';
+import { useDispatch, useSelector } from 'react-redux';
+import { AppState } from '../../../state';
+import { getJourneySteps } from '../../../state/actions/journey';
 import { JourneyState } from '../../../state/reducers/journey';
 
 export const useCheckSteps = (): JourneyState => {
   const { checkGroupId } = useParams<{ checkGroupId: string }>();
+  const dispatch = useDispatch();
 
-  const { data, status, error } = useFetcher(() => {
-    return fetchJourneySteps({
-      checkGroup: checkGroupId,
-    });
-  }, [checkGroupId]);
+  useEffect(() => {
+    dispatch(
+      getJourneySteps({
+        checkGroup: checkGroupId,
+      })
+    );
+  }, [checkGroupId, dispatch]);
+
+  const checkGroup = useSelector((state: AppState) => {
+    return state.journeys[checkGroupId];
+  });
 
   return {
-    error,
     checkGroup: checkGroupId,
-    steps: data?.steps ?? [],
-    details: data?.details,
-    loading: status === FETCH_STATUS.LOADING || status === FETCH_STATUS.PENDING,
+    steps: checkGroup?.steps ?? [],
+    details: checkGroup?.details,
+    loading: checkGroup?.loading ?? false,
+    error: checkGroup?.error,
   };
 };

x-pack/plugins/uptime/public/lib/helper/helper_with_redux.tsx

@@ -6,19 +6,36 @@
  */
 
 import React from 'react';
-import { Provider as ReduxProvider } from 'react-redux';
-import { AppState } from '../../state';
+import { createStore as createReduxStore, applyMiddleware } from 'redux';
 
-export const MountWithReduxProvider: React.FC<{ state?: AppState }> = ({ children, state }) => (
-  <ReduxProvider
-    store={{
-      dispatch: jest.fn(),
-      getState: jest.fn().mockReturnValue(state || { selectedFilters: null }),
-      subscribe: jest.fn(),
-      replaceReducer: jest.fn(),
-      [Symbol.observable]: jest.fn(),
-    }}
-  >
-    {children}
-  </ReduxProvider>
-);
+import { Provider as ReduxProvider } from 'react-redux';
+import createSagaMiddleware from 'redux-saga';
+
+import { AppState } from '../../state';
+import { rootReducer } from '../../state/reducers';
+import { rootEffect } from '../../state/effects';
+
+const createRealStore = () => {
+  const sagaMW = createSagaMiddleware();
+  const store = createReduxStore(rootReducer, applyMiddleware(sagaMW));
+  sagaMW.run(rootEffect);
+  return store;
+};
+
+export const MountWithReduxProvider: React.FC<{ state?: AppState; useRealStore?: boolean }> = ({
+  children,
+  state,
+  useRealStore,
+}) => {
+  const store = useRealStore
+    ? createRealStore()
+    : {
+        dispatch: jest.fn(),
+        getState: jest.fn().mockReturnValue(state || { selectedFilters: null }),
+        subscribe: jest.fn(),
+        replaceReducer: jest.fn(),
+        [Symbol.observable]: jest.fn(),
+      };
+
+  return <ReduxProvider store={store}>{children}</ReduxProvider>;
+};

x-pack/plugins/uptime/public/lib/helper/rtl_helpers.tsx

@@ -14,7 +14,7 @@ import {
   RenderOptions,
   Nullish,
 } from '@testing-library/react';
-import { Router } from 'react-router-dom';
+import { Router, Route } from 'react-router-dom';
 import { merge } from 'lodash';
 import { createMemoryHistory, History } from 'history';
 import { CoreStart } from 'kibana/public';
@@ -57,6 +57,7 @@ interface MockKibanaProviderProps<ExtraCore> extends KibanaProviderOptions<Extra
 
 interface MockRouterProps<ExtraCore> extends MockKibanaProviderProps<ExtraCore> {
   history?: History;
+  path?: string;
 }
 
 type Url =
@@ -71,6 +72,7 @@ interface RenderRouterOptions<ExtraCore> extends KibanaProviderOptions<ExtraCore
   renderOptions?: Omit<RenderOptions, 'queries'>;
   state?: Partial<AppState> | DeepPartial<AppState>;
   url?: Url;
+  path?: string;
 }
 
 function getSetting<T = any>(key: string): T {
@@ -121,6 +123,9 @@ const mockCore: () => Partial<CoreStart> = () => {
       get: getSetting,
       get$: setSetting$,
     },
+    usageCollection: {
+      reportUiCounter: () => {},
+    },
     triggersActionsUi: triggersActionsUiMock.createStart(),
     storage: createMockStore(),
     data: dataPluginMock.createStartContract(),
@@ -163,19 +168,46 @@ export function MockKibanaProvider<ExtraCore>({
 export function MockRouter<ExtraCore>({
   children,
   core,
+  path,
   history = createMemoryHistory(),
   kibanaProps,
 }: MockRouterProps<ExtraCore>) {
   return (
     <Router history={history}>
       <MockKibanaProvider core={core} kibanaProps={kibanaProps}>
-        {children}
+        <Route path={path}>{children}</Route>
       </MockKibanaProvider>
     </Router>
   );
 }
 configure({ testIdAttribute: 'data-test-subj' });
 
+export const MockRedux = ({
+  state,
+  history = createMemoryHistory(),
+  children,
+  path,
+}: {
+  state: Partial<AppState>;
+  history?: History;
+  children: React.ReactNode;
+  path?: string;
+  useRealStore?: boolean;
+}) => {
+  const testState: AppState = {
+    ...mockState,
+    ...state,
+  };
+
+  return (
+    <MountWithReduxProvider state={testState}>
+      <MockRouter path={path} history={history}>
+        {children}
+      </MockRouter>
+    </MountWithReduxProvider>
+  );
+};
+
 /* Custom react testing library render */
 export function render<ExtraCore>(
   ui: ReactElement,
@@ -186,7 +218,9 @@ export function render<ExtraCore>(
     renderOptions,
     state,
     url,
-  }: RenderRouterOptions<ExtraCore> = {}
+    path,
+    useRealStore,
+  }: RenderRouterOptions<ExtraCore> & { useRealStore?: boolean } = {}
 ) {
   const testState: AppState = merge({}, mockState, state);
 
@@ -196,8 +230,8 @@ export function render<ExtraCore>(
 
   return {
     ...reactTestLibRender(
-      <MountWithReduxProvider state={testState}>
-        <MockRouter history={history} kibanaProps={kibanaProps} core={core}>
+      <MountWithReduxProvider state={testState} useRealStore={useRealStore}>
+        <MockRouter path={path} history={history} kibanaProps={kibanaProps} core={core}>
           {ui}
         </MockRouter>
       </MountWithReduxProvider>,

x-pack/plugins/uptime/public/pages/synthetics/synthetics_checks.test.tsx

@@ -7,53 +7,95 @@
 
 import React from 'react';
 import { render } from '../../lib/helper/rtl_helpers';
-import { spyOnUseFetcher } from '../../lib/helper/spy_use_fetcher';
 import {
   SyntheticsCheckSteps,
   SyntheticsCheckStepsPageHeader,
   SyntheticsCheckStepsPageRightSideItem,
 } from './synthetics_checks';
+import { fetchJourneySteps } from '../../state/api/journey';
+import { createMemoryHistory } from 'history';
+import { SYNTHETIC_CHECK_STEPS_ROUTE } from '../../../common/constants';
+
+jest.mock('../../state/api/journey', () => ({
+  fetchJourneySteps: jest.fn(),
+}));
+
+// We must mock all other API calls because we're using the real store
+// in this test. Using the real store causes actions and effects to actually
+// run, which could trigger API calls.
+jest.mock('../../state/api/utils.ts', () => ({
+  apiService: { get: jest.fn().mockResolvedValue([]) },
+}));
+
+const getRelevantPageHistory = () => {
+  const history = createMemoryHistory();
+  const checkStepsHistoryFrame = SYNTHETIC_CHECK_STEPS_ROUTE.replace(
+    /:checkGroupId/g,
+    'my-check-group-id'
+  );
+
+  history.push(checkStepsHistoryFrame);
+
+  return history;
+};
 
 describe('SyntheticsCheckStepsPageHeader component', () => {
-  it('returns the monitor name', () => {
-    spyOnUseFetcher({
-      details: {
-        journey: {
-          monitor: {
-            name: 'test-name',
-            id: 'test-id',
-          },
-        },
-      },
-    });
-    const { getByText } = render(<SyntheticsCheckStepsPageHeader />);
-    expect(getByText('test-name'));
+  afterAll(() => {
+    jest.restoreAllMocks();
   });
 
-  it('returns the monitor ID when no name is provided', () => {
-    spyOnUseFetcher({
+  it('returns the monitor name', async () => {
+    (fetchJourneySteps as jest.Mock).mockResolvedValueOnce({
+      checkGroup: 'my-check-group-id',
       details: {
         journey: {
-          monitor: {
-            id: 'test-id',
-          },
+          monitor: { name: 'test-name' },
         },
       },
     });
-    const { getByText } = render(<SyntheticsCheckStepsPageHeader />);
-    expect(getByText('test-id'));
+
+    const { findByText } = render(<SyntheticsCheckStepsPageHeader />, {
+      history: getRelevantPageHistory(),
+      path: SYNTHETIC_CHECK_STEPS_ROUTE,
+      useRealStore: true,
+    });
+
+    expect(await findByText('test-name'));
+  });
+
+  it('returns the monitor ID when no name is provided', async () => {
+    (fetchJourneySteps as jest.Mock).mockResolvedValueOnce({
+      checkGroup: 'my-check-group-id',
+      details: {
+        journey: {
+          monitor: { name: 'test-id' },
+        },
+      },
+    });
+
+    const { findByText } = render(<SyntheticsCheckStepsPageHeader />, {
+      history: getRelevantPageHistory(),
+      path: SYNTHETIC_CHECK_STEPS_ROUTE,
+      useRealStore: true,
+    });
+    expect(await findByText('test-id'));
   });
 });
 
 describe('SyntheticsCheckStepsPageRightSideItem component', () => {
   it('returns null when there are no details', () => {
-    spyOnUseFetcher(null);
-    const { container } = render(<SyntheticsCheckStepsPageRightSideItem />);
+    (fetchJourneySteps as jest.Mock).mockResolvedValueOnce(null);
+    const { container } = render(<SyntheticsCheckStepsPageRightSideItem />, {
+      history: getRelevantPageHistory(),
+      path: SYNTHETIC_CHECK_STEPS_ROUTE,
+      useRealStore: true,
+    });
     expect(container.firstChild).toBeNull();
   });
 
-  it('renders navigation element if details exist', () => {
-    spyOnUseFetcher({
+  it('renders navigation element if details exist', async () => {
+    (fetchJourneySteps as jest.Mock).mockResolvedValueOnce({
+      checkGroup: 'my-check-group-id',
       details: {
         timestamp: '20031104',
         journey: {
@@ -64,22 +106,54 @@ describe('SyntheticsCheckStepsPageRightSideItem component', () => {
         },
       },
     });
-    const { getByText } = render(<SyntheticsCheckStepsPageRightSideItem />);
-    expect(getByText('Nov 4, 2003 12:00:00 AM'));
-    expect(getByText('Next check'));
-    expect(getByText('Previous check'));
+
+    const { findByText } = render(<SyntheticsCheckStepsPageRightSideItem />, {
+      history: getRelevantPageHistory(),
+      path: SYNTHETIC_CHECK_STEPS_ROUTE,
+      useRealStore: true,
+    });
+    expect(await findByText('Nov 4, 2003 12:00:00 AM'));
+    expect(await findByText('Next check'));
+    expect(await findByText('Previous check'));
   });
 });
 
 describe('SyntheticsCheckSteps component', () => {
-  it('renders empty steps list', () => {
-    const { getByText } = render(<SyntheticsCheckSteps />);
-    expect(getByText('0 Steps - all failed or skipped'));
-    expect(getByText('This journey did not contain any steps.'));
+  it('renders empty steps list', async () => {
+    (fetchJourneySteps as jest.Mock).mockResolvedValueOnce({
+      checkGroup: 'my-check-group-id',
+      details: {
+        timestamp: '20031104',
+        journey: {
+          monitor: {
+            name: 'test-name',
+            id: 'test-id',
+          },
+        },
+      },
+    });
+
+    const { findByText } = render(<SyntheticsCheckSteps />, {
+      history: getRelevantPageHistory(),
+      path: SYNTHETIC_CHECK_STEPS_ROUTE,
+      useRealStore: true,
+    });
+    expect(await findByText('0 Steps - all failed or skipped'));
+    expect(await findByText('This journey did not contain any steps.'));
   });
 
-  it('renders steps', () => {
-    spyOnUseFetcher({
+  it('renders steps', async () => {
+    (fetchJourneySteps as jest.Mock).mockResolvedValueOnce({
+      checkGroup: 'my-check-group-id',
+      details: {
+        timestamp: '20031104',
+        journey: {
+          monitor: {
+            name: 'test-name',
+            id: 'test-id',
+          },
+        },
+      },
       steps: [
         {
           _id: 'step-1',
@@ -94,7 +168,12 @@ describe('SyntheticsCheckSteps component', () => {
         },
       ],
     });
-    const { getByText } = render(<SyntheticsCheckSteps />);
-    expect(getByText('1 Steps - all failed or skipped'));
+
+    const { findByText } = render(<SyntheticsCheckSteps />, {
+      history: getRelevantPageHistory(),
+      path: SYNTHETIC_CHECK_STEPS_ROUTE,
+      useRealStore: true,
+    });
+    expect(await findByText('1 Steps - all failed or skipped'));
   });
 });

x-pack/plugins/uptime/public/state/effects/journey.test.ts

@@ -0,0 +1,103 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import createSagaMiddleware from 'redux-saga';
+import { createStore as createReduxStore, applyMiddleware } from 'redux';
+
+import { rootReducer } from '../reducers';
+import { fetchJourneyStepsEffect } from '../effects/journey';
+
+import { getJourneySteps } from '../actions/journey';
+
+import { fetchJourneySteps } from '../api/journey';
+
+jest.mock('../api/journey', () => ({
+  fetchJourneySteps: jest.fn(),
+}));
+
+const createTestStore = () => {
+  const sagaMW = createSagaMiddleware();
+  const store = createReduxStore(rootReducer, applyMiddleware(sagaMW));
+  sagaMW.run(fetchJourneyStepsEffect);
+  return store;
+};
+
+describe('journey effect', () => {
+  afterEach(() => jest.resetAllMocks());
+  afterAll(() => jest.restoreAllMocks());
+
+  it('fetches only once when dispatching multiple getJourneySteps for a particular ID', () => {
+    (fetchJourneySteps as jest.Mock).mockResolvedValue({
+      checkGroup: 'saga-test',
+      details: {
+        journey: {
+          monitor: { name: 'test-name' },
+        },
+      },
+    });
+
+    const store = createTestStore();
+
+    // Actually dispatched
+    store.dispatch(getJourneySteps({ checkGroup: 'saga-test' }));
+
+    // Skipped
+    store.dispatch(getJourneySteps({ checkGroup: 'saga-test' }));
+
+    expect(fetchJourneySteps).toHaveBeenCalledTimes(1);
+  });
+
+  it('fetches multiple times for different IDs', () => {
+    (fetchJourneySteps as jest.Mock).mockResolvedValue({
+      checkGroup: 'saga-test',
+      details: {
+        journey: {
+          monitor: { name: 'test-name' },
+        },
+      },
+    });
+
+    const store = createTestStore();
+
+    // Actually dispatched
+    store.dispatch(getJourneySteps({ checkGroup: 'saga-test' }));
+
+    // Skipped
+    store.dispatch(getJourneySteps({ checkGroup: 'saga-test' }));
+
+    // Actually dispatched because it has a different ID
+    store.dispatch(getJourneySteps({ checkGroup: 'saga-test-second' }));
+
+    expect(fetchJourneySteps).toHaveBeenCalledTimes(2);
+  });
+
+  it('can re-fetch after an ID is fetched', async () => {
+    (fetchJourneySteps as jest.Mock).mockResolvedValue({
+      checkGroup: 'saga-test',
+      details: {
+        journey: {
+          monitor: { name: 'test-name' },
+        },
+      },
+    });
+
+    const store = createTestStore();
+
+    const waitForStateUpdate = (): Promise<void> =>
+      new Promise((resolve) => store.subscribe(() => resolve()));
+
+    // Actually dispatched
+    store.dispatch(getJourneySteps({ checkGroup: 'saga-test' }));
+
+    await waitForStateUpdate();
+
+    // Also dispatched given its initial request is not in-flight anymore
+    store.dispatch(getJourneySteps({ checkGroup: 'saga-test' }));
+
+    expect(fetchJourneySteps).toHaveBeenCalledTimes(2);
+  });
+});

x-pack/plugins/uptime/public/state/effects/journey.ts

@@ -6,7 +6,7 @@
  */
 
 import { Action } from 'redux-actions';
-import { call, put, takeLatest } from 'redux-saga/effects';
+import { call, put, takeEvery } from 'redux-saga/effects';
 import {
   getJourneySteps,
   getJourneyStepsSuccess,
@@ -14,14 +14,25 @@ import {
   FetchJourneyStepsParams,
 } from '../actions/journey';
 import { fetchJourneySteps } from '../api/journey';
+import type { SyntheticsJourneyApiResponse } from '../../../common/runtime_types';
+
+const inFlightStepRequests: Record<FetchJourneyStepsParams['checkGroup'], boolean> = {};
 
 export function* fetchJourneyStepsEffect() {
-  yield takeLatest(getJourneySteps, function* (action: Action<FetchJourneyStepsParams>) {
+  yield takeEvery(getJourneySteps, function* (action: Action<FetchJourneyStepsParams>) {
+    if (inFlightStepRequests[action.payload.checkGroup]) return;
+
     try {
-      const response = yield call(fetchJourneySteps, action.payload);
+      inFlightStepRequests[action.payload.checkGroup] = true;
+      const response = (yield call(
+        fetchJourneySteps,
+        action.payload
+      )) as SyntheticsJourneyApiResponse;
       yield put(getJourneyStepsSuccess(response));
     } catch (e) {
       yield put(getJourneyStepsFail({ checkGroup: action.payload.checkGroup, error: e }));
+    } finally {
+      delete inFlightStepRequests[action.payload.checkGroup];
     }
   });
 }