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] Update wordings and breadcrumb for timelines page #90809

Merged
merged 14 commits into from
Feb 25, 2021
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,7 @@ export const getBreadcrumbsForRoute = (
}
if (isTimelinesRoutes(spyState) && object.navTabs) {
const tempNav: SearchNavTab = { urlKey: 'timeline', isDetailPage: false };
let urlStateKeys = [getOr(tempNav, spyState.pageName, object.navTabs)];
if (spyState.tabName != null) {
urlStateKeys = [...urlStateKeys, getOr(tempNav, spyState.tabName, object.navTabs)];
}
const urlStateKeys = [getOr(tempNav, spyState.pageName, object.navTabs)];

return [
siemRootBreadcrumb,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ const StatefulRecentTimelinesComponent: React.FC<Props> = ({ apolloClient, filte
<RecentTimelines
noTimelinesMessage={noTimelinesMessage}
onOpenTimeline={onOpenTimeline}
timelines={timelines}
timelines={timelines ?? []}
/>
)}
<EuiHorizontalRule margin="s" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,26 @@

import { mountWithIntl } from '@kbn/test/jest';
import React from 'react';
import { useParams } from 'react-router-dom';

import { DeleteTimelineModal } from './delete_timeline_modal';

import * as i18n from '../translations';
import { TimelineType } from '../../../../../common/types/timeline';

jest.mock('react-router-dom', () => {
const actual = jest.requireActual('react-router-dom');
return {
...actual,
useParams: jest.fn(),
};
});

describe('DeleteTimelineModal', () => {
beforeAll(() => {
(useParams as jest.Mock).mockReturnValue({ tabName: TimelineType.default });
});

test('it renders the expected title when a timeline is selected', () => {
const wrapper = mountWithIntl(
<DeleteTimelineModal
Expand Down Expand Up @@ -80,7 +94,9 @@ describe('DeleteTimelineModal', () => {
/>
);

expect(wrapper.find('[data-test-subj="warning"]').first().text()).toEqual(i18n.DELETE_WARNING);
expect(wrapper.find('[data-test-subj="warning"]').first().text()).toEqual(
i18n.DELETE_TIMELINE_WARNING
);
});

test('it invokes closeModal when the Cancel button is clicked', () => {
Expand Down Expand Up @@ -115,3 +131,23 @@ describe('DeleteTimelineModal', () => {
expect(onDelete).toBeCalled();
});
});

describe('DeleteTimelineTemplateModal', () => {
beforeAll(() => {
(useParams as jest.Mock).mockReturnValue({ tabName: TimelineType.template });
});

test('it renders a deletion warning', () => {
const wrapper = mountWithIntl(
<DeleteTimelineModal
title="Privilege Escalation"
onDelete={jest.fn()}
closeModal={jest.fn()}
/>
);

expect(wrapper.find('[data-test-subj="warning"]').first().text()).toEqual(
i18n.DELETE_TIMELINE_TEMPLATE_WARNING
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import { FormattedMessage } from '@kbn/i18n/react';
import React, { useCallback } from 'react';
import { isEmpty } from 'lodash/fp';

import { useParams } from 'react-router-dom';
import * as i18n from '../translations';
import { TimelineType } from '../../../../../common/types/timeline';

interface Props {
title?: string | null;
Expand All @@ -24,6 +26,12 @@ export const DELETE_TIMELINE_MODAL_WIDTH = 600; // px
* Renders a modal that confirms deletion of a timeline
*/
export const DeleteTimelineModal = React.memo<Props>(({ title, closeModal, onDelete }) => {
const { tabName } = useParams<{ tabName: TimelineType }>();
const warning =
tabName === TimelineType.template
? i18n.DELETE_TIMELINE_TEMPLATE_WARNING
: i18n.DELETE_TIMELINE_WARNING;

const getTitle = useCallback(() => {
const trimmedTitle = title != null ? title.trim() : '';
const titleResult = !isEmpty(trimmedTitle) ? trimmedTitle : i18n.UNTITLED_TIMELINE;
Expand All @@ -48,7 +56,7 @@ export const DeleteTimelineModal = React.memo<Props>(({ title, closeModal, onDel
onConfirm={onDelete}
title={getTitle()}
>
<div data-test-subj="warning">{i18n.DELETE_WARNING}</div>
<div data-test-subj="warning">{warning}</div>
</EuiConfirmModal>
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,18 @@

import { mountWithIntl } from '@kbn/test/jest';
import React from 'react';
import { useParams } from 'react-router-dom';

import { DeleteTimelineModalOverlay } from '.';
import { TimelineType } from '../../../../../common/types/timeline';

jest.mock('react-router-dom', () => {
const actual = jest.requireActual('react-router-dom');
return {
...actual,
useParams: jest.fn(),
};
});

describe('DeleteTimelineModal', () => {
const savedObjectId = 'abcd';
Expand All @@ -20,6 +30,10 @@ describe('DeleteTimelineModal', () => {
title: 'Privilege Escalation',
};

beforeAll(() => {
(useParams as jest.Mock).mockReturnValue({ tabName: TimelineType.default });
});

describe('showModalState', () => {
test('it does NOT render the modal when isModalOpen is false', () => {
const testProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ export const OpenTimeline = React.memo<OpenTimelineProps>(

const onRefreshBtnClick = useCallback(() => {
if (refetch != null) {
refetch(searchResults, totalSearchResultsCount);
refetch();
}
}, [refetch, searchResults, totalSearchResultsCount]);
}, [refetch]);

const handleCloseModal = useCallback(() => {
if (setImportDataModalToggle != null) {
Expand All @@ -137,9 +137,9 @@ export const OpenTimeline = React.memo<OpenTimelineProps>(
setImportDataModalToggle(false);
}
if (refetch != null) {
refetch(searchResults, totalSearchResultsCount);
refetch();
}
}, [setImportDataModalToggle, refetch, searchResults, totalSearchResultsCount]);
}, [setImportDataModalToggle, refetch]);

const actionTimelineToShow = useMemo<ActionTimelineToShow[]>(() => {
const timelineActions: ActionTimelineToShow[] = ['createFrom', 'duplicate'];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@ import { getActionsColumns } from './actions_columns';
import { getCommonColumns } from './common_columns';
import { getExtendedColumns } from './extended_columns';
import { getIconHeaderColumns } from './icon_header_columns';
import { TimelineTypeLiteralWithNull, TimelineStatus } from '../../../../../common/types/timeline';
import {
TimelineTypeLiteralWithNull,
TimelineStatus,
TimelineType,
} from '../../../../../common/types/timeline';

// there are a number of type mismatches across this file
const EuiBasicTable: any = _EuiBasicTable; // eslint-disable-line @typescript-eslint/no-explicit-any
Expand Down Expand Up @@ -103,7 +107,7 @@ export interface TimelinesTableProps {
onToggleShowNotes: OnToggleShowNotes;
pageIndex: number;
pageSize: number;
searchResults: OpenTimelineResult[];
searchResults: OpenTimelineResult[] | null;
showExtendedColumns: boolean;
sortDirection: 'asc' | 'desc';
sortField: string;
Expand Down Expand Up @@ -196,6 +200,13 @@ export const TimelinesTable = React.memo<TimelinesTableProps>(
]
);

const noItemsMessage =
isLoading || searchResults == null
? i18n.LOADING
: timelineType === TimelineType.template
? i18n.ZERO_TIMELINE_TEMPLATES_MATCH
: i18n.ZERO_TIMELINES_MATCH;

return (
<BasicTable
columns={columns}
Expand All @@ -204,9 +215,9 @@ export const TimelinesTable = React.memo<TimelinesTableProps>(
isSelectable={actionTimelineToShow.includes('selectable')}
itemId="savedObjectId"
itemIdToExpandedRowMap={itemIdToExpandedNotesRowMap}
items={searchResults}
items={searchResults ?? []}
loading={isLoading}
noItemsMessage={i18n.ZERO_TIMELINES_MATCH}
noItemsMessage={noItemsMessage}
onChange={onTableChange}
pagination={pagination}
selection={actionTimelineToShow.includes('selectable') ? selection : undefined}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,21 @@ export const DELETE_SELECTED = i18n.translate(
}
);

export const DELETE_WARNING = i18n.translate(
export const DELETE_TIMELINE_WARNING = i18n.translate(
'xpack.securitySolution.open.timeline.deleteWarningLabel',
{
defaultMessage: 'You will not be able to recover this timeline or its notes once deleted.',
}
);

export const DELETE_TIMELINE_TEMPLATE_WARNING = i18n.translate(
'xpack.securitySolution.open.timeline.deleteTemplateWarningLabel',
{
defaultMessage:
'You will not be able to recover this timeline template or its notes once deleted.',
}
);

export const DESCRIPTION = i18n.translate(
'xpack.securitySolution.open.timeline.descriptionTableHeader',
{
Expand Down Expand Up @@ -204,13 +212,24 @@ export const WITH = i18n.translate('xpack.securitySolution.open.timeline.withLab
defaultMessage: 'with',
});

export const LOADING = i18n.translate('xpack.securitySolution.open.timeline.loadingLabel', {
defaultMessage: 'Loading...',
});

export const ZERO_TIMELINES_MATCH = i18n.translate(
'xpack.securitySolution.open.timeline.zeroTimelinesMatchLabel',
{
defaultMessage: '0 timelines match the search criteria',
}
);

export const ZERO_TIMELINE_TEMPLATES_MATCH = i18n.translate(
'xpack.securitySolution.open.timeline.zeroTimelineTemplatesMatchLabel',
{
defaultMessage: '0 timeline templates match the search criteria',
}
);

export const SINGLE_TIMELINE = i18n.translate(
'xpack.securitySolution.open.timeline.singleTimelineLabel',
{
Expand Down Expand Up @@ -305,14 +324,14 @@ export const FILTER_CUSTOM_TIMELINES = i18n.translate(
export const IMPORT_TIMELINE_BTN_TITLE = i18n.translate(
'xpack.securitySolution.timelines.components.importTimelineModal.importTimelineTitle',
{
defaultMessage: 'Import timeline',
defaultMessage: 'Import',
}
);

export const SELECT_TIMELINE = i18n.translate(
'xpack.securitySolution.timelines.components.importTimelineModal.selectTimelineDescription',
{
defaultMessage: 'Select a Security timeline (as exported from the Timeline view) to import',
defaultMessage: 'Select a security timeline or timeline template file to import',
Copy link
Contributor

Choose a reason for hiding this comment

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

(copy help needed)
Note: As we can import multiple timelines / templates in a file at a time, so I change the wording to Select a security timeline or timeline template file to import instead

The following text incorporates the multiple selection feature noted above:

Select one or more timeline or timeline template files to import

The word security is omitted in the suggestion above, which leaves open the possibility to import timelines or templates in an non-security context in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, omitted security sounds good, but at the moment we still only support importing a single ndjson file at a time. (However the ndjson file might contain more than one timeline or timeline template's json object in it.)
So I think just removing the security would be fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the clarification re: multiple selection vs multiple timelines in a single file. In that case, please consider the following suggestion:

Select a timeline or timeline template file to import

}
);

Expand Down Expand Up @@ -343,14 +362,14 @@ export const SUCCESSFULLY_IMPORTED_TIMELINES = (totalCount: number) =>
export const IMPORT_FAILED = i18n.translate(
'xpack.securitySolution.timelines.components.importTimelineModal.importFailedTitle',
{
defaultMessage: 'Failed to import timelines',
defaultMessage: 'Failed to import',
}
);

export const IMPORT_TIMELINE = i18n.translate(
'xpack.securitySolution.timelines.components.importTimelineModal.importTitle',
{
defaultMessage: 'Import timeline…',
defaultMessage: 'Import…',
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,9 @@ export interface OpenTimelineProps {
/** The currently applied search criteria */
query: string;
/** Refetch table */
refetch?: (existingTimeline?: OpenTimelineResult[], existingCount?: number) => void;
/** The results of executing a search */
searchResults: OpenTimelineResult[];
refetch?: () => void;
/** The results of executing a search, null is the status before data fatched */
searchResults: OpenTimelineResult[] | null;
/** the currently-selected timelines in the table */
selectedItems: OpenTimelineResult[];
/** Toggle export timelines modal*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ const SelectableTimelineComponent: React.FC<SelectableTimelineProps> = ({
windowProps: {
onScroll: ({ scrollOffset }) =>
handleOnScroll(
timelines.filter((t) => !hideUntitled || t.title !== '').length,
(timelines ?? []).filter((t) => !hideUntitled || t.title !== '').length,
timelineCount,
scrollOffset
),
Expand Down Expand Up @@ -254,15 +254,15 @@ const SelectableTimelineComponent: React.FC<SelectableTimelineProps> = ({
<EuiSelectable
data-test-subj="selectable-input"
height={POPOVER_HEIGHT}
isLoading={loading && timelines.length === 0}
isLoading={loading && timelines == null}
listProps={listProps}
renderOption={renderTimelineOption}
onChange={handleTimelineChange}
searchable
searchProps={searchProps}
singleSelection={true}
options={getSelectableOptions({
timelines,
timelines: timelines ?? [],
onlyFavorites,
searchTimelineValue,
timelineType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export interface AllTimelinesArgs {
status,
timelineType,
}: AllTimelinesVariables) => void;
timelines: OpenTimelineResult[];
timelines: OpenTimelineResult[] | null;
loading: boolean;
totalCount: number;
customTemplateTimelineCount: number;
Expand Down Expand Up @@ -105,7 +105,7 @@ export const useGetAllTimeline = (): AllTimelinesArgs => {
const [allTimelines, setAllTimelines] = useState<Omit<AllTimelinesArgs, 'fetchAllTimeline'>>({
loading: false,
totalCount: 0,
timelines: [],
timelines: null, // use null as initial state to distinguish between empty result and haven't started loading.
customTemplateTimelineCount: 0,
defaultTimelineCount: 0,
elasticTemplateTimelineCount: 0,
Expand All @@ -128,7 +128,10 @@ export const useGetAllTimeline = (): AllTimelinesArgs => {
const fetchData = async () => {
try {
if (apolloClient != null) {
setAllTimelines((prevState) => ({ ...prevState, loading: true }));
setAllTimelines((prevState) => ({
...prevState,
loading: true,
}));

const variables: GetAllTimeline.Variables = {
onlyUserFavorite,
Expand Down
Loading