fix: dashboard - textbox default variable not working (#9843)

This commit is contained in:
Ashwin Bhatkal
2026-01-19 18:23:17 +05:30
committed by GitHub
parent 325974292f
commit 31e9e896ec
9 changed files with 501 additions and 62 deletions

View File

@@ -211,7 +211,10 @@ describe('VariableItem Integration Tests', () => {
await user.clear(textInput);
await user.type(textInput, 'new-text-value');
// Should call onValueUpdate after debounce
// Blur the input to trigger the value update
await user.tab();
// Should call onValueUpdate after blur
await waitFor(
() => {
expect(mockOnValueUpdate).toHaveBeenCalledWith(

View File

@@ -1,6 +1,12 @@
/* eslint-disable sonarjs/no-identical-functions */
/* eslint-disable sonarjs/no-duplicate-string */
import { fireEvent, render, screen, waitFor } from 'tests/test-utils';
import {
fireEvent,
render,
screen,
userEvent,
waitFor,
} from 'tests/test-utils';
import {
IDashboardVariable,
TSortVariableValuesType,
@@ -639,4 +645,186 @@ describe('VariableItem Component', () => {
await expectCircularDependencyError();
});
});
describe('Textbox Variable Default Value Handling', () => {
test('saves textbox variable with defaultValue and selectedValue set to textboxValue', async () => {
const user = userEvent.setup();
const textboxVariable: IDashboardVariable = {
id: TEST_VAR_IDS.VAR1,
name: TEST_VAR_NAMES.VAR1,
description: 'Test Textbox Variable',
type: 'TEXTBOX',
textboxValue: 'my-default-value',
...VARIABLE_DEFAULTS,
order: 0,
};
renderVariableItem(textboxVariable);
// Click save button
const saveButton = screen.getByText(SAVE_BUTTON_TEXT);
await user.click(saveButton);
// Verify that onSave was called with defaultValue and selectedValue equal to textboxValue
expect(onSave).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
type: 'TEXTBOX',
textboxValue: 'my-default-value',
defaultValue: 'my-default-value',
selectedValue: 'my-default-value',
}),
expect.anything(),
);
});
test('saves textbox variable with empty values when textboxValue is empty', async () => {
const user = userEvent.setup();
const textboxVariable: IDashboardVariable = {
id: TEST_VAR_IDS.VAR1,
name: TEST_VAR_NAMES.VAR1,
description: 'Test Textbox Variable',
type: 'TEXTBOX',
textboxValue: '',
...VARIABLE_DEFAULTS,
order: 0,
};
renderVariableItem(textboxVariable);
// Click save button
const saveButton = screen.getByText(SAVE_BUTTON_TEXT);
await user.click(saveButton);
// Verify that onSave was called with empty defaultValue and selectedValue
expect(onSave).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
type: 'TEXTBOX',
textboxValue: '',
defaultValue: '',
selectedValue: '',
}),
expect.anything(),
);
});
test('updates textbox defaultValue and selectedValue when user changes textboxValue input', async () => {
const user = userEvent.setup();
const textboxVariable: IDashboardVariable = {
id: TEST_VAR_IDS.VAR1,
name: TEST_VAR_NAMES.VAR1,
description: 'Test Textbox Variable',
type: 'TEXTBOX',
textboxValue: 'initial-value',
...VARIABLE_DEFAULTS,
order: 0,
};
renderVariableItem(textboxVariable);
// Change the textbox value
const textboxInput = screen.getByPlaceholderText(
'Enter a default value (if any)...',
);
await user.clear(textboxInput);
await user.type(textboxInput, 'updated-value');
// Click save button
const saveButton = screen.getByText(SAVE_BUTTON_TEXT);
await user.click(saveButton);
// Verify that onSave was called with the updated defaultValue and selectedValue
expect(onSave).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
type: 'TEXTBOX',
textboxValue: 'updated-value',
defaultValue: 'updated-value',
selectedValue: 'updated-value',
}),
expect.anything(),
);
});
test('non-textbox variables use variableDefaultValue instead of textboxValue', async () => {
const user = userEvent.setup();
const queryVariable: IDashboardVariable = {
id: TEST_VAR_IDS.VAR1,
name: TEST_VAR_NAMES.VAR1,
description: 'Test Query Variable',
type: 'QUERY',
queryValue: 'SELECT * FROM test',
textboxValue: 'should-not-be-used',
defaultValue: 'query-default-value',
...VARIABLE_DEFAULTS,
order: 0,
};
renderVariableItem(queryVariable);
// Click save button
const saveButton = screen.getByText(SAVE_BUTTON_TEXT);
await user.click(saveButton);
// Verify that onSave was called with defaultValue not being textboxValue
expect(onSave).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
type: 'QUERY',
defaultValue: 'query-default-value',
}),
expect.anything(),
);
// Verify that defaultValue is NOT the textboxValue
const savedVariable = onSave.mock.calls[0][1];
expect(savedVariable.defaultValue).not.toBe('should-not-be-used');
});
test('switching to textbox type sets defaultValue and selectedValue correctly on save', async () => {
const user = userEvent.setup();
// Start with a QUERY variable
const queryVariable: IDashboardVariable = {
id: TEST_VAR_IDS.VAR1,
name: TEST_VAR_NAMES.VAR1,
description: 'Test Variable',
type: 'QUERY',
queryValue: 'SELECT * FROM test',
...VARIABLE_DEFAULTS,
order: 0,
};
renderVariableItem(queryVariable);
// Switch to TEXTBOX type
const textboxButton = findButtonByText(TEXT.TEXTBOX);
expect(textboxButton).toBeInTheDocument();
if (textboxButton) {
await user.click(textboxButton);
}
// Enter a default value in the textbox input
const textboxInput = screen.getByPlaceholderText(
'Enter a default value (if any)...',
);
await user.type(textboxInput, 'new-textbox-default');
// Click save button
const saveButton = screen.getByText(SAVE_BUTTON_TEXT);
await user.click(saveButton);
// Verify that onSave was called with type TEXTBOX and correct defaultValue and selectedValue
expect(onSave).toHaveBeenCalledWith(
expect.anything(),
expect.objectContaining({
type: 'TEXTBOX',
textboxValue: 'new-textbox-default',
defaultValue: 'new-textbox-default',
selectedValue: 'new-textbox-default',
}),
expect.anything(),
);
});
});
});

