[Security Solution][Notes] Make MAX_UNASSOCIATED_NOTES an advanced Ki…

…bana setting (#194947) ## Summary Fixes: #193097 Adds a new Kibana advanced setting that allows users to limit the maximum amount of unassociated notes. The max value for that used to be hard coded before. https://github.com/user-attachments/assets/34af7f67-9109-4251-a5a3-a1af68f123fe ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Elastic Machine <[email protected]>
elastic · Oct 14, 2024 · 925329e · 925329e
1 parent 424ffba
commit 925329e
Show file tree

Hide file tree

Showing 16 changed files with 220 additions and 77 deletions.
diff --git a/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts b/src/plugins/kibana_usage_collection/server/collectors/management/schema.ts
@@ -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.' },

diff --git a/src/plugins/kibana_usage_collection/server/collectors/management/types.ts b/src/plugins/kibana_usage_collection/server/collectors/management/types.ts
@@ -183,5 +183,6 @@ export interface UsageStats {
   'aiAssistant:preferredAIAssistantType': string;
   'observability:profilingFetchTopNFunctionsFromStacktraces': boolean;
   'securitySolution:excludedDataTiersForRuleExecution': string[];
+  'securitySolution:maxUnassociatedNotes': number;
   'observability:searchExcludedDataTiers': string[];
 }
diff --git a/src/plugins/telemetry/schema/oss_plugins.json b/src/plugins/telemetry/schema/oss_plugins.json
@@ -9888,6 +9888,12 @@
             }
           }
         },
+        "securitySolution:maxUnassociatedNotes": {
+          "type": "integer",
+          "_meta": {
+            "description": "The maximum number of allowed unassociated notes"
+          }
+        },
         "securitySolution:defaultThreatIndex": {
           "type": "keyword",
           "_meta": {
@@ -10050,7 +10056,7 @@
             "description": "Non-default value of setting."
           }
         },
-        "securitySolution:enableVisualizationsInFlyout":{
+        "securitySolution:enableVisualizationsInFlyout": {
           "type": "boolean",
           "_meta": {
             "description": "Non-default value of setting."

diff --git a/x-pack/plugins/security_solution/common/constants.ts b/x-pack/plugins/security_solution/common/constants.ts
@@ -71,6 +71,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.
@@ -200,6 +201,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;
+
 /** 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;

diff --git a/x-pack/plugins/security_solution/public/notes/api/api.test.ts b/x-pack/plugins/security_solution/public/notes/api/api.test.ts
@@ -0,0 +1,53 @@
+/*
+ * 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 type { PersistNoteRouteResponse } from '../../../common/api/timeline';
+import { KibanaServices } from '../../common/lib/kibana';
+import * as api from './api';
+
+jest.mock('../../common/lib/kibana', () => {
+  return {
+    KibanaServices: {
+      get: jest.fn(),
+    },
+  };
+});
+
+describe('Notes API client', () => {
+  beforeAll(() => {
+    jest.resetAllMocks();
+  });
+  describe('create note', () => {
+    it('should throw an error when a response code other than 200 is returned', async () => {
+      const errorResponse: PersistNoteRouteResponse = {
+        data: {
+          persistNote: {
+            code: 500,
+            message: 'Internal server error',
+            note: {
+              timelineId: '1',
+              noteId: '2',
+              version: '3',
+            },
+          },
+        },
+      };
+      (KibanaServices.get as jest.Mock).mockReturnValue({
+        http: {
+          patch: jest.fn().mockReturnValue(errorResponse),
+        },
+      });
+
+      expect(async () =>
+        api.createNote({
+          note: {
+            timelineId: '1',
+          },
+        })
+      ).rejects.toThrow();
+    });
+  });
+});
diff --git a/x-pack/plugins/security_solution/public/notes/api/api.ts b/x-pack/plugins/security_solution/public/notes/api/api.ts
@@ -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';
 
@@ -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;
   } catch (err) {
-    throw new Error(`Failed to stringify query: ${JSON.stringify(err)}`);
+    throw new Error(('message' in err && err.message) || 'Request failed');
   }
 };
 
@@ -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, {
     body: JSON.stringify({ noteIds }),
     version: '2023-10-31',
   });

diff --git a/x-pack/plugins/security_solution/public/notes/components/add_note.test.tsx b/x-pack/plugins/security_solution/public/notes/components/add_note.test.tsx
@@ -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: {
@@ -112,7 +113,7 @@ describe('AddNote', () => {
         },
         error: {
           ...mockGlobalState.notes.error,
-          createNote: { type: 'http', status: 500 },
+          createNote: createNoteError,
         },
       },
     });
@@ -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 () => {

diff --git a/x-pack/plugins/security_solution/public/notes/components/add_note.tsx b/x-pack/plugins/security_solution/public/notes/components/add_note.tsx
@@ -25,6 +25,7 @@ import {
   ReqStatus,
   selectCreateNoteError,
   selectCreateNoteStatus,
+  userClosedCreateErrorToast,
 } from '../store/notes.slice';
 import { MarkdownEditor } from '../../common/components/markdown_editor';
 
@@ -101,14 +102,19 @@ export const AddNote = memo(
       setEditorValue('');
     }, [dispatch, editorValue, eventId, telemetry, timelineId, onNoteAdd]);
 
+    const resetError = useCallback(() => {
+      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,
         });
+        resetError();
       }
-    }, [addErrorToast, createError, createStatus]);
+    }, [addErrorToast, createError, createStatus, resetError]);
 
     const buttonDisabled = useMemo(
       () => disableButton || editorValue.trim().length === 0 || isMarkdownInvalid,

diff --git a/x-pack/plugins/security_solution/public/notes/store/notes.slice.test.ts b/x-pack/plugins/security_solution/public/notes/store/notes.slice.test.ts
@@ -46,6 +46,7 @@ import {
   fetchNotesBySavedObjectIds,
   selectNotesBySavedObjectId,
   selectSortedNotesBySavedObjectId,
+  userClosedCreateErrorToast,
 } from './notes.slice';
 import type { NotesState } from './notes.slice';
 import { mockGlobalState } from '../../common/mock';
@@ -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('userSelectedNotesForDeletion', () => {
       it('should set correct id when user selects a note to delete', () => {
         const action = { type: userSelectedNotesForDeletion.type, payload: '1' };