[Security Solution] [Cases] Deleting files from security solution and removing feature flag #95176
Conversation
stephmilovic
added
v8.0.0
release_note:skip
| @@ -5,4 +5,4 @@ | |||
| * 2.0. | |||
| */ | |||
|
|
|||
| export * from '../../translations'; | |||
| export * from '../../public/containers/types'; | |||
There was a problem hiding this comment.
this is weird. this is a brand new file, but github thinks it comes from one of my deleted files?
| const onTableRowClick = memoize(() => { | ||
| const onTableRowClick = memoize(async () => { | ||
| if (alertData != null) { | ||
| await postComment({ |
There was a problem hiding this comment.
this was previously in security_solution/public/cases/components/timeline_actions/add_to_case_action.tsx
There was a problem hiding this comment.
Imo, this shouldn't be inside the component. It is better to be implemented inside the onRowClick where the consumer of the component pass it as a prop. If security solution, needs access to the postComment function we can provided it through our casesClient (backend).
There was a problem hiding this comment.
i didnt want the ui components to rely on any api methods since actions dont, they should just work
There was a problem hiding this comment.
I see what you mean. I talk about it with @XavierM and we think that we should break the AllCases components into smaller components and remove the isModal logic. This way we can move the onTableRowClick and alertData logic inside a new modal called, for example, AttachToCaseModal which can be exported through the CasesUiClient.
Example:
// AllCases component
interface Props {
configureCasesHref: string;
createCaseHref: string;
disabledStatuses?: CaseStatuses[];
getCaseDetailsHref: (caseDetails: CaseDetailsHrefSchema) => string;
onCaseDetailsNavClick: (caseDetails: CaseDetailsHrefSchema) => void;
onConfigureCasesNavClick?: (ev: React.MouseEvent) => void;
onCreateCaseNavClick?: (ev: React.MouseEvent) => void;
userCanCrud: boolean;
}
<>
<AllCasesHeader>
<AllCasesFilters>
<AllCasesTable>
</>
// AttachToCase modal
interface Props {
onAttachment: () => void;
}
<>
<AllCasesFilters>
<AllCasesTable>
</>
I think is a good opportunity to do it in this PR. It will be much cleaner. The AllCases is too compact, does a lot of things without reason.
| @@ -36,8 +36,6 @@ const CaseParamsFields: React.FunctionComponent<ActionParamsProps<CaseActionPara | |||
| actionParams, | |||
| editAction, | |||
| index, | |||
| errors, | |||
There was a problem hiding this comment.
these were unused
| @@ -26,21 +27,36 @@ const Container = styled.div` | |||
|
|
|||
| export interface CreateCaseProps { | |||
| afterCaseCreated?: (theCase: Case) => Promise<void>; | |||
| caseType?: CaseType; | |||
There was a problem hiding this comment.
i missed a few properties from create case from modal/flyout
| perPage: number; | ||
| } | ||
|
|
||
| const RecentCases = ({ |
There was a problem hiding this comment.
| @@ -131,15 +131,15 @@ export interface UseGetCases extends UseGetCasesState { | |||
| } | |||
|
|
|||
| export const useGetCases = ( | |||
| initialQueryParams?: QueryParams, | |||
| initialFilterOptions?: FilterOptions | |||
| initialQueryParams: Partial<QueryParams> = {}, | |||
There was a problem hiding this comment.
changed this so components passing arguments don't need to import DEFAULT_FILTER_OPTIONS or DEFAULT_QUERY_PARAMS, only need to pass the params that are different from default
| useInsertTimeline(formData[descriptionFieldName] ?? '', onTimelineAttached); | ||
| return null; | ||
| }; | ||
| // TO DO: Cases RAC UI, reimplement this shiz |
| userCanCrud={userPermissions?.crud ?? false} | ||
| /> | ||
| )} | ||
| <CaseView |
| @@ -7,3 +7,4 @@ | |||
|
|
|||
| export * from './constants'; | |||
| export * from './api'; | |||
| export * from './ui/types'; | |||
There was a problem hiding this comment.
Why the ui types are being exported? Will they be used by another plugin?
There was a problem hiding this comment.
yes, security solution uses the types on the UI
|
|
||
| UI component: | ||
| ![Recent Cases Component][recent-cases-img] | ||
|
|
There was a problem hiding this comment.
I know is out of the scope of this PR but could you add a note that the case action type is disabled by default and the connector is not supported?
| @@ -5,13 +5,16 @@ | |||
| * 2.0. | |||
| */ | |||
|
|
|||
| import { CoreStart, Plugin, PluginInitializerContext } from 'src/core/public'; | |||
| import { CoreSetup, CoreStart, Plugin, PluginInitializerContext } from 'src/core/public'; | |||
x-pack/plugins/security_solution/public/overview/components/recent_cases/index.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/cases/public/components/recent_cases/loading_placeholders.tsx
Show resolved
Hide resolved
x-pack/plugins/cases/public/components/recent_cases/no_cases/index.test.tsx
Outdated
Show resolved
Hide resolved
| const ref = useRef(); | ||
| useEffect(() => { | ||
| (ref.current as unknown) = value; | ||
| }); |
There was a problem hiding this comment.
This will run on every render, right?
There was a problem hiding this comment.
i think so, copy/pasted over from @andrew-goldstein
| }; | ||
|
|
||
| // eslint-disable-next-line import/no-default-export | ||
| export { RecentCases as default }; |
There was a problem hiding this comment.
Why don't we export React.memo(RecentCases); ?
There was a problem hiding this comment.
why tho? what benefit does wrapping it in React.memo give?
...ck/plugins/security_solution/public/cases/components/timeline_actions/add_to_case_action.tsx
Show resolved
Hide resolved
...ck/plugins/security_solution/public/cases/components/timeline_actions/add_to_case_action.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Very good PR. I left some comments. The most important one is the one about the alertData we are passing to the AllCases component.
I did the following tests:
- Recent cases widget:
- Create case button 🟢
- Switch between filters 🟢
- View all cases 🟢
- All cases:
- KPI 🟢
- Bulk actions 🟢
- Change status from actions 🟢
- Filter by status, tags, reporters 🟢
- Search bar 🟢
- Pagination 🟢
- Correct data on the table 🟢
- Sort by alerts or comments 🔴 (I think this is a know issue)
- Configuration page:
- Create connectors 🟢
- Select connector 🟢
- Go back to all cases 🟢
- Change closure options 🟢
- Creation form:
- Create case 🟢
- Cancel 🟢
- Default connector 🟢
- Connectors' fields 🟢
- View case
- Add comment 🟢
- Edit tags, title, description, connector 🟢
- User actions 🟢
- Push case to all supported connectors 🟢
- Delete case 🟢
- View external service incident 🟢
- Alerts:
- See attach alerts on a case 🟢
- Show alerts details inside a case 🟢
- Navigate to alert's rule from a case 🟢
- Alert status is changed when changing the status of a case and sync is on.
- Attach alert to a new case (flyout) 🟢
- Attach alert to an existing case (modal) 🟢
- Timeline
- Cannot attach a timeline to a new case 🔴 (I think this is a know issue)
- Cannot attach a timeline to an existing case 🔴 (I think this is a know issue)
- Timeline closes after pressing attach to an existing case 🔴 (even thought the application is being navigated to the case view page).
Bugs:
- When you change the status of a case the push button appears. This issue in unrelated to this PR.
- The title of the case is not being shown on the path
- Comments tooltip in the recent cases widget is not being shown correctly
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
Deleting files from security solution and removing feature flag. Security Solution now fully relies upon Cases for all UI.
More moved:
plugins.triggersActionsUi.actionTypeRegistry.register(getCaseConnectorUi());is now called fromcasesrather thansecurity_solutionx-pack/plugins/security_solution/public/cases/components/create/flyout.tsxx-pack/plugins/security_solution/public/overview/components/recent_cases/was moved tox-pack/plugins/cases/public/components/recent_cases/. theindex.tsxinsecurity_solutionnow calls a new method,cases.getRecentCasesNew method:
getRecentCasesArguments:
string;href of all cases pagestring;(caseDetails: CaseDetailsHrefSchema) => string;(ev: React.MouseEvent) => void;callback for all cases link click(caseDetails: CaseDetailsHrefSchema) => void;callback for case details nav clicknumber;number of cases to show in widgetUI component:
