-
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
[Fleet] IngestManager Plugin interface for registering UI extensions #82783
[Fleet] IngestManager Plugin interface for registering UI extensions #82783
Conversation
…has configuration options)
…gest-ui-extension-registration # Conflicts: # x-pack/plugins/ingest_manager/public/applications/ingest_manager/sections/agent_policy/create_package_policy_page/step_configure_package.tsx
…gest-ui-extension-registration # Conflicts: # x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/components/custom_package_policy.tsx # x-pack/plugins/fleet/public/index.ts # x-pack/plugins/fleet/public/plugin.ts # x-pack/plugins/security_solution/public/common/mock/endpoint/dependencies_start_mock.ts # x-pack/plugins/security_solution/public/management/pages/policy/view/ingest_manager_integration/endpoint_policy_edit_extension.tsx
…gest-ui-extension-registration
a3380c7
to
77250da
Compare
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 overall architecture looks good and makes sense. I left comments on some areas of concern that I spotted, happy to have discussions for any of them!
x-pack/plugins/fleet/public/applications/fleet/hooks/use_ui_extension.ts
Outdated
Show resolved
Hide resolved
...s/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/index.tsx
Outdated
Show resolved
Hide resolved
...ins/fleet/public/applications/fleet/sections/agent_policy/edit_package_policy_page/index.tsx
Outdated
Show resolved
Hide resolved
@@ -55,6 +63,10 @@ export const StepConfigurePackagePolicy: React.FunctionComponent<{ | |||
validationResults, | |||
submitAttempted, | |||
}) => { | |||
const hasUiExtension = | |||
useUIExtension(packageInfo.name, 'integration-policy', from === 'edit' ? 'edit' : 'create') !== | |||
undefined; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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 believe it will be a boolean. It's the result of something !== undefined, which should be a boolean.
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.
doh 🤦🏻♀️ I read it as a nested ternary
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 case for replacing the shorthand, if it's initially hard to parse scrolling through
...c/management/pages/policy/view/ingest_manager_integration/endpoint_policy_edit_extension.tsx
Outdated
Show resolved
Hide resolved
const { registerExtension } = plugins.ingestManager; | ||
|
||
registerExtension({ | ||
integration: 'endpoint', | ||
type: 'integration-policy', | ||
view: 'edit', | ||
component: LazyEndpointPolicyEditExtension, | ||
}); | ||
|
||
registerExtension({ | ||
integration: 'endpoint', | ||
type: 'integration-policy', | ||
view: 'create', | ||
component: LazyEndpointPolicyCreateExtension, | ||
}); |
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.
nice 👍🏻
...ins/fleet/public/applications/fleet/sections/agent_policy/edit_package_policy_page/index.tsx
Show resolved
Hide resolved
...plications/fleet/sections/agent_policy/create_package_policy_page/step_configure_package.tsx
Outdated
Show resolved
Hide resolved
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.
Tested locally works as expected, looks good to me 🚀
…gest-ui-extension-registration
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.
One minor comment but otherwise LGTM. Thanks for making the review changes; this is a great foundation for all the extensions to come. Nice work!
x-pack/plugins/fleet/public/applications/fleet/types/ui_extensions.ts
Outdated
Show resolved
Hide resolved
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Distributable file count
Page load bundle
History
To update your PR or re-run it, just comment with: |
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.
looks good, nothing left to say but nits
…lastic#82783) * Expose `registerExtension()` interface on `Plugin#start` * Refactor use of `CustomConfigurePackagePolicy` to the new registerExtension approach * Refactor to always show registered ui extension (even if Integration has configuration options)
* master: [ML] Update apidoc config with the Trained models endpoints (elastic#83274) [Fleet] IngestManager Plugin interface for registering UI extensions (elastic#82783) [Alerting] Moves the Index & Geo Threshold UIs into the Stack Alerts Public Plugin (elastic#82951)
Summary
Plugins#start
interface for registering Fleet UI extensions (removes prior single-purpose method)closes #82478
Checklist
Delete any items that are not applicable to this PR.