-
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] Tabular UI for installed integrations #212582
[Fleet] Tabular UI for installed integrations #212582
Conversation
0e7a038
to
5783ba9
Compare
5783ba9
to
284b64f
Compare
…ed-integrations-tabular-ui
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
PR Cloud deployment started at: https://buildkite.com/elastic/kibana-deploy-cloud-from-pr/builds/89 |
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --include-path /api/dashboards --update'
Project deployed, see credentials at: https://buildkite.com/elastic/kibana-deploy-project-from-pr/builds/344 |
…m:nchaulet/kibana into feature-installed-integrations-tabular-ui
Pinging @elastic/fleet (Team:Fleet) |
@elasticmachine merge upstream |
...sections/epm/screens/installed_integrations/components/installed_integrations_search_bar.tsx
Outdated
Show resolved
Hide resolved
...ions/sections/epm/screens/installed_integrations/components/installed_integrations_table.tsx
Outdated
Show resolved
Hide resolved
...ntegrations/sections/epm/screens/installed_integrations/hooks/use_installed_integrations.tsx
Outdated
Show resolved
Hide resolved
...fleet/public/applications/integrations/sections/epm/screens/installed_integrations/types.tsx
Outdated
Show resolved
Hide resolved
const res = await getPackages({ | ||
savedObjectsClient, | ||
...request.query, | ||
}); | ||
const flattenedRes = res.map((pkg) => soToInstallationInfo(pkg)) as PackageList; | ||
|
||
if (request.query.withPackagePoliciesCount) { | ||
const countByPackage = await getPackagePoliciesCountByPackageName( |
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.
curious why not use packagePolicyService.list
instead of a new function?
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.
that function does an aggregation in elasticsearch
on package policy name as we do not need to return package policies, we could eventually update the list method to support passing aggregations but we will still probably need an extra function to abstract that aggregation wdyt?
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 also missed that getPackagePoliciesCountByPackageName
keys by package name, not that it returns a count based on a package name param (that's why I thought we could use packagePolicyService.list
kuery + total
). thanks for clarifying
…rations/sections/epm/screens/installed_integrations/components/installed_integrations_table.tsx Co-authored-by: Jen Huang <its.jenetic@gmail.com>
…rations/sections/epm/screens/installed_integrations/components/installed_integrations_search_bar.tsx Co-authored-by: Jen Huang <its.jenetic@gmail.com>
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
|
{ count_by_package_name: { buckets: Array<{ key: string; doc_count: number }> } } | ||
>({ | ||
type: savedObjectType, | ||
perPage: 0, |
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.
should we add size:0
since hits
are not used?
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 👍 I will make the change
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.
actually it's already done with the perPage: 0
it's a saved object client not an ES client
…ed-integrations-tabular-ui
label: ( | ||
<FormattedMessage | ||
id="xpack.fleet.epmInstalledIntegrations.upgradeAvailableFilterLabel" | ||
defaultMessage="Upgrade" |
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 Upgrade available
would be more expressive, though it might take up too much space.
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.
That a good point noting this to do a design review with christian when this is technically done.
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.
Thanks for the changes, LGTM 🚀
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
<EuiFlexItem grow={false}> | ||
<EuiFilterGroup> | ||
{statuses.map((item) => ( | ||
<EuiFilterButton |
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 could be extracted to a small component
Summary
Related to #209861
The feature is behind a new feature
installedIntegrationsTabularUI
Implementation details
All the packages are fetched in memory, and the pagination filtering is done client side, in the
useInstalledIntegrations
hook, that hook is unit tested.API Changes
To be able to populate the
View policies
count, I update the package API with a newwithPackagePoliciesCount
query parameter default to false, that populate an packagePolicyInfo property on the package result.UI Changes
Not Done in that PR
Todo