Skip to content

Commit

Permalink
[Security Solution][Notes] - disable add note button in flyout is use…
Browse files Browse the repository at this point in the history
…r lacks privileges (elastic#201707)

## Summary

This PR fixes a small issue where users could click on the add button in
the alert details flyout even if they did not have the correct
privileges.

When the user has the correct privileges, the UI does not change. In the
flyout:
- if no notes have previously been created for the document, we show a
`Add note` button
- if some notes have previously been created for the document, we show
the number of notes and a plus button icon


https://github.com/user-attachments/assets/d9a27b70-99b1-4562-8224-4f5c2f25b001

When the user does not have the correct privileges, the flyout UI now
shows the following:
- if no notes have previously been created for the document, we show a
`-`
- if one or more notes have been created for the document, we show the
number of notes followed by a `View note(s)` button, that - when clicked
- opens the left panel for the user to view the notes


https://github.com/user-attachments/assets/8ebe8bf5-16ab-4652-b4d3-47507c2d3673

elastic#201702

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ] [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
  • Loading branch information
PhilippeOberti authored Nov 26, 2024
1 parent 97318c9 commit c7b5b35
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
NOTES_COUNT_TEST_ID,
NOTES_LOADING_TEST_ID,
NOTES_TITLE_TEST_ID,
NOTES_VIEW_NOTES_BUTTON_TEST_ID,
} from './test_ids';
import { FETCH_NOTES_ERROR, Notes } from './notes';
import { mockContextValue } from '../../shared/mocks/mock_context';
Expand All @@ -24,8 +25,10 @@ import { useExpandableFlyoutApi } from '@kbn/expandable-flyout';
import { DocumentDetailsLeftPanelKey } from '../../shared/constants/panel_keys';
import { LeftPanelNotesTab } from '../../left';
import { getEmptyValue } from '../../../../common/components/empty_value';
import { useUserPrivileges } from '../../../../common/components/user_privileges';

jest.mock('@kbn/expandable-flyout');
jest.mock('../../../../common/components/user_privileges');

