Skip to content

Commit 48fd5c0

Browse files
authored
[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
1 parent 25cedbe commit 48fd5c0

File tree

11 files changed

+384
-117
lines changed

11 files changed

+384
-117
lines changed

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

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,4 @@ interface ServerFailedToReturnPolicyListData {
2121
payload: ServerApiError;
2222
}
2323

24-
interface UserPaginatedPolicyListTable {
25-
type: 'userPaginatedPolicyListTable';
26-
payload: {
27-
pageSize: number;
28-
pageIndex: number;
29-
};
30-
}
31-
32-
export type PolicyListAction =
33-
| ServerReturnedPolicyListData
34-
| UserPaginatedPolicyListTable
35-
| ServerFailedToReturnPolicyListData;
24+
export type PolicyListAction = ServerReturnedPolicyListData | ServerFailedToReturnPolicyListData;

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

Lines changed: 142 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,76 +4,192 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { PolicyListState } from '../../types';
7+
import { EndpointAppLocation, PolicyListState } from '../../types';
88
import { applyMiddleware, createStore, Dispatch, Store } from 'redux';
99
import { AppAction } from '../action';
1010
import { policyListReducer } from './reducer';
1111
import { policyListMiddlewareFactory } from './middleware';
1212
import { coreMock } from '../../../../../../../../src/core/public/mocks';
13-
import { CoreStart } from 'kibana/public';
14-
import { selectIsLoading } from './selectors';
13+
import { isOnPolicyListPage, selectIsLoading, urlSearchParams } from './selectors';
1514
import { DepsStartMock, depsStartMock } from '../../mocks';
15+
import {
16+
createSpyMiddleware,
17+
MiddlewareActionSpyHelper,
18+
setPolicyListApiMockImplementation,
19+
} from './test_mock_utils';
20+
import { INGEST_API_DATASOURCES } from './services/ingest';
1621

1722
describe('policy list store concerns', () => {
18-
const sleep = () => new Promise(resolve => setTimeout(resolve, 1000));
19-
let fakeCoreStart: jest.Mocked<CoreStart>;
23+
let fakeCoreStart: ReturnType<typeof coreMock.createStart>;
2024
let depsStart: DepsStartMock;
2125
let store: Store<PolicyListState>;
2226
let getState: typeof store['getState'];
2327
let dispatch: Dispatch<AppAction>;
28+
let waitForAction: MiddlewareActionSpyHelper['waitForAction'];
2429

2530
beforeEach(() => {
2631
fakeCoreStart = coreMock.createStart({ basePath: '/mock' });
2732
depsStart = depsStartMock();
33+
setPolicyListApiMockImplementation(fakeCoreStart.http);
34+
let actionSpyMiddleware;
35+
({ actionSpyMiddleware, waitForAction } = createSpyMiddleware<PolicyListState>());
36+
2837
store = createStore(
2938
policyListReducer,
30-
applyMiddleware(policyListMiddlewareFactory(fakeCoreStart, depsStart))
39+
applyMiddleware(policyListMiddlewareFactory(fakeCoreStart, depsStart), actionSpyMiddleware)
3140
);
3241
getState = store.getState;
3342
dispatch = store.dispatch;
3443
});
3544

36-
// https://github.com/elastic/kibana/issues/58972
37-
test.skip('it sets `isLoading` when `userNavigatedToPage`', async () => {
38-
expect(selectIsLoading(getState())).toBe(false);
39-
dispatch({ type: 'userNavigatedToPage', payload: 'policyListPage' });
40-
expect(selectIsLoading(getState())).toBe(true);
41-
await sleep();
42-
expect(selectIsLoading(getState())).toBe(false);
45+
it('it does nothing on `userChangedUrl` if pathname is NOT `/policy`', async () => {
46+
const state = getState();
47+
expect(isOnPolicyListPage(state)).toBe(false);
48+
dispatch({
49+
type: 'userChangedUrl',
50+
payload: {
51+
pathname: '/foo',
52+
search: '',
53+
hash: '',
54+
} as EndpointAppLocation,
55+
});
56+
expect(getState()).toEqual(state);
4357
});
4458

45-
// https://github.com/elastic/kibana/issues/58896
46-
test.skip('it sets `isLoading` when `userPaginatedPolicyListTable`', async () => {
59+
it('it reports `isOnPolicyListPage` correctly when router pathname is `/policy`', async () => {
60+
dispatch({
61+
type: 'userChangedUrl',
62+
payload: {
63+
pathname: '/policy',
64+
search: '',
65+
hash: '',
66+
},
67+
});
68+
expect(isOnPolicyListPage(getState())).toBe(true);
69+
});
70+
71+
it('it sets `isLoading` when `userChangedUrl`', async () => {
4772
expect(selectIsLoading(getState())).toBe(false);
4873
dispatch({
49-
type: 'userPaginatedPolicyListTable',
74+
type: 'userChangedUrl',
5075
payload: {
51-
pageSize: 10,
52-
pageIndex: 1,
76+
pathname: '/policy',
77+
search: '',
78+
hash: '',
5379
},
5480
});
5581
expect(selectIsLoading(getState())).toBe(true);
56-
await sleep();
82+
await waitForAction('serverReturnedPolicyListData');
5783
expect(selectIsLoading(getState())).toBe(false);
5884
});
5985

60-
test('it resets state on `userNavigatedFromPage` action', async () => {
86+
it('it resets state on `userChangedUrl` and pathname is NOT `/policy`', async () => {
87+
dispatch({
88+
type: 'userChangedUrl',
89+
payload: {
90+
pathname: '/policy',
91+
search: '',
92+
hash: '',
93+
},
94+
});
95+
await waitForAction('serverReturnedPolicyListData');
6196
dispatch({
62-
type: 'serverReturnedPolicyListData',
97+
type: 'userChangedUrl',
6398
payload: {
64-
policyItems: [],
65-
pageIndex: 20,
66-
pageSize: 50,
67-
total: 200,
99+
pathname: '/foo',
100+
search: '',
101+
hash: '',
68102
},
69103
});
70-
dispatch({ type: 'userNavigatedFromPage', payload: 'policyListPage' });
71104
expect(getState()).toEqual({
105+
apiError: undefined,
106+
location: undefined,
72107
policyItems: [],
73108
isLoading: false,
74109
pageIndex: 0,
75110
pageSize: 10,
76111
total: 0,
77112
});
78113
});
114+
it('uses default pagination params when not included in url', async () => {
115+
dispatch({
116+
type: 'userChangedUrl',
117+
payload: {
118+
pathname: '/policy',
119+
search: '',
120+
hash: '',
121+
},
122+
});
123+
await waitForAction('serverReturnedPolicyListData');
124+
expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
125+
query: { kuery: 'datasources.package.name: endpoint', page: 1, perPage: 10 },
126+
});
127+
});
128+
129+
describe('when url contains search params', () => {
130+
const dispatchUserChangedUrl = (searchParams: string = '') =>
131+
dispatch({
132+
type: 'userChangedUrl',
133+
payload: {
134+
pathname: '/policy',
135+
search: searchParams,
136+
hash: '',
137+
},
138+
});
139+
140+
it('uses pagination params from url', async () => {
141+
dispatchUserChangedUrl('?page_size=50&page_index=0');
142+
await waitForAction('serverReturnedPolicyListData');
143+
expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
144+
query: { kuery: 'datasources.package.name: endpoint', page: 1, perPage: 50 },
145+
});
146+
});
147+
it('uses defaults for params not in url', async () => {
148+
dispatchUserChangedUrl('?page_index=99');
149+
expect(urlSearchParams(getState())).toEqual({
150+
page_index: 99,
151+
page_size: 10,
152+
});
153+
dispatchUserChangedUrl('?page_size=50');
154+
expect(urlSearchParams(getState())).toEqual({
155+
page_index: 0,
156+
page_size: 50,
157+
});
158+
});
159+
it('accepts only positive numbers for page_index and page_size', async () => {
160+
dispatchUserChangedUrl('?page_size=-50&page_index=-99');
161+
await waitForAction('serverReturnedPolicyListData');
162+
expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
163+
query: { kuery: 'datasources.package.name: endpoint', page: 1, perPage: 10 },
164+
});
165+
});
166+
it('it ignores non-numeric values for page_index and page_size', async () => {
167+
dispatchUserChangedUrl('?page_size=fifty&page_index=ten');
168+
await waitForAction('serverReturnedPolicyListData');
169+
expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
170+
query: { kuery: 'datasources.package.name: endpoint', page: 1, perPage: 10 },
171+
});
172+
});
173+
it('accepts only known values for `page_size`', async () => {
174+
dispatchUserChangedUrl('?page_size=300&page_index=10');
175+
await waitForAction('serverReturnedPolicyListData');
176+
expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
177+
query: { kuery: 'datasources.package.name: endpoint', page: 11, perPage: 10 },
178+
});
179+
});
180+
it(`ignores unknown url search params`, async () => {
181+
dispatchUserChangedUrl('?page_size=20&page_index=10&foo=bar');
182+
expect(urlSearchParams(getState())).toEqual({
183+
page_index: 10,
184+
page_size: 20,
185+
});
186+
});
187+
it(`uses last param value if param is defined multiple times`, async () => {
188+
dispatchUserChangedUrl('?page_size=20&page_size=50&page_index=20&page_index=40');
189+
expect(urlSearchParams(getState())).toEqual({
190+
page_index: 20,
191+
page_size: 20,
192+
});
193+
});
194+
});
79195
});

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

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,18 @@
66

