[ML] Single Metric Viewer: Fix job check.#55191
Merged
walterra merged 6 commits intoelastic:masterfrom Jan 20, 2020
Merged
Conversation
Contributor
|
Pinging @elastic/ml-ui (:ml) |
21 tasks
alvarezmelissa87
approved these changes
Jan 17, 2020
Contributor
alvarezmelissa87
left a comment
There was a problem hiding this comment.
Tested and LGTM ⚡️
Contributor
Author
|
@elasticmachine merge upstream |
| <> | ||
| <NavigationMenu tabId="timeseriesexplorer" /> | ||
| {/* Show animated progress bar while loading */} | ||
| {loading === true && ( |
Member
There was a problem hiding this comment.
total nit, this could be a ternary
| setGlobalState('ml', { jobIds: [validSelectedJobIds[0]] }); | ||
| return true; | ||
| } else { | ||
| // if a group has been loaded |
Member
There was a problem hiding this comment.
is this else reachable?
Contributor
Author
There was a problem hiding this comment.
Good spot that must have been some quite old code I copied over. When picking a group to display in Anomaly Explorer, the job ids will also be added to the URL. Using the UI and coming to Single Metric Viewer, you cannot have a URL that just has groups but not jobs populated. I cleaned up the code in 6017f59.
Contributor
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
This was referenced Jan 20, 2020
walterra
added a commit
to walterra/kibana
that referenced
this pull request
Jan 20, 2020
Fixes switching via navigation to Single Metric Viewer from Anomaly Explorer for a job which isn't supported in the Single Metric Viewer.
walterra
added a commit
to walterra/kibana
that referenced
this pull request
Jan 20, 2020
Fixes switching via navigation to Single Metric Viewer from Anomaly Explorer for a job which isn't supported in the Single Metric Viewer.
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jan 20, 2020
* upstream/master: [ML] Single Metric Viewer: Fix job check. (elastic#55191)
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Jan 20, 2020
* master: (108 commits) [ML] Single Metric Viewer: Fix job check. (elastic#55191) Show error page when accessing unavailable app (elastic#54656) [ML] Improving job wizards with datafeed aggregations (elastic#55180) remove flaly assetion. a license presence tested anyway (elastic#55289) fix commonly used ranges uptime (elastic#54930) [SIEM] Use proper icons on Detections view (elastic#55215) Fix: invalid translation referenced (elastic#54901) [State Management] Remove AppState from edit_index_pattern page (elastic#54104) Implements `getStartServices` on server-side (elastic#55156) Move vis_vega_type/data_model tests to jest (elastic#55186) [SIEM] [Detection Engine] Update status on rule details page (elastic#55201) Fix KQL value suggestions for nested fields (elastic#54820) Enforce camelCase format for a plugin id (elastic#53759) [SIEM] Detection engine cleanup for rule details/creation/edit page (elastic#55069) Remove nested root from index pattern (elastic#54978) [Reporting/Migration] ReportingSetup, LegacySetup (elastic#54198) [SIEM] [Detection Engine] Fixes duplicate rule action (elastic#55252) [SIEM] Detections add alert & signal tab (elastic#55127) Management API - redirect on disabled app path (elastic#55136) [SIEM][Detection Engine] Fixes critical regression on the backend with immutable and tags ...
walterra
added a commit
that referenced
this pull request
Jan 20, 2020
walterra
added a commit
that referenced
this pull request
Jan 20, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part of #52986.
Fixes switching via navigation to Single Metric Viewer from Anomaly Explorer for a job which isn't supported in the Single Metric Viewer.
checkJobSelection()method fromtimeseriesexplorer.jsto a stand-alone function asvalidateJobSelection()intimeseriesexplorer_utils. This allows us to move the check to<TimeSeriesExplorerUrlStateManager />further up to this hook based outer component. Based on that we no longer need to pass on some of the stillundefinedprops to the inner components which should result in some less renders and stricter props with less optional attributes. With this in place we now pass on the current job asselectedJobIdtotimeseriesexplorer.jsand not the raw array we get fromglobalStateanymore.TimeSeriesExplorerPagecomponent formtimeseriesexplorer.jsto its own file to make it reusable.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsThis was checked for keyboard-only and screenreader accessibilityFor maintainers