-
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
[Logs+] Add title / icons to installed packages API #161777
[Logs+] Add title / icons to installed packages API #161777
Conversation
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
Pinging @elastic/fleet (Team:Fleet) |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
@elasticmachine merge upstream |
x-pack/plugins/discover_log_explorer/public/components/dataset_selector/utils.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/discover_log_explorer/public/components/dataset_selector/utils.tsx
Outdated
Show resolved
Hide resolved
Slightly more elegant for access. Shouldn't be less performant vs multiple finds.
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, thanks for the changes!
Slightly changes the types so the properties are optional vs just defining undefined in the union. This is mostly to appease fromSelection() as it tries to reconstruct from a partial integration payload. Once the hydration function is used (it's only call location) we should probably expand the payload for the integration properties. Also appeases storybook. # Please enter the commit message for your changes. Lines starting
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @Kerry350 |
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
Summary
Implements #160935.
This PR adds the
title
andicons
(alsodescription
since it was there) properties to the installed packages API, these are extracted from the matching manifest assets (so that we don't have to call the registry, and can query for multiple items at once). Discover has also been updated to use these new properties.List:
With selection:
I've made this robust against a matching manifest asset not being found - I don't think this is actually possible, but just incase.
Testing