feat: Filter parameter: Improve loose type validation for booleans (#10702)
Some checks are pending
Test Master / install-and-build (push) Waiting to run
Test Master / Unit tests (18.x) (push) Blocked by required conditions
Test Master / Unit tests (20.x) (push) Blocked by required conditions
Test Master / Unit tests (22.4) (push) Blocked by required conditions
Test Master / Lint (push) Blocked by required conditions
Test Master / Notify Slack on failure (push) Blocked by required conditions
Benchmark Docker Image CI / build (push) Waiting to run

This commit is contained in:
Elias Meire 2024-09-09 09:54:36 +02:00 committed by GitHub
parent b18313f219
commit e9b8d99084
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 261 additions and 22 deletions

View file

@ -42,6 +42,8 @@ const i18n = useI18n();
const ndvStore = useNDVStore();
const { debounce } = useDebounce();
const debouncedEmitChange = debounce(emitChange, { debounceTime: 1000 });
function createCondition(): FilterConditionValue {
return { id: uuid(), leftValue: '', rightValue: '', operator: DEFAULT_OPERATOR_VALUE };
}
@ -86,7 +88,7 @@ watch(
try {
newOptions = {
...DEFAULT_FILTER_OPTIONS,
...resolveParameter(typeOptions as NodeParameterValue),
...resolveParameter(typeOptions as unknown as NodeParameterValue),
};
} catch (error) {}
@ -117,8 +119,6 @@ function emitChange() {
});
}
const debouncedEmitChange = debounce(emitChange, { debounceTime: 1000 });
function addCondition(): void {
state.paramValue.conditions.push(createCondition());
debouncedEmitChange();

View file

@ -8,6 +8,7 @@ export const DEFAULT_FILTER_OPTIONS: FilterOptionsValue = {
caseSensitive: true,
leftValue: '',
typeValidation: 'strict',
version: 1,
};
export const OPERATORS_BY_ID = {

View file

@ -13,13 +13,14 @@ export class Filter extends VersionedNodeType {
iconColor: 'light-blue',
group: ['transform'],
description: 'Remove items matching a condition',
defaultVersion: 2.1,
defaultVersion: 2.2,
};
const nodeVersions: IVersionedNodeType['nodeVersions'] = {
1: new FilterV1(baseDescription),
2: new FilterV2(baseDescription),
2.1: new FilterV2(baseDescription),
2.2: new FilterV2(baseDescription),
};
super(nodeVersions, baseDescription);

View file

@ -19,7 +19,7 @@ export class FilterV2 implements INodeType {
constructor(baseDescription: INodeTypeBaseDescription) {
this.description = {
...baseDescription,
version: [2, 2.1],
version: [2, 2.1, 2.2],
defaults: {
name: 'Filter',
color: '#229eff',
@ -39,6 +39,7 @@ export class FilterV2 implements INodeType {
filter: {
caseSensitive: '={{!$parameter.options.ignoreCase}}',
typeValidation: getTypeValidationStrictness(2.1),
version: '={{ $nodeVersion >= 2.2 ? 2 : 1 }}',
},
},
},

View file

@ -13,13 +13,14 @@ export class If extends VersionedNodeType {
iconColor: 'green',
group: ['transform'],
description: 'Route items to different branches (true/false)',
defaultVersion: 2.1,
defaultVersion: 2.2,
};
const nodeVersions: IVersionedNodeType['nodeVersions'] = {
1: new IfV1(baseDescription),
2: new IfV2(baseDescription),
2.1: new IfV2(baseDescription),
2.2: new IfV2(baseDescription),
};
super(nodeVersions, baseDescription);

View file

@ -19,7 +19,7 @@ export class IfV2 implements INodeType {
constructor(baseDescription: INodeTypeBaseDescription) {
this.description = {
...baseDescription,
version: [2, 2.1],
version: [2, 2.1, 2.2],
defaults: {
name: 'If',
color: '#408000',
@ -39,6 +39,7 @@ export class IfV2 implements INodeType {
filter: {
caseSensitive: '={{!$parameter.options.ignoreCase}}',
typeValidation: getTypeValidationStrictness(2.1),
version: '={{ $nodeVersion >= 2.2 ? 2 : 1 }}',
},
},
},

View file

@ -0,0 +1,174 @@
{
"name": "Filter test: boolean",
"nodes": [
{
"parameters": {},
"id": "9e2c2dc5-bd37-460b-a5a4-943860dcc03e",
"name": "When clicking \"Execute Workflow\"",
"type": "n8n-nodes-base.manualTrigger",
"typeVersion": 1,
"position": [
-720,
160
]
},
{
"parameters": {},
"id": "3184fda2-b1d0-400a-a882-5844bbe99ae3",
"name": "false",
"type": "n8n-nodes-base.noOp",
"typeVersion": 1,
"position": [
0,
260
]
},
{
"parameters": {
"jsCode": "return [\n {\n email: \"shane@yahoo.com\",\n admin: false\n },\n {\n email: \"sharon@yahoo.com\",\n admin: true\n },\n {\n email: \"sarah@gmail.com\",\n admin: 'false'\n },\n {\n email: \"tom@gmail.com\",\n admin: '0'\n },\n {\n email: \"jane@gmail.com\",\n admin: 1\n }\n]"
},
"id": "85de5f5c-0a4c-4da1-805b-9e056089bcd5",
"name": "Code",
"type": "n8n-nodes-base.code",
"typeVersion": 2,
"position": [
-500,
160
]
},
{
"parameters": {},
"id": "8577ab3b-b9f8-4c4d-a3a1-abb6a9269473",
"name": "true",
"type": "n8n-nodes-base.noOp",
"typeVersion": 1,
"position": [
0,
100
]
},
{
"parameters": {
"conditions": {
"options": {
"caseSensitive": true,
"leftValue": "",
"typeValidation": "loose",
"version": 2
},
"conditions": [
{
"id": "307e4ea0-3a82-4722-aca6-68d882115e8b",
"leftValue": "={{ $json.admin }}",
"rightValue": "",
"operator": {
"type": "boolean",
"operation": "true",
"singleValue": true
}
}
],
"combinator": "and"
},
"looseTypeValidation": true,
"options": {}
},
"type": "n8n-nodes-base.if",
"typeVersion": 2.2,
"position": [
-280,
160
],
"id": "d5d17556-45e6-44a1-8580-a08395ca38c4",
"name": "loose"
}
],
"pinData": {
"true": [
{
"json": {
"email": "sharon@yahoo.com",
"admin": true
}
},
{
"json": {
"email": "jane@gmail.com",
"admin": 1
}
}
],
"false": [
{
"json": {
"email": "shane@yahoo.com",
"admin": false
}
},
{
"json": {
"email": "sarah@gmail.com",
"admin": "false"
}
},
{
"json": {
"email": "tom@gmail.com",
"admin": "0"
}
}
]
},
"connections": {
"When clicking \"Execute Workflow\"": {
"main": [
[
{
"node": "Code",
"type": "main",
"index": 0
}
]
]
},
"Code": {
"main": [
[
{
"node": "loose",
"type": "main",
"index": 0
}
]
]
},
"loose": {
"main": [
[
{
"node": "true",
"type": "main",
"index": 0
}
],
[
{
"node": "false",
"type": "main",
"index": 0
}
]
]
}
},
"active": false,
"settings": {
"executionOrder": "v1"
},
"versionId": "35631b37-dc5e-4155-a54f-41b38584f38e",
"meta": {
"instanceId": "27cc9b56542ad45b38725555722c50a1c3fee1670bbb67980558314ee08517c4"
},
"id": "JQsdJ4gnZtuDb7Oo",
"tags": []
}

View file

@ -14,7 +14,7 @@ export class Switch extends VersionedNodeType {
iconColor: 'light-blue',
group: ['transform'],
description: 'Route items depending on defined expression or rules',
defaultVersion: 3.1,
defaultVersion: 3.2,
};
const nodeVersions: IVersionedNodeType['nodeVersions'] = {
@ -22,6 +22,7 @@ export class Switch extends VersionedNodeType {
2: new SwitchV2(baseDescription),
3: new SwitchV3(baseDescription),
3.1: new SwitchV3(baseDescription),
3.2: new SwitchV3(baseDescription),
};
super(nodeVersions, baseDescription);

View file

@ -50,7 +50,7 @@ export class SwitchV3 implements INodeType {
this.description = {
...baseDescription,
subtitle: `=mode: {{(${capitalize})($parameter["mode"])}}`,
version: [3, 3.1],
version: [3, 3.1, 3.2],
defaults: {
name: 'Switch',
color: '#506000',
@ -160,6 +160,7 @@ export class SwitchV3 implements INodeType {
filter: {
caseSensitive: '={{!$parameter.options.ignoreCase}}',
typeValidation: getTypeValidationStrictness(3.1),
version: '={{ $nodeVersion >= 3.2 ? 2 : 1 }}',
},
},
},

View file

@ -4,4 +4,4 @@ export const NODE_RAN_MULTIPLE_TIMES_WARNING =
export const LOCALHOST = '127.0.0.1';
export const ENABLE_LESS_STRICT_TYPE_VALIDATION =
"Try changing the type of comparison. Alternatively you can enable 'Less Strict Type Validation' in the options.";
"Try changing the type of comparison. Alternatively you can enable 'Convert Value Types'.";

View file

@ -34,7 +34,7 @@ export const returnAllOrLimit: INodeProperties[] = [
];
export const looseTypeValidationProperty: INodeProperties = {
displayName: 'Less Strict Type Validation',
displayName: 'Convert Value Types',
description: 'Whether to try casting value types based on the selected operator',
name: 'looseTypeValidation',
type: 'boolean',

View file

@ -1292,13 +1292,14 @@ type NonEmptyArray<T> = [T, ...T[]];
export type FilterTypeCombinator = 'and' | 'or';
export type FilterTypeOptions = Partial<{
caseSensitive: boolean | string; // default = true
leftValue: string; // when set, user can't edit left side of condition
allowedCombinators: NonEmptyArray<FilterTypeCombinator>; // default = ['and', 'or']
maxConditions: number; // default = 10
typeValidation: 'strict' | 'loose' | {}; // default = strict, `| {}` is a TypeScript trick to allow custom strings, but still give autocomplete
}>;
export type FilterTypeOptions = {
version: 1 | 2 | {}; // required so nodes are pinned on a version
caseSensitive?: boolean | string; // default = true
leftValue?: string; // when set, user can't edit left side of condition
allowedCombinators?: NonEmptyArray<FilterTypeCombinator>; // default = ['and', 'or']
maxConditions?: number; // default = 10
typeValidation?: 'strict' | 'loose' | {}; // default = strict, `| {}` is a TypeScript trick to allow custom strings (expressions), but still give autocomplete
};
export type AssignmentTypeOptions = Partial<{
hideType?: boolean; // visible by default
@ -2554,6 +2555,7 @@ export type FilterOptionsValue = {
caseSensitive: boolean;
leftValue: string;
typeValidation: 'strict' | 'loose';
version: 1 | 2;
};
export type FilterValue = {

View file

@ -32,12 +32,18 @@ function parseSingleFilterValue(
value: unknown,
type: FilterOperatorType,
strict = false,
version: FilterOptionsValue['version'] = 1,
): ValidationResult {
if (type === 'any' || value === null || value === undefined) {
return { valid: true, newValue: value } as ValidationResult;
}
if (type === 'boolean' && !strict) {
if (version >= 2) {
const result = validateFieldType('filter', value, type);
if (result.valid) return result;
}
return { valid: true, newValue: Boolean(value) };
}
@ -53,6 +59,7 @@ const withIndefiniteArticle = (noun: string): string => {
return `${article} ${noun}`;
};
// eslint-disable-next-line complexity
function parseFilterConditionValues(
condition: FilterConditionValue,
options: FilterOptionsValue,
@ -62,10 +69,16 @@ function parseFilterConditionValues(
const itemIndex = metadata.itemIndex ?? 0;
const errorFormat = metadata.errorFormat ?? 'full';
const strict = options.typeValidation === 'strict';
const version = options.version ?? 1;
const { operator } = condition;
const rightType = operator.rightType ?? operator.type;
const parsedLeftValue = parseSingleFilterValue(condition.leftValue, operator.type, strict);
const parsedRightValue = parseSingleFilterValue(condition.rightValue, rightType, strict);
const parsedLeftValue = parseSingleFilterValue(
condition.leftValue,
operator.type,
strict,
version,
);
const parsedRightValue = parseSingleFilterValue(condition.rightValue, rightType, strict, version);
const leftValid =
parsedLeftValue.valid ||
(metadata.unresolvedExpressions &&
@ -96,7 +109,7 @@ function parseFilterConditionValues(
const getTypeDescription = (isStrict: boolean) => {
if (isStrict)
return 'Try changing the type of the comparison, or enabling less strict type validation.';
return "Try changing the type of comparison. Alternatively you can enable 'Convert Value Types'.";
return 'Try changing the type of the comparison.';
};
@ -122,7 +135,7 @@ function parseFilterConditionValues(
return `
<p>Try either:</p>
<ol>
<li>Enabling less strict type validation</li>
<li>Enabling 'Convert Value Types'</li>
<li>Converting the ${valuePosition} field to ${expectedType}${suggestFunction}</li>
</ol>
`;

View file

@ -19,6 +19,7 @@ const filterFactory = (data: DeepPartial<FilterValue> = {}): FilterValue =>
combinator: 'and',
conditions: [],
options: {
version: 1,
leftValue: '',
caseSensitive: false,
typeValidation: 'strict',
@ -234,6 +235,48 @@ describe('FilterParameter', () => {
});
});
describe('options.version', () => {
describe('version 1', () => {
it('should parse "false" as true', () => {
expect(
executeFilter(
filterFactory({
conditions: [
{
id: '1',
leftValue: 'false',
rightValue: false,
operator: { operation: 'equals', type: 'boolean' },
},
],
options: { typeValidation: 'loose', version: 1 },
}),
),
).toEqual(false);
});
});
describe('version 2', () => {
it('should parse "false" as false', () => {
expect(
executeFilter(
filterFactory({
conditions: [
{
id: '1',
leftValue: 'false',
rightValue: false,
operator: { operation: 'equals', type: 'boolean' },
},
],
options: { typeValidation: 'loose', version: 2 },
}),
),
).toEqual(true);
});
});
});
describe('operators', () => {
describe('exists', () => {
it.each([