const mockAddError = jest.fn();
jest.mock('../../../../common/hooks/use_app_toasts', () => ({
Expand All @@ -45,6 +48,9 @@ jest.mock('react-redux', () => {

describe('<Notes />', () => {
beforeEach(() => {
(useUserPrivileges as jest.Mock).mockReturnValue({
kibanaSecuritySolutionsPrivileges: { crud: true },
});
jest.clearAllMocks();
});

Expand Down Expand Up @@ -297,4 +303,69 @@ describe('<Notes />', () => {
title: FETCH_NOTES_ERROR,
});
});

it('should show View note button if user does not have the correct privileges but notes have already been created', () => {
(useUserPrivileges as jest.Mock).mockReturnValue({
kibanaSecuritySolutionsPrivileges: { crud: false },
});

const mockOpenLeftPanel = jest.fn();
(useExpandableFlyoutApi as jest.Mock).mockReturnValue({ openLeftPanel: mockOpenLeftPanel });

const contextValue = {
...mockContextValue,
eventId: '1',
};

const { getByTestId, queryByTestId } = render(
<TestProviders>
<DocumentDetailsContext.Provider value={contextValue}>
<Notes />
</DocumentDetailsContext.Provider>
</TestProviders>
);

expect(mockDispatch).toHaveBeenCalled();

expect(getByTestId(NOTES_COUNT_TEST_ID)).toBeInTheDocument();
expect(getByTestId(NOTES_COUNT_TEST_ID)).toHaveTextContent('1');

expect(queryByTestId(NOTES_ADD_NOTE_ICON_BUTTON_TEST_ID)).not.toBeInTheDocument();
expect(queryByTestId(NOTES_ADD_NOTE_BUTTON_TEST_ID)).not.toBeInTheDocument();
const button = getByTestId(NOTES_VIEW_NOTES_BUTTON_TEST_ID);
expect(button).toBeInTheDocument();

button.click();

expect(mockOpenLeftPanel).toHaveBeenCalledWith({
id: DocumentDetailsLeftPanelKey,
path: { tab: LeftPanelNotesTab },
params: {
id: contextValue.eventId,
indexName: mockContextValue.indexName,
scopeId: mockContextValue.scopeId,
},
});
});

it('should show a - if user does not have the correct privileges and no notes have been created', () => {
(useUserPrivileges as jest.Mock).mockReturnValue({
kibanaSecuritySolutionsPrivileges: { crud: false },
});

const { getByText, queryByTestId } = render(
<TestProviders>
<DocumentDetailsContext.Provider value={mockContextValue}>
<Notes />
</DocumentDetailsContext.Provider>
</TestProviders>
);

expect(mockDispatch).toHaveBeenCalled();

expect(queryByTestId(NOTES_ADD_NOTE_ICON_BUTTON_TEST_ID)).not.toBeInTheDocument();
expect(queryByTestId(NOTES_ADD_NOTE_BUTTON_TEST_ID)).not.toBeInTheDocument();
expect(queryByTestId(NOTES_COUNT_TEST_ID)).not.toBeInTheDocument();
expect(getByText(getEmptyValue())).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { memo, useCallback, useEffect } from 'react';
import React, { memo, useCallback, useEffect, useMemo } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { FormattedMessage } from '@kbn/i18n-react';
import { i18n } from '@kbn/i18n';
Expand All @@ -19,6 +19,7 @@ import {
} from '@elastic/eui';
import { css } from '@emotion/react';
import { useExpandableFlyoutApi } from '@kbn/expandable-flyout';
import { useUserPrivileges } from '../../../../common/components/user_privileges';
import { getEmptyTagValue } from '../../../../common/components/empty_value';
import { DocumentDetailsLeftPanelKey } from '../../shared/constants/panel_keys';
import { FormattedCount } from '../../../../common/components/formatted_number';
Expand All @@ -29,6 +30,7 @@ import {
NOTES_COUNT_TEST_ID,
NOTES_LOADING_TEST_ID,
NOTES_TITLE_TEST_ID,
NOTES_VIEW_NOTES_BUTTON_TEST_ID,
} from './test_ids';
import type { State } from '../../../../common/store';
import type { Note } from '../../../../../common/api/timeline';
Expand All @@ -55,6 +57,12 @@ export const ADD_NOTE_BUTTON = i18n.translate(
defaultMessage: 'Add note',
}
);
export const VIEW_NOTES_BUTTON_ARIA_LABEL = i18n.translate(
'xpack.securitySolution.flyout.right.notes.viewNoteButtonAriaLabel',
{
defaultMessage: 'View notes',
}
);

/**
* Renders a block with the number of notes for the event
Expand All @@ -64,6 +72,7 @@ export const Notes = memo(() => {
const dispatch = useDispatch();
const { eventId, indexName, scopeId, isPreview, isPreviewMode } = useDocumentDetailsContext();
const { addError: addErrorToast } = useAppToasts();
const { kibanaSecuritySolutionsPrivileges } = useUserPrivileges();

const { openLeftPanel } = useExpandableFlyoutApi();
const openExpandedFlyoutNotesTab = useCallback(
Expand Down Expand Up @@ -101,6 +110,61 @@ export const Notes = memo(() => {
}
}, [addErrorToast, fetchError, fetchStatus]);

const viewNotesButton = useMemo(
() => (
<EuiButtonEmpty
onClick={openExpandedFlyoutNotesTab}
size="s"
disabled={isPreviewMode || isPreview}
aria-label={VIEW_NOTES_BUTTON_ARIA_LABEL}
data-test-subj={NOTES_VIEW_NOTES_BUTTON_TEST_ID}
>
<FormattedMessage
id="xpack.securitySolution.flyout.right.notes.viewNoteButtonLabel"
defaultMessage="View {count, plural, one {note} other {notes}}"
values={{ count: notes.length }}
/>
</EuiButtonEmpty>
),
[isPreview, isPreviewMode, notes.length, openExpandedFlyoutNotesTab]
);
const addNoteButton = useMemo(
() => (
<EuiButtonEmpty
iconType="plusInCircle"
onClick={openExpandedFlyoutNotesTab}
size="s"
disabled={isPreviewMode || isPreview}
aria-label={ADD_NOTE_BUTTON}
data-test-subj={NOTES_ADD_NOTE_BUTTON_TEST_ID}
>
{ADD_NOTE_BUTTON}
</EuiButtonEmpty>
),
[isPreview, isPreviewMode, openExpandedFlyoutNotesTab]
);
const addNoteButtonIcon = useMemo(
() => (
<EuiButtonIcon
onClick={openExpandedFlyoutNotesTab}
iconType="plusInCircle"
disabled={isPreviewMode || isPreview || !kibanaSecuritySolutionsPrivileges.crud}
css={css`
margin-left: ${euiTheme.size.xs};
`}
aria-label={ADD_NOTE_BUTTON}
data-test-subj={NOTES_ADD_NOTE_ICON_BUTTON_TEST_ID}
/>
),
[
euiTheme.size.xs,
isPreview,
isPreviewMode,
kibanaSecuritySolutionsPrivileges.crud,
openExpandedFlyoutNotesTab,
]
);

return (
<AlertHeaderBlock
title={
Expand All @@ -120,32 +184,14 @@ export const Notes = memo(() => {
) : (
<>
{notes.length === 0 ? (
<EuiButtonEmpty
iconType="plusInCircle"
onClick={openExpandedFlyoutNotesTab}
size="s"
disabled={isPreviewMode || isPreview}
aria-label={ADD_NOTE_BUTTON}
data-test-subj={NOTES_ADD_NOTE_BUTTON_TEST_ID}
>
{ADD_NOTE_BUTTON}
</EuiButtonEmpty>
<>{kibanaSecuritySolutionsPrivileges.crud ? addNoteButton : getEmptyTagValue()}</>
) : (
<EuiFlexGroup responsive={false} alignItems="center" gutterSize="none">
<EuiFlexItem data-test-subj={NOTES_COUNT_TEST_ID}>
<FormattedCount count={notes.length} />
</EuiFlexItem>
<EuiFlexItem>
<EuiButtonIcon
onClick={openExpandedFlyoutNotesTab}
iconType="plusInCircle"
disabled={isPreviewMode || isPreview}
css={css`
margin-left: ${euiTheme.size.xs};
`}
aria-label={ADD_NOTE_BUTTON}
data-test-subj={NOTES_ADD_NOTE_ICON_BUTTON_TEST_ID}
/>
{kibanaSecuritySolutionsPrivileges.crud ? addNoteButtonIcon : viewNotesButton}
</EuiFlexItem>
</EuiFlexGroup>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export const CHAT_BUTTON_TEST_ID = 'newChatByTitle' as const;

export const NOTES_TITLE_TEST_ID = `${FLYOUT_HEADER_TEST_ID}NotesTitle` as const;
export const NOTES_ADD_NOTE_BUTTON_TEST_ID = `${FLYOUT_HEADER_TEST_ID}NotesAddNoteButton` as const;
export const NOTES_VIEW_NOTES_BUTTON_TEST_ID =
`${FLYOUT_HEADER_TEST_ID}NotesViewNotesButton` as const;
export const NOTES_ADD_NOTE_ICON_BUTTON_TEST_ID =
`${FLYOUT_HEADER_TEST_ID}NotesAddNoteIconButton` as const;
export const NOTES_COUNT_TEST_ID = `${FLYOUT_HEADER_TEST_ID}NotesCount` as const;
Expand Down

0 comments on commit c7b5b35

Please sign in to comment.