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 15 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 initialCasesContextState = (): CasesContextState => {
academo marked this conversation as resolved.
Show resolved Hide resolved
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,21 @@
/*
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 the component that decides to show or not the flyout based on the state and eventually the select case modal too and any other integration the cases plugin would offer.

* 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 CasesContextUI = ({ state }: { state: CasesContextState }) => {
semd marked this conversation as resolved.
Show resolved Hide resolved
return (
<>
{state.createCaseFlyout.isFlyoutOpen && state.createCaseFlyout.props !== undefined
? getCreateCaseFlyoutLazyNoProvider(state.createCaseFlyout.props)
: null}
</>
);
};
CasesContextUI.displayName = 'CasesContextUi';
64 changes: 47 additions & 17 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,38 @@
* 2.0.
*/

import React, { useState, useEffect } from 'react';
import React, { useState, useEffect, useReducer, useMemo } 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,
CasesContextState,
initialCasesContextState,
} from './cases_context_reducer';
import { CasesContextFeatures, CasesFeatures } from '../../containers/types';
import { CasesContextUI } from './cases_context_ui';

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;
state: CasesContextState;
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,16 +47,19 @@ export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({
value: { owner, userCanCrud, basePath = DEFAULT_BASE_PATH, features = {} },
}) => {
const { appId, appTitle } = useApplication();
const [value, setValue] = useState<CasesContextStateValue>(() => ({
owner,
userCanCrud,
basePath,
/**
* The empty object at the beginning avoids the mutation
* of the DEFAULT_FEATURES object
*/
features: merge({}, DEFAULT_FEATURES, features),
}));
const [state, dispatch] = useReducer(casesContextReducer, initialCasesContextState());
const [baseValue, setBaseValue] = useState<Omit<CasesContextStateValue, 'state' | 'dispatch'>>(
() => ({
owner,
userCanCrud,
basePath,
/**
* The empty object at the beginning avoids the mutation
* of the DEFAULT_FEATURES object
*/
features: merge({}, DEFAULT_FEATURES, features),
})
);
semd marked this conversation as resolved.
Show resolved Hide resolved

/**
* `userCanCrud` prop may change by the parent plugin.
Expand All @@ -48,7 +68,7 @@ export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({
*/
useEffect(() => {
if (appId && appTitle) {
setValue((prev) => ({
setBaseValue((prev) => ({
...prev,
appId,
appTitle,
Expand All @@ -57,12 +77,22 @@ export const CasesProvider: React.FC<{ value: CasesContextProps }> = ({
}
}, [appTitle, appId, userCanCrud]);

const value = useMemo(() => {
return { ...baseValue, state, dispatch };
}, [baseValue, state]);

return isCasesContextValue(value) ? (
<CasesContext.Provider value={value}>{children}</CasesContext.Provider>
<CasesContext.Provider value={value}>
academo marked this conversation as resolved.
Show resolved Hide resolved
<CasesContextUI state={value.state} />
semd marked this conversation as resolved.
Show resolved Hide resolved
{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,45 @@
/*
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 { CasesContextStoreActionsList } from '../../cases_context/cases_context_reducer';
import { useCasesContext } from '../../cases_context/use_cases_context';
import { CreateCaseFlyoutProps } from './create_case_flyout';

export const useCasesAddToNewCasesFlyout = (props: CreateCaseFlyoutProps) => {
const context = useCasesContext();
const closeFlyout = () => {
academo marked this conversation as resolved.
Show resolved Hide resolved
context.dispatch({
type: CasesContextStoreActionsList.CLOSE_CREATE_CASE_FLYOUT,
});
};
const openFlyout = () => {
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);
}
},
},
});
};
return {
open: openFlyout,
close: closeFlyout,
};
};

export type UseCasesAddToNewCasesFlyout = typeof useCasesAddToNewCasesFlyout;
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
34 changes: 34 additions & 0 deletions x-pack/plugins/cases/public/methods/get_cases_context.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lazy component to load the CasesContext.

The CasesContext is required to render the cases flyout and eventually the modals. You use the useCreateCaseFlyout hook to interact with this context.

Plugins should wrap the components that will show the flyout or modal on this.

* 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 { EuiLoadingSpinner } from '@elastic/eui';
import React, { lazy, ReactNode, Suspense } from 'react';
import { CasesContextProps } from '../components/cases_context';

export type GetCasesContextProps = CasesContextProps;

const CasesProviderLazy: React.FC<{ value: GetCasesContextProps }> = lazy(
() => import('../components/cases_context')
);

const CasesProviderLazyWrapper = ({
owner,
userCanCrud,
features,
children,
}: GetCasesContextProps & { children: ReactNode }) => {
return (
<Suspense fallback={<EuiLoadingSpinner />}>
<CasesProviderLazy value={{ owner, userCanCrud, features }}>{children}</CasesProviderLazy>
</Suspense>
);
};
CasesProviderLazyWrapper.displayName = 'CasesProviderLazyWrapper';

export const getCasesContextLazy = () => {
return CasesProviderLazyWrapper;
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { CasesProvider, CasesContextProps } from '../components/cases_context';

export type GetCreateCaseFlyoutProps = CreateCaseFlyoutProps & CasesContextProps;

const CreateCaseFlyoutLazy: React.FC<CreateCaseFlyoutProps> = lazy(
export const CreateCaseFlyoutLazy: React.FC<CreateCaseFlyoutProps> = lazy(
() => import('../components/create/flyout')
);
export const getCreateCaseFlyoutLazy = ({
Expand All @@ -35,3 +35,9 @@ export const getCreateCaseFlyoutLazy = ({
</Suspense>
</CasesProvider>
);

export const getCreateCaseFlyoutLazyNoProvider = (props: CreateCaseFlyoutProps) => (
academo marked this conversation as resolved.
Show resolved Hide resolved
<Suspense fallback={<EuiLoadingSpinner />}>
<CreateCaseFlyoutLazy {...props} />
</Suspense>
);
5 changes: 5 additions & 0 deletions x-pack/plugins/cases/public/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ import { CasesUiStart } from './types';
const createStartContract = (): jest.Mocked<CasesUiStart> => ({
canUseCases: jest.fn(),
getCases: jest.fn(),
getCasesContext: jest.fn(),
getAllCasesSelectorModal: jest.fn(),
getCreateCaseFlyout: jest.fn(),
getRecentCases: jest.fn(),
getCreateCaseFlyoutNoProvider: jest.fn(),
hooks: {
getUseCasesAddToNewCasesFlyout: jest.fn(),
},
});

export const casesPluginMock = {
Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/cases/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@ import {
getAllCasesSelectorModalLazy,
getCreateCaseFlyoutLazy,
canUseCases,
getCreateCaseFlyoutLazyNoProvider,
} from './methods';
import { CasesUiConfigType } from '../common/ui/types';
import { getCasesContextLazy } from './methods/get_cases_context';
import { useCasesAddToNewCasesFlyout } from './components/create/flyout/use_cases_add_to_new_case_flyout';

/**
* @public
Expand All @@ -35,9 +38,14 @@ export class CasesUiPlugin implements Plugin<void, CasesUiStart, SetupPlugins, S
return {
canUseCases: canUseCases(core.application.capabilities),
getCases: getCasesLazy,
getCasesContext: getCasesContextLazy,
getRecentCases: getRecentCasesLazy,
getCreateCaseFlyout: getCreateCaseFlyoutLazy,
getAllCasesSelectorModal: getAllCasesSelectorModalLazy,
getCreateCaseFlyoutNoProvider: getCreateCaseFlyoutLazyNoProvider,
academo marked this conversation as resolved.
Show resolved Hide resolved
hooks: {
getUseCasesAddToNewCasesFlyout: useCasesAddToNewCasesFlyout,
},
};
}

Expand Down
Loading