[Security Solution][Notes] - switch the securitySolutionNotesEnables …

…feature flag to securitySolutionNotesDisabled (elastic#196778) ## Summary This PR switches the `securitySolutionNotesEnabled` to `securitySolutionNotesDisabled` (with a `false` value by default) to enable the new Notes functionality in `8.16`. Customers can set the new `securitySolutionNotesDisabled` feature flag to true in their environment if they want to go back to the old notes system. The PR also fixes a tiny bug with the badge showing the number of notes in the Timeline Notes tab. The new system was not taking into account a timeline description, so if the timeline had a description the number of notes was always 1 lower than the actual number of notes displayed below. This issue was highlighted by a Cypress test! The goal is to remove the old system entirely within a few releases (maybe `8.18` or `9.0`). ### 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 elastic#189879
viduni94 · Oct 29, 2024 · 4fb4282 · 4fb4282
1 parent 81856bc
commit 4fb4282
Show file tree

Hide file tree

Showing 27 changed files with 157 additions and 129 deletions.
diff --git a/x-pack/plugins/security_solution/common/experimental_features.ts b/x-pack/plugins/security_solution/common/experimental_features.ts
@@ -94,9 +94,9 @@ export const allowedExperimentalValues = Object.freeze({
   endpointManagementSpaceAwarenessEnabled: false,
 
   /**
-   * Enables new notes
+   * Disables new notes
    */
-  securitySolutionNotesEnabled: false,
+  securitySolutionNotesDisabled: false,
 
   /**
    * Disables entity and alert previews

diff --git a/...k/plugins/security_solution/public/common/components/control_columns/row_action/index.tsx b/...k/plugins/security_solution/public/common/components/control_columns/row_action/index.tsx
@@ -93,8 +93,8 @@ const RowActionComponent = ({
     [columnHeaders, timelineNonEcsData]
   );
 
-  const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
-    'securitySolutionNotesEnabled'
+  const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
+    'securitySolutionNotesDisabled'
   );
 
   const handleOnEventDetailPanelOpened = useCallback(() => {
@@ -175,12 +175,12 @@ const RowActionComponent = ({
           showCheckboxes={showCheckboxes}
           tabType={tabType}
           timelineId={tableId}
-          toggleShowNotes={securitySolutionNotesEnabled ? toggleShowNotes : undefined}
+          toggleShowNotes={securitySolutionNotesDisabled ? undefined : toggleShowNotes}
           width={width}
           setEventsLoading={setEventsLoading}
           setEventsDeleted={setEventsDeleted}
           refetch={refetch}
-          showNotes={securitySolutionNotesEnabled ? true : false}
+          showNotes={!securitySolutionNotesDisabled}
         />
       )}
     </>

diff --git a/...gins/security_solution/public/common/components/events_tab/events_query_tab_body.test.tsx b/...gins/security_solution/public/common/components/events_tab/events_query_tab_body.test.tsx
@@ -16,6 +16,9 @@ import { useGlobalFullScreen } from '../../containers/use_full_screen';
 import { licenseService } from '../../hooks/use_license';
 import { mockHistory } from '../../mock/router';
 import { DEFAULT_EVENTS_STACK_BY_VALUE } from './histogram_configurations';
+import { useIsExperimentalFeatureEnabled } from '../../hooks/use_experimental_features';
+
+jest.mock('../../hooks/use_experimental_features');
 
 const mockGetDefaultControlColumn = jest.fn();
 jest.mock('../../../timelines/components/timeline/body/control_columns', () => ({
@@ -95,6 +98,7 @@ describe('EventsQueryTabBody', () => {
   };
 
   beforeEach(() => {
+    (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(false);
     jest.clearAllMocks();
   });
 
@@ -106,7 +110,7 @@ describe('EventsQueryTabBody', () => {
     );
 
     expect(queryByText('MockedStatefulEventsViewer')).toBeInTheDocument();
-    expect(mockGetDefaultControlColumn).toHaveBeenCalledWith(4);
+    expect(mockGetDefaultControlColumn).toHaveBeenCalledWith(5);
   });
 
   it('renders the matrix histogram when globalFullScreen is false', () => {
@@ -186,7 +190,19 @@ describe('EventsQueryTabBody', () => {
     expect(spy).toHaveBeenCalled();
   });
 
-  it('only have 4 columns on Action bar for non-Enterprise user', () => {
+  it('should have 5 columns on Action bar for non-Enterprise user', () => {
+    render(
+      <TestProviders>
+        <EventsQueryTabBody {...commonProps} />
+      </TestProviders>
+    );
+
+    expect(mockGetDefaultControlColumn).toHaveBeenCalledWith(5);
+  });
+
+  it('should have 4 columns on Action bar for non-Enterprise user and securitySolutionNotesDisabled is true', () => {
+    (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(true);
+
     render(
       <TestProviders>
         <EventsQueryTabBody {...commonProps} />
@@ -196,10 +212,23 @@ describe('EventsQueryTabBody', () => {
     expect(mockGetDefaultControlColumn).toHaveBeenCalledWith(4);
   });
 
-  it('shows 5 columns on Action bar for Enterprise user', () => {
+  it('should 6 columns on Action bar for Enterprise user', () => {
     const licenseServiceMock = licenseService as jest.Mocked<typeof licenseService>;
+    licenseServiceMock.isEnterprise.mockReturnValue(true);
 
+    render(
+      <TestProviders>
+        <EventsQueryTabBody {...commonProps} />
+      </TestProviders>
+    );
+
+    expect(mockGetDefaultControlColumn).toHaveBeenCalledWith(6);
+  });
+
+  it('should 6 columns on Action bar for Enterprise user and securitySolutionNotesDisabled is true', () => {
+    const licenseServiceMock = licenseService as jest.Mocked<typeof licenseService>;
     licenseServiceMock.isEnterprise.mockReturnValue(true);
+    (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(true);
 
     render(
       <TestProviders>

diff --git a/...k/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx b/...k/plugins/security_solution/public/common/components/events_tab/events_query_tab_body.tsx
@@ -75,10 +75,10 @@ const EventsQueryTabBodyComponent: React.FC<EventsQueryTabBodyComponentProps> =
   const [defaultNumberFormat] = useUiSetting$<string>(DEFAULT_NUMBER_FORMAT);
   const isEnterprisePlus = useLicense().isEnterprise();
   let ACTION_BUTTON_COUNT = isEnterprisePlus ? 6 : 5;
-  const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
-    'securitySolutionNotesEnabled'
+  const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
+    'securitySolutionNotesDisabled'
   );
-  if (!securitySolutionNotesEnabled) {
+  if (securitySolutionNotesDisabled) {
     ACTION_BUTTON_COUNT--;
   }
   const leadingControlColumns = useMemo(

diff --git a/x-pack/plugins/security_solution/public/common/components/header_actions/actions.tsx b/x-pack/plugins/security_solution/public/common/components/header_actions/actions.tsx
@@ -255,8 +255,8 @@ const ActionsComponent: React.FC<ActionProps> = ({
     onEventDetailsPanelOpened();
   }, [activeStep, incrementStep, isTourAnchor, isTourShown, onEventDetailsPanelOpened]);
 
-  const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
-    'securitySolutionNotesEnabled'
+  const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
+    'securitySolutionNotesDisabled'
   );
 
   /* only applicable for new event based notes */
@@ -270,7 +270,7 @@ const ActionsComponent: React.FC<ActionProps> = ({
 
   /* note ids associated with the document AND attached to the current timeline, used for pinning */
   const timelineNoteIds = useMemo(() => {
-    if (securitySolutionNotesEnabled) {
+    if (!securitySolutionNotesDisabled) {
       // if timeline is unsaved, there is no notes associated to timeline yet
       return savedObjectId ? documentBasedNotesInTimeline.map((note) => note.noteId) : [];
     }
@@ -280,13 +280,13 @@ const ActionsComponent: React.FC<ActionProps> = ({
     eventId,
     documentBasedNotesInTimeline,
     savedObjectId,
-    securitySolutionNotesEnabled,
+    securitySolutionNotesDisabled,
   ]);
 
   /* note count of the document */
   const notesCount = useMemo(
-    () => (securitySolutionNotesEnabled ? documentBasedNotes.length : timelineNoteIds.length),
-    [documentBasedNotes, timelineNoteIds, securitySolutionNotesEnabled]
+    () => (securitySolutionNotesDisabled ? timelineNoteIds.length : documentBasedNotes.length),
+    [documentBasedNotes, timelineNoteIds, securitySolutionNotesDisabled]
   );
 
   // we hide the analyzer icon if the data is not available for the resolver

diff --git a/...urity_solution/public/detections/hooks/trigger_actions_alert_table/use_actions_column.tsx b/...urity_solution/public/detections/hooks/trigger_actions_alert_table/use_actions_column.tsx
@@ -47,10 +47,10 @@ export const getUseActionColumnHook =
     }
 
     // we only want to show the note icon if the new notes system feature flag is enabled
-    const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
-      'securitySolutionNotesEnabled'
+    const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
+      'securitySolutionNotesDisabled'
     );
-    if (!securitySolutionNotesEnabled) {
+    if (securitySolutionNotesDisabled) {
       ACTION_BUTTON_COUNT--;
     }
 

diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/left/index.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/left/index.tsx
@@ -36,8 +36,8 @@ export const LeftPanel: FC<Partial<DocumentDetailsProps>> = memo(({ path }) => {
   const { openLeftPanel } = useExpandableFlyoutApi();
   const { eventId, indexName, scopeId, getFieldsData, isPreview } = useDocumentDetailsContext();
   const eventKind = getField(getFieldsData('event.kind'));
-  const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
-    'securitySolutionNotesEnabled'
+  const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
+    'securitySolutionNotesDisabled'
   );
 
   const [visualizationInFlyoutEnabled] = useUiSetting$<boolean>(
@@ -49,14 +49,14 @@ export const LeftPanel: FC<Partial<DocumentDetailsProps>> = memo(({ path }) => {
       eventKind === EventKind.signal
         ? [tabs.insightsTab, tabs.investigationTab, tabs.responseTab]
         : [tabs.insightsTab];
-    if (securitySolutionNotesEnabled && !isPreview) {
+    if (!securitySolutionNotesDisabled && !isPreview) {
       tabList.push(tabs.notesTab);
     }
     if (visualizationInFlyoutEnabled && !isPreview) {
       return [tabs.visualizeTab, ...tabList];
     }
     return tabList;
-  }, [eventKind, isPreview, securitySolutionNotesEnabled, visualizationInFlyoutEnabled]);
+  }, [eventKind, isPreview, securitySolutionNotesDisabled, visualizationInFlyoutEnabled]);
 
   const selectedTabId = useMemo(() => {
     const defaultTab = tabsDisplayed[0].id;

diff --git a/...rity_solution/public/flyout/document_details/right/components/alert_header_title.test.tsx b/...rity_solution/public/flyout/document_details/right/components/alert_header_title.test.tsx
@@ -78,7 +78,7 @@ describe('<AlertHeaderTitle />', () => {
   });
 
   it('should render notes section if experimental flag is enabled', () => {
-    (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(true);
+    (useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(false);
 
     const { getByTestId } = renderHeader(mockContextValue);
     expect(getByTestId(NOTES_TITLE_TEST_ID)).toBeInTheDocument();

diff --git a/.../security_solution/public/flyout/document_details/right/components/alert_header_title.tsx b/.../security_solution/public/flyout/document_details/right/components/alert_header_title.tsx
@@ -40,8 +40,8 @@ export const AlertHeaderTitle = memo(() => {
     refetchFlyoutData,
     getFieldsData,
   } = useDocumentDetailsContext();
-  const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
-    'securitySolutionNotesEnabled'
+  const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
+    'securitySolutionNotesDisabled'
   );
 
   const { isAlert, ruleName, timestamp, ruleId } = useBasicDataFromDetailsData(
@@ -98,7 +98,30 @@ export const AlertHeaderTitle = memo(() => {
         />
       )}
       <EuiSpacer size="m" />
-      {securitySolutionNotesEnabled ? (
+      {securitySolutionNotesDisabled ? (
+        <EuiFlexGroup
+          direction="row"
+          gutterSize="s"
+          responsive={false}
+          wrap
+          data-test-subj={ALERT_SUMMARY_PANEL_TEST_ID}
+        >
+          <EuiFlexItem>
+            <DocumentStatus />
+          </EuiFlexItem>
+          <EuiFlexItem>
+            <RiskScore />
+          </EuiFlexItem>
+          <EuiFlexItem>
+            <Assignees
+              eventId={eventId}
+              assignedUserIds={alertAssignees}
+              onAssigneesUpdated={onAssigneesUpdated}
+              isPreview={isPreview}
+            />
+          </EuiFlexItem>
+        </EuiFlexGroup>
+      ) : (
         <EuiFlexGroup
           direction="row"
           gutterSize="s"
@@ -132,29 +155,6 @@ export const AlertHeaderTitle = memo(() => {
             </EuiFlexGroup>
           </EuiFlexItem>
         </EuiFlexGroup>
-      ) : (
-        <EuiFlexGroup
-          direction="row"
-          gutterSize="s"
-          responsive={false}
-          wrap
-          data-test-subj={ALERT_SUMMARY_PANEL_TEST_ID}
-        >
-          <EuiFlexItem>
-            <DocumentStatus />
-          </EuiFlexItem>
-          <EuiFlexItem>
-            <RiskScore />
-          </EuiFlexItem>
-          <EuiFlexItem>
-            <Assignees
-              eventId={eventId}
-              assignedUserIds={alertAssignees}
-              onAssigneesUpdated={onAssigneesUpdated}
-              isPreview={isPreview}
-            />
-          </EuiFlexItem>
-        </EuiFlexGroup>
       )}
     </>
   );

diff --git a/x-pack/plugins/security_solution/public/management/links.ts b/x-pack/plugins/security_solution/public/management/links.ts
@@ -229,7 +229,7 @@ export const links: LinkItem = {
       path: NOTES_PATH,
       skipUrlState: true,
       hideTimeline: true,
-      experimentalKey: 'securitySolutionNotesEnabled',
+      hideWhenExperimentalKey: 'securitySolutionNotesDisabled',
     },
   ],
 };

diff --git a/x-pack/plugins/security_solution/public/management/pages/index.tsx b/x-pack/plugins/security_solution/public/management/pages/index.tsx
@@ -88,8 +88,8 @@ const NotesTelemetry = () => (
 );
 
 export const ManagementContainer = memo(() => {
-  const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
-    'securitySolutionNotesEnabled'
+  const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
+    'securitySolutionNotesDisabled'
   );
 
   const {
@@ -162,7 +162,7 @@ export const ManagementContainer = memo(() => {
         hasPrivilege={canReadActionsLogManagement}
       />
 
-      {securitySolutionNotesEnabled && (
+      {!securitySolutionNotesDisabled && (
         <Route path={MANAGEMENT_ROUTING_NOTES_PATH} component={NotesTelemetry} />
       )}
 

diff --git a/x-pack/plugins/security_solution/public/notes/hooks/use_fetch_notes.test.ts b/x-pack/plugins/security_solution/public/notes/hooks/use_fetch_notes.test.ts
@@ -45,24 +45,24 @@ describe('useFetchNotes', () => {
     expect(typeof result.current.onLoad).toBe('function');
   });
 
-  it('should not dispatch action when securitySolutionNotesEnabled is false', () => {
-    mockedUseIsExperimentalFeatureEnabled.mockReturnValue(false);
+  it('should not dispatch action when securitySolutionNotesDisabled is true', () => {
+    mockedUseIsExperimentalFeatureEnabled.mockReturnValue(true);
     const { result } = renderHook(() => useFetchNotes());
 
     result.current.onLoad([{ _id: '1' }]);
     expect(mockDispatch).not.toHaveBeenCalled();
   });
 
   it('should not dispatch action when events array is empty', () => {
-    mockedUseIsExperimentalFeatureEnabled.mockReturnValue(true);
+    mockedUseIsExperimentalFeatureEnabled.mockReturnValue(false);
     const { result } = renderHook(() => useFetchNotes());
 
     result.current.onLoad([]);
     expect(mockDispatch).not.toHaveBeenCalled();
   });
 
   it('should dispatch fetchNotesByDocumentIds with correct ids when conditions are met', () => {
-    mockedUseIsExperimentalFeatureEnabled.mockReturnValue(true);
+    mockedUseIsExperimentalFeatureEnabled.mockReturnValue(false);
     const { result } = renderHook(() => useFetchNotes());
 
     const events = [{ _id: '1' }, { _id: '2' }, { _id: '3' }];
@@ -74,7 +74,7 @@ describe('useFetchNotes', () => {
   });
 
   it('should memoize onLoad function', () => {
-    mockedUseIsExperimentalFeatureEnabled.mockReturnValue(true);
+    mockedUseIsExperimentalFeatureEnabled.mockReturnValue(false);
     const { result, rerender } = renderHook(() => useFetchNotes());
 
     const firstOnLoad = result.current.onLoad;
@@ -84,7 +84,7 @@ describe('useFetchNotes', () => {
     expect(firstOnLoad).toBe(secondOnLoad);
   });
 
-  it('should update onLoad when securitySolutionNotesEnabled changes', () => {
+  it('should update onLoad when securitySolutionNotesDisabled changes', () => {
     mockedUseIsExperimentalFeatureEnabled.mockReturnValue(true);
     const { result, rerender } = renderHook(() => useFetchNotes());
 

diff --git a/x-pack/plugins/security_solution/public/notes/hooks/use_fetch_notes.ts b/x-pack/plugins/security_solution/public/notes/hooks/use_fetch_notes.ts
@@ -22,19 +22,19 @@ export interface UseFetchNotesResult {
  */
 export const useFetchNotes = (): UseFetchNotesResult => {
   const dispatch = useDispatch();
-  const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
-    'securitySolutionNotesEnabled'
+  const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
+    'securitySolutionNotesDisabled'
   );
   const onLoad = useCallback(
     (events: Array<Partial<{ _id: string }>>) => {
-      if (!securitySolutionNotesEnabled || events.length === 0) return;
+      if (securitySolutionNotesDisabled || events.length === 0) return;
 
       const eventIds: string[] = events
         .map((event) => event._id)
         .filter((id) => id != null) as string[];
       dispatch(fetchNotesByDocumentIds({ documentIds: eventIds }));
     },
-    [dispatch, securitySolutionNotesEnabled]
+    [dispatch, securitySolutionNotesDisabled]
   );
 
   return { onLoad };

diff --git a/x-pack/plugins/security_solution/public/notes/links.ts b/x-pack/plugins/security_solution/public/notes/links.ts
@@ -21,5 +21,5 @@ export const links: LinkItem = {
     }),
   ],
   links: [],
-  experimentalKey: 'securitySolutionNotesEnabled',
+  hideWhenExperimentalKey: 'securitySolutionNotesDisabled',
 };