Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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 @@ -43,6 +43,10 @@ export interface ToastOptions {
* How long should the toast remain on screen.
*/
toastLifeTimeMs?: number;
/**
* Perform an action after the toast has been closed.
*/
onClose?: ToastInputFields['onClose'];
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ export const stackManagementSchema: MakeSchemaFrom<UsageStats> = {
_meta: { description: 'Non-default value of setting.' },
},
},
'securitySolution:maxUnassociatedNotes': {
type: 'integer',
_meta: { description: 'The maximum number of allowed unassociated notes' },
},
'securitySolution:defaultThreatIndex': {
type: 'keyword',
_meta: { description: 'Default value of the setting was changed.' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,5 +182,6 @@ export interface UsageStats {
'aiAssistant:preferredAIAssistantType': string;
'observability:profilingFetchTopNFunctionsFromStacktraces': boolean;
'securitySolution:excludedDataTiersForRuleExecution': string[];
'securitySolution:maxUnassociatedNotes': number;
'observability:searchExcludedDataTiers': string[];
}
8 changes: 7 additions & 1 deletion src/plugins/telemetry/schema/oss_plugins.json
Original file line number Diff line number Diff line change
Expand Up @@ -9757,6 +9757,12 @@
}
}
},
"securitySolution:maxUnassociatedNotes": {
"type": "integer",
"_meta": {
"description": "The maximum number of allowed unassociated notes"
}
},
"securitySolution:defaultThreatIndex": {
"type": "keyword",
"_meta": {
Expand Down Expand Up @@ -9919,7 +9925,7 @@
"description": "Non-default value of setting."
}
},
"securitySolution:enableVisualizationsInFlyout":{
"securitySolution:enableVisualizationsInFlyout": {
"type": "boolean",
"_meta": {
"description": "Non-default value of setting."
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/security_solution/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export const SECURITY_TAG_NAME = 'Security Solution' as const;
export const SECURITY_TAG_DESCRIPTION = 'Security Solution auto-generated tag' as const;
export const DEFAULT_SPACE_ID = 'default' as const;
export const DEFAULT_RELATIVE_DATE_THRESHOLD = 24 as const;
export const DEFAULT_MAX_UNASSOCIATED_NOTES = 1000 as const;

// Document path where threat indicator fields are expected. Fields are used
// to enrich signals, and are copied to threat.enrichments.
Expand Down Expand Up @@ -199,6 +200,9 @@ export const ENABLE_ASSET_CRITICALITY_SETTING = 'securitySolution:enableAssetCri
export const EXCLUDED_DATA_TIERS_FOR_RULE_EXECUTION =
'securitySolution:excludedDataTiersForRuleExecution' as const;

/** This Kibana Advances setting allows users to define the maximum amount of unassociated notes (notes without a `timelineId`) */
export const MAX_UNASSOCIATED_NOTES = 'securitySolution:maxUnassociatedNotes' as const;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider renaming this to something as below because current name feels like it is the number of MAX_NOTES but not the setting name.

Suggested change
export const MAX_UNASSOCIATED_NOTES = 'securitySolution:maxUnassociatedNotes' as const;
export const MAX_UNASSOCIATED_NOTES_SETTING = 'securitySolution:maxUnassociatedNotes' as const;


/** This Kibana Advanced Setting allows users to enable/disable the Visualizations in Flyout feature */
export const ENABLE_VISUALIZATIONS_IN_FLYOUT_SETTING =
'securitySolution:enableVisualizationsInFlyout' as const;
Expand Down
68 changes: 33 additions & 35 deletions x-pack/plugins/security_solution/public/notes/api/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
* 2.0.
*/

import type { BareNote, Note } from '../../../common/api/timeline';
import type {
BareNote,
DeleteNoteResponse,
GetNotesResponse,
PersistNoteRouteResponse,
} from '../../../common/api/timeline';
import { KibanaServices } from '../../common/lib/kibana';
import { NOTE_URL } from '../../../common/constants';

Expand All @@ -16,16 +21,18 @@ import { NOTE_URL } from '../../../common/constants';
*/
export const createNote = async ({ note }: { note: BareNote }) => {
try {
const response = await KibanaServices.get().http.patch<{
data: { persistNote: { code: number; message: string; note: Note } };
}>(NOTE_URL, {
const response = await KibanaServices.get().http.patch<PersistNoteRouteResponse>(NOTE_URL, {
method: 'PATCH',
body: JSON.stringify({ note }),
version: '2023-10-31',
});
return response.data.persistNote.note;
const noteResponse = response.data.persistNote;
if (noteResponse.code !== 200) {
throw new Error(noteResponse.message);
}
return noteResponse.note;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider adding unit tests for this case.

} catch (err) {
throw new Error(`Failed to stringify query: ${JSON.stringify(err)}`);
throw new Error(('message' in err && err.message) || 'Request failed');
}
};

Expand All @@ -44,56 +51,47 @@ export const fetchNotes = async ({
filter: string;
search: string;
}) => {
const response = await KibanaServices.get().http.get<{ totalCount: number; notes: Note[] }>(
NOTE_URL,
{
query: {
page,
perPage,
sortField,
sortOrder,
filter,
search,
},
version: '2023-10-31',
}
);
const response = await KibanaServices.get().http.get<GetNotesResponse>(NOTE_URL, {
query: {
page,
perPage,
sortField,
sortOrder,
filter,
search,
},
version: '2023-10-31',
});
return response;
};

/**
* Fetches all the notes for an array of document ids
*/
export const fetchNotesByDocumentIds = async (documentIds: string[]) => {
const response = await KibanaServices.get().http.get<{ notes: Note[]; totalCount: number }>(
NOTE_URL,
{
query: { documentIds },
version: '2023-10-31',
}
);
const response = await KibanaServices.get().http.get<GetNotesResponse>(NOTE_URL, {
query: { documentIds },
version: '2023-10-31',
});
return response;
};

/**
* Fetches all the notes for an array of saved object ids
*/
export const fetchNotesBySaveObjectIds = async (savedObjectIds: string[]) => {
const response = await KibanaServices.get().http.get<{ notes: Note[]; totalCount: number }>(
NOTE_URL,
{
query: { savedObjectIds },
version: '2023-10-31',
}
);
const response = await KibanaServices.get().http.get<GetNotesResponse>(NOTE_URL, {
query: { savedObjectIds },
version: '2023-10-31',
});
return response;
};

/**
* Deletes multiple notes
*/
export const deleteNotes = async (noteIds: string[]) => {
const response = await KibanaServices.get().http.delete<{ data: unknown }>(NOTE_URL, {
const response = await KibanaServices.get().http.delete<DeleteNoteResponse>(NOTE_URL, {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👌 on adding all the types.

body: JSON.stringify({ noteIds }),
version: '2023-10-31',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ describe('AddNote', () => {
});

it('should render error toast if create a note fails', () => {
const createNoteError = new Error('This error comes from the backend');
const store = createMockStore({
...mockGlobalState,
notes: {
Expand All @@ -112,7 +113,7 @@ describe('AddNote', () => {
},
error: {
...mockGlobalState.notes.error,
createNote: { type: 'http', status: 500 },
createNote: createNoteError,
},
},
});
Expand All @@ -123,9 +124,12 @@ describe('AddNote', () => {
</TestProviders>
);

expect(mockAddError).toHaveBeenCalledWith(null, {
title: CREATE_NOTE_ERROR,
});
expect(mockAddError).toHaveBeenCalledWith(
createNoteError,
expect.objectContaining({
title: CREATE_NOTE_ERROR,
})
);
});

it('should call onNodeAdd callback when it is available', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
ReqStatus,
selectCreateNoteError,
selectCreateNoteStatus,
userClosedCreateErrorToast,
} from '../store/notes.slice';
import { MarkdownEditor } from '../../common/components/markdown_editor';

Expand Down Expand Up @@ -101,14 +102,19 @@ export const AddNote = memo(
setEditorValue('');
}, [dispatch, editorValue, eventId, telemetry, timelineId, onNoteAdd]);

const resetError = useCallback(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

currently if the user tries to close the error toast using the cross icon in the top-right corner nothing happens. You're forced to click on the See the full error button that opens the modal then it clears

Screen.Recording.2024-10-10.at.4.35.21.PM.mov

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is now fixed, thanks!

dispatch(userClosedCreateErrorToast());
}, [dispatch]);

// show a toast if the create note call fails
useEffect(() => {
if (createStatus === ReqStatus.Failed && createError) {
addErrorToast(null, {
addErrorToast(createError, {
title: CREATE_NOTE_ERROR,
onClose: resetError,
});
}
}, [addErrorToast, createError, createStatus]);
}, [addErrorToast, createError, createStatus, resetError]);

const buttonDisabled = useMemo(
() => disableButton || editorValue.trim().length === 0 || isMarkdownInvalid,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
fetchNotesBySavedObjectIds,
selectNotesBySavedObjectId,
selectSortedNotesBySavedObjectId,
userClosedCreateErrorToast,
} from './notes.slice';
import type { NotesState } from './notes.slice';
import { mockGlobalState } from '../../common/mock';
Expand Down Expand Up @@ -533,6 +534,25 @@ describe('notesSlice', () => {
});
});

describe('userClosedCreateErrorToast', () => {
it('should reset create note error', () => {
const action = { type: userClosedCreateErrorToast.type };

expect(
notesReducer(
{
...initalEmptyState,
error: {
...initalEmptyState.error,
createNote: new Error(),
},
},
action
).error.createNote
).toBe(null);
});
});

describe('userSelectedRowForDeletion', () => {
it('should set correct id when user selects a row', () => {
const action = { type: userSelectedRowForDeletion.type, payload: '1' };
Expand Down
21 changes: 14 additions & 7 deletions x-pack/plugins/security_solution/public/notes/store/notes.slice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export const fetchNotesByDocumentIds = createAsyncThunk<
>('notes/fetchNotesByDocumentIds', async (args) => {
const { documentIds } = args;
const res = await fetchNotesByDocumentIdsApi(documentIds);
return normalizeEntities(res.notes);
return normalizeEntities('notes' in res ? res.notes : []);
});

export const fetchNotesBySavedObjectIds = createAsyncThunk<
Expand All @@ -113,7 +113,7 @@ export const fetchNotesBySavedObjectIds = createAsyncThunk<
>('notes/fetchNotesBySavedObjectIds', async (args) => {
const { savedObjectIds } = args;
const res = await fetchNotesBySaveObjectIdsApi(savedObjectIds);
return normalizeEntities(res.notes);
return normalizeEntities('notes' in res ? res.notes : []);
});

export const fetchNotes = createAsyncThunk<
Expand All @@ -130,7 +130,10 @@ export const fetchNotes = createAsyncThunk<
>('notes/fetchNotes', async (args) => {
const { page, perPage, sortField, sortOrder, filter, search } = args;
const res = await fetchNotesApi({ page, perPage, sortField, sortOrder, filter, search });
return { ...normalizeEntities(res.notes), totalCount: res.totalCount };
return {
...normalizeEntities('notes' in res ? res.notes : []),
totalCount: 'totalCount' in res ? res.totalCount : 0,
Comment thread
PhilippeOberti marked this conversation as resolved.
};
});

export const createNote = createAsyncThunk<NormalizedEntity<Note>, { note: BareNote }, {}>(
Expand Down Expand Up @@ -199,6 +202,9 @@ const notesSlice = createSlice({
userSelectedBulkDelete: (state) => {
state.pendingDeleteIds = state.selectedIds;
},
userClosedCreateErrorToast: (state) => {
state.error.createNote = null;
},
},
extraReducers(builder) {
builder
Expand Down Expand Up @@ -308,12 +314,12 @@ export const selectFetchNotesError = (state: State) => state.notes.error.fetchNo
export const selectFetchNotesStatus = (state: State) => state.notes.status.fetchNotes;

export const selectNotesByDocumentId = createSelector(
[selectAllNotes, (state: State, documentId: string) => documentId],
[selectAllNotes, (_: State, documentId: string) => documentId],
Comment thread
PhilippeOberti marked this conversation as resolved.
(notes, documentId) => notes.filter((note) => note.eventId === documentId)
);

export const selectNotesBySavedObjectId = createSelector(
[selectAllNotes, (state: State, savedObjectId: string) => savedObjectId],
[selectAllNotes, (_: State, savedObjectId: string) => savedObjectId],
(notes, savedObjectId) =>
savedObjectId.length > 0 ? notes.filter((note) => note.timelineId === savedObjectId) : []
);
Expand All @@ -334,7 +340,7 @@ export const selectSortedNotesByDocumentId = createSelector(
[
selectAllNotes,
(
state: State,
_: State,
{
documentId,
sort,
Expand All @@ -359,7 +365,7 @@ export const selectSortedNotesBySavedObjectId = createSelector(
[
selectAllNotes,
(
state: State,
_: State,
{
savedObjectId,
sort,
Expand Down Expand Up @@ -391,6 +397,7 @@ export const {
userSearchedNotes,
userSelectedRow,
userClosedDeleteModal,
userClosedCreateErrorToast,
userSelectedRowForDeletion,
userSelectedBulkDelete,
} = notesSlice.actions;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { NOTE_URL } from '../../../../common/constants';
import type { BareNote, Note } from '../../../../common/api/timeline';
import type { BareNote, PersistNoteRouteResponse } from '../../../../common/api/timeline';
import { KibanaServices } from '../../../common/lib/kibana';

export const persistNote = async ({
Expand All @@ -27,7 +27,7 @@ export const persistNote = async ({
} catch (err) {
return Promise.reject(new Error(`Failed to stringify query: ${JSON.stringify(err)}`));
}
const response = await KibanaServices.get().http.patch<Note[]>(NOTE_URL, {
const response = await KibanaServices.get().http.patch<PersistNoteRouteResponse>(NOTE_URL, {
method: 'PATCH',
body: requestBody,
version: '2023-10-31',
Expand Down
Loading