Skip to content

[Streams] Downsampling flyout fixes #254178

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.

Sign up for GitHub

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

Merged
damian-polewski merged 12 commits into from
Feb 23, 2026
Merged

[Streams] Downsampling flyout fixes #254178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a965ebb
fix: adjust tab border color if has errors
damian-polewski Feb 19, 2026
04d76cc
fix: change add button color to text
damian-polewski Feb 19, 2026
1035aa6
fix: improve min age validation, add save tooltip
damian-polewski Feb 20, 2026
c7fc31f
fix: double value from previous phase as default, adjust validation w…
damian-polewski Feb 20, 2026
1474449
feat: add onChange debounce
damian-polewski Feb 20, 2026
d4cf84a
fix: default after to 1 if there is no previous step
damian-polewski Feb 20, 2026
a6cd02e
fix: remove readonly when downsampling is enabled
damian-polewski Feb 20, 2026
67461b5
fix: save enabled in dsl after adding new step despite error
damian-polewski Feb 20, 2026
6a9b5b9
fix: reindex dsl errors in step tabs row after removal
damian-polewski Feb 23, 2026
78edd97
fix: remove unneeded timeout
damian-polewski Feb 23, 2026
82e2978
Merge branch 'main' into streams/downsampling_flyout_fixes
damian-polewski Feb 23, 2026
c17063e
fix: save button enabled after clearing min_age value
damian-polewski Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
223 changes: 222 additions & 1 deletion ...stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ const renderFlyout = (
};
const onSelectedStepIndexChange = jest.fn();

render(
const { unmount } = render(
<Wrapper
initialSteps={initialSteps}
onClose={onClose}
Expand All @@ -66,6 +66,7 @@ const renderFlyout = (
onSave,
initialSteps,
onSelectedStepIndexChange,
unmount,
setSelectedStepIndex: (index: number | undefined) => {
act(() => {
if (!setSelectedStepIndexRef.current) {
Expand Down Expand Up @@ -115,6 +116,7 @@ const Wrapper = ({
onClose={onClose}
onChange={onChange}
onSave={onSave}
onChangeDebounceMs={0}
{...props}
/>
</>
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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();
}
});
});
});
68 changes: 41 additions & 27 deletions ...ment/stream_detail_lifecycle/downsampling/edit_dsl_steps_flyout/edit_dsl_steps_flyout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
EuiButtonEmpty,
EuiFlexGroup,
EuiFlexItem,
EuiToolTip,
} from '@elastic/eui';
import type { IngestStreamLifecycleDSL } from '@kbn/streams-schema';
import {
Expand All @@ -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}</>;

Expand All @@ -58,6 +49,7 @@ export const EditDslStepsFlyout = ({
onChange,
onSave,
onClose,
onChangeDebounceMs = 250,
isSaving,
'data-test-subj': dataTestSubjProp,
}: EditDslStepsFlyoutProps) => {
Expand Down Expand Up @@ -97,7 +89,8 @@ export const EditDslStepsFlyout = ({
const pendingOnChangeOutputRef = useRef<IngestStreamLifecycleDSL | null>(null);
const pendingOnChangeTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);

const { onStepFieldErrorsChange, tabHasErrors, pruneToStepPaths } = useDslStepsFlyoutTabErrors();
const { onStepFieldErrorsChange, tabHasErrors, pruneToStepPaths, reindexErrorsAfterRemoval } =
useDslStepsFlyoutTabErrors();

useEffect(() => {
const sub = form.subscribe(({ data }) => {
Expand Down Expand Up @@ -138,7 +131,7 @@ export const EditDslStepsFlyout = ({
if (isEqual(toEmit, lastEmittedOutputRef.current)) return;
lastEmittedOutputRef.current = toEmit;
onChangeRef.current(toEmit);
}, 0);
}, onChangeDebounceMs);
});

return () => {
Expand All @@ -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 = (
<EuiButton
fill
isLoading={Boolean(isSaving) || form.isSubmitting}
data-test-subj={`${dataTestSubj}SaveButton`}
onClick={() => form.submit()}
disabled={isSaveDisabled}
>
{i18n.translate('xpack.streams.editDslStepsFlyout.save', {
defaultMessage: 'Save',
})}
</EuiButton>
);

return isSaveDisabledDueToInvalid ? (
<EuiToolTip
content={i18n.translate('xpack.streams.editDslStepsFlyout.saveDisabledTooltip', {
defaultMessage: 'Fix the form errors before saving.',
})}
>
{button}
</EuiToolTip>
) : (
button
);
};

return (
<EuiFlyout
Expand Down Expand Up @@ -180,6 +205,7 @@ export const EditDslStepsFlyout = ({
setSelectedStepIndex={setSelectedStepIndex}
tabHasErrors={tabHasErrors}
pruneToStepPaths={pruneToStepPaths}
reindexErrorsAfterRemoval={reindexErrorsAfterRemoval}
/>
)}
</UseArray>
Expand All @@ -204,19 +230,7 @@ export const EditDslStepsFlyout = ({
})}
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButton
fill
isLoading={Boolean(isSaving) || form.isSubmitting}
data-test-subj={`${dataTestSubj}SaveButton`}
onClick={() => form.submit()}
disabled={(form.isSubmitted && form.isValid === false) || form.isSubmitting}
>
{i18n.translate('xpack.streams.editDslStepsFlyout.save', {
defaultMessage: 'Save',
})}
</EuiButton>
</EuiFlexItem>
<EuiFlexItem grow={false}>{renderSaveButton()}</EuiFlexItem>
</EuiFlexGroup>
</EuiFlyoutFooter>
</EuiFlyout>
Expand Down
Loading
Loading