-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solution] [Cases] Move create page components and dependencies to Cases #94444
[Security Solution] [Cases] Move create page components and dependencies to Cases #94444
Conversation
…/kibana into cases_moving_create
…into cases_moving_create
Pinging @elastic/security-solution (Team: SecuritySolution) |
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
@@ -5,8 +5,7 @@ | |||
* 2.0. | |||
*/ | |||
|
|||
import { DEFAULT_MAX_SIGNALS } from '../../security_solution/common/constants'; | |||
|
|||
const DEFAULT_MAX_SIGNALS = 100; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should put a comment here and in security solution that there is a dependency between the two plugins and if changed in security solution it should change in cases also.
}; | ||
} | ||
|
||
export interface SecurityAppError extends AppError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be CasesAppError
right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good call. i just did a search for the word security/siem and got rid of it in my next commit
|
||
private static throwUninitializedError(): never { | ||
throw new Error( | ||
'Kibana services not initialized - are you trying to import this module from outside of the SIEM app?' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: SIEM
-> Cases
@@ -0,0 +1,33 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this file for cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes! we import the form from es_ui_shared
@@ -0,0 +1,12 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we use this file for cases? If not I think we should remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. the function removeExternalLinkText
is used in markdown_editor/renderer.test.ts
.
@@ -0,0 +1,8 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right. this is not used ;)
export const getCreateCaseLazy = (props: CreateCaseProps) => { | ||
const CreateCaseLazy = lazy(() => import('./components/create')); | ||
return ( | ||
<Suspense fallback={null}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why null
as a fallback? You think we shouldn't show a loading spinner and leave it up to the consumer of the component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copied from actions, ill change this to have the spinner
x-pack/plugins/cases/public/types.ts
Outdated
} | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-empty-interface | ||
export interface PluginStart {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we expose getCreateCase
and casesComponent
we should fill this interface accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i deleted it, i had a different type defining this CasesUiStart
`} | ||
`; | ||
|
||
export interface CreateCaseProps { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it make sense to export also afterCaseCreated
? It is a callback that is being called after the creation of the case but before the push of the case. onSuccess
is being called after the creation and the push of the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the link didn't jump to the code. can you show me where this gets called?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Its amazing!
…-action * 'master' of github.com:elastic/kibana: (44 commits) Migrate the optimizer mixin to core (#94272) Replace EuiCodeBlock with JsonCodeEditor in DiscoverGrid (#92442) [ML] Anomaly Detection: Migrate validation messages links to use docLinks. (#94568) [Lists][Exceptions] - Adding basic linting, i18n and storybook support (#94772) [Fleet] Add test/fix for invalid/missing ids in bulk agent reassign (#94632) [Security Solution] [Cases] Move create page components and dependencies to Cases (#94444) [ML] Data Frame Analytics accessibility tests: fix flaky outlier creation test (#94735) [Security Solutions] Remove commented out old linter rules (#94753) [App Search] Role mappings migration part 2 (#94461) [CI] Update Backport action inputs to match updated ones (#94721) [chore] Remove the infra team from CODEOWNERS (#94740) [Connectors UI] Make UI use new connector APIs (#94180) [ML] Use indices options in anomaly detection job wizards (#91830) Remove `string` as a valid registry var type value (#94174) [Alerts] Replaces legacy es client with the ElasticsearchClient for alerts and triggers_actions_ui plugins. (#93364) [Reporting-CSV Export] Re-write CSV Export using SearchSource (#88303) chore(NA): upgrade bazel rules nodejs to v3.2.2 (#94726) [APM] Settings: Update layout and update/add descriptions (#94398) skip flaky suite (#94666) [XY axis] Integrates legend color picker with the eui palette (#90589) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/cold_phase/cold_phase.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared_fields/searchable_snapshot_field/searchable_snapshot_field.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/constants.ts # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/configuration_context.tsx # x-pack/plugins/index_lifecycle_management/public/shared_imports.ts
Friendly reminder: Looks like this PR hasn’t been backported yet. |
Friendly reminder: Looks like this PR hasn’t been backported yet. |
Reverted by #94975. No need to backport. |
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_management/component_templates·ts.apis management index management Component templates Delete should return an error for any component templates not sucessfully deletedStandard Out
Stack Trace
Kibana Pipeline / general / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_management/component_templates·ts.apis management index management Component templates Delete should return an error for any component templates not sucessfully deletedStandard Out
Stack Trace
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
Summary
Moves the create page components and dependencies to Cases. @michaelolo24 and I successfully posted a case with a Jira connector from the create page components housed in CasesUi.
Reviewer, be not afraid! The files in
plugins/cases/public/components
,plugins/cases/public/containers
, andplugins/cases/public/common
were copy/pasted fromsecurity_solution
. The changes made to these files were to update import paths and remove timeline from the markdown components. The changes insecurity_solution
are largely file path changes as well. Here are 212 files of just one line import changes: 4ff74fbThe reason for a lot of the import changes is because when we added cases as a
requiredPlugin
insecurity_solution
, and we got errors yelling at us about importing from a non-public directory everywhere we were importing from cases, a lot of places. Socommon
got added as a public plugin export in cases and the import insecurity_solution
needed to be shortened.Resolves #92416
Resolves #94131
Still To Do on Create Page:
New Cases UI Method
getCreateCase
Arguments:
(theCase: Case) => Promise<void>;
callback passing newly created case before pushCaseToExternalService is called() => void;
callback when create case is canceled(theCase: Case) => Promise<void>;
callback passing newly created case after pushCaseToExternalService is calledUI component:
Cases-RAC-UI is behind a Feature Flag
DO NOT MERGE if
export const USE_RAC_CASES_UI = true;
.USE_RAC_CASES_UI
must befalse
!Checklist
Delete any items that are not applicable to this PR.