fix: format numeric strings without quotes, preserve quoted values (#9637)
* fix: format numeric strings without quotes, preserve quoted values * chore: updated filter creation logic and updated tests * chore: tsc fix --------- Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>
This commit is contained in:
@@ -13,6 +13,7 @@ import {
|
||||
convertAggregationToExpression,
|
||||
convertFiltersToExpression,
|
||||
convertFiltersToExpressionWithExistingQuery,
|
||||
formatValueForExpression,
|
||||
removeKeysFromExpression,
|
||||
} from '../utils';
|
||||
|
||||
@@ -1193,3 +1194,220 @@ describe('removeKeysFromExpression', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('formatValueForExpression', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('Variable values', () => {
|
||||
it('should return variable values as-is', () => {
|
||||
expect(formatValueForExpression('$variable')).toBe('$variable');
|
||||
expect(formatValueForExpression('$env')).toBe('$env');
|
||||
expect(formatValueForExpression(' $variable ')).toBe(' $variable ');
|
||||
});
|
||||
|
||||
it('should return variable arrays as-is', () => {
|
||||
expect(formatValueForExpression(['$var1', '$var2'])).toBe('$var1,$var2');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Numeric string values', () => {
|
||||
it('should return numeric strings with quotes', () => {
|
||||
expect(formatValueForExpression('123')).toBe("'123'");
|
||||
expect(formatValueForExpression('0')).toBe("'0'");
|
||||
expect(formatValueForExpression('100000')).toBe("'100000'");
|
||||
expect(formatValueForExpression('-42')).toBe("'-42'");
|
||||
expect(formatValueForExpression('3.14')).toBe("'3.14'");
|
||||
expect(formatValueForExpression(' 456 ')).toBe("' 456 '");
|
||||
});
|
||||
|
||||
it('should handle numeric strings with IN operator', () => {
|
||||
expect(formatValueForExpression('123', 'IN')).toBe("['123']");
|
||||
expect(formatValueForExpression(['123', '456'], 'IN')).toBe(
|
||||
"['123', '456']",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Quoted string values', () => {
|
||||
it('should return already quoted strings as-is', () => {
|
||||
expect(formatValueForExpression("'quoted'")).toBe("'quoted'");
|
||||
expect(formatValueForExpression('"double-quoted"')).toBe('"double-quoted"');
|
||||
expect(formatValueForExpression('`backticked`')).toBe('`backticked`');
|
||||
expect(formatValueForExpression("'100000'")).toBe("'100000'");
|
||||
});
|
||||
|
||||
it('should preserve quoted strings in arrays', () => {
|
||||
expect(formatValueForExpression(["'value1'", "'value2'"])).toBe(
|
||||
"['value1', 'value2']",
|
||||
);
|
||||
expect(formatValueForExpression(["'100000'", "'200000'"], 'IN')).toBe(
|
||||
"['100000', '200000']",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Regular string values', () => {
|
||||
it('should wrap regular strings in single quotes', () => {
|
||||
expect(formatValueForExpression('hello')).toBe("'hello'");
|
||||
expect(formatValueForExpression('api-gateway')).toBe("'api-gateway'");
|
||||
expect(formatValueForExpression('test value')).toBe("'test value'");
|
||||
});
|
||||
|
||||
it('should escape single quotes in strings', () => {
|
||||
expect(formatValueForExpression("user's data")).toBe("'user\\'s data'");
|
||||
expect(formatValueForExpression("John's")).toBe("'John\\'s'");
|
||||
expect(formatValueForExpression("it's a test")).toBe("'it\\'s a test'");
|
||||
});
|
||||
|
||||
it('should handle empty strings', () => {
|
||||
expect(formatValueForExpression('')).toBe("''");
|
||||
});
|
||||
|
||||
it('should handle strings with special characters', () => {
|
||||
expect(formatValueForExpression('/api/v1/users')).toBe("'/api/v1/users'");
|
||||
expect(formatValueForExpression('user@example.com')).toBe(
|
||||
"'user@example.com'",
|
||||
);
|
||||
expect(formatValueForExpression('Contains "quotes"')).toBe(
|
||||
'\'Contains "quotes"\'',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Number values', () => {
|
||||
it('should convert numbers to strings without quotes', () => {
|
||||
expect(formatValueForExpression(123)).toBe('123');
|
||||
expect(formatValueForExpression(0)).toBe('0');
|
||||
expect(formatValueForExpression(-42)).toBe('-42');
|
||||
expect(formatValueForExpression(100000)).toBe('100000');
|
||||
expect(formatValueForExpression(3.14)).toBe('3.14');
|
||||
});
|
||||
|
||||
it('should handle numbers with IN operator', () => {
|
||||
expect(formatValueForExpression(123, 'IN')).toBe('[123]');
|
||||
expect(formatValueForExpression([100, 200] as any, 'IN')).toBe('[100, 200]');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Boolean values', () => {
|
||||
it('should convert booleans to strings without quotes', () => {
|
||||
expect(formatValueForExpression(true)).toBe('true');
|
||||
expect(formatValueForExpression(false)).toBe('false');
|
||||
});
|
||||
|
||||
it('should handle booleans with IN operator', () => {
|
||||
expect(formatValueForExpression(true, 'IN')).toBe('[true]');
|
||||
expect(formatValueForExpression([true, false] as any, 'IN')).toBe(
|
||||
'[true, false]',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Array values', () => {
|
||||
it('should format array of strings', () => {
|
||||
expect(formatValueForExpression(['a', 'b', 'c'])).toBe("['a', 'b', 'c']");
|
||||
expect(formatValueForExpression(['service1', 'service2'])).toBe(
|
||||
"['service1', 'service2']",
|
||||
);
|
||||
});
|
||||
|
||||
it('should format array of numeric strings', () => {
|
||||
expect(formatValueForExpression(['123', '456', '789'])).toBe(
|
||||
"['123', '456', '789']",
|
||||
);
|
||||
});
|
||||
|
||||
it('should format array of numbers', () => {
|
||||
expect(formatValueForExpression([1, 2, 3] as any)).toBe('[1, 2, 3]');
|
||||
expect(formatValueForExpression([100, 200, 300] as any)).toBe(
|
||||
'[100, 200, 300]',
|
||||
);
|
||||
});
|
||||
|
||||
it('should format mixed array types', () => {
|
||||
expect(formatValueForExpression(['hello', 123, true] as any)).toBe(
|
||||
"['hello', 123, true]",
|
||||
);
|
||||
});
|
||||
|
||||
it('should format array with quoted values', () => {
|
||||
expect(formatValueForExpression(["'quoted'", 'regular'])).toBe(
|
||||
"['quoted', 'regular']",
|
||||
);
|
||||
});
|
||||
|
||||
it('should format array with empty strings', () => {
|
||||
expect(formatValueForExpression(['', 'value'])).toBe("['', 'value']");
|
||||
});
|
||||
});
|
||||
|
||||
describe('IN and NOT IN operators', () => {
|
||||
it('should format single value as array for IN operator', () => {
|
||||
expect(formatValueForExpression('value', 'IN')).toBe("['value']");
|
||||
expect(formatValueForExpression(123, 'IN')).toBe('[123]');
|
||||
expect(formatValueForExpression('123', 'IN')).toBe("['123']");
|
||||
});
|
||||
|
||||
it('should format array for IN operator', () => {
|
||||
expect(formatValueForExpression(['a', 'b'], 'IN')).toBe("['a', 'b']");
|
||||
expect(formatValueForExpression(['123', '456'], 'IN')).toBe(
|
||||
"['123', '456']",
|
||||
);
|
||||
});
|
||||
|
||||
it('should format single value as array for NOT IN operator', () => {
|
||||
expect(formatValueForExpression('value', 'NOT IN')).toBe("['value']");
|
||||
expect(formatValueForExpression('value', 'not in')).toBe("['value']");
|
||||
});
|
||||
|
||||
it('should format array for NOT IN operator', () => {
|
||||
expect(formatValueForExpression(['a', 'b'], 'NOT IN')).toBe("['a', 'b']");
|
||||
});
|
||||
});
|
||||
|
||||
describe('Edge cases', () => {
|
||||
it('should handle strings that look like numbers but have quotes', () => {
|
||||
expect(formatValueForExpression("'123'")).toBe("'123'");
|
||||
expect(formatValueForExpression('"456"')).toBe('"456"');
|
||||
expect(formatValueForExpression('`789`')).toBe('`789`');
|
||||
});
|
||||
|
||||
it('should handle strings with leading/trailing whitespace', () => {
|
||||
expect(formatValueForExpression(' hello ')).toBe("' hello '");
|
||||
expect(formatValueForExpression(' 123 ')).toBe("' 123 '");
|
||||
});
|
||||
|
||||
it('should handle very large numbers', () => {
|
||||
expect(formatValueForExpression('999999999')).toBe("'999999999'");
|
||||
expect(formatValueForExpression(999999999)).toBe('999999999');
|
||||
});
|
||||
|
||||
it('should handle decimal numbers', () => {
|
||||
expect(formatValueForExpression('123.456')).toBe("'123.456'");
|
||||
expect(formatValueForExpression(123.456)).toBe('123.456');
|
||||
});
|
||||
|
||||
it('should handle negative numbers', () => {
|
||||
expect(formatValueForExpression('-100')).toBe("'-100'");
|
||||
expect(formatValueForExpression(-100)).toBe('-100');
|
||||
});
|
||||
|
||||
it('should handle strings that are not valid numbers', () => {
|
||||
expect(formatValueForExpression('123abc')).toBe("'123abc'");
|
||||
expect(formatValueForExpression('abc123')).toBe("'abc123'");
|
||||
expect(formatValueForExpression('12.34.56')).toBe("'12.34.56'");
|
||||
});
|
||||
|
||||
it('should handle empty array', () => {
|
||||
expect(formatValueForExpression([])).toBe('[]');
|
||||
expect(formatValueForExpression([], 'IN')).toBe('[]');
|
||||
});
|
||||
|
||||
it('should handle array with single element', () => {
|
||||
expect(formatValueForExpression(['single'])).toBe("['single']");
|
||||
expect(formatValueForExpression([123] as any)).toBe('[123]');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -24,7 +24,7 @@ import {
|
||||
import { EQueryType } from 'types/common/dashboard';
|
||||
import { DataSource, ReduceOperators } from 'types/common/queryBuilder';
|
||||
import { extractQueryPairs } from 'utils/queryContextUtils';
|
||||
import { unquote } from 'utils/stringUtils';
|
||||
import { isQuoted, unquote } from 'utils/stringUtils';
|
||||
import { isFunctionOperator, isNonValueOperator } from 'utils/tokenUtils';
|
||||
import { v4 as uuid } from 'uuid';
|
||||
|
||||
@@ -38,49 +38,57 @@ const isArrayOperator = (operator: string): boolean => {
|
||||
return arrayOperators.includes(operator);
|
||||
};
|
||||
|
||||
const isVariable = (value: string | string[] | number | boolean): boolean => {
|
||||
const isVariable = (
|
||||
value: (string | number | boolean)[] | string | number | boolean,
|
||||
): boolean => {
|
||||
if (Array.isArray(value)) {
|
||||
return value.some((v) => typeof v === 'string' && v.trim().startsWith('$'));
|
||||
}
|
||||
return typeof value === 'string' && value.trim().startsWith('$');
|
||||
};
|
||||
|
||||
/**
|
||||
* Formats a single value for use in expression strings.
|
||||
* Strings are quoted and escaped, while numbers and booleans are converted to strings.
|
||||
*/
|
||||
const formatSingleValue = (v: string | number | boolean): string => {
|
||||
if (typeof v === 'string') {
|
||||
// Preserve already-quoted strings
|
||||
if (isQuoted(v)) {
|
||||
return v;
|
||||
}
|
||||
// Quote and escape single quotes in strings
|
||||
return `'${v.replace(/'/g, "\\'")}'`;
|
||||
}
|
||||
// Convert numbers and booleans to strings without quotes
|
||||
return String(v);
|
||||
};
|
||||
|
||||
/**
|
||||
* Format a value for the expression string
|
||||
* @param value - The value to format
|
||||
* @param operator - The operator being used (to determine if array is needed)
|
||||
* @returns Formatted value string
|
||||
*/
|
||||
const formatValueForExpression = (
|
||||
value: string[] | string | number | boolean,
|
||||
export const formatValueForExpression = (
|
||||
value: (string | number | boolean)[] | string | number | boolean,
|
||||
operator?: string,
|
||||
): string => {
|
||||
if (isVariable(value)) {
|
||||
return String(value);
|
||||
}
|
||||
|
||||
// For IN operators, ensure value is always an array
|
||||
if (isArrayOperator(operator || '')) {
|
||||
const arrayValue = Array.isArray(value) ? value : [value];
|
||||
return `[${arrayValue
|
||||
.map((v) =>
|
||||
typeof v === 'string' ? `'${v.replace(/'/g, "\\'")}'` : String(v),
|
||||
)
|
||||
.join(', ')}]`;
|
||||
return `[${arrayValue.map(formatSingleValue).join(', ')}]`;
|
||||
}
|
||||
|
||||
if (Array.isArray(value)) {
|
||||
// Handle array values (e.g., for IN operations)
|
||||
return `[${value
|
||||
.map((v) =>
|
||||
typeof v === 'string' ? `'${v.replace(/'/g, "\\'")}'` : String(v),
|
||||
)
|
||||
.join(', ')}]`;
|
||||
return `[${value.map(formatSingleValue).join(', ')}]`;
|
||||
}
|
||||
|
||||
if (typeof value === 'string') {
|
||||
// Add single quotes around all string values and escape internal single quotes
|
||||
return `'${value.replace(/'/g, "\\'")}'`;
|
||||
return formatSingleValue(value);
|
||||
}
|
||||
|
||||
return String(value);
|
||||
@@ -136,14 +144,43 @@ export const convertFiltersToExpression = (
|
||||
};
|
||||
};
|
||||
|
||||
const formatValuesForFilter = (value: string | string[]): string | string[] => {
|
||||
if (Array.isArray(value)) {
|
||||
return value.map((v) => (typeof v === 'string' ? unquote(v) : String(v)));
|
||||
}
|
||||
/**
|
||||
* Converts a string value to its appropriate type (number, boolean, or string)
|
||||
* for use in filter objects. This is the inverse of formatSingleValue.
|
||||
*/
|
||||
function formatSingleValueForFilter(
|
||||
value: string | number | boolean,
|
||||
): string | number | boolean {
|
||||
if (typeof value === 'string') {
|
||||
return unquote(value);
|
||||
const trimmed = value.trim();
|
||||
|
||||
// Try to convert numeric strings to numbers
|
||||
if (trimmed !== '' && !Number.isNaN(Number(trimmed))) {
|
||||
return Number(trimmed);
|
||||
}
|
||||
|
||||
// Convert boolean strings to booleans
|
||||
if (trimmed === 'true' || trimmed === 'false') {
|
||||
return trimmed === 'true';
|
||||
}
|
||||
}
|
||||
return String(value);
|
||||
|
||||
// Return non-string values as-is, or string values that couldn't be converted
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Formats values for filter objects, converting string representations
|
||||
* to their proper types (numbers, booleans) when appropriate.
|
||||
*/
|
||||
const formatValuesForFilter = (
|
||||
value: (string | number | boolean)[] | number | boolean | string,
|
||||
): (string | number | boolean)[] | number | boolean | string => {
|
||||
if (Array.isArray(value)) {
|
||||
return value.map(formatSingleValueForFilter);
|
||||
}
|
||||
|
||||
return formatSingleValueForFilter(value);
|
||||
};
|
||||
|
||||
export const convertExpressionToFilters = (
|
||||
|
||||
@@ -178,7 +178,7 @@ export default function CheckboxFilter(props: ICheckboxProps): JSX.Element {
|
||||
if (SELECTED_OPERATORS.includes(filterSync.op)) {
|
||||
if (isArray(filterSync.value)) {
|
||||
filterSync.value.forEach((val) => {
|
||||
filterState[val] = true;
|
||||
filterState[String(val)] = true;
|
||||
});
|
||||
} else if (typeof filterSync.value === 'string') {
|
||||
filterState[filterSync.value] = true;
|
||||
@@ -191,7 +191,7 @@ export default function CheckboxFilter(props: ICheckboxProps): JSX.Element {
|
||||
filterState = setDefaultValues(attributeValues, true);
|
||||
if (isArray(filterSync.value)) {
|
||||
filterSync.value.forEach((val) => {
|
||||
filterState[val] = false;
|
||||
filterState[String(val)] = false;
|
||||
});
|
||||
} else if (typeof filterSync.value === 'string') {
|
||||
filterState[filterSync.value] = false;
|
||||
|
||||
@@ -74,7 +74,7 @@ export interface ITag {
|
||||
id?: string;
|
||||
key: BaseAutocompleteData;
|
||||
op: string;
|
||||
value: string[] | string | number | boolean;
|
||||
value: (string | number | boolean)[] | string | number | boolean;
|
||||
}
|
||||
|
||||
interface CustomTagProps {
|
||||
@@ -300,7 +300,8 @@ function QueryBuilderSearchV2(
|
||||
currentFilterItem?.key?.dataType ?? DataTypes.EMPTY,
|
||||
tagType: currentFilterItem?.key?.type ?? '',
|
||||
searchText: isArray(currentFilterItem?.value)
|
||||
? currentFilterItem?.value?.[currentFilterItem.value.length - 1] || ''
|
||||
? String(currentFilterItem?.value?.[currentFilterItem.value.length - 1]) ||
|
||||
''
|
||||
: currentFilterItem?.value?.toString() || '',
|
||||
},
|
||||
{
|
||||
|
||||
@@ -63,7 +63,8 @@ export function convertOperatorLabelForExceptions(
|
||||
export function formatStringValuesForTrace(
|
||||
val: TagFilterItem['value'] = [],
|
||||
): string[] {
|
||||
return !Array.isArray(val) ? [String(val)] : val;
|
||||
// IN QB V5 we can pass array of all (boolean, number, string) values. To make this compatible with the old version, we need to convert the array to a string array.
|
||||
return !Array.isArray(val) ? [String(val)] : val.map((item) => String(item));
|
||||
}
|
||||
|
||||
export const convertCompositeQueryToTraceSelectedTags = (
|
||||
|
||||
@@ -35,7 +35,7 @@ export interface TagFilterItem {
|
||||
id: string;
|
||||
key?: BaseAutocompleteData;
|
||||
op: string;
|
||||
value: string[] | string | number | boolean;
|
||||
value: (string | number | boolean)[] | string | number | boolean;
|
||||
}
|
||||
|
||||
export interface TagFilter {
|
||||
|
||||
@@ -11,3 +11,8 @@ export function unquote(str: string): string {
|
||||
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
export function isQuoted(str: string): boolean {
|
||||
const trimmed = str.trim();
|
||||
return trimmed.length >= 2 && /^(["'`])(.*)\1$/.test(trimmed);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user