[ML] [AIOps] Log Rate Analysis: Adds support to restore baseline/deviation from url state on page refresh #171398
Conversation
walterra
added
release_note:enhancement
:ml
v8.12.0
Feature:ML/AIOps
|
Pinging @elastic/ml-ui (:ml) |
walterra
requested review from
alvarezmelissa87
and removed request for
jgowdyelastic
| @@ -15,25 +15,13 @@ import { isPopulatedObject } from '@kbn/ml-is-populated-object'; | |||
| * @typedef {WindowParameters} | |||
| */ | |||
| export interface WindowParameters { | |||
| /** | |||
There was a problem hiding this comment.
Removed the JSDoc type annotations here because it's overly verbose and isn't needed to satisfy the docs script. There's also not really an IDE/Intellisense benefit when the file is TS anyway.
| */ | ||
|
|
||
| import { getDefaultAiOpsListState, type AiOpsFullIndexBasedAppState } from './common'; | ||
|
|
There was a problem hiding this comment.
This file was broke out from above application/url_state/common.ts
peteharverson
changed the title
| /** | ||
| * Transforms a full window parameters object to the abbreviated url state version. | ||
| */ | ||
| export const windowParameters2AppState = (wp?: WindowParameters): LogRateAnalysisAppState['wp'] => |
There was a problem hiding this comment.
Is it worth doing any basic validation of the parameters? For example, here I manually edited the URL so that the baseline / deviation were outside the page time range:
Admittedly this is an edge case, so maybe not worth the extra work?
There was a problem hiding this comment.
I documented this in a separate issue to follow up with: #171657
The problem here is that the full time range doesn't include any data, we should recognize that and not show the brushes and UI below at all since there will be no results anyway. So this can happen by updating the date picker too, it's sort of unrelated to the url state update.
| /** | ||
| * Transforms an abbreviated url state version of window parameters to its full version. | ||
| */ | ||
| export const appState2WindowParameters = ( |
There was a problem hiding this comment.
Nit: I think we should keep function names clear and consistent - imo it would be preferable to have a slightly longer function name appStateToWindowParameters than to introduce a number that has no meaning but to take the place of a word.
There was a problem hiding this comment.
Good suggestion, updated in 907d63b.
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: cc @walterra |
szabosteve
changed the title
Summary
Part of #146166.
Support to restore baseline/deviation time ranges from url state on full page refresh.
Also updates functional tests to include a full page refresh after the first analysis run for each dataset.
Checklist