Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cases] Refactor attach alert to new case flyout #125505

Merged
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d7833bd
WIP
Feb 10, 2022
f9de000
WIP2
Feb 11, 2022
fe21f1a
Use new cases context hooks to open and close the flyout
Feb 14, 2022
55e496a
Update timelines to use new hooks
Feb 14, 2022
e137ba6
CLose flyout on create success
Feb 14, 2022
4c580b9
Add back sucess toast
Feb 14, 2022
c6c89aa
Move code to a dedicated component
Feb 14, 2022
6392bf1
Add CasesContext to observability
Feb 14, 2022
205728e
Remove dependency
Feb 14, 2022
044279c
Small refactor
Feb 14, 2022
04988d6
Use observabilityAppId instead of observabilityFeatureId for buttons
Feb 14, 2022
6392bb3
Merge remote-tracking branch 'upstream/main' into refactor/security-f…
Feb 15, 2022
416d81b
Add CasesContext to timetable
Feb 15, 2022
66a0a0d
Fix detection engine test cases
Feb 15, 2022
13caeb6
Fix broken tests
Feb 15, 2022
4962ed8
Merge remote-tracking branch 'upstream/main' into refactor/security-f…
Feb 16, 2022
7deb110
Fix broken tests
Feb 16, 2022
6cf2996
Rename hook
Feb 16, 2022
38b7f50
Add test cases for cases context ui
Feb 16, 2022
a256d8f
Add test for new hook
Feb 16, 2022
ff64286
Remove state from the provider context
Feb 16, 2022
9cf248d
Remove basevalue
Feb 16, 2022
8b159d2
apply suggested renaming
Feb 16, 2022
1486a88
Add usecallback
Feb 16, 2022
a297fbf
Add reducer types, fix test type, remove redundant check
Feb 17, 2022
314cdf0
Merge remote-tracking branch 'upstream/main' into refactor/security-f…
Feb 17, 2022
a5dddde
Merge branch 'main' into refactor/security-flyout-modal-cases-alert
kibanamachine Feb 21, 2022
aa6e16c
Merge branch 'main' into refactor/security-flyout-modal-cases-alert
kibanamachine Feb 21, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions x-pack/plugins/cases/common/ui/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,6 @@ export interface CasesContextFeatures {

export type CasesFeatures = Partial<CasesContextFeatures>;

export interface CasesContextValue {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was moved out of here as it is not a common type for BE and FE but only FE.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@academo Out of curiosity what does BE and FE stand for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BE = Backend, FE = Frontend.

owner: string[];
appId: string;
appTitle: string;
userCanCrud: boolean;
basePath: string;
features: CasesContextFeatures;
}

export interface CasesUiConfigType {
markdownPlugins: {
lens: boolean;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
Copy link
Contributor Author

@academo academo Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reducer will be only to handle the state for the CasesContext which will handle the flyout, modals and in general third integrations. It is not intended to be used for all cases plugin purposes.

* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
academo marked this conversation as resolved.
Show resolved Hide resolved
* 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 { CreateCaseFlyoutProps } from '../create/flyout';

export const getInitialCasesContextState = (): CasesContextState => {
return {
createCaseFlyout: {
isFlyoutOpen: false,
},
};
};

export interface CasesContextState {
createCaseFlyout: {
isFlyoutOpen: boolean;
props?: CreateCaseFlyoutProps;
};
}

export enum CasesContextStoreActionsList {
OPEN_CREATE_CASE_FLYOUT,
CLOSE_CREATE_CASE_FLYOUT,
}
export type CasesContextStoreAction =
| {
type: CasesContextStoreActionsList.OPEN_CREATE_CASE_FLYOUT;
payload: CreateCaseFlyoutProps;
}
| { type: CasesContextStoreActionsList.CLOSE_CREATE_CASE_FLYOUT };

export const casesContextReducer: React.Reducer<CasesContextState, CasesContextStoreAction> = (
state: CasesContextState,
action: CasesContextStoreAction
): CasesContextState => {
switch (action.type) {
case CasesContextStoreActionsList.OPEN_CREATE_CASE_FLYOUT: {
return { ...state, createCaseFlyout: { isFlyoutOpen: true, props: action.payload } };
}
case CasesContextStoreActionsList.CLOSE_CREATE_CASE_FLYOUT: {
return { ...state, createCaseFlyout: { isFlyoutOpen: false } };
}
default:
return state;
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* 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 React from 'react';
import { AppMockRenderer, createAppMockRenderer } from '../../common/mock';
import { getCreateCaseFlyoutLazyNoProvider } from '../../methods/get_create_case_flyout';
import { CasesGlobalComponents } from './cases_global_components';

jest.mock('../../methods/get_create_case_flyout');

const getCreateCaseFlyoutLazyNoProviderMock = getCreateCaseFlyoutLazyNoProvider as jest.Mock;

describe('Cases context UI', () => {
let appMock: AppMockRenderer;

beforeEach(() => {
appMock = createAppMockRenderer();
getCreateCaseFlyoutLazyNoProviderMock.mockClear();
});

describe('create case flyout', () => {
it('should render the create case flyout when isFlyoutOpen is true', async () => {
const state = {
createCaseFlyout: {
isFlyoutOpen: true,
props: {
attachments: [],
},
},
};
appMock.render(<CasesGlobalComponents state={state} />);
expect(getCreateCaseFlyoutLazyNoProviderMock).toHaveBeenCalledWith({ attachments: [] });
});
it('should not render the create case flyout when isFlyoutOpen is true', async () => {
academo marked this conversation as resolved.
Show resolved Hide resolved
const state = {
createCaseFlyout: {
isFlyoutOpen: false,
},
};
appMock.render(<CasesGlobalComponents state={state} />);
expect(getCreateCaseFlyoutLazyNoProviderMock).not.toHaveBeenCalled();
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* 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 React from 'react';
import { getCreateCaseFlyoutLazyNoProvider } from '../../methods';
import { CasesContextState } from './cases_context_reducer';

export const CasesGlobalComponents = React.memo(({ state }: { state: CasesContextState }) => {
return (
<>
{state.createCaseFlyout.isFlyoutOpen && state.createCaseFlyout.props !== undefined
? getCreateCaseFlyoutLazyNoProvider(state.createCaseFlyout.props)
: null}
</>
);
});
CasesGlobalComponents.displayName = 'CasesContextUi';
35 changes: 29 additions & 6 deletions x-pack/plugins/cases/public/components/cases_context/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,36 @@
* 2.0.
*/

import React, { useState, useEffect } from 'react';
import React, { useState, useEffect, useReducer } from 'react';
import { merge } from 'lodash';
import { CasesContextValue, CasesFeatures } from '../../../common/ui/types';
import { DEFAULT_FEATURES } from '../../../common/constants';
import { DEFAULT_BASE_PATH } from '../../common/navigation';
import { useApplication } from './use_application';
import {
CasesContextStoreAction,
casesContextReducer,
getInitialCasesContextState,
} from './cases_context_reducer';
import { CasesContextFeatures, CasesFeatures } from '../../containers/types';
import { CasesGlobalComponents } from './cases_global_components';

export const CasesContext = React.createContext<CasesContextValue | undefined>(undefined);
export interface CasesContextValue {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brought from common types.

owner: string[];
appId: string;
appTitle: string;
userCanCrud: boolean;
basePath: string;
features: CasesContextFeatures;
dispatch: (action: CasesContextStoreAction) => void;
academo marked this conversation as resolved.
Show resolved Hide resolved
}

export interface CasesContextProps
extends Omit<CasesContextValue, 'appId' | 'appTitle' | 'basePath' | 'features'> {
export interface CasesContextProps extends Pick<CasesContextValue, 'owner' | 'userCanCrud'> {
basePath?: string;
features?: CasesFeatures;
}

export const CasesContext = React.createContext<CasesContextValue | undefined>(undefined);

export interface CasesContextStateValue extends Omit<CasesContextValue, 'appId' | 'appTitle'> {
appId?: string;
appTitle?: string;
Expand All @@ -30,6 +45,7 @@ export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({
value: { owner, userCanCrud, basePath = DEFAULT_BASE_PATH, features = {} },
}) => {
const { appId, appTitle } = useApplication();
const [state, dispatch] = useReducer(casesContextReducer, getInitialCasesContextState());
const [value, setValue] = useState<CasesContextStateValue>(() => ({
owner,
userCanCrud,
Expand All @@ -39,6 +55,7 @@ export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({
* of the DEFAULT_FEATURES object
*/
features: merge({}, DEFAULT_FEATURES, features),
dispatch,
}));

/**
Expand All @@ -58,11 +75,17 @@ export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({
}, [appTitle, appId, userCanCrud]);

return isCasesContextValue(value) ? (
<CasesContext.Provider value={value}>{children}</CasesContext.Provider>
<CasesContext.Provider value={value}>
academo marked this conversation as resolved.
Show resolved Hide resolved
<CasesGlobalComponents state={state} />
{children}
</CasesContext.Provider>
) : null;
};
CasesProvider.displayName = 'CasesProvider';

function isCasesContextValue(value: CasesContextStateValue): value is CasesContextValue {
return value.appId != null && value.appTitle != null && value.userCanCrud != null;
}

// eslint-disable-next-line import/no-default-export
export default CasesProvider;
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import { UsePostComment } from '../../../containers/use_post_comment';

export interface CreateCaseFlyoutProps {
afterCaseCreated?: (theCase: Case, postComment: UsePostComment['postComment']) => Promise<void>;
onClose: () => void;
onSuccess: (theCase: Case) => Promise<void>;
onClose?: () => void;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These attributes are now optional because third party plugins might decide to not need them. The idea is they could simply say: "Open the modal, attach this info to a case" and might or not care about the outcome of the operation.

onSuccess?: (theCase: Case) => Promise<void>;
attachments?: CreateCaseAttachment;
}

Expand Down Expand Up @@ -66,6 +66,8 @@ const FormWrapper = styled.div`

export const CreateCaseFlyout = React.memo<CreateCaseFlyoutProps>(
({ afterCaseCreated, onClose, onSuccess, attachments }) => {
const handleCancel = onClose || function () {};
const handleOnSuccess = onSuccess || async function () {};
return (
<>
<GlobalStyle />
Expand All @@ -85,8 +87,8 @@ export const CreateCaseFlyout = React.memo<CreateCaseFlyoutProps>(
<CreateCaseForm
afterCaseCreated={afterCaseCreated}
attachments={attachments}
onCancel={onClose}
onSuccess={onSuccess}
onCancel={handleCancel}
onSuccess={handleOnSuccess}
withSteps={false}
/>
</FormWrapper>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* 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.
*/

/* eslint-disable react/display-name */

import { renderHook } from '@testing-library/react-hooks';
import React from 'react';
import { CasesContext } from '../../cases_context';
import { CasesContextStoreActionsList } from '../../cases_context/cases_context_reducer';
import { useCasesAddToNewCaseFlyout } from './use_cases_add_to_new_case_flyout';

describe('use cases add to new case flyout hook', () => {
const dispatch = jest.fn();
let wrapper: React.FC;
beforeEach(() => {
dispatch.mockReset();
wrapper = ({ children }) => {
return (
<CasesContext.Provider
academo marked this conversation as resolved.
Show resolved Hide resolved
value={{
owner: ['test'],
userCanCrud: true,
appId: 'test',
appTitle: 'jest',
basePath: '/jest',
dispatch,
features: { alerts: { sync: true }, metrics: [] },
}}
>
{children}
</CasesContext.Provider>
);
};
});

it('should throw if called outside of a cases context', () => {
const { result } = renderHook(() => {
useCasesAddToNewCaseFlyout({});
});
expect(result.error?.message).toContain(
'useCasesContext must be used within a CasesProvider and have a defined value'
);
});

it('should dispatch the open action when invoked', () => {
const { result } = renderHook(
() => {
return useCasesAddToNewCaseFlyout({});
},
{ wrapper }
);
result.current.open();
expect(dispatch).toHaveBeenCalledWith(
expect.objectContaining({
type: CasesContextStoreActionsList.OPEN_CREATE_CASE_FLYOUT,
})
);
});

it('should dispatch the close action when invoked', () => {
const { result } = renderHook(
() => {
return useCasesAddToNewCaseFlyout({});
},
{ wrapper }
);
result.current.close();
expect(dispatch).toHaveBeenCalledWith(
expect.objectContaining({
type: CasesContextStoreActionsList.CLOSE_CREATE_CASE_FLYOUT,
})
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hook is the direct integration with third party plugins. Plugins will load this hook and use it to open or close the flyout as required.

* 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 { useCallback } from 'react';
import { CasesContextStoreActionsList } from '../../cases_context/cases_context_reducer';
import { useCasesContext } from '../../cases_context/use_cases_context';
import { CreateCaseFlyoutProps } from './create_case_flyout';

export const useCasesAddToNewCaseFlyout = (props: CreateCaseFlyoutProps) => {
const context = useCasesContext();

const closeFlyout = useCallback(() => {
context.dispatch({
type: CasesContextStoreActionsList.CLOSE_CREATE_CASE_FLYOUT,
});
}, [context]);

const openFlyout = useCallback(() => {
context.dispatch({
type: CasesContextStoreActionsList.OPEN_CREATE_CASE_FLYOUT,
payload: {
...props,
onClose: () => {
closeFlyout();
if (props.onClose) {
return props.onClose();
}
},
afterCaseCreated: async (...args) => {
closeFlyout();
if (props.afterCaseCreated) {
return props.afterCaseCreated(...args);
}
},
},
});
}, [closeFlyout, context, props]);
return {
open: openFlyout,
close: closeFlyout,
};
};

export type UseCasesAddToNewCaseFlyout = typeof useCasesAddToNewCaseFlyout;
1 change: 1 addition & 0 deletions x-pack/plugins/cases/public/components/create/form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const MySpinner = styled(EuiLoadingSpinner)`
`;
export type SupportedCreateCaseAttachment = CommentRequestAlertType | CommentRequestUserType;
export type CreateCaseAttachment = SupportedCreateCaseAttachment[];
export type CaseAttachments = SupportedCreateCaseAttachment[];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a type to replace CreateCaseAttachment. That will come in cleanup PRs to make this PR smaller.


export interface CreateCaseFormFieldsProps {
connectors: ActionConnector[];
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/cases/public/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export type { GetCreateCaseFlyoutProps } from './methods/get_create_case_flyout'
export type { GetAllCasesSelectorModalProps } from './methods/get_all_cases_selector_modal';
export type { GetRecentCasesProps } from './methods/get_recent_cases';

export type { CaseAttachments } from './components/create/form';

export type { ICasesDeepLinkId } from './common/navigation';
export {
getCasesDeepLinks,
Expand Down
Loading