View File

@@ -320,6 +320,10 @@ function VariableItem({
]);
const variableValue = useMemo(() => {
if (queryType === 'TEXTBOX') {
return variableTextboxValue;
}
if (variableMultiSelect) {
let value = variableData.selectedValue;
if (isEmpty(value)) {
@@ -352,6 +356,8 @@ function VariableItem({
variableData.selectedValue,
variableData.showALLOption,
variableDefaultValue,
variableTextboxValue,
queryType,
previewValues,
]);
@@ -367,13 +373,10 @@ function VariableItem({
multiSelect: variableMultiSelect,
showALLOption: queryType === 'DYNAMIC' ? true : variableShowALLOption,
sort: variableSortType,
...(queryType === 'TEXTBOX' && {
selectedValue: (variableData.selectedValue ||
variableTextboxValue) as never,
}),
...(queryType !== 'TEXTBOX' && {
defaultValue: variableDefaultValue as never,
}),
// the reason we need to do this is because defaultValues are treated differently in case of textbox type
// They are the exact same and not like the other types where defaultValue is a separate field
defaultValue:
queryType === 'TEXTBOX' ? variableTextboxValue : variableDefaultValue,
modificationUUID: generateUUID(),
id: variableData.id || generateUUID(),
order: variableData.order,

View File

@@ -25,6 +25,12 @@
}
}
&.focused {
.variable-value {
outline: 1px solid var(--bg-robin-400);
}
}
.variable-value {
display: flex;
min-width: 120px;
@@ -93,6 +99,12 @@
.lightMode {
.variable-item {
&.focused {
.variable-value {
border: 1px solid var(--bg-robin-400);
}
}
.variable-name {
border: 1px solid var(--bg-vanilla-300);
background: var(--bg-vanilla-100);

View File

@@ -80,10 +80,12 @@ describe('VariableItem', () => {
/>
</MockQueryClientProvider>,
);
expect(screen.getByPlaceholderText('Enter value')).toBeInTheDocument();
expect(
screen.getByTestId('variable-textbox-test_variable'),
).toBeInTheDocument();
});
test('calls onChange event handler when Input value changes', async () => {
test('calls onValueUpdate when Input value changes and blurs', async () => {
render(
<MockQueryClientProvider>
<VariableItem
@@ -102,13 +104,19 @@ describe('VariableItem', () => {
</MockQueryClientProvider>,
);
const inputElement = screen.getByTestId('variable-textbox-test_variable');
// Change the value
act(() => {
const inputElement = screen.getByPlaceholderText('Enter value');
fireEvent.change(inputElement, { target: { value: 'newValue' } });
});
// Blur the input to trigger the update
act(() => {
fireEvent.blur(inputElement);
});
await waitFor(() => {
// expect(mockOnValueUpdate).toHaveBeenCalledTimes(1);
expect(mockOnValueUpdate).toHaveBeenCalledWith(
'testVariable',
'test_variable',

View File

@@ -8,14 +8,14 @@ import './DashboardVariableSelection.styles.scss';
import { orange } from '@ant-design/colors';
import { InfoCircleOutlined, WarningOutlined } from '@ant-design/icons';
import { Input, Popover, Tooltip, Typography } from 'antd';
import { Input, InputRef, Popover, Tooltip, Typography } from 'antd';
import dashboardVariablesQuery from 'api/dashboard/variables/dashboardVariablesQuery';
import { CustomMultiSelect, CustomSelect } from 'components/NewSelect';
import { REACT_QUERY_KEY } from 'constants/reactQueryKeys';
import { commaValuesParser } from 'lib/dashbaordVariables/customCommaValuesParser';
import sortValues from 'lib/dashbaordVariables/sortVariableValues';
import { debounce, isArray, isEmpty, isString } from 'lodash-es';
import { memo, useCallback, useEffect, useMemo, useState } from 'react';
import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { useQuery } from 'react-query';
import { useSelector } from 'react-redux';
import { AppState } from 'store/reducers';
@@ -71,6 +71,15 @@ function VariableItem({
string | string[] | undefined
>(undefined);
// Local state for textbox input to ensure smooth editing experience
const [textboxInputValue, setTextboxInputValue] = useState<string>(
(variableData.selectedValue?.toString() ||
variableData.defaultValue?.toString()) ??
'',
);
const [isTextboxFocused, setIsTextboxFocused] = useState<boolean>(false);
const textboxInputRef = useRef<InputRef>(null);
const { maxTime, minTime } = useSelector<AppState, GlobalReducer>(
(state) => state.globalTime,
);
@@ -371,7 +380,7 @@ function VariableItem({
}, [variableData.type, variableData.customValue]);
return (
<div className="variable-item">
<div className={`variable-item${isTextboxFocused ? ' focused' : ''}`}>
<Typography.Text className="variable-name" ellipsis>
${variableData.name}
{variableData.description && (
@@ -384,16 +393,40 @@ function VariableItem({
<div className="variable-value">
{variableData.type === 'TEXTBOX' ? (
<Input
ref={textboxInputRef}
placeholder="Enter value"
data-testid={`variable-textbox-${variableData.id}`}
bordered={false}
key={variableData.selectedValue?.toString()}
defaultValue={variableData.selectedValue?.toString()}
value={textboxInputValue}
title={textboxInputValue}
onChange={(e): void => {
debouncedHandleChange(e.target.value || '');
setTextboxInputValue(e.target.value);
}}
style={{
width:
50 + ((variableData.selectedValue?.toString()?.length || 0) * 7 || 50),
onFocus={(): void => {
setIsTextboxFocused(true);
}}
onBlur={(e): void => {
setIsTextboxFocused(false);
const value = e.target.value.trim();
// If empty, reset to default value
if (!value && variableData.defaultValue) {
setTextboxInputValue(variableData.defaultValue.toString());
debouncedHandleChange(variableData.defaultValue.toString());
} else {
debouncedHandleChange(value);
}
}}
onKeyDown={(e): void => {
if (e.key === 'Enter') {
const value = textboxInputValue.trim();
if (!value && variableData.defaultValue) {
setTextboxInputValue(variableData.defaultValue.toString());
debouncedHandleChange(variableData.defaultValue.toString());
} else {
debouncedHandleChange(value);
}
textboxInputRef.current?.blur();
}
}}
/>
) : (

View File

@@ -291,6 +291,10 @@ export function DashboardProvider({
variable.order = order;
existingOrders.add(order);
// ! BWC - Specific case for backward compatibility where textboxValue was used instead of defaultValue
if (variable.type === 'TEXTBOX' && !variable.defaultValue) {
variable.defaultValue = variable.textboxValue || '';
}
}
if (variable.id === undefined) {

View File

@@ -1,5 +1,5 @@
/* eslint-disable sonarjs/no-duplicate-string */
import { render, waitFor } from '@testing-library/react';
import { render, screen, waitFor } from '@testing-library/react';
import getDashboard from 'api/v1/dashboards/id/get';
import { REACT_QUERY_KEY } from 'constants/reactQueryKeys';
import ROUTES from 'constants/routes';
@@ -379,12 +379,9 @@ describe('Dashboard Provider - URL Variables Integration', () => {
// Empty URL variables - tests initialization flow
mockGetUrlVariables.mockReturnValue({});
const { getByTestId } = renderWithDashboardProvider(
`/dashboard/${DASHBOARD_ID}`,
{
dashboardId: DASHBOARD_ID,
},
);
renderWithDashboardProvider(`/dashboard/${DASHBOARD_ID}`, {
dashboardId: DASHBOARD_ID,
});
await waitFor(() => {
expect(mockGetDashboard).toHaveBeenCalledWith({ id: DASHBOARD_ID });
@@ -415,16 +412,14 @@ describe('Dashboard Provider - URL Variables Integration', () => {
});
// Verify dashboard state contains the variables with default values
await waitFor(() => {
const dashboardVariables = getByTestId('dashboard-variables');
const parsedVariables = JSON.parse(dashboardVariables.textContent || '{}');
const dashboardVariables = await screen.findByTestId('dashboard-variables');
const parsedVariables = JSON.parse(dashboardVariables.textContent || '{}');
expect(parsedVariables).toHaveProperty('environment');
expect(parsedVariables).toHaveProperty('services');
// Default allSelected values should be preserved
expect(parsedVariables.environment.allSelected).toBe(false);
expect(parsedVariables.services.allSelected).toBe(false);
});
expect(parsedVariables).toHaveProperty('environment');
expect(parsedVariables).toHaveProperty('services');
// Default allSelected values should be preserved
expect(parsedVariables.environment.allSelected).toBe(false);
expect(parsedVariables.services.allSelected).toBe(false);
});
it('should merge URL variables with dashboard data and normalize values correctly', async () => {
@@ -438,12 +433,9 @@ describe('Dashboard Provider - URL Variables Integration', () => {
.mockReturnValueOnce('development')
.mockReturnValueOnce(['db', 'cache']);
const { getByTestId } = renderWithDashboardProvider(
`/dashboard/${DASHBOARD_ID}`,
{
dashboardId: DASHBOARD_ID,
},
);
renderWithDashboardProvider(`/dashboard/${DASHBOARD_ID}`, {
dashboardId: DASHBOARD_ID,
});
await waitFor(() => {
expect(mockGetDashboard).toHaveBeenCalledWith({ id: DASHBOARD_ID });
@@ -474,18 +466,16 @@ describe('Dashboard Provider - URL Variables Integration', () => {
});
// Verify the dashboard state reflects the normalized URL values
await waitFor(() => {
const dashboardVariables = getByTestId('dashboard-variables');
const parsedVariables = JSON.parse(dashboardVariables.textContent || '{}');
const dashboardVariables = await screen.findByTestId('dashboard-variables');
const parsedVariables = JSON.parse(dashboardVariables.textContent || '{}');
// The selectedValue should be updated with normalized URL values
expect(parsedVariables.environment.selectedValue).toBe('development');
expect(parsedVariables.services.selectedValue).toEqual(['db', 'cache']);
// The selectedValue should be updated with normalized URL values
expect(parsedVariables.environment.selectedValue).toBe('development');
expect(parsedVariables.services.selectedValue).toEqual(['db', 'cache']);
// allSelected should be set to false when URL values override
expect(parsedVariables.environment.allSelected).toBe(false);
expect(parsedVariables.services.allSelected).toBe(false);
});
// allSelected should be set to false when URL values override
expect(parsedVariables.environment.allSelected).toBe(false);
expect(parsedVariables.services.allSelected).toBe(false);
});
it('should handle ALL_SELECTED_VALUE from URL and set allSelected correctly', async () => {
@@ -495,12 +485,9 @@ describe('Dashboard Provider - URL Variables Integration', () => {
mockGetUrlVariables.mockReturnValue(urlVariables);
const { getByTestId } = renderWithDashboardProvider(
`/dashboard/${DASHBOARD_ID}`,
{
dashboardId: DASHBOARD_ID,
},
);
renderWithDashboardProvider(`/dashboard/${DASHBOARD_ID}`, {
dashboardId: DASHBOARD_ID,
});
await waitFor(() => {
expect(mockGetDashboard).toHaveBeenCalledWith({ id: DASHBOARD_ID });
@@ -513,8 +500,8 @@ describe('Dashboard Provider - URL Variables Integration', () => {
);
// Verify that allSelected is set to true for the services variable
await waitFor(() => {
const dashboardVariables = getByTestId('dashboard-variables');
await waitFor(async () => {
const dashboardVariables = await screen.findByTestId('dashboard-variables');
const parsedVariables = JSON.parse(dashboardVariables.textContent || '{}');
expect(parsedVariables.services.allSelected).toBe(true);
@@ -563,3 +550,203 @@ describe('Dashboard Provider - URL Variables Integration', () => {
});
});
});
describe('Dashboard Provider - Textbox Variable Backward Compatibility', () => {
const DASHBOARD_ID = 'test-dashboard-id';
beforeEach(() => {
jest.clearAllMocks();
mockGetUrlVariables.mockReturnValue({});
// eslint-disable-next-line sonarjs/no-identical-functions
mockNormalizeUrlValueForVariable.mockImplementation((urlValue) => {
if (urlValue === undefined || urlValue === null) {
return urlValue;
}
return urlValue as IDashboardVariable['selectedValue'];
});
});
describe('Textbox Variable defaultValue Migration', () => {
it('should set defaultValue from textboxValue for TEXTBOX variables without defaultValue (BWC)', async () => {
// Mock dashboard with TEXTBOX variable that has textboxValue but no defaultValue
// This simulates old data format before the migration
/* eslint-disable @typescript-eslint/no-explicit-any */
mockGetDashboard.mockResolvedValue({
httpStatusCode: 200,
data: {
id: DASHBOARD_ID,
title: 'Test Dashboard',
data: {
variables: {
myTextbox: {
id: 'textbox-id',
name: 'myTextbox',
type: 'TEXTBOX',
textboxValue: 'legacy-default-value',
// defaultValue is intentionally missing to test BWC
multiSelect: false,
showALLOption: false,
sort: 'DISABLED',
} as any,
},
},
},
} as any);
/* eslint-enable @typescript-eslint/no-explicit-any */
renderWithDashboardProvider(`/dashboard/${DASHBOARD_ID}`, {
dashboardId: DASHBOARD_ID,
});
await waitFor(() => {
expect(mockGetDashboard).toHaveBeenCalledWith({ id: DASHBOARD_ID });
});
// Verify that defaultValue is set from textboxValue
await waitFor(async () => {
const dashboardVariables = await screen.findByTestId('dashboard-variables');
const parsedVariables = JSON.parse(dashboardVariables.textContent || '{}');
expect(parsedVariables.myTextbox.type).toBe('TEXTBOX');
expect(parsedVariables.myTextbox.textboxValue).toBe('legacy-default-value');
expect(parsedVariables.myTextbox.defaultValue).toBe('legacy-default-value');
});
});
it('should not override existing defaultValue for TEXTBOX variables', async () => {
// Mock dashboard with TEXTBOX variable that already has defaultValue
/* eslint-disable @typescript-eslint/no-explicit-any */
mockGetDashboard.mockResolvedValue({
httpStatusCode: 200,
data: {
id: DASHBOARD_ID,
title: 'Test Dashboard',
data: {
variables: {
myTextbox: {
id: 'textbox-id',
name: 'myTextbox',
type: 'TEXTBOX',
textboxValue: 'old-textbox-value',
defaultValue: 'existing-default-value',
multiSelect: false,
showALLOption: false,
sort: 'DISABLED',
} as any,
},
},
},
} as any);
/* eslint-enable @typescript-eslint/no-explicit-any */
renderWithDashboardProvider(`/dashboard/${DASHBOARD_ID}`, {
dashboardId: DASHBOARD_ID,
});
await waitFor(() => {
expect(mockGetDashboard).toHaveBeenCalledWith({ id: DASHBOARD_ID });
});
// Verify that existing defaultValue is preserved
await waitFor(async () => {
const dashboardVariables = await screen.findByTestId('dashboard-variables');
const parsedVariables = JSON.parse(dashboardVariables.textContent || '{}');
expect(parsedVariables.myTextbox.type).toBe('TEXTBOX');
expect(parsedVariables.myTextbox.defaultValue).toBe(
'existing-default-value',
);
});
});
it('should set empty defaultValue when textboxValue is also empty for TEXTBOX variables', async () => {
// Mock dashboard with TEXTBOX variable with empty textboxValue and no defaultValue
/* eslint-disable @typescript-eslint/no-explicit-any */
mockGetDashboard.mockResolvedValue({
httpStatusCode: 200,
data: {
id: DASHBOARD_ID,
title: 'Test Dashboard',
data: {
variables: {
myTextbox: {
id: 'textbox-id',
name: 'myTextbox',
type: 'TEXTBOX',
textboxValue: '',
// defaultValue is intentionally missing
multiSelect: false,
showALLOption: false,
sort: 'DISABLED',
} as any,
},
},
},
} as any);
/* eslint-enable @typescript-eslint/no-explicit-any */
renderWithDashboardProvider(`/dashboard/${DASHBOARD_ID}`, {
dashboardId: DASHBOARD_ID,
});
await waitFor(() => {
expect(mockGetDashboard).toHaveBeenCalledWith({ id: DASHBOARD_ID });
});
// Verify that defaultValue is set to empty string
await waitFor(async () => {
const dashboardVariables = await screen.findByTestId('dashboard-variables');
const parsedVariables = JSON.parse(dashboardVariables.textContent || '{}');
expect(parsedVariables.myTextbox.type).toBe('TEXTBOX');
expect(parsedVariables.myTextbox.defaultValue).toBe('');
});
});
it('should not apply BWC logic to non-TEXTBOX variables', async () => {
// Mock dashboard with QUERY variable that has no defaultValue
/* eslint-disable @typescript-eslint/no-explicit-any */
mockGetDashboard.mockResolvedValue({
httpStatusCode: 200,
data: {
id: DASHBOARD_ID,
title: 'Test Dashboard',
data: {
variables: {
myQuery: {
id: 'query-id',
name: 'myQuery',
type: 'QUERY',
queryValue: 'SELECT * FROM test',
textboxValue: 'should-not-be-used',
// defaultValue is intentionally missing
multiSelect: false,
showALLOption: false,
sort: 'DISABLED',
} as any,
},
},
},
} as any);
/* eslint-enable @typescript-eslint/no-explicit-any */
renderWithDashboardProvider(`/dashboard/${DASHBOARD_ID}`, {
dashboardId: DASHBOARD_ID,
});
await waitFor(() => {
expect(mockGetDashboard).toHaveBeenCalledWith({ id: DASHBOARD_ID });
});
// Verify that defaultValue is NOT set from textboxValue for QUERY type
await waitFor(async () => {
const dashboardVariables = await screen.findByTestId('dashboard-variables');
const parsedVariables = JSON.parse(dashboardVariables.textContent || '{}');
expect(parsedVariables.myQuery.type).toBe('QUERY');
// defaultValue should not be set to textboxValue for non-TEXTBOX variables
expect(parsedVariables.myQuery.defaultValue).not.toBe('should-not-be-used');
});
});
});
});

View File

@@ -37,6 +37,7 @@ export interface IDashboardVariable {
// Custom
customValue?: string;
// Textbox
// special case of variable where defaultValue is same as this. Otherwise, defaultValue is a single field
textboxValue?: string;
sort: TSortVariableValuesType;