Skip to content

Commit

Permalink
[Endpoint] Bug fixes to Policy List and Details views (#64568)
Browse files Browse the repository at this point in the history
* Fix Datasource API calls to use correct object type in kuery
* Fix policy details showing toaster multiple times
* Fix taking last url param value (instead of first)
  • Loading branch information
paul-tavares committed Apr 28, 2020
1 parent fa219bc commit f7f245f
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { setPolicyListApiMockImplementation } from './test_mock_utils';
import { INGEST_API_DATASOURCES } from './services/ingest';
import { Immutable } from '../../../../../common/types';
import { createSpyMiddleware, MiddlewareActionSpyHelper } from '../test_utils';
import { DATASOURCE_SAVED_OBJECT_TYPE } from '../../../../../../ingest_manager/common';

describe('policy list store concerns', () => {
let fakeCoreStart: ReturnType<typeof coreMock.createStart>;
Expand Down Expand Up @@ -121,7 +122,11 @@ describe('policy list store concerns', () => {
});
await waitForAction('serverReturnedPolicyListData');
expect(fakeCoreStart.http.get).toHaveBeenCalledWith(INGEST_API_DATASOURCES, {
query: { kuery: 'datasources.package.name: endpoint', page: 1, perPage: 10 },
query: {
kuery: `${DATASOURCE_SAVED_OBJECT_TYPE}.package.name: endpoint`,
page: 1,
perPage: 10,
},
});
});

Expand All @@ -140,7 +145,11 @@ describe('policy list store concerns', () => {
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 },
query: {
kuery: `${DATASOURCE_SAVED_OBJECT_TYPE}.package.name: endpoint`,
page: 1,
perPage: 50,
},
});
});
it('uses defaults for params not in url', async () => {
Expand All @@ -159,21 +168,33 @@ describe('policy list store concerns', () => {
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 },
query: {
kuery: `${DATASOURCE_SAVED_OBJECT_TYPE}.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 },
query: {
kuery: `${DATASOURCE_SAVED_OBJECT_TYPE}.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 },
query: {
kuery: `${DATASOURCE_SAVED_OBJECT_TYPE}.package.name: endpoint`,
page: 11,
perPage: 10,
},
});
});
it(`ignores unknown url search params`, async () => {
Expand All @@ -186,8 +207,8 @@ describe('policy list store concerns', () => {
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,
page_index: 40,
page_size: 50,
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,16 @@ export const urlSearchParams: (
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
// once parsed, would be an array. In these case, we take the last value defined
searchParams.page_index = Number(
(Array.isArray(query.page_index) ? query.page_index[0] : query.page_index) ?? 0
(Array.isArray(query.page_index)
? query.page_index[query.page_index.length - 1]
: query.page_index) ?? 0
);
searchParams.page_size = Number(
(Array.isArray(query.page_size) ? query.page_size[0] : query.page_size) ?? 10
(Array.isArray(query.page_size)
? query.page_size[query.page_size.length - 1]
: query.page_size) ?? 10
);

// If pageIndex is not a valid positive integer, set it to 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { sendGetDatasource, sendGetEndpointSpecificDatasources } from './ingest';
import { httpServiceMock } from '../../../../../../../../../src/core/public/mocks';
import { DATASOURCE_SAVED_OBJECT_TYPE } from '../../../../../../../ingest_manager/common';

describe('ingest service', () => {
let http: ReturnType<typeof httpServiceMock.createStartContract>;
Expand All @@ -19,7 +20,7 @@ describe('ingest service', () => {
await sendGetEndpointSpecificDatasources(http);
expect(http.get).toHaveBeenCalledWith('/api/ingest_manager/datasources', {
query: {
kuery: 'datasources.package.name: endpoint',
kuery: `${DATASOURCE_SAVED_OBJECT_TYPE}.package.name: endpoint`,
},
});
});
Expand All @@ -29,7 +30,7 @@ describe('ingest service', () => {
});
expect(http.get).toHaveBeenCalledWith('/api/ingest_manager/datasources', {
query: {
kuery: 'someValueHere and datasources.package.name: endpoint',
kuery: `someValueHere and ${DATASOURCE_SAVED_OBJECT_TYPE}.package.name: endpoint`,
perPage: 10,
page: 1,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { HttpFetchOptions, HttpStart } from 'kibana/public';
import {
GetDatasourcesRequest,
GetAgentStatusResponse,
DATASOURCE_SAVED_OBJECT_TYPE,
} from '../../../../../../../ingest_manager/common';
import { GetPolicyListResponse, GetPolicyResponse, UpdatePolicyResponse } from '../../../types';
import { NewPolicyData } from '../../../../../../common/types';
Expand All @@ -33,7 +34,7 @@ export const sendGetEndpointSpecificDatasources = (
...options.query,
kuery: `${
options?.query?.kuery ? options.query.kuery + ' and ' : ''
}datasources.package.name: endpoint`,
}${DATASOURCE_SAVED_OBJECT_TYPE}.package.name: endpoint`,
},
});
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const PolicyDetails = React.memo(() => {
const [showConfirm, setShowConfirm] = useState<boolean>(false);
const policyName = policyItem?.name ?? '';

// Handle showing udpate statuses
// Handle showing update statuses
useEffect(() => {
if (policyUpdateStatus) {
if (policyUpdateStatus.success) {
Expand All @@ -79,7 +79,7 @@ export const PolicyDetails = React.memo(() => {
});
}
}
}, [notifications.toasts, policyItem, policyName, policyUpdateStatus]);
}, [notifications.toasts, policyName, policyUpdateStatus]);

const handleBackToListOnClick = useNavigateByRouterEventHandler('/policy');

Expand Down

0 comments on commit f7f245f

Please sign in to comment.