[Endpoint] Policy list support for URL pagination state (#63291)

* store changes to support pagination via url

* Fix storing location when pagination happens

* Initial set of tests

* Redux spy middleware and async utility

* Add better types to `waitForAction`

* Add more docs

* fix urlSearchParams selector to account for array of values

* full set of tests for policy list store concerns

* More efficient redux spy middleware (no more sleep())

* Set spy middleware `dispatch` to a `jest.fn` and expose `mock` info.

* Fix url param selector to return first param value when it is defined multiple times

* Removed PageId and associated hook

* clean up TODO items

* Fixes post-merge frm `master`

* Address code review comments

x-pack/plugins/endpoint/public/applications/endpoint/store/policy_list/action.ts

@@ -21,15 +21,4 @@ interface ServerFailedToReturnPolicyListData {
   payload: ServerApiError;
 }
 
-interface UserPaginatedPolicyListTable {
-  type: 'userPaginatedPolicyListTable';
-  payload: {
-    pageSize: number;
-    pageIndex: number;
-  };
-}
-
-export type PolicyListAction =
-  | ServerReturnedPolicyListData
-  | UserPaginatedPolicyListTable
-  | ServerFailedToReturnPolicyListData;
+export type PolicyListAction = ServerReturnedPolicyListData | ServerFailedToReturnPolicyListData;

x-pack/plugins/endpoint/public/applications/endpoint/store/policy_list/index.test.ts

@@ -4,76 +4,192 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { PolicyListState } from '../../types';
+import { EndpointAppLocation, PolicyListState } from '../../types';
 import { applyMiddleware, createStore, Dispatch, Store } from 'redux';
 import { AppAction } from '../action';
 import { policyListReducer } from './reducer';
 import { policyListMiddlewareFactory } from './middleware';
 import { coreMock } from '../../../../../../../../src/core/public/mocks';
-import { CoreStart } from 'kibana/public';
-import { selectIsLoading } from './selectors';
+import { isOnPolicyListPage, selectIsLoading, urlSearchParams } from './selectors';
 import { DepsStartMock, depsStartMock } from '../../mocks';
+import {
+  createSpyMiddleware,
+  MiddlewareActionSpyHelper,
+  setPolicyListApiMockImplementation,
+} from './test_mock_utils';
+import { INGEST_API_DATASOURCES } from './services/ingest';
 
 describe('policy list store concerns', () => {
-  const sleep = () => new Promise(resolve => setTimeout(resolve, 1000));
-  let fakeCoreStart: jest.Mocked<CoreStart>;
+  let fakeCoreStart: ReturnType<typeof coreMock.createStart>;
   let depsStart: DepsStartMock;
   let store: Store<PolicyListState>;
   let getState: typeof store['getState'];
   let dispatch: Dispatch<AppAction>;
+  let waitForAction: MiddlewareActionSpyHelper['waitForAction'];
 
   beforeEach(() => {
     fakeCoreStart = coreMock.createStart({ basePath: '/mock' });
     depsStart = depsStartMock();
+    setPolicyListApiMockImplementation(fakeCoreStart.http);
+    let actionSpyMiddleware;
+    ({ actionSpyMiddleware, waitForAction } = createSpyMiddleware<PolicyListState>());
+
     store = createStore(
       policyListReducer,
-      applyMiddleware(policyListMiddlewareFactory(fakeCoreStart, depsStart))
+      applyMiddleware(policyListMiddlewareFactory(fakeCoreStart, depsStart), actionSpyMiddleware)
     );
     getState = store.getState;
     dispatch = store.dispatch;
   });
 
-  // https://github.com/elastic/kibana/issues/58972
-  test.skip('it sets `isLoading` when `userNavigatedToPage`', async () => {
-    expect(selectIsLoading(getState())).toBe(false);
-    dispatch({ type: 'userNavigatedToPage', payload: 'policyListPage' });
-    expect(selectIsLoading(getState())).toBe(true);
-    await sleep();
-    expect(selectIsLoading(getState())).toBe(false);
+  it('it does nothing on `userChangedUrl` if pathname is NOT `/policy`', async () => {
+    const state = getState();
+    expect(isOnPolicyListPage(state)).toBe(false);
+    dispatch({
+      type: 'userChangedUrl',
+      payload: {
+        pathname: '/foo',
+        search: '',
+        hash: '',
+      } as EndpointAppLocation,
+    });
+    expect(getState()).toEqual(state);
   });
 
-  // https://github.com/elastic/kibana/issues/58896
-  test.skip('it sets `isLoading` when `userPaginatedPolicyListTable`', async () => {
+  it('it reports `isOnPolicyListPage` correctly when router pathname is `/policy`', async () => {
+    dispatch({
+      type: 'userChangedUrl',
+      payload: {
+        pathname: '/policy',
+        search: '',
+        hash: '',
+      },
+    });
+    expect(isOnPolicyListPage(getState())).toBe(true);
+  });
+
+  it('it sets `isLoading` when `userChangedUrl`', async () => {
     expect(selectIsLoading(getState())).toBe(false);
     dispatch({
-      type: 'userPaginatedPolicyListTable',
+      type: 'userChangedUrl',
       payload: {
-        pageSize: 10,
-        pageIndex: 1,
+        pathname: '/policy',
+        search: '',
+        hash: '',
       },
     });
     expect(selectIsLoading(getState())).toBe(true);
-    await sleep();
+    await waitForAction('serverReturnedPolicyListData');
     expect(selectIsLoading(getState())).toBe(false);
   });
 
-  test('it resets state on `userNavigatedFromPage` action', async () => {
+  it('it resets state on `userChangedUrl` and pathname is NOT `/policy`', async () => {
+    dispatch({
+      type: 'userChangedUrl',
+      payload: {
+        pathname: '/policy',
+        search: '',
+        hash: '',
+      },
+    });
+    await waitForAction('serverReturnedPolicyListData');
     dispatch({
-      type: 'serverReturnedPolicyListData',
+      type: 'userChangedUrl',
       payload: {
-        policyItems: [],
-        pageIndex: 20,
-        pageSize: 50,
-        total: 200,
+        pathname: '/foo',
+        search: '',
+        hash: '',
       },
     });
-    dispatch({ type: 'userNavigatedFromPage', payload: 'policyListPage' });
     expect(getState()).toEqual({
+      apiError: undefined,
+      location: undefined,
       policyItems: [],
       isLoading: false,
       pageIndex: 0,
       pageSize: 10,
       total: 0,
     });
   });
+  it('uses default pagination params when not included in url', async () => {
+    dispatch({
+      type: 'userChangedUrl',
+      payload: {
+        pathname: '/policy',
+        search: '',
+        hash: '',
+      },
+    });
+    await waitForAction('serverReturnedPolicyListData');
+    expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
+      query: { kuery: 'datasources.package.name: endpoint', page: 1, perPage: 10 },
+    });
+  });
+
+  describe('when url contains search params', () => {
+    const dispatchUserChangedUrl = (searchParams: string = '') =>
+      dispatch({
+        type: 'userChangedUrl',
+        payload: {
+          pathname: '/policy',
+          search: searchParams,
+          hash: '',
+        },
+      });
+
+    it('uses pagination params from url', async () => {
+      dispatchUserChangedUrl('?page_size=50&page_index=0');
+      await waitForAction('serverReturnedPolicyListData');
+      expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
+        query: { kuery: 'datasources.package.name: endpoint', page: 1, perPage: 50 },
+      });
+    });
+    it('uses defaults for params not in url', async () => {
+      dispatchUserChangedUrl('?page_index=99');
+      expect(urlSearchParams(getState())).toEqual({
+        page_index: 99,
+        page_size: 10,
+      });
+      dispatchUserChangedUrl('?page_size=50');
+      expect(urlSearchParams(getState())).toEqual({
+        page_index: 0,
+        page_size: 50,
+      });
+    });
+    it('accepts only positive numbers for page_index and page_size', async () => {
+      dispatchUserChangedUrl('?page_size=-50&page_index=-99');
+      await waitForAction('serverReturnedPolicyListData');
+      expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
+        query: { kuery: 'datasources.package.name: endpoint', page: 1, perPage: 10 },
+      });
+    });
+    it('it ignores non-numeric values for page_index and page_size', async () => {
+      dispatchUserChangedUrl('?page_size=fifty&page_index=ten');
+      await waitForAction('serverReturnedPolicyListData');
+      expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
+        query: { kuery: 'datasources.package.name: endpoint', page: 1, perPage: 10 },
+      });
+    });
+    it('accepts only known values for `page_size`', async () => {
+      dispatchUserChangedUrl('?page_size=300&page_index=10');
+      await waitForAction('serverReturnedPolicyListData');
+      expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
+        query: { kuery: 'datasources.package.name: endpoint', page: 11, perPage: 10 },
+      });
+    });
+    it(`ignores unknown url search params`, async () => {
+      dispatchUserChangedUrl('?page_size=20&page_index=10&foo=bar');
+      expect(urlSearchParams(getState())).toEqual({
+        page_index: 10,
+        page_size: 20,
+      });
+    });
+    it(`uses last param value if param is defined multiple times`, async () => {
+      dispatchUserChangedUrl('?page_size=20&page_size=50&page_index=20&page_index=40');
+      expect(urlSearchParams(getState())).toEqual({
+        page_index: 20,
+        page_size: 20,
+      });
+    });
+  });
 });

