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

[Security Solution] Updates the text in the Actions log and Status command to match UX #135935

Merged
merged 13 commits into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from 8 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 @@ -7,9 +7,9 @@

import React from 'react';
import { EuiBadge } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import { HostStatus } from '../../../../common/endpoint/types';
import { HOST_STATUS_TO_BADGE_COLOR } from '../../../management/pages/endpoint_hosts/view/host_constants';
import { agentStatusText } from './agent_status_text';

export const AgentStatus = React.memo(({ hostStatus }: { hostStatus: HostStatus }) => {
return (
Expand All @@ -18,11 +18,7 @@ export const AgentStatus = React.memo(({ hostStatus }: { hostStatus: HostStatus
data-test-subj="rowHostStatus"
className="eui-textTruncate"
>
<FormattedMessage
id="xpack.securitySolution.endpoint.list.hostStatusValue"
defaultMessage="{hostStatus, select, healthy {Healthy} unhealthy {Unhealthy} updating {Updating} offline {Offline} inactive {Inactive} unenrolled {Unenrolled} other {Unhealthy}}"
values={{ hostStatus }}
/>
{agentStatusText(hostStatus)}
</EuiBadge>
);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { i18n } from '@kbn/i18n';
import { HostStatus } from '../../../../common/endpoint/types';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { HostStatus } from '../../../../common/endpoint/types';
import type { HostStatus } from '../../../../common/endpoint/types';


export const agentStatusText = (hostStatus: HostStatus) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there is no JSX code here this file can be a .ts extension.

Copy link
Contributor

Choose a reason for hiding this comment

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

Name suggestion: getAgentStatusText

return i18n.translate('xpack.securitySolution.endpoint.list.hostStatusValue', {
defaultMessage:
'{hostStatus, select, healthy {Healthy} unhealthy {Unhealthy} updating {Updating} offline {Offline} inactive {Inactive} unenrolled {Unenrolled} other {Unhealthy}}',
values: { hostStatus },
});
};
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export const BadArgument = memo<CommandExecutionComponentProps<{}, { errorMessag
</div>
<div>
<ConsoleCodeBlock>
<EuiSpacer size="m" />
<FormattedMessage
id="xpack.securitySolution.console.badArgument.helpMessage"
defaultMessage="Enter {helpCmd} for further assistance."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React, { memo, useMemo } from 'react';
import { EuiDescriptionList, EuiPanel, EuiSpacer, EuiText } from '@elastic/eui';
import { EuiDescriptionList, EuiPanel, EuiSpacer } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { ConsoleCodeBlock } from './console_code_block';
import { getArgumentsForCommand } from '../service/parsed_command_input';
Expand All @@ -20,11 +20,12 @@ const additionalProps = {

export const CommandInputUsage = memo<Pick<CommandUsageProps, 'commandDef'>>(({ commandDef }) => {
const usageHelp = useMemo(() => {
return getArgumentsForCommand(commandDef).map((usage) => {
return getArgumentsForCommand(commandDef).map((usage, index) => {
return (
<EuiText size="s">
<>
{index > 0 && <EuiSpacer size="xs" />}
<ConsoleCodeBlock>{`${commandDef.name} ${usage}`}</ConsoleCodeBlock>
</EuiText>
</>
);
});
}, [commandDef]);
Expand Down Expand Up @@ -129,7 +130,7 @@ export const CommandUsage = memo<CommandUsageProps>(({ commandDef }) => {
}, [commandDef.args, hasArgs]);

const parametersDescriptionList = (title: string, parameters: CommandDetails) => {
const description = parameters.map((item) => (
const description = parameters.map((item, index) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

index is not needed here

<div>
<ConsoleCodeBlock bold inline>
{item.title}
Expand All @@ -139,7 +140,7 @@ export const CommandUsage = memo<CommandUsageProps>(({ commandDef }) => {
));
return (
<>
<EuiSpacer />
<EuiSpacer size="s" />
{commandDef.args && (
<EuiDescriptionList
compressed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React, { memo, ReactNode } from 'react';
import { EuiText, EuiTextColor } from '@elastic/eui';
import { EuiText, EuiTextColor, EuiSpacer } from '@elastic/eui';
import { i18n } from '@kbn/i18n';

const UNSUPPORTED_LABEL = i18n.translate(
Expand All @@ -27,9 +27,8 @@ export const UnsupportedMessageCallout = memo<UnsupportedMessageCalloutProps>(
<EuiText size="s">
<EuiTextColor color="danger">{header}</EuiTextColor>
</EuiText>
<EuiText size="s">
<EuiTextColor color="subdued">{children}</EuiTextColor>
</EuiText>
<EuiSpacer size="s" />
{children}
</div>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,12 @@ const ConsoleWindow = styled.div`
&.euiDescriptionList {
> .euiDescriptionList__title {
width: 20%;
margin-top: ${({ theme: { eui } }) => eui.euiSizeS};
}

> .euiDescriptionList__description {
width: 80%;
margin-top: ${({ theme: { eui } }) => eui.euiSizeS};
ashokaditya marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const getEndpointResponseActionsConsoleCommands = (
{
name: 'kill-process',
about: i18n.translate('xpack.securitySolution.endpointConsoleCommands.killProcess.about', {
defaultMessage: 'Kill a running process',
defaultMessage: 'Kill a running process. Accepts either a PID or an entity id.',
}),
RenderComponent: KillProcessActionResult,
meta: {
Expand All @@ -97,8 +97,7 @@ export const getEndpointResponseActionsConsoleCommands = (
allowMultiples: false,
exclusiveOr: true,
about: i18n.translate('xpack.securitySolution.endpointConsoleCommands.pid.arg.comment', {
defaultMessage:
'A PID representing the process to kill. You can enter a pid or an entity id, but not both.',
defaultMessage: 'A PID representing the process to kill',
}),
validate: emptyArgumentValidator,
},
Expand All @@ -109,8 +108,7 @@ export const getEndpointResponseActionsConsoleCommands = (
about: i18n.translate(
'xpack.securitySolution.endpointConsoleCommands.entityId.arg.comment',
{
defaultMessage:
'An entity id representing the process to kill. You can enter a pid or an entity id, but not both.',
defaultMessage: 'An entity id representing the process to kill',
}
),
validate: emptyArgumentValidator,
Expand All @@ -120,7 +118,7 @@ export const getEndpointResponseActionsConsoleCommands = (
{
name: 'suspend-process',
about: i18n.translate('xpack.securitySolution.endpointConsoleCommands.suspendProcess.about', {
defaultMessage: 'Suspend a running process',
defaultMessage: 'Suspend a running process. Accepts either a PID or an entity id.',
}),
RenderComponent: SuspendProcessActionResult,
meta: {
Expand All @@ -145,8 +143,7 @@ export const getEndpointResponseActionsConsoleCommands = (
about: i18n.translate(
'xpack.securitySolution.endpointConsoleCommands.suspendProcess.pid.arg.comment',
{
defaultMessage:
'A PID representing the process to suspend. You can enter a pid or an entity id, but not both.',
defaultMessage: 'A PID representing the process to suspend',
}
),
validate: emptyArgumentValidator,
Expand All @@ -158,8 +155,7 @@ export const getEndpointResponseActionsConsoleCommands = (
about: i18n.translate(
'xpack.securitySolution.endpointConsoleCommands.suspendProcess.entityId.arg.comment',
{
defaultMessage:
'An entity id representing the process to suspend. You can enter a pid or an entity id, but not both.',
defaultMessage: 'An entity id representing the process to suspend',
}
),
validate: emptyArgumentValidator,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
*/

import React, { memo, useEffect, useMemo } from 'react';
import { EuiFlexGroup, EuiFlexItem, EuiText } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import { EuiDescriptionList } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import type { HttpFetchError } from '@kbn/core/public';
import { v4 as uuidV4 } from 'uuid';
Expand All @@ -16,11 +15,12 @@ import type { EndpointCommandDefinitionMeta } from './types';
import { EndpointHostIsolationStatusProps } from '../../../common/components/endpoint/host_isolation';
import { useGetEndpointPendingActionsSummary } from '../../hooks/endpoint/use_get_endpoint_pending_actions_summary';
import { FormattedDate } from '../../../common/components/formatted_date';
import { EndpointAppliedPolicyStatus } from '../endpoint_applied_policy_status';
import { EndpointAgentAndIsolationStatus } from '../endpoint_agent_and_isolation_status';
import { useGetEndpointDetails } from '../../hooks';
import type { CommandExecutionComponentProps } from '../console/types';
import { FormattedError } from '../formatted_error';
import { ConsoleCodeBlock } from '../console/components/console_code_block';
import { POLICY_STATUS_TO_TEXT } from '../../pages/endpoint_hosts/view/host_constants';
import { agentStatusText } from '../../../common/components/endpoint/agent_status_text';

export const EndpointStatusActionResult = memo<
CommandExecutionComponentProps<
Expand Down Expand Up @@ -50,7 +50,7 @@ export const EndpointStatusActionResult = memo<

const { data: fetchedPendingActionsSummary } = useGetEndpointPendingActionsSummary([endpointId], {
enabled: isPending,
queryKey,
queryKey: [queryKey, endpointId],
});

const pendingIsolationActions = useMemo<
Expand Down Expand Up @@ -127,50 +127,83 @@ export const EndpointStatusActionResult = memo<
return <ResultComponent showAs="pending" />;
}

return (
<ResultComponent showTitle={false}>
<EuiFlexGroup wrap={false} responsive={false}>
<EuiFlexItem grow={false}>
<EuiText size="s">
<FormattedMessage
id="xpack.securitySolution.endpointResponseActions.status.agentStatus"
defaultMessage="Agent status"
/>
</EuiText>
<EndpointAgentAndIsolationStatus
status={endpointDetails.host_status}
isIsolated={Boolean(endpointDetails.metadata.Endpoint.state?.isolation)}
{...pendingIsolationActions}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText size="s">
<FormattedMessage
id="xpack.securitySolution.endpointResponseActions.status.version"
defaultMessage="Version"
/>
</EuiText>
<EuiText>{endpointDetails.metadata.agent.version}</EuiText>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiText size="s">
<FormattedMessage
id="xpack.securitySolution.endpointResponseActions.status.policyStatus"
defaultMessage="Policy status"
/>
</EuiText>
<EndpointAppliedPolicyStatus
policyApplied={endpointDetails.metadata.Endpoint.policy.applied}
/>
</EuiFlexItem>
<EuiFlexItem>
<EuiText size="s">
<FormattedMessage
id="xpack.securitySolution.endpointResponseActions.status.lastActive"
defaultMessage="Last active"
/>
</EuiText>
<EuiText>
const statusDescriptionList = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest you move this inside a useCallback and change the name by something like getStatusDescriptionList

const agentStatus = () => {
let isolateStatus = '';

if (pendingIsolationActions.pendingIsolate > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not hold up on this PR, but this logic here for actions no longer makes sense now that we introduced more actions. Not sure that isolateStatus should be set to isolating or releasing when there are other actions also pending.

(FYI: I also found that our component that normally show the isolation status/pending actions is not showing all pending actions in the tooltip - opening issue now)

isolateStatus = i18n.translate(
'xpack.securitySolution.endpointResponseActions.status.isolating',
{
defaultMessage: 'Isolating',
}
);
} else if (pendingIsolationActions.pendingUnIsolate > 0) {
isolateStatus = i18n.translate(
'xpack.securitySolution.endpointResponseActions.status.releasing',
{
defaultMessage: 'Releasing',
}
);
} else if (endpointDetails.metadata.Endpoint.state?.isolation) {
isolateStatus = i18n.translate(
'xpack.securitySolution.endpointResponseActions.status.isolated',
{
defaultMessage: 'Isolated',
}
);
}

return `${agentStatusText(endpointDetails.host_status)}${
isolateStatus.length > 0 ? ` - ${isolateStatus}` : ''
}`;
};

const statusData = [
Copy link
Contributor

Choose a reason for hiding this comment

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

This has also a dynamic result, move it in a useMemo will avoid unnecessary code execution during renders.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dasansol92 if I wrap the containing function into the useCallBack() and the deps array contains the dynamic data, won't this be taken care of? If I put this as useMemo() we'd have a hook in a hook.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohh! You are totally right! Github diff hided me the useCallback. No need for useMemo here then. Apologies for the noise.

{
title: (
<ConsoleCodeBlock>
{i18n.translate('xpack.securitySolution.endpointResponseActions.status.agentStatus', {
defaultMessage: 'Agent status',
})}
</ConsoleCodeBlock>
),
description: <ConsoleCodeBlock>{agentStatus()}</ConsoleCodeBlock>,
},
{
title: (
<ConsoleCodeBlock>
{i18n.translate('xpack.securitySolution.endpointResponseActions.status.version', {
defaultMessage: 'Version',
})}
</ConsoleCodeBlock>
),
description: endpointDetails.metadata.agent.version,
},
{
title: (
<ConsoleCodeBlock>
{i18n.translate('xpack.securitySolution.endpointResponseActions.status.policyStatus', {
defaultMessage: 'Policy status',
})}
</ConsoleCodeBlock>
),
description: (
<ConsoleCodeBlock>
{POLICY_STATUS_TO_TEXT[endpointDetails.metadata.Endpoint.policy.applied.status]}
</ConsoleCodeBlock>
),
},
{
title: (
<ConsoleCodeBlock>
{i18n.translate('xpack.securitySolution.endpointResponseActions.status.lastActive', {
defaultMessage: 'Last active',
})}
</ConsoleCodeBlock>
),
description: (
<ConsoleCodeBlock>
<FormattedDate
fieldName={i18n.translate(
'xpack.securitySolution.endpointResponseActions.status.lastActive',
Expand All @@ -179,10 +212,21 @@ export const EndpointStatusActionResult = memo<
value={endpointDetails.metadata['@timestamp']}
className="eui-textTruncate"
/>
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
</ResultComponent>
);
</ConsoleCodeBlock>
),
},
];
return (
<EuiDescriptionList
compressed
type="column"
className="descriptionList-20_80"
listItems={statusData}
data-test-subj={'agent-status-console-output'}
/>
);
};

return <ResultComponent showTitle={false}>{statusDescriptionList()}</ResultComponent>;
});
EndpointStatusActionResult.displayName = 'EndpointStatusActionResult';
Loading