diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.test.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.test.tsx index 82ef28ff30969..445f3a207e23b 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.test.tsx +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.test.tsx @@ -47,7 +47,7 @@ const renderFlyout = ( }; const onSelectedStepIndexChange = jest.fn(); - render( + const { unmount } = render( { act(() => { if (!setSelectedStepIndexRef.current) { @@ -115,6 +116,7 @@ const Wrapper = ({ onClose={onClose} onChange={onChange} onSave={onSave} + onChangeDebounceMs={0} {...props} /> @@ -276,6 +278,83 @@ describe('EditDslStepsFlyout', () => { expect(getTab(1).querySelector('[data-euiicon-type="warning"]')).not.toBeNull(); }); + it('keeps Save disabled when adding a step while an existing step has errors', async () => { + const { onSave } = renderFlyout({ + initialSteps: { + dsl: { + data_retention: '30d', + downsample: [{ after: '30d', fixed_interval: '1h' }], + }, + }, + }); + + await tick(); + + const panel = withinStep(0); + fireEvent.change(panel.getByTestId(`${DATA_TEST_SUBJ}FixedIntervalValue`), { + target: { value: '1.5' }, + }); + + // Trigger validation via submit attempt to ensure the form has recorded blocking errors. + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)); + await tick(); + expect(onSave).toHaveBeenCalledTimes(0); + expect(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)).toBeDisabled(); + + // Add another step while the first one is still invalid. + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}AddTabButton`)); + await waitFor(() => expect(getTab(2)).toBeInTheDocument()); + + // Save should remain disabled because there are still form errors. + await waitFor(() => expect(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)).toBeDisabled()); + }); + + it('re-enables Save after removing the invalid step', async () => { + const { onSave } = renderFlyout( + { + initialSteps: { + dsl: { + data_retention: '30d', + downsample: [ + { after: '30d', fixed_interval: '1h' }, + { after: '40d', fixed_interval: '5d' }, + ], + }, + }, + }, + { initialSelectedStepIndex: 1 } + ); + + await tick(); + + const invalidPanel = withinStep(1); + fireEvent.change(invalidPanel.getByTestId(`${DATA_TEST_SUBJ}FixedIntervalValue`), { + target: { value: '1.5' }, + }); + + // Trigger validation via submit attempt to ensure the form has recorded blocking errors. + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)); + await tick(); + expect(onSave).toHaveBeenCalledTimes(0); + expect(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)).toBeDisabled(); + + // Remove the invalid step -> save should become enabled again. + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}RemoveStepButton-step-2`)); + await waitFor(() => expect(queryTab(2)).not.toBeInTheDocument()); + await waitFor(() => + expect(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)).not.toBeDisabled() + ); + + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)); + await waitFor(() => expect(onSave).toHaveBeenCalledTimes(1)); + expect(onSave).toHaveBeenCalledWith({ + dsl: { + data_retention: '30d', + downsample: [{ after: '30d', fixed_interval: '1h' }], + }, + }); + }); + it('prevents saving when fixed_interval is less than 5 minutes', async () => { const { onSave } = renderFlyout({ initialSteps: { @@ -400,6 +479,45 @@ describe('EditDslStepsFlyout', () => { expect(panel.getByTestId(`${DATA_TEST_SUBJ}FixedIntervalValue`)).toHaveValue(3); expect(panel.getByTestId(`${DATA_TEST_SUBJ}FixedIntervalUnit`)).toHaveValue('h'); }); + + it('does not keep stale tab errors when removing an invalid middle step', async () => { + const { onSave } = renderFlyout( + { + initialSteps: { + dsl: { + data_retention: '30d', + downsample: [ + { after: '10d', fixed_interval: '1h' }, + { after: '20d', fixed_interval: '2h' }, + { after: '30d', fixed_interval: '3h' }, + ], + }, + }, + }, + { initialSelectedStepIndex: 1 } + ); + + await tick(); + + // Make step 2 invalid. + const step2Panel = withinStep(1); + fireEvent.change(step2Panel.getByTestId(`${DATA_TEST_SUBJ}FixedIntervalValue`), { + target: { value: '1.5' }, + }); + + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)); + await tick(); + expect(onSave).toHaveBeenCalledTimes(0); + expect(getTab(2).querySelector('[data-euiicon-type="warning"]')).not.toBeNull(); + + // Remove step 2; step 3 shifts to step 2 and should not keep the stale warning icon. + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}RemoveStepButton-step-2`)); + await waitFor(() => expect(queryTab(3)).not.toBeInTheDocument()); + + await waitFor(() => + expect(getTab(2).querySelector('[data-euiicon-type="warning"]')).toBeNull() + ); + }); }); describe('onChange emission', () => { @@ -438,5 +556,108 @@ describe('EditDslStepsFlyout', () => { }, }); }); + + it('debounces rapid user edits into a single onChange', async () => { + jest.useFakeTimers(); + const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout'); + try { + const onChange = jest.fn(); + renderFlyout({ + onChange, + onChangeDebounceMs: 100, + initialSteps: { + dsl: { + data_retention: '30d', + downsample: [{ after: '30d', fixed_interval: '1h' }], + }, + }, + }); + + // Flush initial mount work. + await act(async () => { + jest.runOnlyPendingTimers(); + }); + onChange.mockClear(); + clearTimeoutSpy.mockClear(); + + const panel = withinStep(0); + fireEvent.change(panel.getByTestId(`${DATA_TEST_SUBJ}FixedIntervalValue`), { + target: { value: '1' }, + }); + fireEvent.change(panel.getByTestId(`${DATA_TEST_SUBJ}FixedIntervalValue`), { + target: { value: '2' }, + }); + fireEvent.change(panel.getByTestId(`${DATA_TEST_SUBJ}FixedIntervalValue`), { + target: { value: '3' }, + }); + + expect(onChange).toHaveBeenCalledTimes(0); + + await act(async () => { + jest.advanceTimersByTime(99); + }); + expect(onChange).toHaveBeenCalledTimes(0); + + await act(async () => { + jest.advanceTimersByTime(1); + }); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenLastCalledWith({ + dsl: { + data_retention: '30d', + downsample: [{ after: '30d', fixed_interval: '3h' }], + }, + }); + + // Intermediate edits should clear the previous pending debounce timer. + expect(clearTimeoutSpy).toHaveBeenCalled(); + } finally { + clearTimeoutSpy.mockRestore(); + jest.useRealTimers(); + } + }); + + it('cleans up a pending debounced onChange on unmount', async () => { + jest.useFakeTimers(); + const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout'); + try { + const onChange = jest.fn(); + const { unmount } = renderFlyout({ + onChange, + onChangeDebounceMs: 100, + initialSteps: { + dsl: { + data_retention: '30d', + downsample: [{ after: '30d', fixed_interval: '1h' }], + }, + }, + }); + + await act(async () => { + jest.runOnlyPendingTimers(); + }); + onChange.mockClear(); + clearTimeoutSpy.mockClear(); + + const panel = withinStep(0); + fireEvent.change(panel.getByTestId(`${DATA_TEST_SUBJ}FixedIntervalValue`), { + target: { value: '2' }, + }); + + unmount(); + + await act(async () => { + jest.runOnlyPendingTimers(); + jest.advanceTimersByTime(100); + }); + + expect(onChange).toHaveBeenCalledTimes(0); + expect(clearTimeoutSpy).toHaveBeenCalled(); + } finally { + clearTimeoutSpy.mockRestore(); + jest.useRealTimers(); + } + }); }); }); diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.tsx index 75d478232a2ba..078ca31ce7f1c 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.tsx +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.tsx @@ -14,6 +14,7 @@ import { EuiButtonEmpty, EuiFlexGroup, EuiFlexItem, + EuiToolTip, } from '@elastic/eui'; import type { IngestStreamLifecycleDSL } from '@kbn/streams-schema'; import { @@ -34,17 +35,7 @@ import { } from './form'; import { DslStepsFlyoutArrayView } from './sections'; import { useStyles } from './use_styles'; - -export interface EditDslStepsFlyoutProps { - initialSteps: IngestStreamLifecycleDSL; - selectedStepIndex: number | undefined; - setSelectedStepIndex: (index: number | undefined) => void; - onChange: (next: IngestStreamLifecycleDSL) => void; - onSave: (next: IngestStreamLifecycleDSL) => void; - onClose: () => void; - isSaving?: boolean; - 'data-test-subj'?: string; -} +import type { EditDslStepsFlyoutProps } from './types'; const FragmentFormWrapper = ({ children }: React.PropsWithChildren) => <>{children}; @@ -58,6 +49,7 @@ export const EditDslStepsFlyout = ({ onChange, onSave, onClose, + onChangeDebounceMs = 250, isSaving, 'data-test-subj': dataTestSubjProp, }: EditDslStepsFlyoutProps) => { @@ -97,7 +89,8 @@ export const EditDslStepsFlyout = ({ const pendingOnChangeOutputRef = useRef(null); const pendingOnChangeTimeoutRef = useRef | null>(null); - const { onStepFieldErrorsChange, tabHasErrors, pruneToStepPaths } = useDslStepsFlyoutTabErrors(); + const { onStepFieldErrorsChange, tabHasErrors, pruneToStepPaths, reindexErrorsAfterRemoval } = + useDslStepsFlyoutTabErrors(); useEffect(() => { const sub = form.subscribe(({ data }) => { @@ -138,7 +131,7 @@ export const EditDslStepsFlyout = ({ if (isEqual(toEmit, lastEmittedOutputRef.current)) return; lastEmittedOutputRef.current = toEmit; onChangeRef.current(toEmit); - }, 0); + }, onChangeDebounceMs); }); return () => { @@ -149,7 +142,39 @@ export const EditDslStepsFlyout = ({ pendingOnChangeOutputRef.current = null; sub.unsubscribe(); }; - }, [form]); + }, [form, onChangeDebounceMs]); + + const hasFormErrors = form.getErrors().length > 0; + const isSaveDisabledDueToInvalid = form.isValid === false || hasFormErrors; + const isSaveDisabled = isSaveDisabledDueToInvalid || form.isSubmitting; + + const renderSaveButton = () => { + const button = ( + form.submit()} + disabled={isSaveDisabled} + > + {i18n.translate('xpack.streams.editDslStepsFlyout.save', { + defaultMessage: 'Save', + })} + + ); + + return isSaveDisabledDueToInvalid ? ( + + {button} + + ) : ( + button + ); + }; return ( )} @@ -204,19 +230,7 @@ export const EditDslStepsFlyout = ({ })} - - form.submit()} - disabled={(form.isSubmitted && form.isValid === false) || form.isSubmitting} - > - {i18n.translate('xpack.streams.editDslStepsFlyout.save', { - defaultMessage: 'Save', - })} - - + {renderSaveButton()} diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/form/error_tracking.test.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/form/error_tracking.test.tsx new file mode 100644 index 0000000000000..1f74aceebb839 --- /dev/null +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/form/error_tracking.test.tsx @@ -0,0 +1,237 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; +import { act, renderHook } from '@testing-library/react'; +import { + OnStepFieldErrorsChangeProvider, + useDslStepsFlyoutTabErrors, + useOnStepFieldErrorsChange, +} from './error_tracking'; + +const stepPath = (index: number) => `_meta.downsampleSteps[${index}]`; + +describe('useDslStepsFlyoutTabErrors', () => { + const useHook = () => { + const renderCountRef = React.useRef(0); + renderCountRef.current += 1; + return { ...useDslStepsFlyoutTabErrors(), renderCount: renderCountRef.current }; + }; + + it('reports no errors by default', () => { + const { result } = renderHook(() => useHook()); + expect(result.current.tabHasErrors(stepPath(0))).toBe(false); + expect(result.current.tabHasErrors(stepPath(1))).toBe(false); + }); + + it('tracks after and fixed_interval errors per step', () => { + const { result } = renderHook(() => useHook()); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'after', ['required']); + }); + expect(result.current.tabHasErrors(stepPath(0))).toBe(true); + expect(result.current.tabHasErrors(stepPath(1))).toBe(false); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'after', null); + }); + expect(result.current.tabHasErrors(stepPath(0))).toBe(false); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(1), 'fixed_interval', ['integer']); + }); + expect(result.current.tabHasErrors(stepPath(0))).toBe(false); + expect(result.current.tabHasErrors(stepPath(1))).toBe(true); + }); + + it('does not re-render when setting the same errors (isEqual shortcut)', () => { + const { result } = renderHook(() => useHook()); + expect(result.current.renderCount).toBe(1); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'fixed_interval', ['integer']); + }); + expect(result.current.renderCount).toBe(2); + + // Same content (different reference) should not trigger an update. + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'fixed_interval', ['integer']); + }); + expect(result.current.renderCount).toBe(2); + }); + + it('prunes errors to the provided step paths', () => { + const { result } = renderHook(() => useHook()); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'after', ['required']); + result.current.onStepFieldErrorsChange(stepPath(1), 'fixed_interval', ['integer']); + result.current.onStepFieldErrorsChange(stepPath(2), 'after', ['required']); + }); + + expect(result.current.tabHasErrors(stepPath(0))).toBe(true); + expect(result.current.tabHasErrors(stepPath(1))).toBe(true); + expect(result.current.tabHasErrors(stepPath(2))).toBe(true); + + act(() => { + result.current.pruneToStepPaths([stepPath(0), stepPath(2)]); + }); + + expect(result.current.tabHasErrors(stepPath(0))).toBe(true); + expect(result.current.tabHasErrors(stepPath(1))).toBe(false); + expect(result.current.tabHasErrors(stepPath(2))).toBe(true); + }); + + it('does not re-render when pruning to the same set of step paths', () => { + const { result } = renderHook(() => useHook()); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'after', ['required']); + result.current.onStepFieldErrorsChange(stepPath(2), 'after', ['required']); + }); + + const rendersBefore = result.current.renderCount; + act(() => { + result.current.pruneToStepPaths([stepPath(0), stepPath(2)]); + }); + expect(result.current.renderCount).toBe(rendersBefore); + }); + + it('reindexes errors after removing a middle step (drops removed, shifts higher indices down)', () => { + const { result } = renderHook(() => useHook()); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'after', ['e0']); + result.current.onStepFieldErrorsChange(stepPath(1), 'fixed_interval', ['e1']); + result.current.onStepFieldErrorsChange(stepPath(2), 'after', ['e2']); + }); + + expect(result.current.tabHasErrors(stepPath(0))).toBe(true); + expect(result.current.tabHasErrors(stepPath(1))).toBe(true); + expect(result.current.tabHasErrors(stepPath(2))).toBe(true); + + act(() => { + result.current.reindexErrorsAfterRemoval(1); + }); + + // step 0 unchanged + expect(result.current.tabHasErrors(stepPath(0))).toBe(true); + // old step 1 was removed => no longer has errors + // old step 2 shifted to step 1 => still has errors + expect(result.current.tabHasErrors(stepPath(1))).toBe(true); + // old step 2 moved, so step 2 is now empty in the error map + expect(result.current.tabHasErrors(stepPath(2))).toBe(false); + }); + + it('does not keep removed-step field errors when reindexing', () => { + const { result } = renderHook(() => useHook()); + + act(() => { + // Removed step has a fixed_interval error. + result.current.onStepFieldErrorsChange(stepPath(1), 'fixed_interval', ['integer']); + // Next step has an after error (this should shift down). + result.current.onStepFieldErrorsChange(stepPath(2), 'after', ['required']); + }); + + act(() => { + result.current.reindexErrorsAfterRemoval(1); + }); + + // New step 1 should only have the shifted "after" error. + expect(result.current.tabHasErrors(stepPath(1))).toBe(true); + + act(() => { + // Clearing the shifted "after" error should clear all errors for step 1. + result.current.onStepFieldErrorsChange(stepPath(1), 'after', null); + }); + expect(result.current.tabHasErrors(stepPath(1))).toBe(false); + }); + + it('reindexes errors after removing the first step (shifts everything down)', () => { + const { result } = renderHook(() => useHook()); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'after', ['e0']); + result.current.onStepFieldErrorsChange(stepPath(1), 'fixed_interval', ['e1']); + }); + + act(() => { + result.current.reindexErrorsAfterRemoval(0); + }); + + // old step 1 -> new step 0 + expect(result.current.tabHasErrors(stepPath(0))).toBe(true); + // old step 0 removed and old step 1 shifted => step 1 should be empty + expect(result.current.tabHasErrors(stepPath(1))).toBe(false); + }); + + it('does nothing for negative removed indices', () => { + const { result } = renderHook(() => useHook()); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'after', ['e0']); + }); + expect(result.current.tabHasErrors(stepPath(0))).toBe(true); + + const rendersBefore = result.current.renderCount; + act(() => { + result.current.reindexErrorsAfterRemoval(-1); + }); + + expect(result.current.renderCount).toBe(rendersBefore); + expect(result.current.tabHasErrors(stepPath(0))).toBe(true); + }); + + it('does nothing when reindexing a removed index beyond existing step paths', () => { + const { result } = renderHook(() => useHook()); + + act(() => { + result.current.onStepFieldErrorsChange(stepPath(0), 'after', ['e0']); + }); + + const rendersBefore = result.current.renderCount; + act(() => { + result.current.reindexErrorsAfterRemoval(10); + }); + + expect(result.current.renderCount).toBe(rendersBefore); + expect(result.current.tabHasErrors(stepPath(0))).toBe(true); + }); + + it('preserves errors for non-step paths when reindexing', () => { + const { result } = renderHook(() => useHook()); + const unknownPath = '_meta.someOtherThing'; + + act(() => { + result.current.onStepFieldErrorsChange(unknownPath, 'after', ['x']); + result.current.onStepFieldErrorsChange(stepPath(1), 'fixed_interval', ['e1']); + }); + + expect(result.current.tabHasErrors(unknownPath)).toBe(true); + expect(result.current.tabHasErrors(stepPath(1))).toBe(true); + + act(() => { + result.current.reindexErrorsAfterRemoval(0); + }); + + expect(result.current.tabHasErrors(unknownPath)).toBe(true); + }); +}); + +describe('OnStepFieldErrorsChangeProvider / useOnStepFieldErrorsChange', () => { + it('provides the callback via context', () => { + const callback = jest.fn(); + + const wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} + ); + + const { result } = renderHook(() => useOnStepFieldErrorsChange(), { wrapper }); + expect(result.current).toBe(callback); + }); +}); diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/form/error_tracking.ts b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/form/error_tracking.ts index 6d4fd953757d2..be624bc9a62bd 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/form/error_tracking.ts +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/form/error_tracking.ts @@ -7,6 +7,7 @@ import React, { createContext, useCallback, useContext, useState } from 'react'; import { isEqual } from 'lodash'; +import { getStepIndexFromArrayItemPath } from './utils'; export type StepFieldKey = 'after' | 'fixed_interval'; export type OnStepFieldErrorsChange = ( @@ -66,5 +67,37 @@ export const useDslStepsFlyoutTabErrors = () => { }); }, []); - return { onStepFieldErrorsChange, tabHasErrors, pruneToStepPaths }; + const reindexErrorsAfterRemoval = useCallback((removedIndex: number) => { + setErrorsByStepPath((prev) => { + if (!Number.isFinite(removedIndex) || removedIndex < 0) return prev; + + const next: typeof prev = {}; + + for (const [stepPath, stepErrors] of Object.entries(prev)) { + const index = getStepIndexFromArrayItemPath(stepPath); + if (index < 0) { + next[stepPath] = stepErrors; + continue; + } + + if (index < removedIndex) { + next[stepPath] = stepErrors; + continue; + } + + if (index === removedIndex) { + // Removed step - drop its errors. + continue; + } + + // Steps after the removed index shift down by 1. + const nextPath = `_meta.downsampleSteps[${index - 1}]`; + next[nextPath] = stepErrors; + } + + return isEqual(prev, next) ? prev : next; + }); + }, []); + + return { onStepFieldErrorsChange, tabHasErrors, pruneToStepPaths, reindexErrorsAfterRemoval }; }; diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/index.ts b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/index.ts new file mode 100644 index 0000000000000..a4db2f85dd5dc --- /dev/null +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/index.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export { EditDslStepsFlyout } from './edit_dsl_steps_flyout'; +export type { EditDslStepsFlyoutProps } from './types'; diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/sections/dsl_steps_flyout_array_view.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/sections/dsl_steps_flyout_array_view.tsx index 63fb4c4f0573e..797dcfde6eabf 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/sections/dsl_steps_flyout_array_view.tsx +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/sections/dsl_steps_flyout_array_view.tsx @@ -24,8 +24,8 @@ import { type DslStepMetaFields, type DslStepsFlyoutFormInternal, type PreservedTimeUnit, - toMilliseconds, } from '../form'; +import { getDoubledDurationFromPrevious, toMilliseconds } from '../../shared'; import { TIME_UNIT_OPTIONS } from '../constants'; import { useStyles } from '../use_styles'; import { StepPanel } from './step_panel'; @@ -47,6 +47,7 @@ export interface DslStepsFlyoutArrayViewProps { setSelectedStepIndex: (index: number | undefined) => void; tabHasErrors: (stepPath: string) => boolean; pruneToStepPaths: (stepPaths: string[]) => void; + reindexErrorsAfterRemoval: (removedIndex: number) => void; } export const DslStepsFlyoutArrayView = ({ @@ -57,6 +58,7 @@ export const DslStepsFlyoutArrayView = ({ setSelectedStepIndex, tabHasErrors, pruneToStepPaths, + reindexErrorsAfterRemoval, }: DslStepsFlyoutArrayViewProps) => { const { items, form } = arrayField; const { sectionStyles, headerStyles, headerNoStepsStyles } = useStyles(); @@ -102,31 +104,34 @@ export const DslStepsFlyoutArrayView = ({ (previousStep?: DslStepMetaFields): DslStepMetaFields => { if (previousStep === undefined) { return { - afterValue: '', + afterValue: '1', afterUnit: 'd', - afterToMilliSeconds: -1, + afterToMilliSeconds: toMilliseconds('1', 'd'), fixedIntervalValue: '1', fixedIntervalUnit: 'd', }; } const previousAfterUnit = previousStep.afterUnit ?? 'd'; - const previousAfterNum = Number(previousStep.afterValue); - const safePreviousAfterNum = - Number.isFinite(previousAfterNum) && previousAfterNum >= 0 ? previousAfterNum : 0; - const nextAfterValue = String(safePreviousAfterNum * 2); - const nextAfterMs = toMilliseconds(nextAfterValue, previousAfterUnit); + const { value: nextAfterValue, ms: nextAfterMs } = getDoubledDurationFromPrevious({ + previousValue: previousStep.afterValue, + previousUnit: previousAfterUnit, + previousValueFallback: 0, + previousValueMinInclusive: 0, + }); const previousFixedUnit = previousStep.fixedIntervalUnit ?? 'd'; - const previousFixedNum = Number(previousStep.fixedIntervalValue); - const safePreviousFixedNum = - Number.isFinite(previousFixedNum) && previousFixedNum > 0 ? previousFixedNum : 1; - const nextFixedIntervalValue = String(safePreviousFixedNum * 2); + const { value: nextFixedIntervalValue } = getDoubledDurationFromPrevious({ + previousValue: previousStep.fixedIntervalValue, + previousUnit: previousFixedUnit, + previousValueFallback: 1, + previousValueMinExclusive: 0, + }); return { afterValue: nextAfterValue, afterUnit: previousAfterUnit, - afterToMilliSeconds: Number.isFinite(nextAfterMs) ? nextAfterMs : -1, + afterToMilliSeconds: nextAfterMs, fixedIntervalValue: nextFixedIntervalValue, fixedIntervalUnit: previousFixedUnit, }; @@ -156,6 +161,7 @@ export const DslStepsFlyoutArrayView = ({ _meta: { __dslStepsFlyout: true, downsampleSteps: nextSteps }, }; form.updateFieldValues(payload, { runDeserializer: false }); + void form.validate(); }, [ clampStepIndex, @@ -256,6 +262,7 @@ export const DslStepsFlyoutArrayView = ({ _meta: { __dslStepsFlyout: true, downsampleSteps: nextSteps }, }; form.updateFieldValues(payload, { runDeserializer: false }); + void form.validate(); setSelectedStepIndex(nextIndex); }, [createNextStepFromPrevious, form, getCurrentSteps, items.length, setSelectedStepIndex]); @@ -263,11 +270,14 @@ export const DslStepsFlyoutArrayView = ({ (stepIndex: number) => { const oldLength = items.length; + reindexErrorsAfterRemoval(stepIndex); + const nextSteps = getCurrentSteps().filter((_, index) => index !== stepIndex); const payload: DslStepsFlyoutFormFieldUpdate = { _meta: { __dslStepsFlyout: true, downsampleSteps: nextSteps }, }; form.updateFieldValues(payload, { runDeserializer: false }); + void form.validate(); if (selectedStepIndex === undefined) return; const newLength = oldLength - 1; @@ -285,7 +295,14 @@ export const DslStepsFlyoutArrayView = ({ setSelectedStepIndex(Math.min(selectedStepIndex, newLength - 1)); } }, - [form, getCurrentSteps, items.length, selectedStepIndex, setSelectedStepIndex] + [ + form, + getCurrentSteps, + items.length, + reindexErrorsAfterRemoval, + selectedStepIndex, + setSelectedStepIndex, + ] ); return ( diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/sections/step_tabs_row.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/sections/step_tabs_row.tsx index a4615359b4c6a..6f43b38596423 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/sections/step_tabs_row.tsx +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/sections/step_tabs_row.tsx @@ -43,7 +43,7 @@ export const StepTabsRow = ({ dataTestSubj, }: StepTabsRowProps) => { const tabsScrollCss = useEuiOverflowScroll('x', true); - const { tabsContainerStyles } = useStyles(); + const { tabsContainerStyles, tabsErrorSelectedUnderlineStyles } = useStyles(); const tabs = useMemo(() => { return items.map((item, index) => { @@ -58,8 +58,24 @@ export const StepTabsRow = ({ key={item.id} onClick={() => setSelectedStepIndex(index)} isSelected={index === selectedStepIndex} + className={hasErrors ? 'streamsDslStepsTab--hasErrors' : undefined} data-test-subj={`${dataTestSubj}Tab-step-${index + 1}`} - prepend={hasErrors ? : undefined} + prepend={ + hasErrors ? ( + + ) : undefined + } > {hasErrors ? {label} : label} @@ -75,7 +91,7 @@ export const StepTabsRow = ({ defaultMessage: 'Add downsampling step', })} size="xs" - color="primary" + color="text" data-test-subj={`${dataTestSubj}AddTabButton`} onClick={onAddStep} disabled={isAddDisabled} @@ -99,7 +115,10 @@ export const StepTabsRow = ({ return ( - + {tabs} {renderAddButton()} diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/types.ts b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/types.ts new file mode 100644 index 0000000000000..2f38c62c253c3 --- /dev/null +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/types.ts @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { IngestStreamLifecycleDSL } from '@kbn/streams-schema'; + +export interface EditDslStepsFlyoutProps { + initialSteps: IngestStreamLifecycleDSL; + selectedStepIndex: number | undefined; + setSelectedStepIndex: (index: number | undefined) => void; + onChange: (next: IngestStreamLifecycleDSL) => void; + onSave: (next: IngestStreamLifecycleDSL) => void; + onClose: () => void; + onChangeDebounceMs?: number; + isSaving?: boolean; + 'data-test-subj'?: string; +} diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/use_styles.ts b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/use_styles.ts index 88fbc2dda75c1..e65abe7004f94 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/use_styles.ts +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/use_styles.ts @@ -35,6 +35,19 @@ export const useStyles = () => { min-width: 0; `; - return { sectionStyles, headerStyles, headerNoStepsStyles, footerStyles, tabsContainerStyles }; - }, [euiTheme.size.l, euiTheme.size.m]); + const tabsErrorSelectedUnderlineStyles = css` + .streamsDslStepsTab--hasErrors.euiTab-isSelected::after { + border-color: ${euiTheme.colors.danger}; + } + `; + + return { + sectionStyles, + headerStyles, + headerNoStepsStyles, + footerStyles, + tabsContainerStyles, + tabsErrorSelectedUnderlineStyles, + }; + }, [euiTheme.colors.danger, euiTheme.size.l, euiTheme.size.m]); }; diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/constants.ts b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/constants.ts index a706bac06699c..fbf19ef041432 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/constants.ts +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/constants.ts @@ -42,19 +42,12 @@ export const PHASE_MOUNT_PATHS: Record> = { '_meta.hot.enabled', '_meta.hot.sizeInBytes', '_meta.hot.rollover', - '_meta.hot.readonlyEnabled', '_meta.hot.downsampleEnabled', ], - warm: [ - '_meta.warm.enabled', - '_meta.warm.sizeInBytes', - '_meta.warm.readonlyEnabled', - '_meta.warm.downsampleEnabled', - ], + warm: ['_meta.warm.enabled', '_meta.warm.sizeInBytes', '_meta.warm.downsampleEnabled'], cold: [ '_meta.cold.enabled', '_meta.cold.sizeInBytes', - '_meta.cold.readonlyEnabled', '_meta.cold.downsampleEnabled', '_meta.cold.searchableSnapshotEnabled', ], diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/edit_ilm_phases_flyout.test.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/edit_ilm_phases_flyout.test.tsx index b7f5048d9ef53..fb93ee45e66ee 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/edit_ilm_phases_flyout.test.tsx +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/edit_ilm_phases_flyout.test.tsx @@ -46,15 +46,7 @@ const tick = async () => { const getTab = (phase: string) => screen.getByTestId(`${DATA_TEST_SUBJ}Tab-${phase}`); const queryTab = (phase: string) => screen.queryByTestId(`${DATA_TEST_SUBJ}Tab-${phase}`); const getPanel = (phase: string) => screen.getByTestId(`${DATA_TEST_SUBJ}Panel-${phase}`); -const getPhaseContainer = (phase: string) => { - const panel = getPanel(phase); - const container = panel.parentElement; - if (!container) { - throw new Error(`Could not find phase container for "${phase}"`); - } - return container as HTMLElement; -}; -const withinPhase = (phase: string) => within(getPhaseContainer(phase)); +const withinPhase = (phase: string) => within(getPanel(phase)); const renderFlyout = ( props: Partial> = {}, @@ -94,13 +86,14 @@ const renderFlyout = ( onClose={onClose} onChange={onChange} onSave={onSave} + onChangeDebounceMs={0} {...props} /> ); }; - render(); + const { unmount } = render(); return { onClose, @@ -108,6 +101,7 @@ const renderFlyout = ( onSave, initialPhases, onSelectedPhaseChange, + unmount, setSelectedPhase: (phase: PhaseName | undefined) => { act(() => { if (!setSelectedPhaseRef.current) { @@ -247,6 +241,74 @@ describe('EditIlmPhasesFlyout', () => { }) ); }); + + it('defaults a newly-enabled phase min_age to 2x the closest enabled previous phase', async () => { + renderFlyout({ + initialPhases: { + hot: { name: 'hot', size_in_bytes: 0, rollover: {} }, + warm: { name: 'warm', size_in_bytes: 0, min_age: '30d' }, + }, + }); + + await tick(); + + // Mocked IlmPhaseSelect adds the cold phase. + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}AddTabButton`)); + await waitFor(() => expect(getTab('cold')).toBeInTheDocument()); + + const coldPanel = withinPhase('cold'); + const valueInput = coldPanel.getByTestId( + `${DATA_TEST_SUBJ}MoveAfterValue` + ) as HTMLInputElement; + const unitSelect = coldPanel.getByTestId( + `${DATA_TEST_SUBJ}MoveAfterUnit` + ) as HTMLSelectElement; + + expect(valueInput.value).toBe('60'); + expect(unitSelect.value).toBe('d'); + }); + + it('prevents saving when cold min_age is cleared and clears the required error when value is set again', async () => { + const onSave = jest.fn(); + renderFlyout( + { + initialPhases: { + hot: { name: 'hot', size_in_bytes: 0, rollover: {} }, + warm: { name: 'warm', size_in_bytes: 0, min_age: '30d' }, + cold: { name: 'cold', size_in_bytes: 0, min_age: '40d' }, + frozen: { + name: 'frozen', + size_in_bytes: 0, + min_age: '50d', + searchable_snapshot: 'repo', + }, + }, + onSave, + }, + { initialSelectedPhase: 'cold' } + ); + + await tick(); + + const coldPanel = withinPhase('cold'); + fireEvent.change(coldPanel.getByTestId(`${DATA_TEST_SUBJ}MoveAfterValue`), { + target: { value: '' }, + }); + + await waitFor(() => expect(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)).toBeDisabled()); + + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)); + expect(onSave).toHaveBeenCalledTimes(0); + + // Set a valid value again -> save should be enabled. + fireEvent.change(coldPanel.getByTestId(`${DATA_TEST_SUBJ}MoveAfterValue`), { + target: { value: '41' }, + }); + + await waitFor(() => + expect(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)).not.toBeDisabled() + ); + }); }); describe('downsampling', () => { @@ -281,6 +343,152 @@ describe('EditIlmPhasesFlyout', () => { expect(warmPanel.getByTestId(`${DATA_TEST_SUBJ}DownsamplingIntervalValue`)).toBeVisible(); }); + + it('defaults warm downsample interval to 2x the previous enabled downsample interval', async () => { + const onChange = jest.fn(); + renderFlyout({ + initialPhases: { + hot: { + name: 'hot', + size_in_bytes: 0, + rollover: {}, + downsample: { after: '0ms', fixed_interval: '1d' }, + }, + warm: { name: 'warm', size_in_bytes: 0, min_age: '30d' }, + }, + onChange, + }); + + await tick(); + fireEvent.click(getTab('warm')); + + const warmPanel = withinPhase('warm'); + fireEvent.click(warmPanel.getByTestId(`${DATA_TEST_SUBJ}DownsamplingSwitch`)); + + await waitFor(() => + expect(onChange).toHaveBeenLastCalledWith({ + hot: { + name: 'hot', + size_in_bytes: 0, + rollover: {}, + downsample: { after: '0ms', fixed_interval: '1d' }, + }, + warm: { + name: 'warm', + size_in_bytes: 0, + min_age: '30d', + downsample: { after: '30d', fixed_interval: '2d' }, + }, + }) + ); + }); + + it('hides and unmounts readonly while downsampling is enabled, and re-adds it when disabled', async () => { + const onChange = jest.fn(); + renderFlyout( + { + initialPhases: { + hot: { name: 'hot', size_in_bytes: 0, rollover: {} }, + warm: { name: 'warm', size_in_bytes: 0, min_age: '30d' }, + }, + onChange, + }, + { initialSelectedPhase: 'warm' } + ); + + await tick(); + const warmPanel = withinPhase('warm'); + + // Initially visible. + expect(warmPanel.getByTestId(`${DATA_TEST_SUBJ}ReadOnlyCheckbox`)).toBeInTheDocument(); + + // Set it to true. + fireEvent.click(warmPanel.getByTestId(`${DATA_TEST_SUBJ}ReadOnlyCheckbox`)); + await tick(); + + // Enable downsampling -> readonly should be cleared and hidden. + fireEvent.click(warmPanel.getByTestId(`${DATA_TEST_SUBJ}DownsamplingSwitch`)); + await tick(); + + expect(warmPanel.queryByTestId(`${DATA_TEST_SUBJ}ReadOnlyCheckbox`)).not.toBeInTheDocument(); + + // Ensure output does not include warm.readonly even if it was previously enabled. + expect(onChange).toHaveBeenLastCalledWith({ + hot: { name: 'hot', size_in_bytes: 0, rollover: {} }, + warm: { + name: 'warm', + size_in_bytes: 0, + min_age: '30d', + downsample: { after: '30d', fixed_interval: '1d' }, + }, + }); + + // Disable downsampling -> readonly should re-appear (re-mounted). + fireEvent.click(warmPanel.getByTestId(`${DATA_TEST_SUBJ}DownsamplingSwitch`)); + await tick(); + + const checkbox = warmPanel.getByTestId(`${DATA_TEST_SUBJ}ReadOnlyCheckbox`); + expect(checkbox).toBeInTheDocument(); + expect(checkbox).not.toBeChecked(); + }); + + it('revalidates cold downsampling interval when re-enabling cold (warm interval changed while cold disabled)', async () => { + const onSave = jest.fn(); + + const { setSelectedPhase } = renderFlyout( + { + initialPhases: { + delete: { name: 'delete', min_age: '60d' }, + }, + onSave, + }, + { initialSelectedPhase: 'delete' } + ); + + await tick(); + + // 1. Add hot and enable downsampling (default 1d). + setSelectedPhase('hot'); + await tick(); + fireEvent.click(withinPhase('hot').getByTestId(`${DATA_TEST_SUBJ}DownsamplingSwitch`)); + await tick(); + + // 2. Add warm and enable downsampling (defaults to 2d). + setSelectedPhase('warm'); + await tick(); + fireEvent.click(withinPhase('warm').getByTestId(`${DATA_TEST_SUBJ}DownsamplingSwitch`)); + await tick(); + + // 3. Add cold and enable downsampling (defaults to 4d). + setSelectedPhase('cold'); + await tick(); + fireEvent.click(withinPhase('cold').getByTestId(`${DATA_TEST_SUBJ}DownsamplingSwitch`)); + await tick(); + + // 4. Remove cold phase. + fireEvent.click(withinPhase('cold').getByTestId(`${DATA_TEST_SUBJ}RemoveItemButton`)); + await tick(); + + // 5. Change warm fixed_interval to 30d (while cold is disabled). + setSelectedPhase('warm'); + await tick(); + fireEvent.change( + withinPhase('warm').getByTestId(`${DATA_TEST_SUBJ}DownsamplingIntervalValue`), + { + target: { value: '30' }, + } + ); + await tick(); + + // 6. Re-enable cold: interval remains 4d, but should now be validated against warm (30d) and block saving. + setSelectedPhase('cold'); + + await waitFor(() => expect(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)).toBeDisabled()); + + // Clicking save should not call onSave because the button is disabled. + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)); + expect(onSave).toHaveBeenCalledTimes(0); + }); }); describe('searchable snapshots', () => { @@ -466,6 +674,43 @@ describe('EditIlmPhasesFlyout', () => { expect(getPanel('hot')).toBeVisible(); }); + it('re-enables saving after removing an invalid phase', async () => { + const initialPhases: IlmPolicyPhases = { + hot: { + name: 'hot', + size_in_bytes: 0, + rollover: {}, + downsample: { after: '0ms', fixed_interval: '1d' }, + }, + warm: { + name: 'warm', + size_in_bytes: 0, + min_age: '30d', + downsample: { after: '30d', fixed_interval: '30d' }, + }, + cold: { + name: 'cold', + size_in_bytes: 0, + min_age: '60d', + downsample: { after: '60d', fixed_interval: '4d' }, // invalid vs warm=30d + }, + }; + + const { onSave } = renderFlyout({ initialPhases }, { initialSelectedPhase: 'cold' }); + await tick(); + + // Trigger validation by attempting to save (invalid). + fireEvent.click(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)); + await waitFor(() => expect(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)).toBeDisabled()); + expect(onSave).toHaveBeenCalledTimes(0); + + // Remove the invalid cold phase -> save should become enabled again. + fireEvent.click(withinPhase('cold').getByTestId(`${DATA_TEST_SUBJ}RemoveItemButton`)); + await waitFor(() => + expect(screen.getByTestId(`${DATA_TEST_SUBJ}SaveButton`)).not.toBeDisabled() + ); + }); + it('disables remove when there is no hot phase and only one non-delete phase', async () => { const onChange = jest.fn(); renderFlyout({ @@ -502,4 +747,102 @@ describe('EditIlmPhasesFlyout', () => { expect(getTab('delete')).toBeInTheDocument(); }); }); + + describe('onChange emission', () => { + it('debounces rapid user edits into a single onChange', async () => { + jest.useFakeTimers(); + const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout'); + try { + const onChange = jest.fn(); + renderFlyout( + { + onChange, + onChangeDebounceMs: 100, + initialPhases: { + hot: { name: 'hot', size_in_bytes: 0, rollover: {} }, + warm: { name: 'warm', size_in_bytes: 0, min_age: '30d' }, + }, + }, + { initialSelectedPhase: 'warm' } + ); + + await act(async () => { + jest.runOnlyPendingTimers(); + }); + onChange.mockClear(); + clearTimeoutSpy.mockClear(); + + const warmPanel = withinPhase('warm'); + fireEvent.change(warmPanel.getByTestId(`${DATA_TEST_SUBJ}MoveAfterValue`), { + target: { value: '1' }, + }); + fireEvent.change(warmPanel.getByTestId(`${DATA_TEST_SUBJ}MoveAfterValue`), { + target: { value: '2' }, + }); + fireEvent.change(warmPanel.getByTestId(`${DATA_TEST_SUBJ}MoveAfterValue`), { + target: { value: '3' }, + }); + + expect(onChange).toHaveBeenCalledTimes(0); + + await act(async () => { + jest.advanceTimersByTime(100); + }); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenLastCalledWith({ + hot: { name: 'hot', size_in_bytes: 0, rollover: {} }, + warm: { name: 'warm', size_in_bytes: 0, min_age: '3d' }, + }); + + expect(clearTimeoutSpy).toHaveBeenCalled(); + } finally { + clearTimeoutSpy.mockRestore(); + jest.useRealTimers(); + } + }); + + it('cleans up a pending debounced onChange on unmount', async () => { + jest.useFakeTimers(); + const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout'); + try { + const onChange = jest.fn(); + const { unmount } = renderFlyout( + { + onChange, + onChangeDebounceMs: 100, + initialPhases: { + hot: { name: 'hot', size_in_bytes: 0, rollover: {} }, + warm: { name: 'warm', size_in_bytes: 0, min_age: '30d' }, + }, + }, + { initialSelectedPhase: 'warm' } + ); + + await act(async () => { + jest.runOnlyPendingTimers(); + }); + onChange.mockClear(); + clearTimeoutSpy.mockClear(); + + const warmPanel = withinPhase('warm'); + fireEvent.change(warmPanel.getByTestId(`${DATA_TEST_SUBJ}MoveAfterValue`), { + target: { value: '10' }, + }); + + unmount(); + + await act(async () => { + jest.runOnlyPendingTimers(); + jest.advanceTimersByTime(100); + }); + + expect(onChange).toHaveBeenCalledTimes(0); + expect(clearTimeoutSpy).toHaveBeenCalled(); + } finally { + clearTimeoutSpy.mockRestore(); + jest.useRealTimers(); + } + }); + }); }); diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/edit_ilm_phases_flyout.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/edit_ilm_phases_flyout.tsx index 8d6f499ab4827..410c69824e292 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/edit_ilm_phases_flyout.tsx +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/edit_ilm_phases_flyout.tsx @@ -18,6 +18,7 @@ import { EuiFlyoutBody, EuiFlyoutFooter, EuiFlyoutHeader, + EuiToolTip, EuiTitle, useGeneratedHtmlId, } from '@elastic/eui'; @@ -30,12 +31,12 @@ import { type IlmPhasesFlyoutFormInternal, OnFieldErrorsChangeProvider, toMilliseconds, - type TimeUnit, useIlmPhasesFlyoutTabErrors, } from './form'; import { DEFAULT_NEW_PHASE_MIN_AGE, ILM_PHASE_ORDER } from './constants'; import { GlobalFieldsMount, PhasePanel, PhaseTabsRow } from './sections'; import { useStyles } from './use_styles'; +import { getDoubledDurationFromPrevious, type PreservedTimeUnit } from '../shared'; export const EditIlmPhasesFlyout = ({ initialPhases, @@ -44,6 +45,7 @@ export const EditIlmPhasesFlyout = ({ onChange, onSave, onClose, + onChangeDebounceMs = 250, isSaving, canCreateRepository = false, searchableSnapshotRepositories = [], @@ -107,47 +109,93 @@ export const EditIlmPhasesFlyout = ({ const { onFieldErrorsChange, tabHasErrors } = useIlmPhasesFlyoutTabErrors(formData); + const onChangeRef = useRef(onChange); + useEffect(() => { + onChangeRef.current = onChange; + }, [onChange]); + const lastEmittedOutputRef = useRef(initialPhasesRef.current); + const pendingOnChangeOutputRef = useRef(null); + const pendingOnChangeTimeoutRef = useRef | null>(null); useEffect(() => { const sub = form.subscribe(({ data }) => { const next = data.format(); if (isEqual(next, lastEmittedOutputRef.current)) return; - lastEmittedOutputRef.current = next; - onChange(next); + pendingOnChangeOutputRef.current = next; + if (pendingOnChangeTimeoutRef.current) { + clearTimeout(pendingOnChangeTimeoutRef.current); + } + + pendingOnChangeTimeoutRef.current = setTimeout(() => { + pendingOnChangeTimeoutRef.current = null; + const toEmit = pendingOnChangeOutputRef.current; + pendingOnChangeOutputRef.current = null; + if (!toEmit) return; + if (isEqual(toEmit, lastEmittedOutputRef.current)) return; + lastEmittedOutputRef.current = toEmit; + onChangeRef.current(toEmit); + }, onChangeDebounceMs); }); return () => { + if (pendingOnChangeTimeoutRef.current) { + clearTimeout(pendingOnChangeTimeoutRef.current); + pendingOnChangeTimeoutRef.current = null; + } + pendingOnChangeOutputRef.current = null; sub.unsubscribe(); }; - }, [form, onChange]); + }, [form, onChangeDebounceMs]); const canSelectFrozen = canCreateRepository || searchableSnapshotRepositories.length > 0; - const getDefaultMinAge = useCallback((): { value: string; unit: TimeUnit } => { - const candidates: Array<'warm' | 'cold' | 'frozen' | 'delete'> = [ - 'warm', - 'cold', - 'frozen', - 'delete', - ]; - let last: { value: string; unit: TimeUnit } | undefined; - - candidates.forEach((p) => { - const isPhaseEnabled = Boolean(form.getFields()[`_meta.${p}.enabled`]?.value); - if (!isPhaseEnabled) return; - - const value = String(form.getFields()[`_meta.${p}.minAgeValue`]?.value ?? '').trim(); - const unit = (form.getFields()[`_meta.${p}.minAgeUnit`]?.value ?? 'd') as TimeUnit; - if (value) { - last = { value, unit }; + const getDefaultMinAgeForPhase = useCallback( + (phase: PhaseName): { value: string; unit: PreservedTimeUnit } => { + const phases: Array<'warm' | 'cold' | 'frozen' | 'delete'> = [ + 'warm', + 'cold', + 'frozen', + 'delete', + ]; + const index = phases.indexOf(phase as (typeof phases)[number]); + if (index <= 0) return DEFAULT_NEW_PHASE_MIN_AGE; + + // Default to 2x the closest enabled previous phase's min_age. + const previousPhases = phases.slice(0, index).reverse(); + const fields = form.getFields(); + + for (const previousPhase of previousPhases) { + const isPhaseEnabled = Boolean(fields[`_meta.${previousPhase}.enabled`]?.value); + if (!isPhaseEnabled) continue; + + const previousValue = String( + fields[`_meta.${previousPhase}.minAgeValue`]?.value ?? '' + ).trim(); + if (previousValue === '') continue; + + const previousUnit = String( + fields[`_meta.${previousPhase}.minAgeUnit`]?.value ?? 'd' + ) as PreservedTimeUnit; + + const previousNum = Number(previousValue); + if (!Number.isFinite(previousNum) || previousNum < 0) continue; + + const { value, unit } = getDoubledDurationFromPrevious({ + previousValue, + previousUnit, + previousValueFallback: previousNum, + previousValueMinInclusive: 0, + }); + return { value, unit }; } - }); - return last ?? DEFAULT_NEW_PHASE_MIN_AGE; - }, [form]); + return DEFAULT_NEW_PHASE_MIN_AGE; + }, + [form] + ); const ensurePhaseEnabledWithDefaults = useCallback( (phase: PhaseName): boolean => { @@ -178,23 +226,44 @@ export const EditIlmPhasesFlyout = ({ const unitField = form.getFields()[unitPath]; // When enabling a previously-disabled phase, preserve existing values. - // Otherwise default to the last configured min_age (or 30d). + // Otherwise default based on the closest enabled previous phase. if (valueField && String(valueField.value ?? '').trim() === '') { - const { value, unit } = getDefaultMinAge(); + const { value, unit } = getDefaultMinAgeForPhase(phase); valueField.setValue(value); unitField?.setValue(unit); } const resolvedValue = String(form.getFields()[valuePath]?.value ?? ''); - const resolvedUnit = String(form.getFields()[unitPath]?.value ?? 'd') as TimeUnit; + const resolvedUnit = String(form.getFields()[unitPath]?.value ?? 'd') as PreservedTimeUnit; const millis = resolvedValue.trim() === '' ? -1 : toMilliseconds(resolvedValue, resolvedUnit); form.setFieldValue(millisPath, millis); } + // Force re-validation for the (re-)enabled phase. + // Phases are not unmounted when disabled, so fields may have been validated while the phase + // was disabled (validators no-op) and would otherwise remain "valid" when re-enabled. + const fieldsToValidate: string[] = []; + if (phase !== 'hot') { + fieldsToValidate.push(`_meta.${phase}.minAgeValue`); + } + if (phase === 'hot' || phase === 'warm' || phase === 'cold') { + fieldsToValidate.push(`_meta.${phase}.downsample.fixedIntervalValue`); + } + if (phase === 'cold' || phase === 'frozen') { + fieldsToValidate.push('_meta.searchableSnapshot.repository'); + } + if (fieldsToValidate.length > 0) { + // Delay to the next tick so `hook_form_lib` has time to propagate the field value changes + // into the form's flattened `formData` snapshot that validators read from. + setTimeout(() => { + void form.validateFields(fieldsToValidate, /* onlyBlocking */ true); + }, 0); + } + return true; }, - [canSelectFrozen, enabledPhases, form, getDefaultMinAge, searchableSnapshotRepositories] + [canSelectFrozen, enabledPhases, form, getDefaultMinAgeForPhase, searchableSnapshotRepositories] ); useEffect(() => { @@ -214,6 +283,38 @@ export const EditIlmPhasesFlyout = ({ } }, [enabledPhases, ensurePhaseEnabledWithDefaults, isMetaReady, selectedPhase, setSelectedPhase]); + const hasFormErrors = form.getErrors().length > 0; + const isSaveDisabledDueToInvalid = form.isValid === false || hasFormErrors; + const isSaveDisabled = isSaveDisabledDueToInvalid || form.isSubmitting; + + const renderSaveButton = () => { + const button = ( + form.submit()} + disabled={isSaveDisabled} + > + {i18n.translate('xpack.streams.editIlmPhasesFlyout.save', { + defaultMessage: 'Save', + })} + + ); + + return isSaveDisabledDueToInvalid ? ( + + {button} + + ) : ( + button + ); + }; + return ( - - form.submit()} - disabled={(form.isSubmitted && form.isValid === false) || form.isSubmitting} - > - {i18n.translate('xpack.streams.editIlmPhasesFlyout.save', { - defaultMessage: 'Save', - })} - - + {renderSaveButton()} diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/form/fields/read_only_toggle_field.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/form/fields/read_only_toggle_field.tsx index 5be5a390a541a..ffa48115eb9c0 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/form/fields/read_only_toggle_field.tsx +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/form/fields/read_only_toggle_field.tsx @@ -16,7 +16,7 @@ import { useGeneratedHtmlId, } from '@elastic/eui'; import type { FormHook } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; -import { useFormData } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; +import { UseField } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; import type { IlmPhasesFlyoutFormInternal } from '../types'; export interface ReadOnlyToggleFieldProps { @@ -32,47 +32,46 @@ export const ReadOnlyToggleField = ({ dataTestSubj, allowedPhases, }: ReadOnlyToggleFieldProps) => { - const watchPath = phaseName ? `_meta.${phaseName}.readonlyEnabled` : `_meta.hot.readonlyEnabled`; - const checkboxId = useGeneratedHtmlId({ prefix: `${dataTestSubj}ReadOnlyCheckbox-${phaseName}`, }); - useFormData({ form, watch: watchPath }); - if (!phaseName) return null; const readonlyAllowed = allowedPhases.includes(phaseName); if (!readonlyAllowed) return null; const path = `_meta.${phaseName}.readonlyEnabled`; - const field = form.getFields()[path]; - if (!field) return null; - - const readonlyValue = Boolean(field.value); return ( - - - { - field.onChange(e); - }} - /> - - - - - + + {(field) => { + const readonlyValue = Boolean(field.value); + return ( + + + { + field.onChange(e); + }} + /> + + + + + + ); + }} + ); }; diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/form/schema.ts b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/form/schema.ts index 145a493bc37e3..6a6278665dcf8 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/form/schema.ts +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/form/schema.ts @@ -32,6 +32,13 @@ const getDownsampleFieldsToValidateOnChange = ( return phasesToValidate.map(getIntervalPath); }; +const getMinAgeFieldsToValidateOnChange = (phase: 'warm' | 'cold' | 'frozen' | 'delete') => { + const ordered = ['warm', 'cold', 'frozen', 'delete'] as const; + const startIndex = ordered.indexOf(phase); + const phasesToValidate = startIndex < 0 ? ordered : ordered.slice(startIndex); + return phasesToValidate.map((p) => `_meta.${p}.minAgeValue`); +}; + /** * Minimal schema for the ILM phases flyout. * @@ -74,14 +81,17 @@ export const getIlmPhasesFlyoutFormSchema = (): FormSchema= 0 ? milliFromForm : computed; + return { - milli: typeof milli === 'number' ? milli : computed, + milli, esFormat, }; }; diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/sections/downsample_field_section.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/sections/downsample_field_section.tsx index 268d70523fde2..5da1621de8a8e 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/sections/downsample_field_section.tsx +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/sections/downsample_field_section.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React from 'react'; +import React, { useCallback, useEffect } from 'react'; import { i18n } from '@kbn/i18n'; import type { IlmPolicyPhases } from '@kbn/streams-schema'; import type { FormHook } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; @@ -19,7 +19,9 @@ import { useGeneratedHtmlId, } from '@elastic/eui'; import type { DownsamplePhase, IlmPhasesFlyoutFormInternal } from '../form'; +import { DOWNSAMPLE_PHASES } from '../form'; import { DownsampleIntervalField } from '../form'; +import { getDoubledDurationFromPrevious, type PreservedTimeUnit } from '../../shared'; import { TIME_UNIT_OPTIONS } from '../constants'; export interface DownsampleFieldSectionProps { @@ -34,14 +36,35 @@ export const DownsampleFieldSection = ({ dataTestSubj, }: DownsampleFieldSectionProps) => { const enabledPath = `_meta.${phaseName}.downsampleEnabled`; + const intervalValuePath = `_meta.${phaseName}.downsample.fixedIntervalValue`; + const intervalUnitPath = `_meta.${phaseName}.downsample.fixedIntervalUnit`; + const readonlyPath = `_meta.${phaseName}.readonlyEnabled`; const titleId = useGeneratedHtmlId({ prefix: dataTestSubj }); useFormData({ form, watch: enabledPath }); const enabledField = form.getFields()[enabledPath]; + const isEnabled = Boolean(enabledField?.value); + const readonlyDefaultValue = form.getFieldDefaultValue(readonlyPath); + const readonlyField = form.getFields()[readonlyPath]; + const isReadonlyEnabled = Boolean(readonlyField?.value ?? readonlyDefaultValue); + + const resetReadonly = useCallback(() => { + // Ensure the "default value on the form" is reset too so that when the readonly field is re-mounted + // (after toggling downsampling off), it comes back unchecked. + const payload: Record = { _meta: { [phaseName]: { readonlyEnabled: false } } }; + form.updateFieldValues(payload, { runDeserializer: false }); + form.setFieldValue(readonlyPath, false); + }, [form, phaseName, readonlyPath]); + + useEffect(() => { + if (isEnabled && isReadonlyEnabled) { + resetReadonly(); + } + }, [isEnabled, isReadonlyEnabled, resetReadonly]); + if (!enabledField) return null; - const isEnabled = Boolean(enabledField.value); return ( @@ -75,7 +98,70 @@ export const DownsampleFieldSection = ({ compressed checked={isEnabled} data-test-subj={`${dataTestSubj}DownsamplingSwitch`} - onChange={(e) => enabledField.setValue(e.target.checked)} + onChange={(e) => { + const nextEnabled = e.target.checked; + enabledField.setValue(nextEnabled); + + if (nextEnabled) { + // Downsampling is incompatible with the ILM "readonly" action in this flyout. + // Clear and unmount the readonly toggle while downsampling is enabled. + resetReadonly(); + } + + // When enabling downsampling, default the fixed_interval to 2x the previous enabled downsample interval. + // Only do this when the current interval is still the schema default (pristine 1d) to avoid clobbering + // existing values when toggling. + if (!nextEnabled) return; + + const fields = form.getFields(); + const valueField = fields[intervalValuePath]; + const unitField = fields[intervalUnitPath]; + if (!valueField || !unitField) return; + + const currentValue = String(valueField.value ?? '').trim(); + const currentUnit = String(unitField.value ?? 'd') as PreservedTimeUnit; + + const isStillDefault = + currentValue === '1' && + currentUnit === 'd' && + valueField.isModified === false && + unitField.isModified === false; + if (!isStillDefault) return; + + const phaseIndex = DOWNSAMPLE_PHASES.indexOf(phaseName); + const previousPhases = + phaseIndex > 0 ? DOWNSAMPLE_PHASES.slice(0, phaseIndex).reverse() : []; + + for (const previousPhase of previousPhases) { + const isPrevEnabled = Boolean(fields[`_meta.${previousPhase}.enabled`]?.value); + const isPrevDownsampleEnabled = Boolean( + fields[`_meta.${previousPhase}.downsampleEnabled`]?.value + ); + if (!isPrevEnabled || !isPrevDownsampleEnabled) continue; + + const previousValue = String( + fields[`_meta.${previousPhase}.downsample.fixedIntervalValue`]?.value ?? '' + ).trim(); + if (previousValue === '') continue; + + const previousUnit = String( + fields[`_meta.${previousPhase}.downsample.fixedIntervalUnit`]?.value ?? 'd' + ) as PreservedTimeUnit; + + const previousNum = Number(previousValue); + if (!Number.isFinite(previousNum) || previousNum <= 0) continue; + + const { value, unit } = getDoubledDurationFromPrevious({ + previousValue, + previousUnit, + previousValueFallback: previousNum, + previousValueMinExclusive: 0, + }); + valueField.setValue(value); + unitField.setValue(unit); + break; + } + }} /> diff --git a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/sections/phase_panel.tsx b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/sections/phase_panel.tsx index 1ab9fd4aa60e6..07653bd9893d4 100644 --- a/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/sections/phase_panel.tsx +++ b/x-pack/platform/plugins/shared/streams_app/public/components/data_management/stream_detail_lifecycle/downsampling/edit_ilm_phases_flyout/sections/phase_panel.tsx @@ -9,6 +9,7 @@ import React from 'react'; import type { SerializedStyles } from '@emotion/react'; import type { IlmPolicyPhases, PhaseName } from '@kbn/streams-schema'; import type { FormHook } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; +import { useFormData } from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib'; import { EuiFlexGroup, EuiFlexItem, EuiHorizontalRule } from '@elastic/eui'; import type { IlmPhasesFlyoutFormInternal } from '../form'; @@ -58,48 +59,51 @@ export const PhasePanel = ({ const showSearchableSnapshotSectionForCold = canCreateRepository || searchableSnapshotRepositories.length > 0; + const downsampleEnabledPath = `_meta.${phase}.downsampleEnabled`; + useFormData({ + form, + watch: isHotPhase || isWarmPhase || isColdPhase ? downsampleEnabledPath : undefined, + }); + const isDownsampleEnabled = Boolean(form.getFields()[downsampleEnabledPath]?.value); + return ( -