diff --git a/x-pack/plugins/cases/public/common/apm/use_cases_transactions.test.ts b/x-pack/plugins/cases/public/common/apm/use_cases_transactions.test.ts index 5438258187a2f..c1cf6b305d48d 100644 --- a/x-pack/plugins/cases/public/common/apm/use_cases_transactions.test.ts +++ b/x-pack/plugins/cases/public/common/apm/use_cases_transactions.test.ts @@ -80,6 +80,15 @@ describe('cases transactions', () => { ); expect(mockAddLabels).toHaveBeenCalledWith({ alert_count: 3 }); }); + + it('should not start any transactions if the app ID is not defined', () => { + const { result } = renderUseCreateCaseWithAttachmentsTransaction(); + + result.current.startTransaction(); + + expect(mockStartTransaction).not.toHaveBeenCalled(); + expect(mockAddLabels).not.toHaveBeenCalled(); + }); }); describe('useAddAttachmentToExistingCaseTransaction', () => { @@ -104,5 +113,14 @@ describe('cases transactions', () => { ); expect(mockAddLabels).toHaveBeenCalledWith({ alert_count: 3 }); }); + + it('should not start any transactions if the app ID is not defined', () => { + const { result } = renderUseAddAttachmentToExistingCaseTransaction(); + + result.current.startTransaction({ attachments: bulkAttachments }); + + expect(mockStartTransaction).not.toHaveBeenCalled(); + expect(mockAddLabels).not.toHaveBeenCalled(); + }); }); }); diff --git a/x-pack/plugins/cases/public/common/apm/use_cases_transactions.ts b/x-pack/plugins/cases/public/common/apm/use_cases_transactions.ts index 891726322cb8e..400f1aa2d956c 100644 --- a/x-pack/plugins/cases/public/common/apm/use_cases_transactions.ts +++ b/x-pack/plugins/cases/public/common/apm/use_cases_transactions.ts @@ -17,8 +17,8 @@ const BULK_ADD_ATTACHMENT_TO_NEW_CASE = 'bulkAddAttachmentsToNewCase' as const; const ADD_ATTACHMENT_TO_EXISTING_CASE = 'addAttachmentToExistingCase' as const; const BULK_ADD_ATTACHMENT_TO_EXISTING_CASE = 'bulkAddAttachmentsToExistingCase' as const; -export type StartCreateCaseWithAttachmentsTransaction = (param: { - appId: string; +export type StartCreateCaseWithAttachmentsTransaction = (param?: { + appId?: string; attachments?: CaseAttachmentsWithoutOwner; }) => Transaction | undefined; @@ -28,11 +28,17 @@ export const useCreateCaseWithAttachmentsTransaction = () => { const startCreateCaseWithAttachmentsTransaction = useCallback( - ({ appId, attachments }) => { + ({ appId, attachments } = {}) => { + if (!appId) { + return; + } + if (!attachments) { return startTransaction(`Cases [${appId}] ${CREATE_CASE}`); } + const alertCount = getAlertCount(attachments); + if (alertCount <= 1) { return startTransaction(`Cases [${appId}] ${ADD_ATTACHMENT_TO_NEW_CASE}`); } @@ -48,7 +54,7 @@ export const useCreateCaseWithAttachmentsTransaction = () => { }; export type StartAddAttachmentToExistingCaseTransaction = (param: { - appId: string; + appId?: string; attachments: CaseAttachmentsWithoutOwner; }) => Transaction | undefined; @@ -59,13 +65,20 @@ export const useAddAttachmentToExistingCaseTransaction = () => { const startAddAttachmentToExistingCaseTransaction = useCallback( ({ appId, attachments }) => { + if (!appId) { + return; + } + const alertCount = getAlertCount(attachments); + if (alertCount <= 1) { return startTransaction(`Cases [${appId}] ${ADD_ATTACHMENT_TO_EXISTING_CASE}`); } + const transaction = startTransaction( `Cases [${appId}] ${BULK_ADD_ATTACHMENT_TO_EXISTING_CASE}` ); + transaction?.addLabels({ alert_count: alertCount }); return transaction; }, diff --git a/x-pack/plugins/cases/public/common/lib/kibana/__mocks__/index.ts b/x-pack/plugins/cases/public/common/lib/kibana/__mocks__/index.ts index 3aa4c02457ef7..7bf4e71e0717a 100644 --- a/x-pack/plugins/cases/public/common/lib/kibana/__mocks__/index.ts +++ b/x-pack/plugins/cases/public/common/lib/kibana/__mocks__/index.ts @@ -25,7 +25,6 @@ export const useKibana = jest.fn().mockReturnValue({ export const useHttp = jest.fn().mockReturnValue(createStartServicesMock().http); export const useTimeZone = jest.fn(); export const useDateFormat = jest.fn(); -export const useBasePath = jest.fn(() => '/test/base/path'); export const useToasts = jest .fn() .mockReturnValue(notificationServiceMock.createStartContract().toasts); diff --git a/x-pack/plugins/cases/public/common/lib/kibana/hooks.ts b/x-pack/plugins/cases/public/common/lib/kibana/hooks.ts index 39b4d3d1edc76..3d72e5ca552b9 100644 --- a/x-pack/plugins/cases/public/common/lib/kibana/hooks.ts +++ b/x-pack/plugins/cases/public/common/lib/kibana/hooks.ts @@ -30,8 +30,6 @@ export const useTimeZone = (): string => { return timeZone === 'Browser' ? moment.tz.guess() : timeZone; }; -export const useBasePath = (): string => useKibana().services.http.basePath.get(); - export const useToasts = (): StartServices['notifications']['toasts'] => useKibana().services.notifications.toasts; @@ -116,12 +114,12 @@ export const useCurrentUser = (): AuthenticatedElasticUser | null => { * Returns a full URL to the provided page path by using * kibana's `getUrlForApp()` */ -export const useAppUrl = (appId: string) => { +export const useAppUrl = (appId?: string) => { const { getUrlForApp } = useKibana().services.application; const getAppUrl = useCallback( (options?: { deepLinkId?: string; path?: string; absolute?: boolean }) => - getUrlForApp(appId, options), + getUrlForApp(appId ?? '', options), [appId, getUrlForApp] ); return { getAppUrl }; @@ -131,7 +129,7 @@ export const useAppUrl = (appId: string) => { * Navigate to any app using kibana's `navigateToApp()` * or by url using `navigateToUrl()` */ -export const useNavigateTo = (appId: string) => { +export const useNavigateTo = (appId?: string) => { const { navigateToApp, navigateToUrl } = useKibana().services.application; const navigateTo = useCallback( @@ -144,7 +142,7 @@ export const useNavigateTo = (appId: string) => { if (url) { navigateToUrl(url); } else { - navigateToApp(appId, options); + navigateToApp(appId ?? '', options); } }, [appId, navigateToApp, navigateToUrl] @@ -156,7 +154,7 @@ export const useNavigateTo = (appId: string) => { * Returns navigateTo and getAppUrl navigation hooks * */ -export const useNavigation = (appId: string) => { +export const useNavigation = (appId?: string) => { const { navigateTo } = useNavigateTo(appId); const { getAppUrl } = useAppUrl(appId); return { navigateTo, getAppUrl }; diff --git a/x-pack/plugins/cases/public/common/lib/kibana/kibana_react.mock.tsx b/x-pack/plugins/cases/public/common/lib/kibana/kibana_react.mock.tsx index 195c1f433a8e7..0223e4648ac93 100644 --- a/x-pack/plugins/cases/public/common/lib/kibana/kibana_react.mock.tsx +++ b/x-pack/plugins/cases/public/common/lib/kibana/kibana_react.mock.tsx @@ -9,6 +9,7 @@ import React from 'react'; import { BehaviorSubject } from 'rxjs'; import type { PublicAppInfo } from '@kbn/core/public'; +import { AppStatus } from '@kbn/core/public'; import type { RecursivePartial } from '@elastic/eui/src/components/common'; import { coreMock } from '@kbn/core/public/mocks'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; @@ -52,7 +53,21 @@ export const createStartServicesMock = ({ license }: StartServiceArgs = {}): Sta services.application.currentAppId$ = new BehaviorSubject('testAppId'); services.application.applications$ = new BehaviorSubject>( - new Map([['testAppId', { category: { label: 'Test' } } as unknown as PublicAppInfo]]) + new Map([ + [ + 'testAppId', + { + id: 'testAppId', + title: 'test-title', + category: { id: 'test-label-id', label: 'Test' }, + status: AppStatus.accessible, + visibleIn: ['globalSearch'], + appRoute: `/app/some-id`, + keywords: [], + deepLinks: [], + }, + ], + ]) ); services.triggersActionsUi.actionTypeRegistry.get = jest.fn().mockReturnValue({ diff --git a/x-pack/plugins/cases/public/common/mock/test_providers.tsx b/x-pack/plugins/cases/public/common/mock/test_providers.tsx index 0361014cbfb68..45239024313db 100644 --- a/x-pack/plugins/cases/public/common/mock/test_providers.tsx +++ b/x-pack/plugins/cases/public/common/mock/test_providers.tsx @@ -19,7 +19,7 @@ import type { ScopedFilesClient } from '@kbn/files-plugin/public'; import { euiDarkVars } from '@kbn/ui-theme'; import { I18nProvider } from '@kbn/i18n-react'; import { createMockFilesClient } from '@kbn/shared-ux-file-mocks'; -import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { QueryClient } from '@tanstack/react-query'; import { KibanaContextProvider } from '@kbn/kibana-react-plugin/public'; import { FilesContext } from '@kbn/shared-ux-file-context'; @@ -108,10 +108,9 @@ const TestProvidersComponent: React.FC = ({ permissions, getFilesClient, }} + queryClient={queryClient} > - - {children} - + {children} @@ -191,8 +190,9 @@ export const createAppMockRenderer = ({ releasePhase, getFilesClient, }} + queryClient={queryClient} > - {children} + {children} diff --git a/x-pack/plugins/cases/public/common/use_cases_toast.test.tsx b/x-pack/plugins/cases/public/common/use_cases_toast.test.tsx index 53fe9118a6aa0..3d9895eea4e8a 100644 --- a/x-pack/plugins/cases/public/common/use_cases_toast.test.tsx +++ b/x-pack/plugins/cases/public/common/use_cases_toast.test.tsx @@ -14,13 +14,16 @@ import { alertComment, basicComment, mockCase } from '../containers/mock'; import React from 'react'; import userEvent from '@testing-library/user-event'; import type { SupportedCaseAttachment } from '../types'; -import { getByTestId } from '@testing-library/react'; +import { getByTestId, queryByTestId, screen } from '@testing-library/react'; import { OWNER_INFO } from '../../common/constants'; +import { useCasesContext } from '../components/cases_context/use_cases_context'; jest.mock('./lib/kibana'); +jest.mock('../components/cases_context/use_cases_context'); const useToastsMock = useToasts as jest.Mock; const useKibanaMock = useKibana as jest.Mocked; +const useCasesContextMock = useCasesContext as jest.Mock; describe('Use cases toast hook', () => { const successMock = jest.fn(); @@ -66,6 +69,8 @@ describe('Use cases toast hook', () => { getUrlForApp, navigateToUrl, }; + + useCasesContextMock.mockReturnValue({ appId: 'testAppId' }); }); describe('showSuccessAttach', () => { @@ -147,6 +152,7 @@ describe('Use cases toast hook', () => { describe('Toast content', () => { let appMockRender: AppMockRenderer; const onViewCaseClick = jest.fn(); + beforeEach(() => { appMockRender = createAppMockRenderer(); onViewCaseClick.mockReset(); @@ -213,9 +219,16 @@ describe('Use cases toast hook', () => { const result = appMockRender.render( ); + userEvent.click(result.getByTestId('toaster-content-case-view-link')); expect(onViewCaseClick).toHaveBeenCalled(); }); + + it('hides the view case link when onViewCaseClick is not defined', () => { + appMockRender.render(); + + expect(screen.queryByTestId('toaster-content-case-view-link')).not.toBeInTheDocument(); + }); }); describe('Toast navigation', () => { @@ -267,6 +280,31 @@ describe('Use cases toast hook', () => { path: '/mock-id', }); }); + + it('does not navigates to a case if the appId is not defined', () => { + useCasesContextMock.mockReturnValue({ appId: undefined }); + + const { result } = renderHook( + () => { + return useCasesToast(); + }, + { wrapper: TestProviders } + ); + + result.current.showSuccessAttach({ + theCase: { ...mockCase, owner: 'in-valid' }, + title: 'Custom title', + }); + + const mockParams = successMock.mock.calls[0][0]; + const el = document.createElement('div'); + mockParams.text(el); + const button = queryByTestId(el, 'toaster-content-case-view-link'); + + expect(button).toBeNull(); + expect(getUrlForApp).not.toHaveBeenCalled(); + expect(navigateToUrl).not.toHaveBeenCalled(); + }); }); }); diff --git a/x-pack/plugins/cases/public/common/use_cases_toast.tsx b/x-pack/plugins/cases/public/common/use_cases_toast.tsx index 93905eaf64213..5eff9b7da849c 100644 --- a/x-pack/plugins/cases/public/common/use_cases_toast.tsx +++ b/x-pack/plugins/cases/public/common/use_cases_toast.tsx @@ -140,13 +140,18 @@ export const useCasesToast = () => { ? OWNER_INFO[theCase.owner].appId : appId; - const url = getUrlForApp(appIdToNavigateTo, { - deepLinkId: 'cases', - path: generateCaseViewPath({ detailName: theCase.id }), - }); + const url = + appIdToNavigateTo != null + ? getUrlForApp(appIdToNavigateTo, { + deepLinkId: 'cases', + path: generateCaseViewPath({ detailName: theCase.id }), + }) + : null; const onViewCaseClick = () => { - navigateToUrl(url); + if (url) { + navigateToUrl(url); + } }; const renderTitle = getToastTitle({ theCase, title, attachments }); @@ -157,7 +162,10 @@ export const useCasesToast = () => { iconType: 'check', title: toMountPoint({renderTitle}), text: toMountPoint( - + ), }); }, @@ -186,7 +194,7 @@ export const CaseToastSuccessContent = ({ onViewCaseClick, content, }: { - onViewCaseClick: () => void; + onViewCaseClick?: () => void; content?: string; }) => { return ( @@ -196,14 +204,16 @@ export const CaseToastSuccessContent = ({ {content} ) : null} - - {VIEW_CASE} - + {onViewCaseClick !== undefined ? ( + + {VIEW_CASE} + + ) : null} ); }; diff --git a/x-pack/plugins/cases/public/components/add_comment/index.tsx b/x-pack/plugins/cases/public/components/add_comment/index.tsx index f29e71673198c..4fea706b89b42 100644 --- a/x-pack/plugins/cases/public/components/add_comment/index.tsx +++ b/x-pack/plugins/cases/public/components/add_comment/index.tsx @@ -75,7 +75,7 @@ export const AddComment = React.memo( const [focusOnContext, setFocusOnContext] = useState(false); const { permissions, owner, appId } = useCasesContext(); const { isLoading, mutate: createAttachments } = useCreateAttachments(); - const draftStorageKey = getMarkdownEditorStorageKey(appId, caseId, id); + const draftStorageKey = getMarkdownEditorStorageKey({ appId, caseId, commentId: id }); const { form } = useForm({ defaultValue: initialCommentValue, diff --git a/x-pack/plugins/cases/public/components/all_cases/table_filter_config/use_filter_config.tsx b/x-pack/plugins/cases/public/components/all_cases/table_filter_config/use_filter_config.tsx index 6c342770a12f3..e8c6a0daf0c79 100644 --- a/x-pack/plugins/cases/public/components/all_cases/table_filter_config/use_filter_config.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/table_filter_config/use_filter_config.tsx @@ -14,7 +14,7 @@ import { LOCAL_STORAGE_KEYS } from '../../../../common/constants'; import type { FilterConfig, FilterConfigState } from './types'; import { useCustomFieldsFilterConfig } from './use_custom_fields_filter_config'; import { useCasesContext } from '../../cases_context/use_cases_context'; -import { deflattenCustomFieldKey, isFlattenCustomField } from '../utils'; +import { deflattenCustomFieldKey, getLocalStorageKey, isFlattenCustomField } from '../utils'; const mergeSystemAndCustomFieldConfigs = ({ systemFilterConfig, @@ -52,7 +52,7 @@ const shouldBeActive = ({ const useActiveByFilterKeyState = ({ filterOptions }: { filterOptions: FilterOptions }) => { const { appId } = useCasesContext(); const [activeByFilterKey, setActiveByFilterKey] = useLocalStorage( - `${appId}.${LOCAL_STORAGE_KEYS.casesTableFiltersConfig}`, + getLocalStorageKey(LOCAL_STORAGE_KEYS.casesTableFiltersConfig, appId), [] ); diff --git a/x-pack/plugins/cases/public/components/all_cases/use_all_cases_state.tsx b/x-pack/plugins/cases/public/components/all_cases/use_all_cases_state.tsx index 39bac2c05d569..dd17191d98a78 100644 --- a/x-pack/plugins/cases/public/components/all_cases/use_all_cases_state.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/use_all_cases_state.tsx @@ -27,6 +27,7 @@ import { parseUrlParams } from './utils/parse_url_params'; import { useCasesContext } from '../cases_context/use_cases_context'; import { sanitizeState } from './utils/sanitize_state'; import { useGetCaseConfiguration } from '../../containers/configure/use_get_case_configuration'; +import { getLocalStorageKey } from './utils'; interface UseAllCasesStateReturn { filterOptions: FilterOptions; @@ -182,8 +183,11 @@ const useAllCasesLocalStorage = (): [ const { appId } = useCasesContext(); const [state, setState] = useLocalStorage( - getAllCasesTableStateLocalStorageKey(appId), - { queryParams: DEFAULT_QUERY_PARAMS, filterOptions: DEFAULT_FILTER_OPTIONS } + getLocalStorageKey(LOCAL_STORAGE_KEYS.casesTableState, appId), + { + queryParams: DEFAULT_QUERY_PARAMS, + filterOptions: DEFAULT_FILTER_OPTIONS, + } ); const sanitizedState = sanitizeState(state); @@ -199,8 +203,3 @@ const useAllCasesLocalStorage = (): [ setState, ]; }; - -const getAllCasesTableStateLocalStorageKey = (appId: string) => { - const key = LOCAL_STORAGE_KEYS.casesTableState; - return `${appId}.${key}`; -}; diff --git a/x-pack/plugins/cases/public/components/all_cases/use_cases_columns_selection.tsx b/x-pack/plugins/cases/public/components/all_cases/use_cases_columns_selection.tsx index 7b81e88c0d383..92abe45c81c86 100644 --- a/x-pack/plugins/cases/public/components/all_cases/use_cases_columns_selection.tsx +++ b/x-pack/plugins/cases/public/components/all_cases/use_cases_columns_selection.tsx @@ -13,18 +13,14 @@ import { LOCAL_STORAGE_KEYS } from '../../../common/constants'; import { useCasesContext } from '../cases_context/use_cases_context'; import { useCasesColumnsConfiguration } from './use_cases_columns_configuration'; import { mergeSelectedColumnsWithConfiguration } from './utils/merge_selected_columns_with_configuration'; - -const getTableColumnsLocalStorageKey = (appId: string) => { - const filteringKey = LOCAL_STORAGE_KEYS.casesTableColumns; - return `${appId}.${filteringKey}`; -}; +import { getLocalStorageKey } from './utils'; export function useCasesColumnsSelection() { const { appId } = useCasesContext(); const casesColumnsConfig = useCasesColumnsConfiguration(); const [selectedColumns, setSelectedColumns] = useLocalStorage( - getTableColumnsLocalStorageKey(appId) + getLocalStorageKey(LOCAL_STORAGE_KEYS.casesTableColumns, appId) ); const columns = selectedColumns || []; diff --git a/x-pack/plugins/cases/public/components/all_cases/utils/index.test.ts b/x-pack/plugins/cases/public/components/all_cases/utils/index.test.ts index 98eeef78f9765..3ef0f47705c15 100644 --- a/x-pack/plugins/cases/public/components/all_cases/utils/index.test.ts +++ b/x-pack/plugins/cases/public/components/all_cases/utils/index.test.ts @@ -11,6 +11,7 @@ import { deflattenCustomFieldKey, stringToInteger, stringToIntegerWithDefault, + getLocalStorageKey, } from '.'; describe('utils', () => { @@ -63,4 +64,14 @@ describe('utils', () => { expect(stringToIntegerWithDefault('foo', 10)).toBe(10); }); }); + + describe('getLocalStorageKey', () => { + it('constructs the local storage key correctly', () => { + expect(getLocalStorageKey('my-key', 'test-app-id')).toBe('test-app-id.my-key'); + }); + + it('constructs the local storage key correctly when the app ID is not defined', () => { + expect(getLocalStorageKey('my-key')).toBe('.my-key'); + }); + }); }); diff --git a/x-pack/plugins/cases/public/components/all_cases/utils/index.ts b/x-pack/plugins/cases/public/components/all_cases/utils/index.ts index 762882834b8ee..0843bd4f2ac09 100644 --- a/x-pack/plugins/cases/public/components/all_cases/utils/index.ts +++ b/x-pack/plugins/cases/public/components/all_cases/utils/index.ts @@ -33,3 +33,7 @@ export const stringToIntegerWithDefault = ( return valueAsInteger && valueAsInteger > 0 ? valueAsInteger : defaultValue; }; + +export const getLocalStorageKey = (localStorageKey: string, appId?: string) => { + return `${appId ?? ''}.${localStorageKey}`; +}; diff --git a/x-pack/plugins/cases/public/components/case_view/components/edit_tags.test.tsx b/x-pack/plugins/cases/public/components/case_view/components/edit_tags.test.tsx index f36620c033b7e..b1ac130d015c6 100644 --- a/x-pack/plugins/cases/public/components/case_view/components/edit_tags.test.tsx +++ b/x-pack/plugins/cases/public/components/case_view/components/edit_tags.test.tsx @@ -25,8 +25,7 @@ const defaultProps: EditTagsProps = { tags: [], }; -// FLAKY: https://github.com/elastic/kibana/issues/175655 -describe.skip('EditTags ', () => { +describe('EditTags ', () => { let appMockRender: AppMockRenderer; const sampleTags = ['coke', 'pepsi']; @@ -62,7 +61,8 @@ describe.skip('EditTags ', () => { userEvent.click(await screen.findByTestId('tag-list-edit-button')); - userEvent.type(await screen.findByRole('combobox'), `${sampleTags[0]}{enter}`); + userEvent.paste(await screen.findByRole('combobox'), `${sampleTags[0]}`); + userEvent.keyboard('{enter}'); userEvent.click(await screen.findByTestId('edit-tags-submit')); @@ -76,7 +76,8 @@ describe.skip('EditTags ', () => { expect(await screen.findByTestId('edit-tags')).toBeInTheDocument(); - userEvent.type(await screen.findByRole('combobox'), 'dude{enter}'); + userEvent.paste(await screen.findByRole('combobox'), 'dude'); + userEvent.keyboard('{enter}'); userEvent.click(await screen.findByTestId('edit-tags-submit')); @@ -90,7 +91,8 @@ describe.skip('EditTags ', () => { expect(await screen.findByTestId('edit-tags')).toBeInTheDocument(); - userEvent.type(await screen.findByRole('combobox'), 'dude {enter}'); + userEvent.paste(await screen.findByRole('combobox'), 'dude '); + userEvent.keyboard('{enter}'); userEvent.click(await screen.findByTestId('edit-tags-submit')); @@ -102,7 +104,8 @@ describe.skip('EditTags ', () => { userEvent.click(await screen.findByTestId('tag-list-edit-button')); - userEvent.type(await screen.findByRole('combobox'), 'new{enter}'); + userEvent.paste(await screen.findByRole('combobox'), 'new'); + userEvent.keyboard('{enter}'); expect(await screen.findByTestId('comboBoxInput')).toHaveTextContent('new'); @@ -122,7 +125,8 @@ describe.skip('EditTags ', () => { expect(await screen.findByTestId('edit-tags')).toBeInTheDocument(); - userEvent.type(await screen.findByRole('combobox'), ' {enter}'); + userEvent.paste(await screen.findByRole('combobox'), ' '); + userEvent.keyboard('{enter}'); expect(await screen.findByText('A tag must contain at least one non-space character.')); }); diff --git a/x-pack/plugins/cases/public/components/case_view/components/edit_tags.tsx b/x-pack/plugins/cases/public/components/case_view/components/edit_tags.tsx index 07bf8ac11c39f..7537042c17246 100644 --- a/x-pack/plugins/cases/public/components/case_view/components/edit_tags.tsx +++ b/x-pack/plugins/cases/public/components/case_view/components/edit_tags.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useState } from 'react'; import { EuiText, EuiHorizontalRule, @@ -17,15 +17,9 @@ import { EuiLoadingSpinner, } from '@elastic/eui'; import styled, { css } from 'styled-components'; -import { isEqual } from 'lodash/fp'; import type { FormSchema } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; -import { - Form, - FormDataProvider, - useForm, - getUseField, -} from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; -import { Field } from '@kbn/es-ui-shared-plugin/static/forms/components'; +import { Form, useForm, UseField } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; +import { ComboBoxField } from '@kbn/es-ui-shared-plugin/static/forms/components'; import * as i18n from '../../tags/translations'; import { useGetTags } from '../../../containers/use_get_tags'; import { Tags } from '../../tags/tags'; @@ -36,8 +30,6 @@ export const schema: FormSchema = { tags: schemaTags, }; -const CommonUseField = getUseField({ component: Field }); - export interface EditTagsProps { isLoading: boolean; onSubmit: (a: string[]) => void; @@ -68,11 +60,13 @@ const ColumnFlexGroup = styled(EuiFlexGroup)` export const EditTags = React.memo(({ isLoading, onSubmit, tags }: EditTagsProps) => { const { permissions } = useCasesContext(); const initialState = { tags }; + const { form } = useForm({ defaultValue: initialState, options: { stripEmptyFields: false }, schema, }); + const { submit } = form; const [isEditTags, setIsEditTags] = useState(false); @@ -89,21 +83,11 @@ export const EditTags = React.memo(({ isLoading, onSubmit, tags }: EditTagsProps }, [onSubmit, submit]); const { data: tagOptions = [] } = useGetTags(); - const [options, setOptions] = useState( - tagOptions.map((label) => ({ - label, - })) - ); - useEffect( - () => - setOptions( - tagOptions.map((label) => ({ - label, - })) - ), - [tagOptions] - ); + const options = tagOptions.map((label) => ({ + label, + })); + return ( @@ -123,7 +107,7 @@ export const EditTags = React.memo(({ isLoading, onSubmit, tags }: EditTagsProps data-test-subj="tag-list-edit-button" aria-label={i18n.EDIT_TAGS_ARIA} iconType={'pencil'} - onClick={setIsEditTags.bind(null, true)} + onClick={() => setIsEditTags(true)} /> )} @@ -140,8 +124,9 @@ export const EditTags = React.memo(({ isLoading, onSubmit, tags }: EditTagsProps
- - - {({ tags: anotherTags }) => { - const current: string[] = options.map((opt) => opt.label); - const newOptions = anotherTags.reduce((acc: string[], item: string) => { - if (!acc.includes(item)) { - return [...acc, item]; - } - return acc; - }, current); - if (!isEqual(current, newOptions)) { - setOptions( - newOptions.map((label: string) => ({ - label, - })) - ); - } - return null; - }} -
@@ -193,7 +159,7 @@ export const EditTags = React.memo(({ isLoading, onSubmit, tags }: EditTagsProps setIsEditTags(false)} size="s" > {i18n.CANCEL} diff --git a/x-pack/plugins/cases/public/components/cases_context/index.tsx b/x-pack/plugins/cases/public/components/cases_context/index.tsx index e1ba6be73e837..f0344f5705082 100644 --- a/x-pack/plugins/cases/public/components/cases_context/index.tsx +++ b/x-pack/plugins/cases/public/components/cases_context/index.tsx @@ -8,13 +8,12 @@ import type { Dispatch, ReactNode } from 'react'; import { merge } from 'lodash'; -import React, { useCallback, useEffect, useState, useReducer } from 'react'; -import useDeepCompareEffect from 'react-use/lib/useDeepCompareEffect'; +import React, { useCallback, useMemo, useReducer } from 'react'; import type { ScopedFilesClient } from '@kbn/files-plugin/public'; - import { FilesContext } from '@kbn/shared-ux-file-context'; +import type { QueryClient } from '@tanstack/react-query'; import { QueryClientProvider } from '@tanstack/react-query'; import type { CasesContextStoreAction } from './cases_context_reducer'; import type { @@ -35,14 +34,14 @@ import { casesContextReducer, getInitialCasesContextState } from './cases_contex import { isRegisteredOwner } from '../../files'; import { casesQueryClient } from './query_client'; -export type CasesContextValueDispatch = Dispatch; +type CasesContextValueDispatch = Dispatch; export interface CasesContextValue { externalReferenceAttachmentTypeRegistry: ExternalReferenceAttachmentTypeRegistry; persistableStateAttachmentTypeRegistry: PersistableStateAttachmentTypeRegistry; owner: string[]; - appId: string; - appTitle: string; + appId?: string; + appTitle?: string; permissions: CasesPermissions; basePath: string; features: CasesFeaturesAllRequired; @@ -66,12 +65,7 @@ export interface CasesContextProps export const CasesContext = React.createContext(undefined); -export interface CasesContextStateValue extends Omit { - appId?: string; - appTitle?: string; -} - -export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({ +export const CasesProvider: React.FC<{ value: CasesContextProps; queryClient?: QueryClient }> = ({ children, value: { externalReferenceAttachmentTypeRegistry, @@ -83,49 +77,61 @@ export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({ releasePhase = 'ga', getFilesClient, }, + queryClient = casesQueryClient, }) => { const { appId, appTitle } = useApplication(); const [state, dispatch] = useReducer(casesContextReducer, getInitialCasesContextState()); - const [value, setValue] = useState(() => ({ - externalReferenceAttachmentTypeRegistry, - persistableStateAttachmentTypeRegistry, - owner, - permissions, - basePath, + + const value: CasesContextValue = useMemo( + () => ({ + appId, + appTitle, + externalReferenceAttachmentTypeRegistry, + persistableStateAttachmentTypeRegistry, + owner, + permissions: { + all: permissions.all, + connectors: permissions.connectors, + create: permissions.create, + delete: permissions.delete, + push: permissions.push, + read: permissions.read, + settings: permissions.settings, + update: permissions.update, + }, + basePath, + /** + * The empty object at the beginning avoids the mutation + * of the DEFAULT_FEATURES object + */ + features: merge( + {}, + DEFAULT_FEATURES, + features + ), + releasePhase, + dispatch, + }), /** - * The empty object at the beginning avoids the mutation - * of the DEFAULT_FEATURES object + * We want to trigger a rerender only if the appId, the appTitle, or + * the permissions will change. The registries, the owner, and the rest + * of the values should not change during the lifecycle of the + * cases application. */ - features: merge( - {}, - DEFAULT_FEATURES, - features - ), - releasePhase, - dispatch, - })); - - /** - * Only update the context if the nested permissions fields changed, this avoids a rerender when the object's reference - * changes. - */ - useDeepCompareEffect(() => { - setValue((prev) => ({ ...prev, permissions })); - }, [permissions]); - - /** - * `appId` and `appTitle` are dynamically retrieved from kibana context. - * We need to update the state if any of these values change, the rest of props are never updated. - */ - useEffect(() => { - if (appId && appTitle) { - setValue((prev) => ({ - ...prev, - appId, - appTitle, - })); - } - }, [appTitle, appId]); + // eslint-disable-next-line react-hooks/exhaustive-deps + [ + appId, + appTitle, + permissions.all, + permissions.connectors, + permissions.create, + permissions.delete, + permissions.push, + permissions.read, + permissions.settings, + permissions.update, + ] + ); const applyFilesContext = useCallback( (contextChildren: ReactNode) => { @@ -148,8 +154,8 @@ export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({ [getFilesClient, owner] ); - return isCasesContextValue(value) ? ( - + return ( + {applyFilesContext( <> @@ -159,13 +165,10 @@ export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({ )} - ) : null; + ); }; -CasesProvider.displayName = 'CasesProvider'; -function isCasesContextValue(value: CasesContextStateValue): value is CasesContextValue { - return value.appId != null && value.appTitle != null && value.permissions != null; -} +CasesProvider.displayName = 'CasesProvider'; // eslint-disable-next-line import/no-default-export export default CasesProvider; diff --git a/x-pack/plugins/cases/public/components/cases_context/use_application.test.tsx b/x-pack/plugins/cases/public/components/cases_context/use_application.test.tsx new file mode 100644 index 0000000000000..59dd790233c3e --- /dev/null +++ b/x-pack/plugins/cases/public/components/cases_context/use_application.test.tsx @@ -0,0 +1,108 @@ +/* + * 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 { PublicAppInfo } from '@kbn/core-application-browser'; +import { AppStatus } from '@kbn/core-application-browser'; +import { renderHook } from '@testing-library/react-hooks'; +import { BehaviorSubject, Subject } from 'rxjs'; +import type { AppMockRenderer } from '../../common/mock'; +import { createAppMockRenderer } from '../../common/mock'; +import { useApplication } from './use_application'; + +describe('useApplication', () => { + let appMockRender: AppMockRenderer; + + beforeEach(() => { + jest.clearAllMocks(); + appMockRender = createAppMockRenderer(); + }); + + const getApp = (props: Partial = {}): PublicAppInfo => ({ + id: 'testAppId', + title: 'Test title', + status: AppStatus.accessible, + visibleIn: ['globalSearch'], + appRoute: `/app/some-id`, + keywords: [], + deepLinks: [], + ...props, + }); + + it('returns the appId and the appTitle correctly', () => { + const { result } = renderHook(() => useApplication(), { + wrapper: appMockRender.AppWrapper, + }); + + expect(result.current).toEqual({ + appId: 'testAppId', + appTitle: 'Test', + }); + }); + + it('returns undefined appId and appTitle if the currentAppId observable is not defined', () => { + appMockRender.coreStart.application.currentAppId$ = new Subject(); + + const { result } = renderHook(() => useApplication(), { + wrapper: appMockRender.AppWrapper, + }); + + expect(result.current).toEqual({}); + }); + + it('returns undefined appTitle if the applications observable is not defined', () => { + appMockRender.coreStart.application.applications$ = new Subject(); + + const { result } = renderHook(() => useApplication(), { + wrapper: appMockRender.AppWrapper, + }); + + expect(result.current).toEqual({ + appId: 'testAppId', + appTitle: undefined, + }); + }); + + it('returns the label as appTitle', () => { + appMockRender.coreStart.application.applications$ = new BehaviorSubject( + new Map([['testAppId', getApp({ category: { id: 'test-label-id', label: 'Test label' } })]]) + ); + + const { result } = renderHook(() => useApplication(), { + wrapper: appMockRender.AppWrapper, + }); + + expect(result.current).toEqual({ + appId: 'testAppId', + appTitle: 'Test label', + }); + }); + + it('returns the title as appTitle if the categories label is missing', () => { + appMockRender.coreStart.application.applications$ = new BehaviorSubject( + new Map([['testAppId', getApp({ title: 'Test title' })]]) + ); + + const { result } = renderHook(() => useApplication(), { + wrapper: appMockRender.AppWrapper, + }); + + expect(result.current).toEqual({ + appId: 'testAppId', + appTitle: 'Test title', + }); + }); + + it('gets the value from the default value of the currentAppId observable if it exists', () => { + appMockRender.coreStart.application.currentAppId$ = new BehaviorSubject('new-test-id'); + + const { result } = renderHook(() => useApplication(), { + wrapper: appMockRender.AppWrapper, + }); + + expect(result.current).toEqual({ appId: 'new-test-id' }); + }); +}); diff --git a/x-pack/plugins/cases/public/components/cases_context/use_application.tsx b/x-pack/plugins/cases/public/components/cases_context/use_application.tsx index f21c40b6ee603..4ea44c087705d 100644 --- a/x-pack/plugins/cases/public/components/cases_context/use_application.tsx +++ b/x-pack/plugins/cases/public/components/cases_context/use_application.tsx @@ -5,6 +5,7 @@ * 2.0. */ import useObservable from 'react-use/lib/useObservable'; +import type { BehaviorSubject, Observable } from 'rxjs'; import { useKibana } from '../../common/lib/kibana'; interface UseApplicationReturn { @@ -12,11 +13,28 @@ interface UseApplicationReturn { appTitle: string | undefined; } +const isBehaviorSubjectObservable = ( + observable: Observable +): observable is BehaviorSubject => 'value' in observable; + +/** + * Checks if the observable is stateful and, in case, sets up `useObservable` with an initial value + */ +const useStatefulObservable = (observable: Observable) => { + let initialValue: T | undefined; + + if (isBehaviorSubjectObservable(observable)) { + initialValue = observable.value; + } + + return useObservable(observable, initialValue); +}; + export const useApplication = (): UseApplicationReturn => { const { currentAppId$, applications$ } = useKibana().services.application; // retrieve the most recent value from the BehaviorSubject - const appId = useObservable(currentAppId$); - const applications = useObservable(applications$); + const appId = useStatefulObservable(currentAppId$); + const applications = useStatefulObservable(applications$); const appTitle = appId ? applications?.get(appId)?.category?.label ?? applications?.get(appId)?.title diff --git a/x-pack/plugins/cases/public/components/create/description.test.tsx b/x-pack/plugins/cases/public/components/create/description.test.tsx index f3fd7342db17e..d0426731f97d9 100644 --- a/x-pack/plugins/cases/public/components/create/description.test.tsx +++ b/x-pack/plugins/cases/public/components/create/description.test.tsx @@ -17,8 +17,7 @@ import { MAX_DESCRIPTION_LENGTH } from '../../../common/constants'; import { FormTestComponent } from '../../common/test_utils'; import type { FormSchema } from '@kbn/index-management-plugin/public/shared_imports'; -// FLAKY: https://github.com/elastic/kibana/issues/175204 -describe.skip('Description', () => { +describe('Description', () => { let appMockRender: AppMockRenderer; const onSubmit = jest.fn(); const draftStorageKey = `cases.caseView.createCase.description.markdownEditor`; diff --git a/x-pack/plugins/cases/public/components/create/form.tsx b/x-pack/plugins/cases/public/components/create/form.tsx index 280adaa66aeec..0948ce289fdd5 100644 --- a/x-pack/plugins/cases/public/components/create/form.tsx +++ b/x-pack/plugins/cases/public/components/create/form.tsx @@ -220,7 +220,11 @@ export const CreateCaseForm: React.FC = React.memo( initialValue, }) => { const { owner, appId } = useCasesContext(); - const draftStorageKey = getMarkdownEditorStorageKey(appId, 'createCase', 'description'); + const draftStorageKey = getMarkdownEditorStorageKey({ + appId, + caseId: 'createCase', + commentId: 'description', + }); const handleOnConfirmationCallback = (): void => { onCancel(); diff --git a/x-pack/plugins/cases/public/components/create/form_context.tsx b/x-pack/plugins/cases/public/components/create/form_context.tsx index 2457b964ac3fc..1fc8f2be98296 100644 --- a/x-pack/plugins/cases/public/components/create/form_context.tsx +++ b/x-pack/plugins/cases/public/components/create/form_context.tsx @@ -41,7 +41,6 @@ const initialCaseValue: FormProps = { connectorId: NONE_CONNECTOR_ID, fields: null, syncAlerts: true, - selectedOwner: null, assignees: [], customFields: {}, }; diff --git a/x-pack/plugins/cases/public/components/create/owner_selector.test.tsx b/x-pack/plugins/cases/public/components/create/owner_selector.test.tsx index 94ce019498078..451207b080dfb 100644 --- a/x-pack/plugins/cases/public/components/create/owner_selector.test.tsx +++ b/x-pack/plugins/cases/public/components/create/owner_selector.test.tsx @@ -6,139 +6,111 @@ */ import React from 'react'; -import { mount } from 'enzyme'; -import { act, waitFor } from '@testing-library/react'; +import { waitFor, screen } from '@testing-library/react'; import { SECURITY_SOLUTION_OWNER } from '../../../common'; -import { OBSERVABILITY_OWNER } from '../../../common/constants'; -import type { FormHook } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; -import { useForm, Form } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; +import { OBSERVABILITY_OWNER, OWNER_INFO } from '../../../common/constants'; import { CreateCaseOwnerSelector } from './owner_selector'; -import type { FormProps } from './schema'; -import { schema } from './schema'; -import { waitForComponentToPaint } from '../../common/test_utils'; - -// FLAKY: https://github.com/elastic/kibana/issues/175570 -describe.skip('Case Owner Selection', () => { - let globalForm: FormHook; - - const MockHookWrapperComponent: React.FC = ({ children }) => { - const { form } = useForm({ - defaultValue: { selectedOwner: '' }, - schema: { - selectedOwner: schema.selectedOwner, - }, - }); - - globalForm = form; +import { FormTestComponent } from '../../common/test_utils'; +import type { AppMockRenderer } from '../../common/mock'; +import { createAppMockRenderer } from '../../common/mock'; +import userEvent from '@testing-library/user-event'; - return
{children}
; - }; +describe('Case Owner Selection', () => { + const onSubmit = jest.fn(); + let appMockRender: AppMockRenderer; beforeEach(() => { jest.clearAllMocks(); + appMockRender = createAppMockRenderer(); }); it('renders', async () => { - const wrapper = mount( - + appMockRender.render( + - + ); - await waitForComponentToPaint(wrapper); - expect(wrapper.find(`[data-test-subj="caseOwnerSelector"]`).exists()).toBeTruthy(); + expect(await screen.findByTestId('caseOwnerSelector')).toBeInTheDocument(); }); it.each([ [OBSERVABILITY_OWNER, SECURITY_SOLUTION_OWNER], [SECURITY_SOLUTION_OWNER, OBSERVABILITY_OWNER], ])('disables %s button if user only has %j', async (disabledButton, permission) => { - const wrapper = mount( - + appMockRender.render( + - + ); - await waitForComponentToPaint(wrapper); - - expect( - wrapper.find(`[data-test-subj="${disabledButton}RadioButton"] input`).first().props().disabled - ).toBeTruthy(); - expect( - wrapper.find(`[data-test-subj="${permission}RadioButton"] input`).first().props().disabled - ).toBeFalsy(); - expect( - wrapper.find(`[data-test-subj="${permission}RadioButton"] input`).first().props().checked - ).toBeTruthy(); + expect(await screen.findByLabelText(OWNER_INFO[disabledButton].label)).toBeDisabled(); + expect(await screen.findByLabelText(OWNER_INFO[permission].label)).not.toBeDisabled(); }); it('defaults to security Solution', async () => { - const wrapper = mount( - + appMockRender.render( + - + + ); + + expect(await screen.findByLabelText('Observability')).not.toBeChecked(); + expect(await screen.findByLabelText('Security')).toBeChecked(); + + userEvent.click(await screen.findByTestId('form-test-component-submit-button')); + + await waitFor(() => { + // data, isValid + expect(onSubmit).toBeCalledWith({ selectedOwner: 'securitySolution' }, true); + }); + }); + + it('defaults to security Solution with empty owners', async () => { + appMockRender.render( + + + ); - await waitForComponentToPaint(wrapper); + expect(await screen.findByLabelText('Observability')).not.toBeChecked(); + expect(await screen.findByLabelText('Security')).toBeChecked(); + + userEvent.click(await screen.findByTestId('form-test-component-submit-button')); - expect( - wrapper.find(`[data-test-subj="observabilityRadioButton"] input`).first().props().checked - ).toBeFalsy(); - expect( - wrapper.find(`[data-test-subj="securitySolutionRadioButton"] input`).first().props().checked - ).toBeTruthy(); + await waitFor(() => { + // data, isValid + expect(onSubmit).toBeCalledWith({ selectedOwner: 'securitySolution' }, true); + }); }); - it('it changes the selection', async () => { - const wrapper = mount( - + it('changes the selection', async () => { + appMockRender.render( + - + ); - await act(async () => { - wrapper - .find(`[data-test-subj="observabilityRadioButton"] input`) - .first() - .simulate('change', OBSERVABILITY_OWNER); - }); + expect(await screen.findByLabelText('Security')).toBeChecked(); + expect(await screen.findByLabelText('Observability')).not.toBeChecked(); - await waitFor(() => { - wrapper.update(); - expect( - wrapper.find(`[data-test-subj="observabilityRadioButton"] input`).first().props().checked - ).toBeTruthy(); - expect( - wrapper.find(`[data-test-subj="securitySolutionRadioButton"] input`).first().props().checked - ).toBeFalsy(); - }); + userEvent.click(await screen.findByLabelText('Observability')); - expect(globalForm.getFormData()).toEqual({ selectedOwner: OBSERVABILITY_OWNER }); + expect(await screen.findByLabelText('Observability')).toBeChecked(); + expect(await screen.findByLabelText('Security')).not.toBeChecked(); - await act(async () => { - wrapper - .find(`[data-test-subj="securitySolutionRadioButton"] input`) - .first() - .simulate('change', SECURITY_SOLUTION_OWNER); - }); + userEvent.click(await screen.findByTestId('form-test-component-submit-button')); await waitFor(() => { - wrapper.update(); - expect( - wrapper.find(`[data-test-subj="securitySolutionRadioButton"] input`).first().props().checked - ).toBeTruthy(); - expect( - wrapper.find(`[data-test-subj="observabilityRadioButton"] input`).first().props().checked - ).toBeFalsy(); + // data, isValid + expect(onSubmit).toBeCalledWith({ selectedOwner: 'observability' }, true); }); - - expect(globalForm.getFormData()).toEqual({ selectedOwner: SECURITY_SOLUTION_OWNER }); }); }); diff --git a/x-pack/plugins/cases/public/components/create/owner_selector.tsx b/x-pack/plugins/cases/public/components/create/owner_selector.tsx index 440fcffcbb5d8..00dd4a03f2664 100644 --- a/x-pack/plugins/cases/public/components/create/owner_selector.tsx +++ b/x-pack/plugins/cases/public/components/create/owner_selector.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React, { memo, useCallback, useEffect } from 'react'; +import React, { memo, useCallback } from 'react'; import { EuiFlexGroup, @@ -61,16 +61,6 @@ const OwnerSelector = ({ const onChange = useCallback((val: string) => field.setValue(val), [field]); - useEffect(() => { - if (!field.value) { - onChange( - availableOwners.includes(SECURITY_SOLUTION_OWNER) - ? SECURITY_SOLUTION_OWNER - : availableOwners[0] - ); - } - }, [availableOwners, field.value, onChange]); - return ( ); }; + OwnerSelector.displayName = 'OwnerSelector'; const CaseOwnerSelector: React.FC = ({ availableOwners, isLoading }) => { + const defaultValue = availableOwners.includes(SECURITY_SOLUTION_OWNER) + ? SECURITY_SOLUTION_OWNER + : availableOwners[0] ?? SECURITY_SOLUTION_OWNER; + return ( diff --git a/x-pack/plugins/cases/public/components/create/severity.test.tsx b/x-pack/plugins/cases/public/components/create/severity.test.tsx index 5431d17d5f54c..927848a9a6a24 100644 --- a/x-pack/plugins/cases/public/components/create/severity.test.tsx +++ b/x-pack/plugins/cases/public/components/create/severity.test.tsx @@ -5,83 +5,76 @@ * 2.0. */ -import { CaseSeverity } from '../../../common/types/domain'; import React from 'react'; +import { screen, waitFor } from '@testing-library/react'; import type { AppMockRenderer } from '../../common/mock'; import { createAppMockRenderer } from '../../common/mock'; -import type { FormHook } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; -import { Form, useForm } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; import { Severity } from './severity'; -import type { FormProps } from './schema'; -import { schema } from './schema'; import userEvent from '@testing-library/user-event'; -import { waitFor } from '@testing-library/react'; import { waitForEuiPopoverOpen } from '@elastic/eui/lib/test/rtl'; +import { FormTestComponent } from '../../common/test_utils'; -let globalForm: FormHook; -const MockHookWrapperComponent: React.FC = ({ children }) => { - const { form } = useForm({ - defaultValue: { severity: CaseSeverity.LOW }, - schema: { - severity: schema.severity, - }, - }); - - globalForm = form; +const onSubmit = jest.fn(); - return
{children}
; -}; -// FLAKY: https://github.com/elastic/kibana/issues/175934 -// FLAKY: https://github.com/elastic/kibana/issues/175935 -describe.skip('Severity form field', () => { +describe('Severity form field', () => { let appMockRender: AppMockRenderer; + beforeEach(() => { appMockRender = createAppMockRenderer(); }); - it('renders', () => { - const result = appMockRender.render( - + + it('renders', async () => { + appMockRender.render( + - + ); - expect(result.getByTestId('caseSeverity')).toBeTruthy(); - expect(result.getByTestId('case-severity-selection')).not.toHaveAttribute('disabled'); + + expect(await screen.findByTestId('caseSeverity')).toBeInTheDocument(); + expect(await screen.findByTestId('case-severity-selection')).not.toHaveAttribute('disabled'); }); // default to LOW in this test configuration - it('defaults to the correct value', () => { - const result = appMockRender.render( - + it('defaults to the correct value', async () => { + appMockRender.render( + - + ); - expect(result.getByTestId('caseSeverity')).toBeTruthy(); - // ID removed for options dropdown here: - // https://github.com/elastic/eui/pull/6630#discussion_r1123657852 - expect(result.getAllByTestId('case-severity-selection-low').length).toBe(1); + + expect(await screen.findByTestId('caseSeverity')).toBeInTheDocument(); + expect(await screen.findByTestId('case-severity-selection-low')).toBeInTheDocument(); }); it('selects the correct value when changed', async () => { - const result = appMockRender.render( - + appMockRender.render( + - + ); - expect(result.getByTestId('caseSeverity')).toBeTruthy(); - userEvent.click(result.getByTestId('case-severity-selection')); + + expect(await screen.findByTestId('caseSeverity')).toBeInTheDocument(); + + userEvent.click(await screen.findByTestId('case-severity-selection')); await waitForEuiPopoverOpen(); - userEvent.click(result.getByTestId('case-severity-selection-high')); + + userEvent.click(await screen.findByTestId('case-severity-selection-high')); + + userEvent.click(await screen.findByTestId('form-test-component-submit-button')); + await waitFor(() => { - expect(globalForm.getFormData()).toEqual({ severity: 'high' }); + // data, isValid + expect(onSubmit).toBeCalledWith({ severity: 'high' }, true); }); }); - it('disables when loading data', () => { - const result = appMockRender.render( - + it('disables when loading data', async () => { + appMockRender.render( + - + ); - expect(result.getByTestId('case-severity-selection')).toHaveAttribute('disabled'); + + expect(await screen.findByTestId('case-severity-selection')).toHaveAttribute('disabled'); }); }); diff --git a/x-pack/plugins/cases/public/components/create/severity.tsx b/x-pack/plugins/cases/public/components/create/severity.tsx index 00c0c3c32642c..b65ec7f6a6350 100644 --- a/x-pack/plugins/cases/public/components/create/severity.tsx +++ b/x-pack/plugins/cases/public/components/create/severity.tsx @@ -8,10 +8,10 @@ import { EuiFormRow } from '@elastic/eui'; import React, { memo } from 'react'; import { + getFieldValidityAndErrorMessage, UseField, - useFormContext, - useFormData, } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; +import { isEmpty } from 'lodash'; import { CaseSeverity } from '../../../common/types/domain'; import { SeveritySelector } from '../severity/selector'; import { SEVERITY_TITLE } from '../severity/translations'; @@ -20,33 +20,38 @@ interface Props { isLoading: boolean; } -const SeverityFieldFormComponent = ({ isLoading }: { isLoading: boolean }) => { - const { setFieldValue } = useFormContext(); - const [{ severity }] = useFormData({ watch: ['severity'] }); - const onSeverityChange = (newSeverity: CaseSeverity) => { - setFieldValue('severity', newSeverity); - }; - return ( - - - - ); -}; -SeverityFieldFormComponent.displayName = 'SeverityFieldForm'; - const SeverityComponent: React.FC = ({ isLoading }) => ( - path={'severity'} - component={SeverityFieldFormComponent} componentProps={{ isLoading, }} - /> + > + {(field) => { + const { isInvalid, errorMessage } = getFieldValidityAndErrorMessage(field); + + const onChange = (newSeverity: CaseSeverity) => { + field.setValue(newSeverity); + }; + + return ( + + + + ); + }} + ); SeverityComponent.displayName = 'SeverityComponent'; diff --git a/x-pack/plugins/cases/public/components/description/index.tsx b/x-pack/plugins/cases/public/components/description/index.tsx index a3e074f6f5867..01fe85d98105b 100644 --- a/x-pack/plugins/cases/public/components/description/index.tsx +++ b/x-pack/plugins/cases/public/components/description/index.tsx @@ -73,7 +73,7 @@ const getDraftDescription = ( caseId: string, commentId: string ): string | null => { - const draftStorageKey = getMarkdownEditorStorageKey(applicationId, caseId, commentId); + const draftStorageKey = getMarkdownEditorStorageKey({ appId: applicationId, caseId, commentId }); return sessionStorage.getItem(draftStorageKey); }; diff --git a/x-pack/plugins/cases/public/components/files/file_delete_button.test.tsx b/x-pack/plugins/cases/public/components/files/file_delete_button.test.tsx index 8c4540aaadbec..5498c81ad4474 100644 --- a/x-pack/plugins/cases/public/components/files/file_delete_button.test.tsx +++ b/x-pack/plugins/cases/public/components/files/file_delete_button.test.tsx @@ -21,8 +21,7 @@ jest.mock('../../containers/use_delete_file_attachment'); const useDeleteFileAttachmentMock = useDeleteFileAttachment as jest.Mock; -// FLAKY: https://github.com/elastic/kibana/issues/175956 -describe.skip('FileDeleteButton', () => { +describe('FileDeleteButton', () => { let appMockRender: AppMockRenderer; const mutate = jest.fn(); @@ -40,8 +39,6 @@ describe.skip('FileDeleteButton', () => { ); expect(await screen.findByTestId('cases-files-delete-button')).toBeInTheDocument(); - - expect(useDeleteFileAttachmentMock).toBeCalledTimes(1); }); it('clicking delete button opens the confirmation modal', async () => { diff --git a/x-pack/plugins/cases/public/components/markdown_editor/editable_markdown_renderer.tsx b/x-pack/plugins/cases/public/components/markdown_editor/editable_markdown_renderer.tsx index 6cbaca3378242..b24d2e4f70432 100644 --- a/x-pack/plugins/cases/public/components/markdown_editor/editable_markdown_renderer.tsx +++ b/x-pack/plugins/cases/public/components/markdown_editor/editable_markdown_renderer.tsx @@ -38,7 +38,8 @@ const EditableMarkDownRenderer = forwardRef< ref ) => { const { appId } = useCasesContext(); - const draftStorageKey = getMarkdownEditorStorageKey(appId, caseId, id); + const draftStorageKey = getMarkdownEditorStorageKey({ appId, caseId, commentId: id }); + const initialState = { content }; const { form } = useForm({ diff --git a/x-pack/plugins/cases/public/components/markdown_editor/utils.test.ts b/x-pack/plugins/cases/public/components/markdown_editor/utils.test.ts index ef1de4a1bc327..8c616b25c69db 100644 --- a/x-pack/plugins/cases/public/components/markdown_editor/utils.test.ts +++ b/x-pack/plugins/cases/public/components/markdown_editor/utils.test.ts @@ -12,7 +12,7 @@ describe('getMarkdownEditorStorageKey', () => { const appId = 'security-solution'; const caseId = 'case-id'; const commentId = 'comment-id'; - const sessionKey = getMarkdownEditorStorageKey(appId, caseId, commentId); + const sessionKey = getMarkdownEditorStorageKey({ appId, caseId, commentId }); expect(sessionKey).toEqual(`cases.${appId}.${caseId}.${commentId}.markdownEditor`); }); @@ -20,7 +20,7 @@ describe('getMarkdownEditorStorageKey', () => { const appId = 'security-solution'; const caseId = 'case-id'; const commentId = ''; - const sessionKey = getMarkdownEditorStorageKey(appId, caseId, commentId); + const sessionKey = getMarkdownEditorStorageKey({ appId, caseId, commentId }); expect(sessionKey).toEqual(`cases.${appId}.${caseId}.comment.markdownEditor`); }); @@ -28,7 +28,7 @@ describe('getMarkdownEditorStorageKey', () => { const appId = 'security-solution'; const caseId = ''; const commentId = 'comment-id'; - const sessionKey = getMarkdownEditorStorageKey(appId, caseId, commentId); + const sessionKey = getMarkdownEditorStorageKey({ appId, caseId, commentId }); expect(sessionKey).toEqual(`cases.${appId}.case.${commentId}.markdownEditor`); }); @@ -36,7 +36,14 @@ describe('getMarkdownEditorStorageKey', () => { const appId = ''; const caseId = 'case-id'; const commentId = 'comment-id'; - const sessionKey = getMarkdownEditorStorageKey(appId, caseId, commentId); + const sessionKey = getMarkdownEditorStorageKey({ appId, caseId, commentId }); + expect(sessionKey).toEqual(`cases.cases.${caseId}.${commentId}.markdownEditor`); + }); + + it('should return default key when app id is undefined', () => { + const caseId = 'case-id'; + const commentId = 'comment-id'; + const sessionKey = getMarkdownEditorStorageKey({ caseId, commentId }); expect(sessionKey).toEqual(`cases.cases.${caseId}.${commentId}.markdownEditor`); }); }); diff --git a/x-pack/plugins/cases/public/components/markdown_editor/utils.ts b/x-pack/plugins/cases/public/components/markdown_editor/utils.ts index 1fb81875b926b..134b2355c9979 100644 --- a/x-pack/plugins/cases/public/components/markdown_editor/utils.ts +++ b/x-pack/plugins/cases/public/components/markdown_editor/utils.ts @@ -5,12 +5,16 @@ * 2.0. */ -export const getMarkdownEditorStorageKey = ( - appId: string, - caseId: string, - commentId: string -): string => { - const appIdKey = appId !== '' ? appId : 'cases'; +export const getMarkdownEditorStorageKey = ({ + caseId, + commentId, + appId, +}: { + caseId: string; + commentId: string; + appId?: string; +}): string => { + const appIdKey = appId && appId !== '' ? appId : 'cases'; const caseIdKey = caseId !== '' ? caseId : 'case'; const commentIdKey = commentId !== '' ? commentId : 'comment'; diff --git a/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx b/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx index e189506a450cb..53ddc22cad30b 100644 --- a/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx +++ b/x-pack/plugins/cases/public/components/user_actions/comment/user.tsx @@ -52,7 +52,7 @@ const hasDraftComment = ( commentId: string, comment: string ): boolean => { - const draftStorageKey = getMarkdownEditorStorageKey(applicationId, caseId, commentId); + const draftStorageKey = getMarkdownEditorStorageKey({ appId: applicationId, caseId, commentId }); const sessionValue = sessionStorage.getItem(draftStorageKey); diff --git a/x-pack/plugins/cases/public/components/user_profiles/user_tooltip.test.tsx b/x-pack/plugins/cases/public/components/user_profiles/user_tooltip.test.tsx index 5fdf94f96c266..a6dc16434a8f2 100644 --- a/x-pack/plugins/cases/public/components/user_profiles/user_tooltip.test.tsx +++ b/x-pack/plugins/cases/public/components/user_profiles/user_tooltip.test.tsx @@ -6,12 +6,12 @@ */ import type { UserProfileWithAvatar } from '@kbn/user-profile-components'; -import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import React from 'react'; import { UserToolTip } from './user_tooltip'; -// FLAKY: https://github.com/elastic/kibana/issues/176643 -describe.skip('UserToolTip', () => { +describe('UserToolTip', () => { it('renders the tooltip when hovering', async () => { const profile: UserProfileWithAvatar = { uid: '1', @@ -34,12 +34,12 @@ describe.skip('UserToolTip', () => { ); - fireEvent.mouseOver(screen.getByText('case user')); + userEvent.hover(await screen.findByText('case user')); - await waitFor(() => screen.getByTestId('user-profile-tooltip')); - expect(screen.getByText('Some Super User')).toBeInTheDocument(); - expect(screen.getByText('some.user@google.com')).toBeInTheDocument(); - expect(screen.getByText('SU')).toBeInTheDocument(); + expect(await screen.findByTestId('user-profile-tooltip')).toBeInTheDocument(); + expect(await screen.findByText('Some Super User')).toBeInTheDocument(); + expect(await screen.findByText('some.user@google.com')).toBeInTheDocument(); + expect(await screen.findByText('SU')).toBeInTheDocument(); }); it('only shows the display name if full name is missing', async () => { @@ -63,12 +63,13 @@ describe.skip('UserToolTip', () => { ); - fireEvent.mouseOver(screen.getByText('case user')); + userEvent.hover(await screen.findByText('case user')); - await waitFor(() => screen.getByTestId('user-profile-tooltip')); + expect(await screen.findByTestId('user-profile-tooltip')).toBeInTheDocument(); + + expect(await screen.findByText('some.user@google.com')).toBeInTheDocument(); + expect(await screen.findByText('SU')).toBeInTheDocument(); expect(screen.queryByText('Some Super User')).not.toBeInTheDocument(); - expect(screen.getByText('some.user@google.com')).toBeInTheDocument(); - expect(screen.getByText('SU')).toBeInTheDocument(); }); it('only shows the full name if display name is missing', async () => { @@ -93,12 +94,12 @@ describe.skip('UserToolTip', () => { ); - fireEvent.mouseOver(screen.getByText('case user')); + userEvent.hover(await screen.findByText('case user')); - await waitFor(() => screen.getByTestId('user-profile-tooltip')); - expect(screen.getByText('Some Super User')).toBeInTheDocument(); - expect(screen.getByText('some.user@google.com')).toBeInTheDocument(); - expect(screen.getByText('SU')).toBeInTheDocument(); + expect(await screen.findByTestId('user-profile-tooltip')).toBeInTheDocument(); + expect(await screen.findByText('Some Super User')).toBeInTheDocument(); + expect(await screen.findByText('some.user@google.com')).toBeInTheDocument(); + expect(await screen.findByText('SU')).toBeInTheDocument(); }); it('only shows the email once when display name and full name are not defined', async () => { @@ -122,12 +123,12 @@ describe.skip('UserToolTip', () => { ); - fireEvent.mouseOver(screen.getByText('case user')); + userEvent.hover(await screen.findByText('case user')); - await waitFor(() => screen.getByTestId('user-profile-tooltip')); + expect(await screen.findByTestId('user-profile-tooltip')).toBeInTheDocument(); + expect(await screen.findByText('some.user@google.com')).toBeInTheDocument(); + expect(await screen.findByText('SU')).toBeInTheDocument(); expect(screen.queryByText('Some Super User')).not.toBeInTheDocument(); - expect(screen.getByText('some.user@google.com')).toBeInTheDocument(); - expect(screen.getByText('SU')).toBeInTheDocument(); }); it('only shows the username once when all other fields are undefined', async () => { @@ -150,13 +151,13 @@ describe.skip('UserToolTip', () => { ); - fireEvent.mouseOver(screen.getByText('case user')); + userEvent.hover(await screen.findByText('case user')); - await waitFor(() => screen.getByTestId('user-profile-tooltip')); + expect(await screen.findByTestId('user-profile-tooltip')).toBeInTheDocument(); expect(screen.queryByText('Some Super User')).not.toBeInTheDocument(); + expect(await screen.findByText('user')).toBeInTheDocument(); + expect(await screen.findByText('SU')).toBeInTheDocument(); expect(screen.queryByText('some.user@google.com')).not.toBeInTheDocument(); - expect(screen.getByText('user')).toBeInTheDocument(); - expect(screen.getByText('SU')).toBeInTheDocument(); }); it('shows an unknown users display name and avatar', async () => { @@ -166,10 +167,10 @@ describe.skip('UserToolTip', () => { ); - fireEvent.mouseOver(screen.getByText('case user')); + userEvent.hover(await screen.findByText('case user')); - await waitFor(() => screen.getByTestId('user-profile-tooltip')); - expect(screen.getByText('Unable to find user profile')).toBeInTheDocument(); - expect(screen.getByText('?')).toBeInTheDocument(); + expect(await screen.findByTestId('user-profile-tooltip')).toBeInTheDocument(); + expect(await screen.findByText('Unable to find user profile')).toBeInTheDocument(); + expect(await screen.findByText('?')).toBeInTheDocument(); }); }); diff --git a/x-pack/plugins/cases/public/containers/use_get_case_files.test.tsx b/x-pack/plugins/cases/public/containers/use_get_case_files.test.tsx index 712dcb5487c5c..2be8dd6052408 100644 --- a/x-pack/plugins/cases/public/containers/use_get_case_files.test.tsx +++ b/x-pack/plugins/cases/public/containers/use_get_case_files.test.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import { renderHook, act } from '@testing-library/react-hooks'; +import { renderHook } from '@testing-library/react-hooks'; import { basicCase } from './mock'; @@ -58,13 +58,12 @@ describe('useGetCaseFiles', () => { }); it('calls filesClient.list with correct arguments', async () => { - await act(async () => { - const { waitForNextUpdate } = renderHook(() => useGetCaseFiles(hookParams), { - wrapper: appMockRender.AppWrapper, - }); - await waitForNextUpdate(); - - expect(appMockRender.getFilesClient().list).toBeCalledWith(expectedCallParams); + const { waitForNextUpdate } = renderHook(() => useGetCaseFiles(hookParams), { + wrapper: appMockRender.AppWrapper, }); + + await waitForNextUpdate(); + + expect(appMockRender.getFilesClient().list).toBeCalledWith(expectedCallParams); }); }); diff --git a/x-pack/plugins/cases/public/containers/use_get_case_metrics.test.tsx b/x-pack/plugins/cases/public/containers/use_get_case_metrics.test.tsx index 8a08b7bc0ab5e..44cec799c9f51 100644 --- a/x-pack/plugins/cases/public/containers/use_get_case_metrics.test.tsx +++ b/x-pack/plugins/cases/public/containers/use_get_case_metrics.test.tsx @@ -6,7 +6,7 @@ */ import React from 'react'; -import { renderHook, act } from '@testing-library/react-hooks'; +import { renderHook } from '@testing-library/react-hooks'; import type { SingleCaseMetricsFeature } from '../../common/ui'; import { useGetCaseMetrics } from './use_get_case_metrics'; import { basicCase } from './mock'; @@ -30,13 +30,13 @@ describe('useGetCaseMetrics', () => { it('calls getSingleCaseMetrics with correct arguments', async () => { const spyOnGetCaseMetrics = jest.spyOn(api, 'getSingleCaseMetrics'); - await act(async () => { - const { waitForNextUpdate } = renderHook(() => useGetCaseMetrics(basicCase.id, features), { - wrapper, - }); - await waitForNextUpdate(); - expect(spyOnGetCaseMetrics).toBeCalledWith(basicCase.id, features, abortCtrl.signal); + + const { waitForNextUpdate } = renderHook(() => useGetCaseMetrics(basicCase.id, features), { + wrapper, }); + + await waitForNextUpdate(); + expect(spyOnGetCaseMetrics).toBeCalledWith(basicCase.id, features, abortCtrl.signal); }); it('shows an error toast when getSingleCaseMetrics throws', async () => { @@ -51,7 +51,9 @@ describe('useGetCaseMetrics', () => { const { waitForNextUpdate } = renderHook(() => useGetCaseMetrics(basicCase.id, features), { wrapper, }); + await waitForNextUpdate(); + expect(spyOnGetCaseMetrics).toBeCalledWith(basicCase.id, features, abortCtrl.signal); expect(addError).toHaveBeenCalled(); }); diff --git a/x-pack/plugins/cases/tsconfig.json b/x-pack/plugins/cases/tsconfig.json index 750ac4f644b88..b217bcb3587de 100644 --- a/x-pack/plugins/cases/tsconfig.json +++ b/x-pack/plugins/cases/tsconfig.json @@ -71,6 +71,7 @@ "@kbn/content-management-plugin", "@kbn/index-management-plugin", "@kbn/rison", + "@kbn/core-application-browser", ], "exclude": [ "target/**/*",