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] add policy data to Host list UI #69202

Merged
merged 7 commits into from
Jun 17, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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 @@ -93,13 +93,40 @@ export const HostDetails = memo(({ details }: { details: HostMetadata }) => {

const policyStatusClickHandler = useNavigateByRouterEventHandler(policyResponseRoutePath);

const [policyDetailsUri, policyDetailsRoutePath] = useMemo(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

are the names switched here?
*Path I normally associate with the router path
*Uri or *Url I normally associated with an Href type of prop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They were, corrected them - my mistake

return [
getManagementUrl({
name: 'policyDetails',
policyId: details.endpoint.policy.applied.id,
excludePrefix: true,
}),
getManagementUrl({
name: 'policyDetails',
policyId: details.endpoint.policy.applied.id,
}),
];
}, [details.endpoint.policy.applied.id]);

const policyDetailsClickHandler = useNavigateByRouterEventHandler(policyDetailsUri);

const detailsResultsPolicy = useMemo(() => {
return [
{
title: i18n.translate('xpack.securitySolution.endpoint.host.details.policy', {
defaultMessage: 'Policy',
}),
description: details.endpoint.policy.applied.id,
description: (
<>
{/* eslint-disable-next-line @elastic/eui/href-or-on-click */}
<EuiLink
data-test-subj="policyDetailsValue"
href={policyDetailsRoutePath}
onClick={policyDetailsClickHandler}
>
{details.endpoint.policy.applied.name}
</EuiLink>
</>
),
},
{
title: i18n.translate('xpack.securitySolution.endpoint.host.details.policyStatus', {
Expand Down Expand Up @@ -128,7 +155,14 @@ export const HostDetails = memo(({ details }: { details: HostMetadata }) => {
),
},
];
}, [details, policyResponseUri, policyStatus, policyStatusClickHandler]);
}, [
details,
policyResponseUri,
policyStatus,
policyStatusClickHandler,
policyDetailsRoutePath,
policyDetailsClickHandler,
]);
const detailsResultsLower = useMemo(() => {
return [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,11 @@ export const POLICY_STATUS_TO_HEALTH_COLOR = Object.freeze<
warning: 'warning',
failure: 'danger',
});

export const POLICY_STATUS_TO_TEXT = Object.freeze<
{ [key in keyof typeof HostPolicyResponseActionStatus]: string }
>({
success: 'Success',
Copy link
Contributor

Choose a reason for hiding this comment

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

need to i18n all of these right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, corrected

warning: 'Warning',
failure: 'Failure',
});
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
} from '../../../../../common/endpoint/types';
import { EndpointDocGenerator } from '../../../../../common/endpoint/generate_data';
import { AppAction } from '../../../../common/store/actions';
import { POLICY_STATUS_TO_HEALTH_COLOR, POLICY_STATUS_TO_TEXT } from './host_constants';

describe('when on the hosts page', () => {
const docGenerator = new EndpointDocGenerator();
Expand Down Expand Up @@ -47,15 +48,23 @@ describe('when on the hosts page', () => {
});
});
describe('when list data loads', () => {
const generatedPolicyStatuses: Array<
HostInfo['metadata']['endpoint']['policy']['applied']['status']
> = [];
let firstPolicyID: string;
beforeEach(() => {
reactTestingLibrary.act(() => {
const hostListData = mockHostResultList({ total: 3 });
firstPolicyID = hostListData.hosts[0].metadata.endpoint.policy.applied.id;
[HostStatus.ERROR, HostStatus.ONLINE, HostStatus.OFFLINE].forEach((status, index) => {
hostListData.hosts[index] = {
metadata: hostListData.hosts[index].metadata,
host_status: status,
};
});
hostListData.hosts.forEach((item, index) => {
generatedPolicyStatuses[index] = item.metadata.endpoint.policy.applied.status;
});
const action: AppAction = {
type: 'serverReturnedHostList',
payload: hostListData,
Expand Down Expand Up @@ -92,6 +101,29 @@ describe('when on the hosts page', () => {
).not.toBeNull();
});

it('should display correct policy status', async () => {
const renderResult = render();
const policyStatuses = await renderResult.findAllByTestId('rowPolicyStatus');

policyStatuses.forEach((status, index) => {
expect(status.textContent).toEqual(POLICY_STATUS_TO_TEXT[generatedPolicyStatuses[index]]);
expect(
status.querySelector(
`[data-euiicon-type][color=${
POLICY_STATUS_TO_HEALTH_COLOR[generatedPolicyStatuses[index]]
}]`
)
).not.toBeNull();
});
});

it('should display policy name as a link', async () => {
const renderResult = render();
const firstPolicyName = (await renderResult.findAllByTestId('policyNameCellLink'))[0];
expect(firstPolicyName).not.toBeNull();
expect(firstPolicyName.getAttribute('href')).toContain(`policy/${firstPolicyID}`);
});

describe('when the user clicks the first hostname in the table', () => {
let renderResult: reactTestingLibrary.RenderResult;
beforeEach(async () => {
Expand Down Expand Up @@ -197,6 +229,26 @@ describe('when on the hosts page', () => {
expect(flyout).not.toBeNull();
});
});
it('should display policy name value as a link', async () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

spacing between the ITs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

spaces added for readability

const renderResult = render();
const policyDetailsLink = await renderResult.findByTestId('policyDetailsValue');
expect(policyDetailsLink).not.toBeNull();
expect(policyDetailsLink.getAttribute('href')).toEqual(
`#/management/policy/${hostDetails.metadata.endpoint.policy.applied.id}`
);
});
it('should update the URL when policy name link is clicked', async () => {
const renderResult = render();
const policyDetailsLink = await renderResult.findByTestId('policyDetailsValue');
const userChangedUrlChecker = middlewareSpy.waitForAction('userChangedUrl');
reactTestingLibrary.act(() => {
reactTestingLibrary.fireEvent.click(policyDetailsLink);
});
const changedUrlAction = await userChangedUrlChecker;
expect(changedUrlAction.payload.pathname).toEqual(
`/management/policy/${hostDetails.metadata.endpoint.policy.applied.id}`
);
});
it('should display policy status value as a link', async () => {
const renderResult = render();
const policyStatusLink = await renderResult.findByTestId('policyStatusValue');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ import { createStructuredSelector } from 'reselect';
import { HostDetailsFlyout } from './details';
import * as selectors from '../store/selectors';
import { useHostSelector } from './hooks';
import { HOST_STATUS_TO_HEALTH_COLOR } from './host_constants';
import {
HOST_STATUS_TO_HEALTH_COLOR,
POLICY_STATUS_TO_HEALTH_COLOR,
POLICY_STATUS_TO_TEXT,
} from './host_constants';
import { useNavigateByRouterEventHandler } from '../../../../common/hooks/endpoint/use_navigate_by_router_event_handler';
import { CreateStructuredSelector } from '../../../../common/store';
import { Immutable, HostInfo } from '../../../../../common/endpoint/types';
Expand All @@ -31,17 +35,18 @@ import { ManagementPageView } from '../../../components/management_page_view';
import { getManagementUrl } from '../../..';
import { FormattedDate } from '../../../../common/components/formatted_date';

const HostLink = memo<{
const HostListNavLink = memo<{
name: string;
href: string;
route: string;
}>(({ name, href, route }) => {
dataTestSubj: string;
}>(({ name, href, route, dataTestSubj }) => {
const clickHandler = useNavigateByRouterEventHandler(route);

return (
// eslint-disable-next-line @elastic/eui/href-or-on-click
<EuiLink
data-test-subj="hostnameCellLink"
data-test-subj={dataTestSubj}
className="eui-textTruncate"
href={href}
onClick={clickHandler}
Expand All @@ -50,7 +55,7 @@ const HostLink = memo<{
</EuiLink>
);
});
HostLink.displayName = 'HostLink';
HostListNavLink.displayName = 'HostListNavLink';

const selector = (createStructuredSelector as CreateStructuredSelector)(selectors);
export const HostList = () => {
Expand Down Expand Up @@ -116,7 +121,14 @@ export const HostList = () => {
name: 'endpointDetails',
selected_host: id,
});
return <HostLink name={hostname} href={toRouteUrl} route={toRoutePath} />;
return (
<HostListNavLink
name={hostname}
href={toRouteUrl}
route={toRoutePath}
dataTestSubj="hostnameCellLink"
/>
);
},
},
{
Expand All @@ -142,43 +154,64 @@ export const HostList = () => {
},
},
{
field: '',
field: 'metadata.endpoint.policy.applied',
name: i18n.translate('xpack.securitySolution.endpointList.policy', {
defaultMessage: 'Policy',
}),
truncateText: true,
// eslint-disable-next-line react/display-name
render: () => {
return <span className="eui-textTruncate">{'Policy Name'}</span>;
render: (policy: HostInfo['metadata']['endpoint']['policy']['applied']) => {
const toRoutePath = getManagementUrl({
name: 'policyDetails',
policyId: policy.id,
excludePrefix: true,
});
const toRouteUrl = getManagementUrl({
name: 'policyDetails',
policyId: policy.id,
});
return (
<HostListNavLink
name={policy.name}
href={toRouteUrl}
route={toRoutePath}
dataTestSubj="policyNameCellLink"
/>
);
},
},
{
field: '',
field: 'metadata.endpoint.policy.applied',
name: i18n.translate('xpack.securitySolution.endpointList.policyStatus', {
defaultMessage: 'Policy Status',
}),
// eslint-disable-next-line react/display-name
render: () => {
render: (policy: HostInfo['metadata']['endpoint']['policy']['applied'], item: HostInfo) => {
const toRoutePath = getManagementUrl({
name: 'endpointPolicyResponse',
selected_host: item.metadata.host.id,
excludePrefix: true,
});
const toRouteUrl = getManagementUrl({
name: 'endpointPolicyResponse',
selected_host: item.metadata.host.id,
});
return (
<EuiHealth color="success" className="eui-textTruncate">
<FormattedMessage
id="xpack.securitySolution.endpointList.policyStatus"
defaultMessage="Policy Status"
<EuiHealth
color={POLICY_STATUS_TO_HEALTH_COLOR[policy.status]}
className="eui-textTruncate"
data-test-subj="rowPolicyStatus"
>
<HostListNavLink
name={POLICY_STATUS_TO_TEXT[policy.status]}
href={toRouteUrl}
route={toRoutePath}
dataTestSubj="policyStatusCellLink"
/>
</EuiHealth>
);
},
},
{
field: '',
name: i18n.translate('xpack.securitySolution.endpointList.alerts', {
defaultMessage: 'Alerts',
}),
dataType: 'number',
render: () => {
return '0';
},
},
{
field: 'metadata.host.os.name',
name: i18n.translate('xpack.securitySolution.endpointList.os', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const policyDetailsMiddlewareFactory: ImmutableMiddlewareFactory<PolicyDe

// Until we get the Default configuration into the Endpoint package so that the datasource has
// the expected data structure, we will add it here manually.
if (!policyItem.inputs.length) {
if (policyItem && !policyItem.inputs.length) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

checks for unit tests

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh... I see why this is needed. Case id is invalid, is that it? in that case, would the API call return success, but .item be undefined/null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's correct

policyItem.inputs = [
{
type: 'endpoint',
Expand All @@ -69,7 +69,7 @@ export const policyDetailsMiddlewareFactory: ImmutableMiddlewareFactory<PolicyDe

// Agent summary is secondary data, so its ok for it to come after the details
// page is populated with the main content
if (policyItem.config_id) {
if (policyItem && policyItem.config_id) {
const { results } = await sendGetFleetAgentStatusForConfig(http, policyItem.config_id);
dispatch({
type: 'serverReturnedPolicyDetailsAgentSummaryData',
Expand Down