Create edit ILM flow#253393
Conversation
|
Pinging @elastic/kibana-management (Team:Kibana Management) |
MichaelMarcialis
left a comment
There was a problem hiding this comment.
@SoniaSanzV, @damian-polewski: Thanks so much for all your hard work that has culminated into this PR. It looks really great so far. I jotted down some comments below for some item to potentially address, but nothing huge. Please have a look and let me know if ya'll have any questions. Note that many of the comments didn't have a logical file for me to attach the comment to in this particular PR, so I just attached them to the first file in the list for easy of viewing/replying.
There was a problem hiding this comment.
In some instances, I was receiving a validation errors when one shouldn't exist. For example, in the attached screenshot, 3d is larger than 0ms, but it's shown as an error. Not entirely sure, but it feels like it might have something to do with the speed at which the user is entering a value in the number input.
There was a problem hiding this comment.
Hey @MichaelMarcialis, could you let me know what was the configuration of this policy?
Code Review SummaryThis is a well-structured PR that introduces the Edit ILM flow in the Streams' retention tab. I've reviewed the changes and have the following observations: Strengths
Minor Observations
Questions for the teamThe reviewer feedback from @MichaelMarcialis raises a good point about read-only access vs downsampling behavior differences between the Streams UI and ILM management UI. This should be clarified before merge to avoid user confusion. Overall, this is solid work that follows Kibana conventions and integrates well with the existing codebase. Reviewed by Cursor |
|
Hi @MichaelMarcialis, thank you for the feedback! Regarding your comments about the flyout I will address them in a separate issue we have for the flyout fixes (#252545). |
There was a problem hiding this comment.
@SoniaSanzV The info label looks good to me, we should probably have a link in there explaining what's needed to use downsampling (e.g. the default metrics-- don't have it, right?)
@damian-polewski Here is a recording for getting into the weird form state:
I think it makes sense, I opened an issue to address that as a follow up #254385 |
I was able to reproduce this issue. Addressed in c17063e. |
flash1293
left a comment
There was a problem hiding this comment.
Fix seems to work fine, LGTM from streams side.
Closes #252545 ## Summary This PR fixes issues with `EditIlmPhasesFlyout` and `EditDslStepsFlyout` components listed below: - (ILM) Hide the readonly field when the downsampling is enabled. - #253393 (comment) - (ILM) Fix the default values for min_age and fixed_interval in ILM flyout. - #253393 (comment) - (ILM/DSL) Add debounce to onChange. - #253393 (comment) - (ILM/DSL) Fix issue with Save button being enabled when errors are present. - #253393 (comment) - (ILM/DSL) Fix border color on tabs with errors. - #253393 (comment) - (ILM/DSL) Fix add button color to text. - #253393 (comment) - (ILM/DSL) Add a tooltip to Save button when it's disabled due to form errors. - #253393 (comment) - (ILM) Fix issue with min_age validation when clearing validated input - #253393 (review)
| const hotRolloverFromActions = getActionValue(phases.hot, 'rollover'); | ||
| const hotDownsampleFromActions = getActionValue(phases.hot, 'downsample'); | ||
| const warmDownsampleFromActions = getActionValue(phases.warm, 'downsample'); | ||
| const coldDownsampleFromActions = getActionValue(phases.cold, 'downsample'); | ||
| const coldSearchableSnapshotFromActions = getActionValue(phases.cold, 'searchable_snapshot'); | ||
| const frozenSearchableSnapshotFromActions = getActionValue( | ||
| phases.frozen, | ||
| 'searchable_snapshot' | ||
| ); | ||
| const deleteAction = getActionValue(phases.delete, 'delete'); | ||
| const hasHotReadonlyAction = getActionValue(phases.hot, 'readonly') !== undefined; | ||
| const hasWarmReadonlyAction = getActionValue(phases.warm, 'readonly') !== undefined; | ||
| const hasColdReadonlyAction = getActionValue(phases.cold, 'readonly') !== undefined; | ||
| const deleteSearchableSnapshotFromActions = asRecord(deleteAction) | ||
| .delete_searchable_snapshot as boolean | undefined; | ||
|
|
||
| const coldSearchableSnapshotRepository = | ||
| phases.cold?.searchable_snapshot ?? | ||
| (asRecord(coldSearchableSnapshotFromActions).snapshot_repository as string | undefined); | ||
| const frozenSearchableSnapshotRepository = | ||
| phases.frozen?.searchable_snapshot ?? | ||
| (asRecord(frozenSearchableSnapshotFromActions).snapshot_repository as string | undefined); | ||
|
|
There was a problem hiding this comment.
The reason additional logic was needed here to extract data from action object is because in the endpoint where we fetch policies we incorrectly cast the response as IlmPoliciesReponse which uses IlmPolicyPhases but in fact it's returning ES Ilm policy shape.
I think two solutions we can consider is either:
- Fix the type so the endpoint returns the ES ilm policy type and adjust the deserialize and flyout to expect this type,
- Normalize the response to
IlmPolicyResponseto keep the intended behavior.
There was a problem hiding this comment.
Also I think it's fine to handle this as a follow-up. Just so we don't block this PR further.
There was a problem hiding this comment.
Good call, @damian-polewski! I've addressed it in 7d3984a, let me know what you think!
There was a problem hiding this comment.
Looks good, thank you for addressing this!
damian-polewski
left a comment
There was a problem hiding this comment.
LGTM from kibana-management side.
💛 Build succeeded, but was flakyFailed CI StepsTest Failures
Metrics [docs]Module Count
Async chunks
Page load bundle
History
cc @SoniaSanzV |
…ps-config-rebase * commit 'f135f030951237c5e9b0251931441aee3121b31d': (163 commits) [CPS] Support data view requests and do not sanitize project_routing in data plugin/resolve indices (elastic#253654) [One Workflow] Execute workflow from historical (elastic#253396) [streams][background tasks] gracefully handle non existing stream (elastic#254683) [Lens API] Waffle/Mosaic get green as a default color (elastic#254304) [Security Solution] Remove prebuilt rules customization callout on Rule Management page (elastic#254386) [Workflows] support passing attachments to run_agent step (elastic#251291) [One Discover][Logs UX] Update OpenTelemetry Semantic Conventions (elastic#254367) [kbn-es] Add --docker flag to yarn es snapshot (elastic#254306) [Workplace AI] Remove Data Source Config (elastic#254521) [Entity Store v2] Add CRUD API (elastic#252052) [CI] Increase type checking machine (elastic#254676) [main] Sync bundled packages with Package Storage (elastic#254232) Skip flaky test elastic#254625 (elastic#254662) Upgrade `@elastic/elasticsearch` to 9.3.1 (elastic#253660) [One Workflow] Migrate http step to new connector (elastic#249004) [Entity Store] Store EUID Scripts (elastic#254515) [APM] Fix Otel missing fields undefined errors (elastic#254271) [Console] Add support for documentation links on Serverless (elastic#254489) Create edit ILM flow (elastic#253393) [Agent Builder] Mid term: minimal recommended model set elastic#12875 (elastic#254560) ...
Closes elastic#248676 ## Summary This PR introduces the Edit ILM flow in the Streams’ retention tab (lifecycle summary), covering edit for ILM phases and downsampling steps. It includes: * Make the Data lifecycle summary actionable: * Show an “Add data phase and downsampling” control (phase picker) when the user can manage lifecycle. * Display edit icons in the phase and downsampling step popovers (when the user can manage lifecycle). * When the user clicks Edit (phase or downsampling step): * Open a right-side “Edit data phases” flyout with phase tabs and validation/error indicators per tab. * While the flyout is open, clicking a phase/step in the summary navigates to the corresponding flyout tab (instead of opening popovers). * Track unsaved changes and show a navigation prompt if the user tries to leave with unsaved edits. * Add searchable snapshot repository support for the frozen phase: * Fetch and list snapshot repositories for selection in the flyout. * Gate adding/selecting Frozen unless repositories exist or the user has privileges to create one, with a link-out to Snapshot Repositories management. **Note**: When there is an error in the flyout, the data phases component is rendering the policy config, even if there is malformed. That would be handled in a follow up but we are still deciding the right approach: elastic#253387 ### How to test * Make sure the retention tab shows the Add phase control and edit/remove icons in the details pop-up. * Open the Edit data phases flyout and verify: * Switching tabs works and tab-level validation state is reflected (warnings on tabs with errors). * Clicking phases/steps in the summary while the flyout is open navigates within the flyout (no popovers). * Navigating away triggers an unsaved changes prompt if you’ve modified settings. * Verify policy impact modals: * If you edit/remove against a policy used by other indices/streams, you get the “in-use/managed” modal. * If you edit/remove against a managed policy, you get notified in a modal. * If you choose Overwrite, the existing policy is updated (and other users of that policy observe the change). * If you choose Save as new a new policy is created and assigned to the current stream. * If saving as new from a managed policy, ensure the new policy is not marked as managed. * Verify snapshot repository behavior: * Frozen is excluded/disabled when there are no repos and the user can’t create one. * If the user can create repos, verify the flyout offers a path to Snapshot Repositories management and refresh works after creating one. * Verify the behavior for Metrics Streams: * The data phases button should display `Add phases and downsampling steps` * Adding a downsampling step to a policy should no display a callout * You should see the downsampling bar * Verify the behavior for non metrics Streams: * The data phases button should display only `Add phases` * Adding a downsampling step to a policy should a any callout * You should not see the downsampling bar ### Demo https://github.com/user-attachments/assets/4a25ecc4-20d1-4a61-b251-e09f21c30f64 **metrics streams vs no metrics streams** https://github.com/user-attachments/assets/bb51e16e-cd7a-4f4d-8f29-bd4deea2c9c9 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
…ut (#254768) Closes #254385 ## Summary As requested in #253393 (review), adding a link to the [Downsampling concepts page](https://www.elastic.co/docs/manage-data/data-store/data-streams/downsampling-concepts ) from the callout we display when a user opens the data phases flyout from a non time series stream <img width="795" height="314" alt="Screenshot 2026-02-24 at 16 38 23" src="https://github.com/user-attachments/assets/797d3e09-554d-4f28-893d-568625a9c5d1" />
Closes #254188 Closes #254852 ## Summary This PR fixes issues with `EditIlmPhasesFlyout` and `EditDslStepsFlyout` components listed below: - (ILM/DSL) Remove overflow fade effect in tabs row, - (ILM/DSL) When different tab is selected externally, scroll to the newly selected tab, - (ILM/DSL) Update fixed_interval help text - #253393 (comment), - (ILM/DSL) Show help text when error is present - #253393 (comment). - (ILM/DSL) Update copy - #254852,
…c#254930) Closes elastic#254188 Closes elastic#254852 ## Summary This PR fixes issues with `EditIlmPhasesFlyout` and `EditDslStepsFlyout` components listed below: - (ILM/DSL) Remove overflow fade effect in tabs row, - (ILM/DSL) When different tab is selected externally, scroll to the newly selected tab, - (ILM/DSL) Update fixed_interval help text - elastic#253393 (comment), - (ILM/DSL) Show help text when error is present - elastic#253393 (comment). - (ILM/DSL) Update copy - elastic#254852,
…c#254930) Closes elastic#254188 Closes elastic#254852 ## Summary This PR fixes issues with `EditIlmPhasesFlyout` and `EditDslStepsFlyout` components listed below: - (ILM/DSL) Remove overflow fade effect in tabs row, - (ILM/DSL) When different tab is selected externally, scroll to the newly selected tab, - (ILM/DSL) Update fixed_interval help text - elastic#253393 (comment), - (ILM/DSL) Show help text when error is present - elastic#253393 (comment). - (ILM/DSL) Update copy - elastic#254852,
…c#254930) Closes elastic#254188 Closes elastic#254852 ## Summary This PR fixes issues with `EditIlmPhasesFlyout` and `EditDslStepsFlyout` components listed below: - (ILM/DSL) Remove overflow fade effect in tabs row, - (ILM/DSL) When different tab is selected externally, scroll to the newly selected tab, - (ILM/DSL) Update fixed_interval help text - elastic#253393 (comment), - (ILM/DSL) Show help text when error is present - elastic#253393 (comment). - (ILM/DSL) Update copy - elastic#254852,
Closes elastic#248676 ## Summary This PR introduces the Edit ILM flow in the Streams’ retention tab (lifecycle summary), covering edit for ILM phases and downsampling steps. It includes: * Make the Data lifecycle summary actionable: * Show an “Add data phase and downsampling” control (phase picker) when the user can manage lifecycle. * Display edit icons in the phase and downsampling step popovers (when the user can manage lifecycle). * When the user clicks Edit (phase or downsampling step): * Open a right-side “Edit data phases” flyout with phase tabs and validation/error indicators per tab. * While the flyout is open, clicking a phase/step in the summary navigates to the corresponding flyout tab (instead of opening popovers). * Track unsaved changes and show a navigation prompt if the user tries to leave with unsaved edits. * Add searchable snapshot repository support for the frozen phase: * Fetch and list snapshot repositories for selection in the flyout. * Gate adding/selecting Frozen unless repositories exist or the user has privileges to create one, with a link-out to Snapshot Repositories management. **Note**: When there is an error in the flyout, the data phases component is rendering the policy config, even if there is malformed. That would be handled in a follow up but we are still deciding the right approach: elastic#253387 ### How to test * Make sure the retention tab shows the Add phase control and edit/remove icons in the details pop-up. * Open the Edit data phases flyout and verify: * Switching tabs works and tab-level validation state is reflected (warnings on tabs with errors). * Clicking phases/steps in the summary while the flyout is open navigates within the flyout (no popovers). * Navigating away triggers an unsaved changes prompt if you’ve modified settings. * Verify policy impact modals: * If you edit/remove against a policy used by other indices/streams, you get the “in-use/managed” modal. * If you edit/remove against a managed policy, you get notified in a modal. * If you choose Overwrite, the existing policy is updated (and other users of that policy observe the change). * If you choose Save as new a new policy is created and assigned to the current stream. * If saving as new from a managed policy, ensure the new policy is not marked as managed. * Verify snapshot repository behavior: * Frozen is excluded/disabled when there are no repos and the user can’t create one. * If the user can create repos, verify the flyout offers a path to Snapshot Repositories management and refresh works after creating one. * Verify the behavior for Metrics Streams: * The data phases button should display `Add phases and downsampling steps` * Adding a downsampling step to a policy should no display a callout * You should see the downsampling bar * Verify the behavior for non metrics Streams: * The data phases button should display only `Add phases` * Adding a downsampling step to a policy should a any callout * You should not see the downsampling bar ### Demo https://github.com/user-attachments/assets/4a25ecc4-20d1-4a61-b251-e09f21c30f64 **metrics streams vs no metrics streams** https://github.com/user-attachments/assets/bb51e16e-cd7a-4f4d-8f29-bd4deea2c9c9 --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
…ut (elastic#254768) Closes elastic#254385 ## Summary As requested in elastic#253393 (review), adding a link to the [Downsampling concepts page](https://www.elastic.co/docs/manage-data/data-store/data-streams/downsampling-concepts ) from the callout we display when a user opens the data phases flyout from a non time series stream <img width="795" height="314" alt="Screenshot 2026-02-24 at 16 38 23" src="https://github.com/user-attachments/assets/797d3e09-554d-4f28-893d-568625a9c5d1" />
…c#254930) Closes elastic#254188 Closes elastic#254852 ## Summary This PR fixes issues with `EditIlmPhasesFlyout` and `EditDslStepsFlyout` components listed below: - (ILM/DSL) Remove overflow fade effect in tabs row, - (ILM/DSL) When different tab is selected externally, scroll to the newly selected tab, - (ILM/DSL) Update fixed_interval help text - elastic#253393 (comment), - (ILM/DSL) Show help text when error is present - elastic#253393 (comment). - (ILM/DSL) Update copy - elastic#254852,

Closes #248676
Summary
This PR introduces the Edit ILM flow in the Streams’ retention tab (lifecycle summary), covering edit for ILM phases and downsampling steps. It includes:
Note: When there is an error in the flyout, the data phases component is rendering the policy config, even if there is malformed. That would be handled in a follow up but we are still deciding the right approach: #253387
How to test
Add phases and downsampling stepsAdd phasesDemo
Screen.Recording.2026-02-17.at.10.18.09.mov
metrics streams vs no metrics streams
Screen.Recording.2026-02-20.at.11.36.58.mov