Skip to content

Commit

Permalink
[Endpoint] Policy list support for URL pagination state (#63291) (#63402
Browse files Browse the repository at this point in the history
)

* 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
  • Loading branch information
paul-tavares committed Apr 13, 2020
1 parent 9287e22 commit 9f49011
Show file tree
Hide file tree
Showing 11 changed files with 384 additions and 117 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { Reducer } from 'redux';
import { PolicyListState } from '../../types';
import { AppAction } from '../action';
import { isOnPolicyListPage } from './selectors';

const initialPolicyListState = (): PolicyListState => {
return {
Expand All @@ -16,6 +17,7 @@ const initialPolicyListState = (): PolicyListState => {
pageIndex: 0,
pageSize: 10,
total: 0,
location: undefined,
};
};

Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
});
Original file line number Diff line number Diff line change
Expand Up @@ -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`;

Expand Down
Loading

0 comments on commit 9f49011

Please sign in to comment.