77
import { MiddlewareFactory, PolicyListState, GetDatasourcesResponse } from '../../types';
88
import { sendGetEndpointSpecificDatasources } from './services/ingest';
9+
import { isOnPolicyListPage, urlSearchParams } from './selectors';
910

1011
export const policyListMiddlewareFactory: MiddlewareFactory<PolicyListState> = coreStart => {
1112
const http = coreStart.http;
1213

1314
return ({ getState, dispatch }) => next => async action => {
1415
next(action);
1516

16-
if (
17-
(action.type === 'userNavigatedToPage' && action.payload === 'policyListPage') ||
18-
action.type === 'userPaginatedPolicyListTable'
19-
) {
20-
const state = getState();
21-
let pageSize: number;
22-
let pageIndex: number;
23-
24-
if (action.type === 'userPaginatedPolicyListTable') {
25-
pageSize = action.payload.pageSize;
26-
pageIndex = action.payload.pageIndex;
27-
} else {
28-
pageSize = state.pageSize;
29-
pageIndex = state.pageIndex;
30-
}
17+
const state = getState();
3118

19+
if (action.type === 'userChangedUrl' && isOnPolicyListPage(state)) {
20+
const { page_index: pageIndex, page_size: pageSize } = urlSearchParams(state);
3221
let response: GetDatasourcesResponse;
3322

3423
try {

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

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import { Reducer } from 'redux';
88
import { PolicyListState } from '../../types';
99
import { AppAction } from '../action';
10+
import { isOnPolicyListPage } from './selectors';
1011

1112
const initialPolicyListState = (): PolicyListState => {
1213
return {
@@ -16,6 +17,7 @@ const initialPolicyListState = (): PolicyListState => {
1617
pageIndex: 0,
1718
pageSize: 10,
1819
total: 0,
20+
location: undefined,
1921
};
2022
};
2123

@@ -39,19 +41,26 @@ export const policyListReducer: Reducer<PolicyListState, AppAction> = (
3941
};
4042
}
4143

42-
if (
43-
action.type === 'userPaginatedPolicyListTable' ||
44-
(action.type === 'userNavigatedToPage' && action.payload === 'policyListPage')
45-
) {
46-
return {
44+
if (action.type === 'userChangedUrl') {
45+
const newState = {
4746
...state,
48-
apiError: undefined,
49-
isLoading: true,
47+
location: action.payload,
5048
};
51-
}
49+
const isCurrentlyOnListPage = isOnPolicyListPage(newState);
50+
const wasPreviouslyOnListPage = isOnPolicyListPage(state);
5251

53-
if (action.type === 'userNavigatedFromPage' && action.payload === 'policyListPage') {
54-
return initialPolicyListState();
52+
// If on the current page, then return new state with location information
53+
// Also adjust some state if user is just entering the policy list view
54+
if (isCurrentlyOnListPage) {
55+
if (!wasPreviouslyOnListPage) {
56+
newState.apiError = undefined;
57+
newState.isLoading = true;
58+
}
59+
return newState;
60+
}
61+
return {
62+
...initialPolicyListState(),
63+
};
5564
}
5665

5766
return state;

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

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { PolicyListState } from '../../types';
7+
import { createSelector } from 'reselect';
8+
import { parse } from 'query-string';
9+
import { PolicyListState, PolicyListUrlSearchParams } from '../../types';
10+
11+
const PAGE_SIZES = Object.freeze([10, 20, 50]);
812

913
export const selectPolicyItems = (state: PolicyListState) => state.policyItems;
1014

@@ -17,3 +21,47 @@ export const selectTotal = (state: PolicyListState) => state.total;
1721
export const selectIsLoading = (state: PolicyListState) => state.isLoading;
1822

1923
export const selectApiError = (state: PolicyListState) => state.apiError;
24+
25+
export const isOnPolicyListPage = (state: PolicyListState) => {
26+
return state.location?.pathname === '/policy';
27+
};
28+
29+
const routeLocation = (state: PolicyListState) => state.location;
30+
31+
/**
32+
* Returns the supported URL search params, populated with defaults if none where present in the URL
33+
*/
34+
export const urlSearchParams: (
35+
state: PolicyListState
36+
) => PolicyListUrlSearchParams = createSelector(routeLocation, location => {
37+
const searchParams = {
38+
page_index: 0,
39+
page_size: 10,
40+
};
41+
if (!location) {
42+
return searchParams;
43+
}
44+
45+
const query = parse(location.search);
46+
47+
// Search params can appear multiple times in the URL, in which case the value for them,
48+
// once parsed, would be an array. In these case, we take the first value defined
49+
searchParams.page_index = Number(
50+
(Array.isArray(query.page_index) ? query.page_index[0] : query.page_index) ?? 0
51+
);
52+
searchParams.page_size = Number(
53+
(Array.isArray(query.page_size) ? query.page_size[0] : query.page_size) ?? 10
54+
);
55+
56+
// If pageIndex is not a valid positive integer, set it to 0
57+
if (!Number.isFinite(searchParams.page_index) || searchParams.page_index < 0) {
58+
searchParams.page_index = 0;
59+
}
60+
61+
// if pageSize is not one of the expected page sizes, reset it to 10
62+
if (!PAGE_SIZES.includes(searchParams.page_size)) {
63+
searchParams.page_size = 10;
64+
}
65+
66+
return searchParams;
67+
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
} from '../../../types';
1818

1919
const INGEST_API_ROOT = `/api/ingest_manager`;
20-
const INGEST_API_DATASOURCES = `${INGEST_API_ROOT}/datasources`;
20+
export const INGEST_API_DATASOURCES = `${INGEST_API_ROOT}/datasources`;
2121
const INGEST_API_FLEET = `${INGEST_API_ROOT}/fleet`;
2222
const INGEST_API_FLEET_AGENT_STATUS = `${INGEST_API_FLEET}/agent-status`;
2323

0 commit comments

Comments
 (0)