Skip to content

Commit

Permalink
[8.16] [Security Solution][Notes] - switch the securitySolutionNotesE…
Browse files Browse the repository at this point in the history
…nables feature flag to securitySolutionNotesDisabled (elastic#196778) (elastic#198205)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Security Solution][Notes] - switch the securitySolutionNotesEnables
feature flag to securitySolutionNotesDisabled
(elastic#196778)](elastic#196778)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Philippe
Oberti","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-29T21:00:20Z","message":"[Security
Solution][Notes] - switch the securitySolutionNotesEnables feature flag
to securitySolutionNotesDisabled (elastic#196778)\n\n## Summary\r\n\r\nThis PR
switches the `securitySolutionNotesEnabled`
to\r\n`securitySolutionNotesDisabled` (with a `false` value by default)
to\r\nenable the new Notes functionality in `8.16`.\r\nCustomers can set
the new `securitySolutionNotesDisabled` feature flag\r\nto true in their
environment if they want to go back to the old
notes\r\nsystem.\r\n\r\nThe PR also fixes a tiny bug with the badge
showing the number of notes\r\nin the Timeline Notes tab. The new system
was not taking into account a\r\ntimeline description, so if the
timeline had a description the number of\r\nnotes was always 1 lower
than the actual number of notes displayed\r\nbelow. This issue was
highlighted by a Cypress test!\r\n\r\nThe goal is to remove the old
system entirely within a few releases\r\n(maybe `8.18` or
`9.0`).\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\nhttps://github.com/elastic/issues/189879","sha":"4fb4282509e0a5f7605433a5ef8f9e9085647282","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport","release_note:skip","v9.0.0","Team:Threat
Hunting:Investigations","v8.16.0"],"title":"[Security Solution][Notes] -
switch the securitySolutionNotesEnables feature flag to
securitySolutionNotesDisabled","number":196778,"url":"https://github.com/elastic/kibana/pull/196778","mergeCommit":{"message":"[Security
Solution][Notes] - switch the securitySolutionNotesEnables feature flag
to securitySolutionNotesDisabled (elastic#196778)\n\n## Summary\r\n\r\nThis PR
switches the `securitySolutionNotesEnabled`
to\r\n`securitySolutionNotesDisabled` (with a `false` value by default)
to\r\nenable the new Notes functionality in `8.16`.\r\nCustomers can set
the new `securitySolutionNotesDisabled` feature flag\r\nto true in their
environment if they want to go back to the old
notes\r\nsystem.\r\n\r\nThe PR also fixes a tiny bug with the badge
showing the number of notes\r\nin the Timeline Notes tab. The new system
was not taking into account a\r\ntimeline description, so if the
timeline had a description the number of\r\nnotes was always 1 lower
than the actual number of notes displayed\r\nbelow. This issue was
highlighted by a Cypress test!\r\n\r\nThe goal is to remove the old
system entirely within a few releases\r\n(maybe `8.18` or
`9.0`).\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\nhttps://github.com/elastic/issues/189879","sha":"4fb4282509e0a5f7605433a5ef8f9e9085647282"}},"sourceBranch":"main","suggestedTargetBranches":["8.16"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196778","number":196778,"mergeCommit":{"message":"[Security
Solution][Notes] - switch the securitySolutionNotesEnables feature flag
to securitySolutionNotesDisabled (elastic#196778)\n\n## Summary\r\n\r\nThis PR
switches the `securitySolutionNotesEnabled`
to\r\n`securitySolutionNotesDisabled` (with a `false` value by default)
to\r\nenable the new Notes functionality in `8.16`.\r\nCustomers can set
the new `securitySolutionNotesDisabled` feature flag\r\nto true in their
environment if they want to go back to the old
notes\r\nsystem.\r\n\r\nThe PR also fixes a tiny bug with the badge
showing the number of notes\r\nin the Timeline Notes tab. The new system
was not taking into account a\r\ntimeline description, so if the
timeline had a description the number of\r\nnotes was always 1 lower
than the actual number of notes displayed\r\nbelow. This issue was
highlighted by a Cypress test!\r\n\r\nThe goal is to remove the old
system entirely within a few releases\r\n(maybe `8.18` or
`9.0`).\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios\r\n\r\nhttps://github.com/elastic/issues/189879","sha":"4fb4282509e0a5f7605433a5ef8f9e9085647282"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Philippe Oberti <[email protected]>
  • Loading branch information
kibanamachine and PhilippeOberti authored Oct 30, 2024
1 parent 8b362b9 commit 80ade8b
Show file tree
Hide file tree
Showing 27 changed files with 157 additions and 129 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ const RowActionComponent = ({
[columnHeaders, timelineNonEcsData]
);

const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
'securitySolutionNotesEnabled'
const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
'securitySolutionNotesDisabled'
);

const handleOnEventDetailPanelOpened = useCallback(() => {
Expand Down Expand Up @@ -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}
/>
)}
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => ({
Expand Down Expand Up @@ -95,6 +98,7 @@ describe('EventsQueryTabBody', () => {
};

beforeEach(() => {
(useIsExperimentalFeatureEnabled as jest.Mock).mockReturnValue(false);
jest.clearAllMocks();
});

Expand All @@ -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', () => {
Expand Down Expand Up @@ -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} />
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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) : [];
}
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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--;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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>
)}
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ export const links: LinkItem = {
path: NOTES_PATH,
skipUrlState: true,
hideTimeline: true,
experimentalKey: 'securitySolutionNotesEnabled',
hideWhenExperimentalKey: 'securitySolutionNotesDisabled',
},
],
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ const NotesTelemetry = () => (
);

export const ManagementContainer = memo(() => {
const securitySolutionNotesEnabled = useIsExperimentalFeatureEnabled(
'securitySolutionNotesEnabled'
const securitySolutionNotesDisabled = useIsExperimentalFeatureEnabled(
'securitySolutionNotesDisabled'
);

const {
Expand Down Expand Up @@ -162,7 +162,7 @@ export const ManagementContainer = memo(() => {
hasPrivilege={canReadActionsLogManagement}
/>

{securitySolutionNotesEnabled && (
{!securitySolutionNotesDisabled && (
<Route path={MANAGEMENT_ROUTING_NOTES_PATH} component={NotesTelemetry} />
)}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' }];
Expand All @@ -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;
Expand All @@ -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());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/public/notes/links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ export const links: LinkItem = {
}),
],
links: [],
experimentalKey: 'securitySolutionNotesEnabled',
hideWhenExperimentalKey: 'securitySolutionNotesDisabled',
};
Loading

0 comments on commit 80ade8b

Please sign in to comment.