x-pack/plugins/endpoint/public/applications/endpoint/store/policy_list/middleware.ts

@@ -6,29 +6,18 @@
 
 import { MiddlewareFactory, PolicyListState, GetDatasourcesResponse } from '../../types';
 import { sendGetEndpointSpecificDatasources } from './services/ingest';
+import { isOnPolicyListPage, urlSearchParams } from './selectors';
 
 export const policyListMiddlewareFactory: MiddlewareFactory<PolicyListState> = coreStart => {
   const http = coreStart.http;
 
   return ({ getState, dispatch }) => next => async action => {
     next(action);
 
-    if (
-      (action.type === 'userNavigatedToPage' && action.payload === 'policyListPage') ||
-      action.type === 'userPaginatedPolicyListTable'
-    ) {
-      const state = getState();
-      let pageSize: number;
-      let pageIndex: number;
-
-      if (action.type === 'userPaginatedPolicyListTable') {
-        pageSize = action.payload.pageSize;
-        pageIndex = action.payload.pageIndex;
-      } else {
-        pageSize = state.pageSize;
-        pageIndex = state.pageIndex;
-      }
+    const state = getState();
 
+    if (action.type === 'userChangedUrl' && isOnPolicyListPage(state)) {
+      const { page_index: pageIndex, page_size: pageSize } = urlSearchParams(state);
       let response: GetDatasourcesResponse;
 
       try {

x-pack/plugins/endpoint/public/applications/endpoint/store/policy_list/reducer.ts

@@ -7,6 +7,7 @@
 import { Reducer } from 'redux';
 import { PolicyListState } from '../../types';
 import { AppAction } from '../action';
+import { isOnPolicyListPage } from './selectors';
 
 const initialPolicyListState = (): PolicyListState => {
   return {
@@ -16,6 +17,7 @@ const initialPolicyListState = (): PolicyListState => {
     pageIndex: 0,
     pageSize: 10,
     total: 0,
+    location: undefined,
   };
 };
 
@@ -39,19 +41,26 @@ export const policyListReducer: Reducer<PolicyListState, AppAction> = (
     };
   }
 
-  if (
-    action.type === 'userPaginatedPolicyListTable' ||
-    (action.type === 'userNavigatedToPage' && action.payload === 'policyListPage')
-  ) {
-    return {
+  if (action.type === 'userChangedUrl') {
+    const newState = {
       ...state,
-      apiError: undefined,
-      isLoading: true,
+      location: action.payload,
     };
-  }
+    const isCurrentlyOnListPage = isOnPolicyListPage(newState);
+    const wasPreviouslyOnListPage = isOnPolicyListPage(state);
 
-  if (action.type === 'userNavigatedFromPage' && action.payload === 'policyListPage') {
-    return initialPolicyListState();
+    // If on the current page, then return new state with location information
+    // Also adjust some state if user is just entering the policy list view
+    if (isCurrentlyOnListPage) {
+      if (!wasPreviouslyOnListPage) {
+        newState.apiError = undefined;
+        newState.isLoading = true;
+      }
+      return newState;
+    }
+    return {
+      ...initialPolicyListState(),
+    };
   }
 
   return state;

x-pack/plugins/endpoint/public/applications/endpoint/store/policy_list/selectors.ts

@@ -4,7 +4,11 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { PolicyListState } from '../../types';
+import { createSelector } from 'reselect';
+import { parse } from 'query-string';
+import { PolicyListState, PolicyListUrlSearchParams } from '../../types';
+
+const PAGE_SIZES = Object.freeze([10, 20, 50]);
 
 export const selectPolicyItems = (state: PolicyListState) => state.policyItems;
 
@@ -17,3 +21,47 @@ export const selectTotal = (state: PolicyListState) => state.total;
 export const selectIsLoading = (state: PolicyListState) => state.isLoading;
 
 export const selectApiError = (state: PolicyListState) => state.apiError;
+
+export const isOnPolicyListPage = (state: PolicyListState) => {
+  return state.location?.pathname === '/policy';
+};
+
+const routeLocation = (state: PolicyListState) => state.location;
+
+/**
+ * Returns the supported URL search params, populated with defaults if none where present in the URL
+ */
+export const urlSearchParams: (
+  state: PolicyListState
+) => PolicyListUrlSearchParams = createSelector(routeLocation, location => {
+  const searchParams = {
+    page_index: 0,
+    page_size: 10,
+  };
+  if (!location) {
+    return searchParams;
+  }
+
+  const query = parse(location.search);
+
+  // Search params can appear multiple times in the URL, in which case the value for them,
+  // once parsed, would be an array. In these case, we take the first value defined
+  searchParams.page_index = Number(
+    (Array.isArray(query.page_index) ? query.page_index[0] : query.page_index) ?? 0
+  );
+  searchParams.page_size = Number(
+    (Array.isArray(query.page_size) ? query.page_size[0] : query.page_size) ?? 10
+  );
+
+  // If pageIndex is not a valid positive integer, set it to 0
+  if (!Number.isFinite(searchParams.page_index) || searchParams.page_index < 0) {
+    searchParams.page_index = 0;
+  }
+
+  // if pageSize is not one of the expected page sizes, reset it to 10
+  if (!PAGE_SIZES.includes(searchParams.page_size)) {
+    searchParams.page_size = 10;
+  }
+
+  return searchParams;
+});

x-pack/plugins/endpoint/public/applications/endpoint/store/policy_list/services/ingest.ts

@@ -17,7 +17,7 @@ import {
 } from '../../../types';
 
 const INGEST_API_ROOT = `/api/ingest_manager`;
-const INGEST_API_DATASOURCES = `${INGEST_API_ROOT}/datasources`;
+export const INGEST_API_DATASOURCES = `${INGEST_API_ROOT}/datasources`;
 const INGEST_API_FLEET = `${INGEST_API_ROOT}/fleet`;
 const INGEST_API_FLEET_AGENT_STATUS = `${INGEST_API_FLEET}/agent-status`;