Skip to content

Commit

Permalink
[Security Solution][Notes] - copy changes for note and timeline + mov…
Browse files Browse the repository at this point in the history
…e the unassociated note advanced setting under the Security Solution section (elastic#197312)

## Summary

This PR tackles 2 small tasks:
- move the unassociated advanced settings introduced in [this
PR](elastic#194947) under the `Security
Solution` category instead of `General`
- make some copy changes on the notes functionality, mainly the
following ([copy changes
document](https://docs.google.com/document/d/10blyxRfkMIR8gk4cw6nFzajA-L63iUzQaxQXHauL8LM/edit#heading=h.mlyibn1i5q84))
  - make sure we don't use a capital `N` for the word `note`
  - make sure that we use a capital `T` for the word `timeline`
  - change some of the no message and callout texts
  - prioritize using `attach` instead of `associate`

All changes have been done with @nastasha-solomon.

elastic#193495
(cherry picked from commit 800e392)
  • Loading branch information
PhilippeOberti committed Oct 22, 2024
1 parent 87f0215 commit 7b1daa7
Show file tree
Hide file tree
Showing 15 changed files with 72 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export const TIMELINE_DESCRIPTION = i18n.translate(
export const NOTE_DESCRIPTION = i18n.translate(
'xpack.securitySolution.navLinks.investigations.note.title',
{
defaultMessage: 'Oversee, revise and revisit the annotations within each document and timeline',
defaultMessage:
'Oversee, revise, and revisit the notes attached to alerts, events and Timelines.',
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const renderTestComponent = (props: Partial<ComponentProps<typeof AddEventNoteAc
const localProps: ComponentProps<typeof AddEventNoteAction> = {
timelineType: TimelineTypeEnum.default,
eventId: 'event-1',
ariaLabel: 'Add Note',
ariaLabel: 'Add note',
toggleShowNotes: toggleShowNotesMock,
notesCount: 2,
...props,
Expand Down Expand Up @@ -76,12 +76,12 @@ describe('AddEventNoteAction', () => {

expect(NotesButtonMock).toHaveBeenCalledWith(
expect.objectContaining({
ariaLabel: 'Add Note',
ariaLabel: 'Add note',
'data-test-subj': 'add-note',
isDisabled: false,
timelineType: TimelineTypeEnum.default,
toggleShowNotes: expect.any(Function),
toolTip: '2 Notes available. Click to view them & add more.',
toolTip: '2 notes available. Click to view them and add more.',
eventId: 'event-1',
notesCount: 2,
}),
Expand All @@ -98,12 +98,12 @@ describe('AddEventNoteAction', () => {

expect(NotesButtonMock).toHaveBeenCalledWith(
expect.objectContaining({
ariaLabel: 'Add Note',
ariaLabel: 'Add note',
'data-test-subj': 'add-note',
isDisabled: false,
timelineType: TimelineTypeEnum.default,
toggleShowNotes: expect.any(Function),
toolTip: '1 Note available. Click to view it & add more.',
toolTip: '1 note available. Click to view it and add more.',
eventId: 'event-2',
notesCount: 1,
}),
Expand All @@ -120,12 +120,12 @@ describe('AddEventNoteAction', () => {

expect(NotesButtonMock).toHaveBeenCalledWith(
expect.objectContaining({
ariaLabel: 'Add Note',
ariaLabel: 'Add note',
'data-test-subj': 'add-note',
isDisabled: false,
timelineType: TimelineTypeEnum.default,
toggleShowNotes: expect.any(Function),
toolTip: 'Add Note',
toolTip: 'Add note',
eventId: 'event-3',
notesCount: 0,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,34 @@
*/

import React, { useMemo } from 'react';
import { i18n } from '@kbn/i18n';
import { NotesButton } from '../../../timelines/components/timeline/properties/helpers';
import { type TimelineType, TimelineTypeEnum } from '../../../../common/api/timeline';
import { useUserPrivileges } from '../user_privileges';
import * as i18n from './translations';
import { ActionIconItem } from './action_icon_item';

const NOTES_DISABLE_TOOLTIP = i18n.translate(
'xpack.securitySolution.timeline.body.notes.disableEventTooltip',
{
defaultMessage: 'Notes cannot be added here while editing a template Timeline.',
}
);
const NOTES_ADD_TOOLTIP = i18n.translate(
'xpack.securitySolution.timeline.body.notes.addNoteTooltip',
{
defaultMessage: 'Add note',
}
);
const NOTES_COUNT_TOOLTIP = ({ notesCount }: { notesCount: number }) =>
i18n.translate(
'xpack.securitySolution.timeline.body.notes.addNote.multipleNotesAvailableTooltip',
{
values: { notesCount },
defaultMessage:
'{notesCount} {notesCount, plural, one {note} other {notes} } available. Click to view {notesCount, plural, one {it} other {them}} and add more.',
}
);

interface AddEventNoteActionProps {
ariaLabel?: string;
timelineType: TimelineType;
Expand All @@ -33,7 +55,7 @@ const AddEventNoteActionComponent: React.FC<AddEventNoteActionProps> = ({
const { kibanaSecuritySolutionsPrivileges } = useUserPrivileges();

const NOTES_TOOLTIP = useMemo(
() => (notesCount > 0 ? i18n.NOTES_COUNT_TOOLTIP({ notesCount }) : i18n.NOTES_ADD_TOOLTIP),
() => (notesCount > 0 ? NOTES_COUNT_TOOLTIP({ notesCount }) : NOTES_ADD_TOOLTIP),
[notesCount]
);

Expand All @@ -45,9 +67,7 @@ const AddEventNoteActionComponent: React.FC<AddEventNoteActionProps> = ({
isDisabled={kibanaSecuritySolutionsPrivileges.crud === false}
timelineType={timelineType}
toggleShowNotes={toggleShowNotes}
toolTip={
timelineType === TimelineTypeEnum.template ? i18n.NOTES_DISABLE_TOOLTIP : NOTES_TOOLTIP
}
toolTip={timelineType === TimelineTypeEnum.template ? NOTES_DISABLE_TOOLTIP : NOTES_TOOLTIP}
eventId={eventId}
notesCount={notesCount}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,6 @@ export const OPEN_SESSION_VIEW = i18n.translate(
}
);

export const NOTES_DISABLE_TOOLTIP = i18n.translate(
'xpack.securitySolution.timeline.body.notes.disableEventTooltip',
{
defaultMessage: 'Notes may not be added here while editing a template timeline',
}
);

export const NOTES_ADD_TOOLTIP = i18n.translate(
'xpack.securitySolution.timeline.body.notes.addNoteTooltip',
{
defaultMessage: 'Add Note',
}
);

export const NOTES_COUNT_TOOLTIP = ({ notesCount }: { notesCount: number }) =>
i18n.translate(
'xpack.securitySolution.timeline.body.notes.addNote.multipleNotesAvailableTooltip',
{
values: { notesCount },
defaultMessage:
'{notesCount} {notesCount, plural, one {Note} other {Notes} } available. Click to view {notesCount, plural, one {it} other {them}} & add more.',
}
);

export const SORT_FIELDS = i18n.translate('xpack.securitySolution.timeline.sortFieldsButton', {
defaultMessage: 'Sort fields',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ describe('AttachToActiveTimeline', () => {

expect(getByTestId(SAVE_TIMELINE_BUTTON_TEST_ID)).toBeInTheDocument();
expect(getByTestId(SAVE_TIMELINE_BUTTON_TEST_ID)).toHaveStyle('background-color: #FEC514');
expect(getByTestId(SAVE_TIMELINE_BUTTON_TEST_ID)).toHaveTextContent('Save timeline');
expect(getByTestId(SAVE_TIMELINE_BUTTON_TEST_ID)).toHaveTextContent('Save current Timeline');
expect(queryByTestId(ATTACH_TO_TIMELINE_CHECKBOX_TEST_ID)).not.toBeInTheDocument();
expect(getByTestId(ATTACH_TO_TIMELINE_CALLOUT_TEST_ID)).toBeInTheDocument();
expect(getByTestId(ATTACH_TO_TIMELINE_CALLOUT_TEST_ID)).toHaveClass('euiCallOut--warning');
expect(getByText('Attach to timeline')).toBeInTheDocument();
expect(getByText('Attach to current Timeline')).toBeInTheDocument();
expect(
getByText('Before attaching a note to the timeline, you need to save the timeline first.')
getByText('You must save the current Timeline before attaching notes to it.')
).toBeInTheDocument();
});

Expand All @@ -76,7 +76,7 @@ describe('AttachToActiveTimeline', () => {
},
});

const { getByTestId, getByText, queryByTestId } = render(
const { getByTestId, getByText, getAllByText, queryByTestId } = render(
<TestProviders store={mockStore}>
<AttachToActiveTimeline
setAttachToTimeline={mockSetAttachToTimeline}
Expand All @@ -89,10 +89,8 @@ describe('AttachToActiveTimeline', () => {
expect(getByTestId(ATTACH_TO_TIMELINE_CHECKBOX_TEST_ID)).toBeInTheDocument();
expect(getByTestId(ATTACH_TO_TIMELINE_CALLOUT_TEST_ID)).toBeInTheDocument();
expect(getByTestId(ATTACH_TO_TIMELINE_CALLOUT_TEST_ID)).toHaveClass('euiCallOut--primary');
expect(getByText('Attach to timeline')).toBeInTheDocument();
expect(
getByText('You can associate the newly created note to the active timeline.')
).toBeInTheDocument();
expect(getAllByText('Attach to current Timeline')).toHaveLength(2);
expect(getByText('Also attach this note to the current Timeline.')).toBeInTheDocument();
});

it('should call the callback when user click on the checkbox', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,31 +26,31 @@ const timelineCheckBoxId = 'xpack.securitySolution.flyout.notes.attachToTimeline
export const ATTACH_TO_TIMELINE_CALLOUT_TITLE = i18n.translate(
'xpack.securitySolution.flyout.left.notes.attachToTimeline.calloutTitle',
{
defaultMessage: 'Attach to timeline',
defaultMessage: 'Attach to current Timeline',
}
);
export const SAVED_TIMELINE_CALLOUT_CONTENT = i18n.translate(
'xpack.securitySolution.flyout.left.notes.attachToTimeline.calloutContent',
{
defaultMessage: 'You can associate the newly created note to the active timeline.',
defaultMessage: 'Also attach this note to the current Timeline.',
}
);
export const UNSAVED_TIMELINE_CALLOUT_CONTENT = i18n.translate(
'xpack.securitySolution.flyout.left.notes.attachToTimeline.calloutContent',
{
defaultMessage: 'Before attaching a note to the timeline, you need to save the timeline first.',
defaultMessage: 'You must save the current Timeline before attaching notes to it.',
}
);
export const ATTACH_TO_TIMELINE_CHECKBOX = i18n.translate(
'xpack.securitySolution.flyout.left.notes.attachToTimeline.checkboxLabel',
{
defaultMessage: 'Attach to active timeline',
defaultMessage: 'Attach to current Timeline',
}
);
export const SAVE_TIMELINE_BUTTON = i18n.translate(
'xpack.securitySolution.flyout.left.notes.savedTimelineButtonLabel',
'xpack.securitySolution.flyout.left.notes.attachToTimeline.savedTimelineButtonLabel',
{
defaultMessage: 'Save timeline',
defaultMessage: 'Save current Timeline',
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const FETCH_NOTES_ERROR = i18n.translate(
);
export const NO_NOTES = (isAlert: boolean) =>
i18n.translate('xpack.securitySolution.flyout.left.notes.noNotesLabel', {
defaultMessage: 'No notes have been created for this {value}',
defaultMessage: 'No notes have been created for this {value}.',
values: { value: isAlert ? 'alert' : 'event' },
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ export const links: LinkItem = {
title: NOTES,
description: i18n.translate('xpack.securitySolution.appLinks.notesDescription', {
defaultMessage:
'Oversee, revise and revisit the annotations within each document and timeline.',
'Oversee, revise, and revisit the notes attached to alerts, events and Timelines.',
}),
landingIcon: 'filebeatApp',
path: NOTES_PATH,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { DocumentDetailsRightPanelKey } from '../../flyout/document_details/shar
export const OPEN_FLYOUT_BUTTON = i18n.translate(
'xpack.securitySolution.notes.openFlyoutButtonLabel',
{
defaultMessage: 'Expand event details',
defaultMessage: 'Expand alert/event details',
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import { ASSOCIATED_NOT_SELECT_TEST_ID, SEARCH_BAR_TEST_ID } from './test_ids';
import { userFilterAssociatedNotes, userSearchedNotes } from '..';
import { AssociatedFilter } from '../../../common/notes/constants';

const FILTER_SELECT = i18n.translate('xpack.securitySolution.notes.management.filterSelect', {
defaultMessage: 'Select filter',
const ATTACH_FILTER = i18n.translate('xpack.securitySolution.notes.management.attachFilter', {
defaultMessage: 'Attached to',
});

const searchBox = {
Expand All @@ -31,11 +31,14 @@ const searchBox = {
'data-test-subj': SEARCH_BAR_TEST_ID,
};
const associatedNoteSelectOptions: EuiSelectOption[] = [
{ value: AssociatedFilter.all, text: 'All' },
{ value: AssociatedFilter.documentOnly, text: 'Attached to document only' },
{ value: AssociatedFilter.savedObjectOnly, text: 'Attached to timeline only' },
{ value: AssociatedFilter.documentAndSavedObject, text: 'Attached to document and timeline' },
{ value: AssociatedFilter.orphan, text: 'Orphan' },
{ value: AssociatedFilter.all, text: 'Anything or nothing' },
{ value: AssociatedFilter.documentOnly, text: 'Alerts or events only' },
{ value: AssociatedFilter.savedObjectOnly, text: 'Timelines only' },
{
value: AssociatedFilter.documentAndSavedObject,
text: 'Alerts or events and Timelines only',
},
{ value: AssociatedFilter.orphan, text: 'Nothing' },
];

export const SearchRow = React.memo(() => {
Expand Down Expand Up @@ -69,8 +72,8 @@ export const SearchRow = React.memo(() => {
id={associatedSelectId}
options={associatedNoteSelectOptions}
onChange={onAssociatedNoteSelectChange}
prepend={FILTER_SELECT}
aria-label={FILTER_SELECT}
prepend={ATTACH_FILTER}
aria-label={ATTACH_FILTER}
data-test-subj={ASSOCIATED_NOT_SELECT_TEST_ID}
/>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ describe('SaveTimelineCallout', () => {

expect(getByTestId(SAVE_TIMELINE_BUTTON_TEST_ID)).toBeInTheDocument();
expect(getByTestId(SAVE_TIMELINE_BUTTON_TEST_ID)).toHaveStyle('background-color: #BD271E');
expect(getByTestId(SAVE_TIMELINE_BUTTON_TEST_ID)).toHaveTextContent('Save timeline');
expect(getByTestId(SAVE_TIMELINE_BUTTON_TEST_ID)).toHaveTextContent('Save Timeline');
expect(getByTestId(SAVE_TIMELINE_CALLOUT_TEST_ID)).toBeInTheDocument();
expect(getAllByText('Save timeline')).toHaveLength(2);
expect(getAllByText('Save Timeline')).toHaveLength(2);
expect(
getByText('You need to save your timeline before creating notes for it.')
getByText('You must save this Timeline before attaching notes to it.')
).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@ import { SaveTimelineButton } from '../modal/actions/save_timeline_button';
export const SAVE_TIMELINE_CALLOUT_TITLE = i18n.translate(
'xpack.securitySolution.timeline.notes.saveTimeline.calloutTitle',
{
defaultMessage: 'Save timeline',
defaultMessage: 'Save Timeline',
}
);
export const SAVE_TIMELINE_CALLOUT_CONTENT = i18n.translate(
'xpack.securitySolution.timeline.notes.saveTimeline.calloutContent',
{
defaultMessage: 'You need to save your timeline before creating notes for it.',
defaultMessage: 'You must save this Timeline before attaching notes to it.',
}
);
export const SAVE_TIMELINE_BUTTON = i18n.translate(
'xpack.securitySolution.flyout.left.notes.savedTimelineButtonLabel',
'xpack.securitySolution.flyout.left.notes.saveTimeline.buttonLabel',
{
defaultMessage: 'Save timeline',
defaultMessage: 'Save Timeline',
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const FETCH_NOTES_ERROR = i18n.translate(
}
);
export const NO_NOTES = i18n.translate('xpack.securitySolution.notes.noNotesLabel', {
defaultMessage: 'No notes have yet been created for this timeline',
defaultMessage: 'No notes have been created for this Timeline.',
});

interface NotesTabContentProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ describe('query tab with unified timeline', () => {
await waitFor(() => {
expect(screen.getByTestId('timeline-notes-tool-tip')).toBeInTheDocument();
expect(screen.getByTestId('timeline-notes-tool-tip')).toHaveTextContent(
'1 Note available. Click to view it & add more.'
'1 note available. Click to view it and add more.'
);
});
},
Expand Down Expand Up @@ -975,7 +975,7 @@ describe('query tab with unified timeline', () => {
await waitFor(() => {
expect(screen.getByTestId('timeline-notes-tool-tip')).toBeVisible();
expect(screen.getByTestId('timeline-notes-tool-tip')).toHaveTextContent(
'1 Note available. Click to view it & add more.'
'1 note available. Click to view it and add more.'
);
});
},
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/security_solution/server/ui_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ export const initUiSettings = (
max: 1000,
defaultValue: DEFAULT_MAX_UNASSOCIATED_NOTES,
}),
category: [APP_ID],
requiresPageReload: false,
},
[EXCLUDED_DATA_TIERS_FOR_RULE_EXECUTION]: {
Expand Down

0 comments on commit 7b1daa7

Please sign in to comment.