Skip to content

Commit 4fa2b1f

Browse files
saikatsarkar056saikatsarkar056
committed
Fix AI Connector form fields resetting to default value when cleared by user (#251095)
## Summary Fixes #250953 This PR fixes a bug in the AI Connector creation flyout where fields with default values (like URL, Model ID) would reset to their default value when the user tried to clear them completely using backspace. **Root cause:** The `useEffect` in `ConfigInputField` and `ConfigNumberField` was checking if the value was empty/null and resetting to `defaultValue`. When a user cleared a field, the form converted the empty string to `null`, which triggered the effect to reset the field back to the default. **Fix:** Modified the `useEffect` to only sync when there's actual external content, preventing the reset when users intentionally clear the field. The initial default is still applied via `useState`. ### Changes - `ConfigInputField`: Updated `useEffect` to not reset to default when value is cleared - `ConfigNumberField`: Same fix applied - Added comprehensive unit tests for both components ## Test plan 1. Go to **Alerts and Insights > Connectors** 2. Click **Create connector** → Select **AI Connector** 3. Select **DeepSeek** as the provider 4. Open **More Options** section 5. Try to backspace/delete the URL field completely 6. **Expected:** Field should clear and stay empty 7. **Before fix:** Field would reset to default URL when last character was deleted ### Before https://github.com/user-attachments/assets/cc8832f5-ca91-4270-825f-023dcbc34615 ### After https://github.com/user-attachments/assets/49a31906-6362-4f5d-aee1-bff10008aabd ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) - [ ] Review the [backport guidelines](https://docs.google.com/document/d/1VyN5k91e5OVumlc0Gb9RPa3h1ewuPE705nRtioPiTvY/edit?usp=sharing) and apply applicable `backport:*` labels. ### Identify risks Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss. Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging. - [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) - [ ] ... ### Release note Fixes AI Connector form fields incorrectly resetting to default values when users clear them using backspace. (cherry picked from commit 6c002da)
1 parent 8e2d0a8 commit 4fa2b1f

2 files changed

Lines changed: 320 additions & 4 deletions

File tree

‎x-pack/platform/packages/shared/kbn-inference-endpoint-ui-common/src/components/configuration/configuration_field.test.tsx‎

Lines changed: 308 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,308 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License
4+
* 2.0; you may not use this file except in compliance with the Elastic License
5+
* 2.0.
6+
*/
7+
8+
import React from 'react';
9+
import { render, screen, fireEvent } from '@testing-library/react';
10+
import { ConfigInputField, ConfigNumberField } from './configuration_field';
11+
import { FieldType } from '../../types/types';
12+
import type { ConfigEntryView } from '../../types/types';
13+
14+
describe('ConfigInputField', () => {
15+
const createConfigEntry = (overrides: Partial<ConfigEntryView> = {}): ConfigEntryView => ({
16+
key: 'url',
17+
isValid: true,
18+
label: 'URL',
19+
description: 'The URL endpoint',
20+
validationErrors: [],
21+
required: false,
22+
sensitive: false,
23+
value: null,
24+
default_value: 'https://api.example.com/v1',
25+
updatable: true,
26+
type: FieldType.STRING,
27+
supported_task_types: ['text_embedding'],
28+
...overrides,
29+
});
30+
31+
const defaultProps = {
32+
isLoading: false,
33+
validateAndSetConfigValue: jest.fn(),
34+
};
35+
36+
beforeEach(() => {
37+
jest.clearAllMocks();
38+
});
39+
40+
it('renders with default value when value is null', () => {
41+
const configEntry = createConfigEntry({ value: null });
42+
render(<ConfigInputField {...defaultProps} configEntry={configEntry} />);
43+
44+
const input = screen.getByTestId('url-input');
45+
expect(input).toHaveValue('https://api.example.com/v1');
46+
});
47+
48+
it('renders with actual value when value is provided', () => {
49+
const configEntry = createConfigEntry({ value: 'https://custom.url.com' });
50+
render(<ConfigInputField {...defaultProps} configEntry={configEntry} />);
51+
52+
const input = screen.getByTestId('url-input');
53+
expect(input).toHaveValue('https://custom.url.com');
54+
});
55+
56+
it('allows user to clear the field completely without resetting to default', () => {
57+
const validateAndSetConfigValue = jest.fn();
58+
const configEntry = createConfigEntry({
59+
value: null,
60+
default_value: 'https://api.example.com/v1',
61+
});
62+
63+
render(
64+
<ConfigInputField
65+
{...defaultProps}
66+
configEntry={configEntry}
67+
validateAndSetConfigValue={validateAndSetConfigValue}
68+
/>
69+
);
70+
71+
const input = screen.getByTestId('url-input');
72+
73+
// User clears the entire field
74+
fireEvent.change(input, { target: { value: '' } });
75+
76+
// The input should be empty, not reset to default
77+
expect(input).toHaveValue('');
78+
expect(validateAndSetConfigValue).toHaveBeenCalledWith('');
79+
});
80+
81+
it('does not reset to default after rerender when field is cleared', () => {
82+
const validateAndSetConfigValue = jest.fn();
83+
const configEntry = createConfigEntry({
84+
value: null,
85+
default_value: 'https://api.example.com/v1',
86+
});
87+
88+
const { rerender } = render(
89+
<ConfigInputField
90+
{...defaultProps}
91+
configEntry={configEntry}
92+
validateAndSetConfigValue={validateAndSetConfigValue}
93+
/>
94+
);
95+
96+
const input = screen.getByTestId('url-input');
97+
98+
// User clears the entire field
99+
fireEvent.change(input, { target: { value: '' } });
100+
101+
// Simulate parent form updating value prop to null (as it converts '' to null)
102+
rerender(
103+
<ConfigInputField
104+
{...defaultProps}
105+
configEntry={{ ...configEntry, value: null }}
106+
validateAndSetConfigValue={validateAndSetConfigValue}
107+
/>
108+
);
109+
110+
// Should still be empty, not reset to default
111+
expect(input).toHaveValue('');
112+
});
113+
114+
it('allows user to type a new value after clearing', () => {
115+
const validateAndSetConfigValue = jest.fn();
116+
const configEntry = createConfigEntry({
117+
value: null,
118+
default_value: 'https://api.example.com/v1',
119+
});
120+
121+
render(
122+
<ConfigInputField
123+
{...defaultProps}
124+
configEntry={configEntry}
125+
validateAndSetConfigValue={validateAndSetConfigValue}
126+
/>
127+
);
128+
129+
const input = screen.getByTestId('url-input');
130+
131+
// User clears and types new value
132+
fireEvent.change(input, { target: { value: '' } });
133+
fireEvent.change(input, { target: { value: 'https://new.url.com' } });
134+
135+
expect(input).toHaveValue('https://new.url.com');
136+
expect(validateAndSetConfigValue).toHaveBeenLastCalledWith('https://new.url.com');
137+
});
138+
139+
it('is disabled when isLoading is true', () => {
140+
const configEntry = createConfigEntry();
141+
render(<ConfigInputField {...defaultProps} configEntry={configEntry} isLoading={true} />);
142+
143+
const input = screen.getByTestId('url-input');
144+
expect(input).toBeDisabled();
145+
});
146+
147+
it('is disabled in edit mode when field is not updatable', () => {
148+
const configEntry = createConfigEntry({ updatable: false });
149+
render(<ConfigInputField {...defaultProps} configEntry={configEntry} isEdit={true} />);
150+
151+
const input = screen.getByTestId('url-input');
152+
expect(input).toBeDisabled();
153+
});
154+
155+
it('shows invalid state when isValid is false', () => {
156+
const configEntry = createConfigEntry({ isValid: false });
157+
render(<ConfigInputField {...defaultProps} configEntry={configEntry} />);
158+
159+
const input = screen.getByTestId('url-input');
160+
expect(input).toHaveAttribute('aria-invalid', 'true');
161+
});
162+
});
163+
164+
describe('ConfigNumberField', () => {
165+
const createConfigEntry = (overrides: Partial<ConfigEntryView> = {}): ConfigEntryView => ({
166+
key: 'max_tokens',
167+
isValid: true,
168+
label: 'Max Tokens',
169+
description: 'Maximum number of tokens',
170+
validationErrors: [],
171+
required: false,
172+
sensitive: false,
173+
value: null,
174+
default_value: 1024,
175+
updatable: true,
176+
type: FieldType.INTEGER,
177+
supported_task_types: ['text_embedding'],
178+
...overrides,
179+
});
180+
181+
const defaultProps = {
182+
isLoading: false,
183+
validateAndSetConfigValue: jest.fn(),
184+
};
185+
186+
beforeEach(() => {
187+
jest.clearAllMocks();
188+
});
189+
190+
it('renders with default value when value is null', () => {
191+
const configEntry = createConfigEntry({ value: null });
192+
render(<ConfigNumberField {...defaultProps} configEntry={configEntry} />);
193+
194+
const input = screen.getByTestId('max_tokens-number');
195+
expect(input).toHaveValue(1024);
196+
});
197+
198+
it('renders with actual value when value is provided', () => {
199+
const configEntry = createConfigEntry({ value: 2048 });
200+
render(<ConfigNumberField {...defaultProps} configEntry={configEntry} />);
201+
202+
const input = screen.getByTestId('max_tokens-number');
203+
expect(input).toHaveValue(2048);
204+
});
205+
206+
it('allows user to clear the field using the clear button', () => {
207+
const validateAndSetConfigValue = jest.fn();
208+
const configEntry = createConfigEntry({
209+
value: null,
210+
default_value: 1024,
211+
});
212+
213+
render(
214+
<ConfigNumberField
215+
{...defaultProps}
216+
configEntry={configEntry}
217+
validateAndSetConfigValue={validateAndSetConfigValue}
218+
/>
219+
);
220+
221+
// Find and click the clear button
222+
const clearButton = screen.getByRole('button', { name: /clear/i });
223+
fireEvent.click(clearButton);
224+
225+
expect(validateAndSetConfigValue).toHaveBeenCalledWith('');
226+
});
227+
228+
it('does not reset to default after rerender when field is cleared', () => {
229+
const validateAndSetConfigValue = jest.fn();
230+
const configEntry = createConfigEntry({
231+
value: null,
232+
default_value: 1024,
233+
});
234+
235+
const { rerender } = render(
236+
<ConfigNumberField
237+
{...defaultProps}
238+
configEntry={configEntry}
239+
validateAndSetConfigValue={validateAndSetConfigValue}
240+
/>
241+
);
242+
243+
// Find and click the clear button
244+
const clearButton = screen.getByRole('button', { name: /clear/i });
245+
fireEvent.click(clearButton);
246+
247+
// Simulate parent form updating value prop to null (as it converts '' to null)
248+
rerender(
249+
<ConfigNumberField
250+
{...defaultProps}
251+
configEntry={{ ...configEntry, value: null }}
252+
validateAndSetConfigValue={validateAndSetConfigValue}
253+
/>
254+
);
255+
256+
// Should still be empty, not reset to default
257+
const input = screen.getByTestId('max_tokens-number');
258+
expect(input).toHaveValue(null);
259+
});
260+
261+
it('allows user to change the value', () => {
262+
const validateAndSetConfigValue = jest.fn();
263+
const configEntry = createConfigEntry({
264+
value: null,
265+
default_value: 1024,
266+
});
267+
268+
render(
269+
<ConfigNumberField
270+
{...defaultProps}
271+
configEntry={configEntry}
272+
validateAndSetConfigValue={validateAndSetConfigValue}
273+
/>
274+
);
275+
276+
const input = screen.getByTestId('max_tokens-number');
277+
fireEvent.change(input, { target: { value: '512' } });
278+
279+
expect(input).toHaveValue(512);
280+
expect(validateAndSetConfigValue).toHaveBeenCalledWith('512');
281+
});
282+
283+
it('is disabled when isLoading is true', () => {
284+
const configEntry = createConfigEntry();
285+
render(<ConfigNumberField {...defaultProps} configEntry={configEntry} isLoading={true} />);
286+
287+
const input = screen.getByTestId('max_tokens-number');
288+
expect(input).toBeDisabled();
289+
});
290+
291+
it('is disabled when isPreconfigured is true', () => {
292+
const configEntry = createConfigEntry();
293+
render(
294+
<ConfigNumberField {...defaultProps} configEntry={configEntry} isPreconfigured={true} />
295+
);
296+
297+
const input = screen.getByTestId('max_tokens-number');
298+
expect(input).toBeDisabled();
299+
});
300+
301+
it('is disabled in edit mode when field is not updatable', () => {
302+
const configEntry = createConfigEntry({ updatable: false });
303+
render(<ConfigNumberField {...defaultProps} configEntry={configEntry} isEdit={true} />);
304+
305+
const input = screen.getByTestId('max_tokens-number');
306+
expect(input).toBeDisabled();
307+
});
308+
});

‎x-pack/platform/packages/shared/kbn-inference-endpoint-ui-common/src/components/configuration/configuration_field.tsx‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,12 @@ export const ConfigInputField: React.FC<ConfigInputFieldProps> = ({
4949
);
5050

5151
useEffect(() => {
52-
setInnerValue(!value || value.toString().length === 0 ? defaultValue : value);
53-
}, [defaultValue, value]);
52+
// Only sync from external value if it has actual content
53+
// Don't reset to default when user clears the field (value becomes null)
54+
if (value != null && String(value).length > 0) {
55+
setInnerValue(value);
56+
}
57+
}, [value]);
5458
return (
5559
<EuiFieldText
5660
disabled={isLoading || (isEdit && !updatable)}
@@ -126,8 +130,12 @@ export const ConfigNumberField: React.FC<ConfigInputFieldProps> = ({
126130
const { isValid, value, default_value: defaultValue, key, updatable } = configEntry;
127131
const [innerValue, setInnerValue] = useState(value ?? defaultValue);
128132
useEffect(() => {
129-
setInnerValue(!value || value.toString().length === 0 ? defaultValue : value);
130-
}, [defaultValue, value]);
133+
// Only sync from external value if it has actual content
134+
// Don't reset to default when user clears the field (value becomes null)
135+
if (value != null && String(value).length > 0) {
136+
setInnerValue(value);
137+
}
138+
}, [value]);
131139
return (
132140
<EuiFormControlLayout
133141
isDisabled={isLoading || (isEdit && !updatable) || isPreconfigured}

0 commit comments

Comments
 (0)