fix: Filter component - improve errors (#10456)

n8n-io · Aug 19, 2024 · 61ac0c7 · 61ac0c7
1 parent f784a4c
commit 61ac0c7
Show file tree

Hide file tree

Showing 10 changed files with 157 additions and 39 deletions.
diff --git a/packages/editor-ui/src/components/Error/NodeErrorView.vue b/packages/editor-ui/src/components/Error/NodeErrorView.vue
@@ -216,17 +216,13 @@ function getErrorDescription(): string {
 function addItemIndexSuffix(message: string): string {
 	let itemIndexSuffix = '';
 
-	const ITEM_INDEX_SUFFIX_TEXT = '[item ';
-
-	if (
-		hasManyInputItems.value &&
-		!message.includes(ITEM_INDEX_SUFFIX_TEXT) &&
-		props.error?.context?.itemIndex !== undefined
-	) {
-		itemIndexSuffix = ` [item ${props.error.context.itemIndex}]`;
+	if (hasManyInputItems.value && props.error?.context?.itemIndex !== undefined) {
+		itemIndexSuffix = `item ${props.error.context.itemIndex}`;
 	}
 
-	return message + itemIndexSuffix;
+	if (message.includes(itemIndexSuffix)) return message;
+
+	return `${message} [${itemIndexSuffix}]`;
 }
 
 function getErrorMessage(): string {

diff --git a/packages/nodes-base/nodes/Filter/Filter.node.ts b/packages/nodes-base/nodes/Filter/Filter.node.ts
@@ -13,11 +13,13 @@ export class Filter extends VersionedNodeType {
 			iconColor: 'light-blue',
 			group: ['transform'],
 			description: 'Remove items matching a condition',
+			defaultVersion: 2.1,
 		};
 
 		const nodeVersions: IVersionedNodeType['nodeVersions'] = {
 			1: new FilterV1(baseDescription),
 			2: new FilterV2(baseDescription),
+			2.1: new FilterV2(baseDescription),
 		};
 
 		super(nodeVersions, baseDescription);

diff --git a/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts b/packages/nodes-base/nodes/Filter/V2/FilterV2.node.ts
@@ -9,14 +9,16 @@ import {
 	type INodeTypeDescription,
 } from 'n8n-workflow';
 import { ENABLE_LESS_STRICT_TYPE_VALIDATION } from '../../../utils/constants';
+import { getTypeValidationParameter, getTypeValidationStrictness } from '../../If/V2/utils';
+import { looseTypeValidationProperty } from '../../../utils/descriptions';
 
 export class FilterV2 implements INodeType {
 	description: INodeTypeDescription;
 
 	constructor(baseDescription: INodeTypeBaseDescription) {
 		this.description = {
 			...baseDescription,
-			version: 2,
+			version: [2, 2.1],
 			defaults: {
 				name: 'Filter',
 				color: '#229eff',
@@ -35,7 +37,16 @@ export class FilterV2 implements INodeType {
 					typeOptions: {
 						filter: {
 							caseSensitive: '={{!$parameter.options.ignoreCase}}',
-							typeValidation: '={{$parameter.options.looseTypeValidation ? "loose" : "strict"}}',
+							typeValidation: getTypeValidationStrictness(2.1),
+						},
+					},
+				},
+				{
+					...looseTypeValidationProperty,
+					default: false,
+					displayOptions: {
+						show: {
+							'@version': [{ _cnd: { gte: 2.1 } }],
 						},
 					},
 				},
@@ -54,11 +65,12 @@ export class FilterV2 implements INodeType {
 							default: true,
 						},
 						{
-							displayName: 'Less Strict Type Validation',
-							description: 'Whether to try casting value types based on the selected operator',
-							name: 'looseTypeValidation',
-							type: 'boolean',
-							default: true,
+							...looseTypeValidationProperty,
+							displayOptions: {
+								show: {
+									'@version': [{ _cnd: { lt: 2.1 } }],
+								},
+							},
 						},
 					],
 				},
@@ -82,7 +94,10 @@ export class FilterV2 implements INodeType {
 						extractValue: true,
 					}) as boolean;
 				} catch (error) {
-					if (!options.looseTypeValidation && !error.description) {
+					if (
+						!getTypeValidationParameter(2.1)(this, itemIndex, options.looseTypeValidation) &&
+						!error.description
+					) {
 						set(error, 'description', ENABLE_LESS_STRICT_TYPE_VALIDATION);
 					}
 					set(error, 'context.itemIndex', itemIndex);

diff --git a/packages/nodes-base/nodes/If/If.node.ts b/packages/nodes-base/nodes/If/If.node.ts
@@ -13,12 +13,13 @@ export class If extends VersionedNodeType {
 			iconColor: 'green',
 			group: ['transform'],
 			description: 'Route items to different branches (true/false)',
-			defaultVersion: 2,
+			defaultVersion: 2.1,
 		};
 
 		const nodeVersions: IVersionedNodeType['nodeVersions'] = {
 			1: new IfV1(baseDescription),
 			2: new IfV2(baseDescription),
+			2.1: new IfV2(baseDescription),
 		};
 
 		super(nodeVersions, baseDescription);

diff --git a/packages/nodes-base/nodes/If/V2/IfV2.node.ts b/packages/nodes-base/nodes/If/V2/IfV2.node.ts
@@ -9,14 +9,16 @@ import {
 	type INodeTypeDescription,
 } from 'n8n-workflow';
 import { ENABLE_LESS_STRICT_TYPE_VALIDATION } from '../../../utils/constants';
+import { looseTypeValidationProperty } from '../../../utils/descriptions';
+import { getTypeValidationParameter, getTypeValidationStrictness } from './utils';
 
 export class IfV2 implements INodeType {
 	description: INodeTypeDescription;
 
 	constructor(baseDescription: INodeTypeBaseDescription) {
 		this.description = {
 			...baseDescription,
-			version: 2,
+			version: [2, 2.1],
 			defaults: {
 				name: 'If',
 				color: '#408000',
@@ -35,7 +37,16 @@ export class IfV2 implements INodeType {
 					typeOptions: {
 						filter: {
 							caseSensitive: '={{!$parameter.options.ignoreCase}}',
-							typeValidation: '={{$parameter.options.looseTypeValidation ? "loose" : "strict"}}',
+							typeValidation: getTypeValidationStrictness(2.1),
+						},
+					},
+				},
+				{
+					...looseTypeValidationProperty,
+					default: false,
+					displayOptions: {
+						show: {
+							'@version': [{ _cnd: { gte: 2.1 } }],
 						},
 					},
 				},
@@ -54,11 +65,12 @@ export class IfV2 implements INodeType {
 							default: true,
 						},
 						{
-							displayName: 'Less Strict Type Validation',
-							description: 'Whether to try casting value types based on the selected operator',
-							name: 'looseTypeValidation',
-							type: 'boolean',
-							default: true,
+							...looseTypeValidationProperty,
+							displayOptions: {
+								show: {
+									'@version': [{ _cnd: { lt: 2.1 } }],
+								},
+							},
 						},
 					],
 				},
@@ -82,7 +94,10 @@ export class IfV2 implements INodeType {
 						extractValue: true,
 					}) as boolean;
 				} catch (error) {
-					if (!options.looseTypeValidation && !error.description) {
+					if (
+						!getTypeValidationParameter(2.1)(this, itemIndex, options.looseTypeValidation) &&
+						!error.description
+					) {
 						set(error, 'description', ENABLE_LESS_STRICT_TYPE_VALIDATION);
 					}
 					set(error, 'context.itemIndex', itemIndex);

diff --git a/packages/nodes-base/nodes/If/V2/utils.ts b/packages/nodes-base/nodes/If/V2/utils.ts
@@ -0,0 +1,15 @@
+import type { IExecuteFunctions } from 'n8n-workflow';
+
+export const getTypeValidationStrictness = (version: number) => {
+	return `={{ ($nodeVersion < ${version} ? $parameter.options.looseTypeValidation :  $parameter.looseTypeValidation) ? "loose" : "strict" }}`;
+};
+
+export const getTypeValidationParameter = (version: number) => {
+	return (context: IExecuteFunctions, itemIndex: number, option: boolean | undefined) => {
+		if (context.getNode().typeVersion < version) {
+			return option;
+		} else {
+			return context.getNodeParameter('looseTypeValidation', itemIndex, false) as boolean;
+		}
+	};
+};
diff --git a/packages/nodes-base/nodes/Switch/Switch.node.ts b/packages/nodes-base/nodes/Switch/Switch.node.ts
@@ -14,13 +14,14 @@ export class Switch extends VersionedNodeType {
 			iconColor: 'light-blue',
 			group: ['transform'],
 			description: 'Route items depending on defined expression or rules',
-			defaultVersion: 3,
+			defaultVersion: 3.1,
 		};
 
 		const nodeVersions: IVersionedNodeType['nodeVersions'] = {
 			1: new SwitchV1(baseDescription),
 			2: new SwitchV2(baseDescription),
 			3: new SwitchV3(baseDescription),
+			3.1: new SwitchV3(baseDescription),
 		};
 
 		super(nodeVersions, baseDescription);

diff --git a/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts b/packages/nodes-base/nodes/Switch/V3/SwitchV3.node.ts
@@ -13,6 +13,8 @@ import { ApplicationError, NodeConnectionType, NodeOperationError } from 'n8n-wo
 import set from 'lodash/set';
 import { capitalize } from '@utils/utilities';
 import { ENABLE_LESS_STRICT_TYPE_VALIDATION } from '../../../utils/constants';
+import { looseTypeValidationProperty } from '../../../utils/descriptions';
+import { getTypeValidationParameter, getTypeValidationStrictness } from '../../If/V2/utils';
 
 const configuredOutputs = (parameters: INodeParameters) => {
 	const mode = parameters.mode as string;
@@ -48,7 +50,7 @@ export class SwitchV3 implements INodeType {
 		this.description = {
 			...baseDescription,
 			subtitle: `=mode: {{(${capitalize})($parameter["mode"])}}`,
-			version: [3],
+			version: [3, 3.1],
 			defaults: {
 				name: 'Switch',
 				color: '#506000',
@@ -157,8 +159,7 @@ export class SwitchV3 implements INodeType {
 										multipleValues: false,
 										filter: {
 											caseSensitive: '={{!$parameter.options.ignoreCase}}',
-											typeValidation:
-												'={{$parameter.options.looseTypeValidation ? "loose" : "strict"}}',
+											typeValidation: getTypeValidationStrictness(3.1),
 										},
 									},
 								},
@@ -184,6 +185,15 @@ export class SwitchV3 implements INodeType {
 						},
 					],
 				},
+				{
+					...looseTypeValidationProperty,
+					default: false,
+					displayOptions: {
+						show: {
+							'@version': [{ _cnd: { gte: 3.1 } }],
+						},
+					},
+				},
 				{
 					displayName: 'Options',
 					name: 'options',
@@ -218,11 +228,12 @@ export class SwitchV3 implements INodeType {
 							default: true,
 						},
 						{
-							displayName: 'Less Strict Type Validation',
-							description: 'Whether to try casting value types based on the selected operator',
-							name: 'looseTypeValidation',
-							type: 'boolean',
-							default: true,
+							...looseTypeValidationProperty,
+							displayOptions: {
+								show: {
+									'@version': [{ _cnd: { lt: 3.1 } }],
+								},
+							},
 						},
 						{
 							displayName: 'Rename Fallback Output',
@@ -349,7 +360,14 @@ export class SwitchV3 implements INodeType {
 								},
 							) as boolean;
 						} catch (error) {
-							if (!options.looseTypeValidation && !error.description) {
+							if (
+								!getTypeValidationParameter(3.1)(
+									this,
+									itemIndex,
+									options.looseTypeValidation as boolean,
+								) &&
+								!error.description
+							) {
 								error.description = ENABLE_LESS_STRICT_TYPE_VALIDATION;
 							}
 							set(error, 'context.itemIndex', itemIndex);

diff --git a/packages/nodes-base/utils/descriptions.ts b/packages/nodes-base/utils/descriptions.ts
@@ -33,6 +33,14 @@ export const returnAllOrLimit: INodeProperties[] = [
 	},
 ];
 
+export const looseTypeValidationProperty: INodeProperties = {
+	displayName: 'Less Strict Type Validation',
+	description: 'Whether to try casting value types based on the selected operator',
+	name: 'looseTypeValidation',
+	type: 'boolean',
+	default: true,
+};
+
 export const encodeDecodeOptions: INodePropertyOptions[] = [
 	{
 		name: 'armscii8',

diff --git a/packages/workflow/src/NodeParameters/FilterParameter.ts b/packages/workflow/src/NodeParameters/FilterParameter.ts
@@ -94,14 +94,61 @@ function parseFilterConditionValues(
 		)} ${suffix}`;
 	};
 
-	const invalidTypeDescription = 'Try changing the type of comparison.';
+	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 the comparison.';
+	};
+
+	const composeInvalidTypeDescription = (
+		type: string,
+		fromType: string,
+		valuePosition: 'first' | 'second',
+	) => {
+		fromType = fromType.toLocaleLowerCase();
+		const expectedType = withIndefiniteArticle(type);
+
+		let convertionFunction = '';
+		if (type === 'string') {
+			convertionFunction = '.toString()';
+		} else if (type === 'number') {
+			convertionFunction = '.toNumber()';
+		} else if (type === 'boolean') {
+			convertionFunction = '.toBoolean()';
+		}
+
+		if (strict && convertionFunction) {
+			const suggestFunction = ` by adding <code>${convertionFunction}</code>`;
+			return `
+<p>Try either:</p>
+<ol>
+  <li>Enabling less strict type validation</li>
+  <li>Converting the ${valuePosition} field to ${expectedType}${suggestFunction}</li>
+</ol>
+			`;
+		}
+
+		return getTypeDescription(strict);
+	};
+
+	if (!leftValid && !rightValid && typeof condition.leftValue === typeof condition.rightValue) {
+		return {
+			ok: false,
+			error: new FilterError(
+				`Comparison type expects ${withIndefiniteArticle(operator.type)} but both fields are ${withIndefiniteArticle(
+					typeof condition.leftValue,
+				)}`,
+				getTypeDescription(strict),
+			),
+		};
+	}
 
 	if (!leftValid) {
 		return {
 			ok: false,
 			error: new FilterError(
 				composeInvalidTypeMessage(operator.type, typeof condition.leftValue, leftValueString),
-				invalidTypeDescription,
+				composeInvalidTypeDescription(operator.type, typeof condition.leftValue, 'first'),
 			),
 		};
 	}
@@ -111,7 +158,7 @@ function parseFilterConditionValues(
 			ok: false,
 			error: new FilterError(
 				composeInvalidTypeMessage(rightType, typeof condition.rightValue, rightValueString),
-				invalidTypeDescription,
+				composeInvalidTypeDescription(rightType, typeof condition.rightValue, 'second'),
 			),
 		};
 	}