-
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
[ML] Fixing missing data visualizer links #103932
[ML] Fixing missing data visualizer links #103932
Conversation
@elasticmachine merge upstream |
Pinging @elastic/ml-ui (:ml) |
...sualizer/public/application/index_data_visualizer/components/actions_panel/actions_panel.tsx
Outdated
Show resolved
Hide resolved
...plugins/data_visualizer/public/application/common/components/results_links/results_links.tsx
Outdated
Show resolved
Hide resolved
Tested and LGTM 🎉 Running the flaky test runner suite for this PR ... |
@elasticmachine merge upstream |
const links: ResultLink[] = useMemo( | ||
() => [ | ||
{ | ||
id: 'create_ml_ad_job', |
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.
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.
If it isn't easy to detect the presence of a timestamp field, could we change the link to point to the start of the job wizard (select job type) page, as we do for the link on the File data visualizer?
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.
Fixed in 04879a8
Index data visualizer does not show the AD job link for non time series indices.
Also the file data vizualizer does not show the link if the imported index is not time based
💚 Build SucceededMetrics [docs]Public APIs missing comments
Any counts in public APIs
Async chunks
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.
Tested latest edits and LGTM
* [ML] Fixing missing data visualizer links * adding index dataviz links * fixing permission * re-enabling tests * fixing typo * adding check for non time based index * catching possible error when getting index pattern Co-authored-by: Kibana Machine <[email protected]>
* [ML] Fixing missing data visualizer links * adding index dataviz links * fixing permission * re-enabling tests * fixing typo * adding check for non time based index * catching possible error when getting index pattern Co-authored-by: Kibana Machine <[email protected]>
* [ML] Fixing missing data visualizer links * adding index dataviz links * fixing permission * re-enabling tests * fixing typo * adding check for non time based index * catching possible error when getting index pattern Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: James Gowdy <[email protected]>
* [ML] Fixing missing data visualizer links * adding index dataviz links * fixing permission * re-enabling tests * fixing typo * adding check for non time based index * catching possible error when getting index pattern Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: James Gowdy <[email protected]>
Adding back in the links to other pages which existed in the original File and Index data visualizers.
These were lost in the recent work to move the these component to separate plugins. (#96408 and #98939)
Note, these links are only present when using these features in ML.
File data visualizer
Index data visualizer
Differences in original versions
File data visualizer
The two added links are now at the end of the list of links, rather than in the middle
Index data visualizer
The
Create job
title has been removed, to add think back in we would need to categorise the links, possibly by supplying a common title in each link passed to the data visualizer component.This additional work seemed out of the scope of what was required. but could be added in if this title is deemed necessary.
The data recognizer cards have not been included. Adding these in requires a lot of additional work.
Original links for comparison:
Relates to #101435
Checklist
Delete any items that are not applicable to this PR.
Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
This was checked for breaking API changes and was labeled appropriately