fix: Improve filter component error handling (#8832)

This commit is contained in:
Elias Meire 2024-03-08 10:10:32 +01:00 committed by GitHub
parent 5d52bda865
commit 76fe960a76
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 46 additions and 49 deletions

View file

@ -16,6 +16,7 @@ import { type FilterOperatorId } from './constants';
import {
getFilterOperator,
handleOperatorChange,
isEmptyInput,
operatorTypeToNodeProperty,
resolveCondition,
} from './utils';
@ -54,12 +55,20 @@ const operatorId = computed<FilterOperatorId>(() => {
});
const operator = computed(() => getFilterOperator(operatorId.value));
const isEmpty = computed(() => {
if (operator.value.singleValue) {
return isEmptyInput(condition.value.leftValue);
}
return isEmptyInput(condition.value.leftValue) && isEmptyInput(condition.value.rightValue);
});
const conditionResult = computed(() =>
resolveCondition({ condition: condition.value, options: props.options }),
);
const allIssues = computed(() => {
if (conditionResult.value.status === 'validation_error') {
if (conditionResult.value.status === 'validation_error' && !isEmpty.value) {
return [conditionResult.value.error];
}

View file

@ -56,6 +56,10 @@ export const handleOperatorChange = ({
return condition;
};
export const isEmptyInput = (value: unknown): boolean => {
return value === '' || value === '=';
};
export const resolveCondition = ({
condition,
options,

View file

@ -83,7 +83,7 @@ export class FilterV2 implements INodeType {
set(
error,
'description',
"Try to change the operator, switch ON the option 'Less Strict Type Validation', or change the type with an expression",
"Try changing the type of comparison. Alternatively you can enable 'Less Strict Type Validation' in the options.",
);
}
set(error, 'context.itemIndex', itemIndex);

View file

@ -83,7 +83,7 @@ export class IfV2 implements INodeType {
set(
error,
'description',
"Try to change the operator, switch ON the option 'Less Strict Type Validation', or change the type with an expression",
"Try changing the type of comparison. Alternatively you can enable 'Less Strict Type Validation' in the options.",
);
}
set(error, 'context.itemIndex', itemIndex);

View file

@ -350,7 +350,7 @@ export class SwitchV3 implements INodeType {
} catch (error) {
if (!options.looseTypeValidation) {
error.description =
"Try to change the operator, switch ON the option 'Less Strict Type Validation', or change the type with an expression";
"Try changing the type of comparison. Alternatively you can enable 'Less Strict Type Validation' in the options.";
}
set(error, 'context.itemIndex', itemIndex);
set(error, 'node', this.getNode());

View file

@ -32,11 +32,16 @@ function parseSingleFilterValue(
type: FilterOperatorType,
strict = false,
): ValidationResult {
return type === 'any' || value === null || value === undefined || value === ''
return type === 'any' || value === null || value === undefined
? ({ valid: true, newValue: value } as ValidationResult)
: validateFieldType('filter', value, type, { strict, parseStrings: true });
}
const withIndefiniteArticle = (noun: string): string => {
const article = 'aeiou'.includes(noun.charAt(0)) ? 'an' : 'a';
return `${article} ${noun}`;
};
function parseFilterConditionValues(
condition: FilterConditionValue,
options: FilterOptionsValue,
@ -63,50 +68,29 @@ function parseFilterConditionValues(
condition.rightValue.startsWith('='));
const leftValueString = String(condition.leftValue);
const rightValueString = String(condition.rightValue);
const errorDescription = 'Try to change the operator, or change the type with an expression';
const inCondition = errorFormat === 'full' ? ` in condition ${index + 1} ` : ' ';
const itemSuffix = `[item ${itemIndex}]`;
const suffix =
errorFormat === 'full' ? `[condition ${index}, item ${itemIndex}]` : `[item ${itemIndex}]`;
if (!leftValid && !rightValid) {
const providedValues = 'The provided values';
let types = `'${operator.type}'`;
if (rightType !== operator.type) {
types = `'${operator.type}' and '${rightType}' respectively`;
}
const composeInvalidTypeMessage = (type: string, fromType: string, value: string) => {
fromType = fromType.toLocaleLowerCase();
if (strict) {
return {
ok: false,
error: new FilterError(
`${providedValues} '${leftValueString}' and '${rightValueString}'${inCondition}are not of the expected type ${types} ${itemSuffix}`,
errorDescription,
),
};
return `Wrong type: '${value}' is ${withIndefiniteArticle(
fromType,
)} but was expecting ${withIndefiniteArticle(type)} ${suffix}`;
}
return {
ok: false,
error: new FilterError(
`${providedValues} '${leftValueString}' and '${rightValueString}'${inCondition}cannot be converted to the expected type ${types} ${itemSuffix}`,
errorDescription,
),
};
}
const composeInvalidTypeMessage = (field: 'left' | 'right', type: string, value: string) => {
const fieldNumber = field === 'left' ? 1 : 2;
if (strict) {
return `The provided value ${fieldNumber} '${value}'${inCondition}is not of the expected type '${type}' ${itemSuffix}`;
}
return `The provided value ${fieldNumber} '${value}'${inCondition}cannot be converted to the expected type '${type}' ${itemSuffix}`;
return `Conversion error: the ${fromType} '${value}' can't be converted to ${withIndefiniteArticle(
type,
)} ${suffix}`;
};
const invalidTypeDescription = 'Try changing the type of comparison.';
if (!leftValid) {
return {
ok: false,
error: new FilterError(
composeInvalidTypeMessage('left', operator.type, leftValueString),
errorDescription,
composeInvalidTypeMessage(operator.type, typeof condition.leftValue, leftValueString),
invalidTypeDescription,
),
};
}
@ -115,8 +99,8 @@ function parseFilterConditionValues(
return {
ok: false,
error: new FilterError(
composeInvalidTypeMessage('right', rightType, rightValueString),
errorDescription,
composeInvalidTypeMessage(rightType, typeof condition.rightValue, rightValueString),
invalidTypeDescription,
),
};
}
@ -301,7 +285,7 @@ export function executeFilterCondition(
switch (condition.operator.operation) {
case 'empty':
return !!left && Object.keys(left).length === 0;
return !left || Object.keys(left).length === 0;
case 'notEmpty':
return !!left && Object.keys(left).length !== 0;
}

View file

@ -116,7 +116,7 @@ describe('FilterParameter', () => {
}),
),
).toThrowError(
"The provided values '15' and 'true' in condition 1 are not of the expected type 'number' [item 0]",
"Wrong type: '15' is a string but was expecting a number [condition 0, item 0]",
);
});
@ -136,7 +136,7 @@ describe('FilterParameter', () => {
}),
),
).toThrowError(
"The provided value 1 '[]' in condition 1 is not of the expected type 'array' [item 0]",
"Wrong type: '[]' is a string but was expecting an array [condition 0, item 0]",
);
});
@ -155,7 +155,7 @@ describe('FilterParameter', () => {
}),
),
).toThrowError(
"The provided value 1 '{}' in condition 1 is not of the expected type 'object' [item 0]",
"Wrong type: '{}' is a string but was expecting an object [condition 0, item 0]",
);
});
});
@ -201,7 +201,7 @@ describe('FilterParameter', () => {
}),
),
).toThrowError(
"The provided values 'a string' and '15' in condition 1 cannot be converted to the expected type 'boolean'",
"Conversion error: the string 'a string' can't be converted to a boolean [condition 0, item 0]",
);
});
});
@ -1011,8 +1011,8 @@ describe('FilterParameter', () => {
it.each([
{ left: {}, expected: true },
{ left: { foo: 'bar' }, expected: false },
{ left: undefined, expected: false },
{ left: null, expected: false },
{ left: undefined, expected: true },
{ left: null, expected: true },
])('object:empty($left) === $expected', ({ left, expected }) => {
const result = executeFilter(
filterFactory({