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.

Sign up for GitHub

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

Jump to bottom

[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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0a2773a
[Security Solution] Update text and layout for status command and act…
kevinlog Jul 7, 2022
ee6c089
update action log tray
kevinlog Jul 7, 2022
681e121
spacing adjustments
kevinlog Jul 7, 2022
8244a73
fix check
kevinlog Jul 7, 2022
9b26a65
adjust spacing for help and errors
kevinlog Jul 7, 2022
013cdd0
Merge branch 'main' into task/status-action-log-text
kibanamachine Jul 7, 2022
84a9842
Merge branch 'main' into task/status-action-log-text
kibanamachine Jul 8, 2022
788df06
Merge branch 'main' of github.com:elastic/kibana into task/status-act…
kevinlog Jul 8, 2022
2bd50b7
pr comments, snapshot test
kevinlog Jul 8, 2022
6930af9
Merge branch 'main' into task/status-action-log-text
kibanamachine Jul 8, 2022
c39f25b
Merge branch 'main' of github.com:elastic/kibana into task/status-act…
kevinlog Jul 11, 2022
2fef0af
Merge branch 'task/status-action-log-text' of https://github.com/kevi…
kevinlog Jul 11, 2022
107effc
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Jul 11, 2022
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
8 changes: 2 additions & 6 deletions x-pack/plugins/security_solution/public/common/components/endpoint/agent_status.tsx
View file
Open in desktop
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 type { HostStatus } from '../../../../common/endpoint/types';
import { HOST_STATUS_TO_BADGE_COLOR } from '../../../management/pages/endpoint_hosts/view/host_constants';
import { getAgentStatusText } 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 }}
/>
{getAgentStatusText(hostStatus)}
paul-tavares marked this conversation as resolved.
Show resolved Hide resolved
</EuiBadge>
);
});
Expand Down
17 changes: 17 additions & 0 deletions x-pack/plugins/security_solution/public/common/components/endpoint/agent_status_text.ts
View file
Open in desktop
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 type { HostStatus } from '../../../../common/endpoint/types';

export const getAgentStatusText = (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.

Ok. I see what you did here... When I saw getAgentStatusText() I though you were using the Fleet helper function.

Since this is under common/components/, can you make it a component? as in:

<EndpointAgentStatusText hostStatus="" />

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My original intention was just to have this generate the text for the status which can be used either in plain text reporting or in a containing component like it is here: https://github.com/elastic/kibana/pull/135935/files#diff-dcba59229f5aeb7b5e1cc30e8b37d4b848460e6e240da640eac479b392e28939R21

Thought - are you OK if I export this text from the above file so that we can either import the text or the entire AgentStatus component based on the need?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, that's fine - nothing that should hold this up. It just feels more like a "service" or "utility" than a component, so I was just highlighting that it probably should not be under component/

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 },
});
};
1 change: 1 addition & 0 deletions ...lugins/security_solution/public/management/components/console/components/bad_argument.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,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
11 changes: 6 additions & 5 deletions ...ugins/security_solution/public/management/components/console/components/command_usage.tsx
View file
Open in desktop
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 @@ -139,7 +140,7 @@ export const CommandUsage = memo<CommandUsageProps>(({ commandDef }) => {
));
return (
<>
<EuiSpacer />
<EuiSpacer size="s" />
{commandDef.args && (
<EuiDescriptionList
compressed
Expand Down
7 changes: 3 additions & 4 deletions ..._solution/public/management/components/console/components/unsupported_message_callout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import type { ReactNode } from 'react';
import React, { memo } 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 @@ -28,9 +28,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
2 changes: 2 additions & 0 deletions x-pack/plugins/security_solution/public/management/components/console/console.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,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
16 changes: 6 additions & 10 deletions ...ic/management/components/endpoint_responder/endpoint_response_actions_console_commands.ts
View file
Open in desktop
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
178 changes: 115 additions & 63 deletions ...ugins/security_solution/public/management/components/endpoint_responder/status_action.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
* 2.0.
*/

import React, { memo, useEffect, useMemo } from 'react';
import { EuiFlexGroup, EuiFlexItem, EuiText } from '@elastic/eui';
import { FormattedMessage } from '@kbn/i18n-react';
import React, { memo, useEffect, useMemo, useCallback } from '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 type { 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 { getAgentStatusText } 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 @@ -115,6 +115,114 @@ export const EndpointStatusActionResult = memo<
}
}, [fetchedPendingActionsSummary, setStore]);

const getStatusDescriptionList = useCallback(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

just FYI: This would be more efficient if it was a useMemo() that would only be updated if the dependencies changed. The way it is now, its called every time there is a re-render of the component

if (!endpointDetails) {
return undefined;
}

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 `${getAgentStatusText(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',
{ defaultMessage: 'Last active' }
)}
value={endpointDetails.metadata['@timestamp']}
className="eui-textTruncate"
/>
</ConsoleCodeBlock>
),
},
];
return (
<EuiDescriptionList
compressed
type="column"
className="descriptionList-20_80"
listItems={statusData}
data-test-subj={'agent-status-console-output'}
/>
);
}, [
pendingIsolationActions.pendingIsolate,
pendingIsolationActions.pendingUnIsolate,
endpointDetails,
]);

if (detailsFetchError) {
return (
<ResultComponent showAs="failure">
Expand All @@ -127,62 +235,6 @@ 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>
<FormattedDate
fieldName={i18n.translate(
'xpack.securitySolution.endpointResponseActions.status.lastActive',
{ defaultMessage: 'Last active' }
)}
value={endpointDetails.metadata['@timestamp']}
className="eui-textTruncate"
/>
</EuiText>
</EuiFlexItem>
</EuiFlexGroup>
</ResultComponent>
);
return <ResultComponent showTitle={false}>{getStatusDescriptionList()}</ResultComponent>;
});
EndpointStatusActionResult.displayName = 'EndpointStatusActionResult';
Loading