Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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';
};

export const routeLocation = (state: PolicyListState) => state.location;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could drop export if you'd like

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. I will remove it in next commit


/**
* 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