fix: Filter component - improve errors (#10456)
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

This commit is contained in:
Michael Kret 2024-08-19 19:01:33 +03:00 committed by GitHub
parent f784a4c95a
commit 61ac0c7775
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 157 additions and 39 deletions

View file

@ -216,17 +216,13 @@ function getErrorDescription(): string {
function addItemIndexSuffix(message: string): string { function addItemIndexSuffix(message: string): string {
let itemIndexSuffix = ''; let itemIndexSuffix = '';
const ITEM_INDEX_SUFFIX_TEXT = '[item '; if (hasManyInputItems.value && props.error?.context?.itemIndex !== undefined) {
itemIndexSuffix = `item ${props.error.context.itemIndex}`;
if (
hasManyInputItems.value &&
!message.includes(ITEM_INDEX_SUFFIX_TEXT) &&
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 { function getErrorMessage(): string {

View file

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

View file

@ -9,6 +9,8 @@ import {
type INodeTypeDescription, type INodeTypeDescription,
} from 'n8n-workflow'; } from 'n8n-workflow';
import { ENABLE_LESS_STRICT_TYPE_VALIDATION } from '../../../utils/constants'; 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 { export class FilterV2 implements INodeType {
description: INodeTypeDescription; description: INodeTypeDescription;
@ -16,7 +18,7 @@ export class FilterV2 implements INodeType {
constructor(baseDescription: INodeTypeBaseDescription) { constructor(baseDescription: INodeTypeBaseDescription) {
this.description = { this.description = {
...baseDescription, ...baseDescription,
version: 2, version: [2, 2.1],
defaults: { defaults: {
name: 'Filter', name: 'Filter',
color: '#229eff', color: '#229eff',
@ -35,7 +37,16 @@ export class FilterV2 implements INodeType {
typeOptions: { typeOptions: {
filter: { filter: {
caseSensitive: '={{!$parameter.options.ignoreCase}}', 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, default: true,
}, },
{ {
displayName: 'Less Strict Type Validation', ...looseTypeValidationProperty,
description: 'Whether to try casting value types based on the selected operator', displayOptions: {
name: 'looseTypeValidation', show: {
type: 'boolean', '@version': [{ _cnd: { lt: 2.1 } }],
default: true, },
},
}, },
], ],
}, },
@ -82,7 +94,10 @@ export class FilterV2 implements INodeType {
extractValue: true, extractValue: true,
}) as boolean; }) as boolean;
} catch (error) { } 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, 'description', ENABLE_LESS_STRICT_TYPE_VALIDATION);
} }
set(error, 'context.itemIndex', itemIndex); set(error, 'context.itemIndex', itemIndex);

View file

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

View file

@ -9,6 +9,8 @@ import {
type INodeTypeDescription, type INodeTypeDescription,
} from 'n8n-workflow'; } from 'n8n-workflow';
import { ENABLE_LESS_STRICT_TYPE_VALIDATION } from '../../../utils/constants'; import { ENABLE_LESS_STRICT_TYPE_VALIDATION } from '../../../utils/constants';
import { looseTypeValidationProperty } from '../../../utils/descriptions';
import { getTypeValidationParameter, getTypeValidationStrictness } from './utils';
export class IfV2 implements INodeType { export class IfV2 implements INodeType {
description: INodeTypeDescription; description: INodeTypeDescription;
@ -16,7 +18,7 @@ export class IfV2 implements INodeType {
constructor(baseDescription: INodeTypeBaseDescription) { constructor(baseDescription: INodeTypeBaseDescription) {
this.description = { this.description = {
...baseDescription, ...baseDescription,
version: 2, version: [2, 2.1],
defaults: { defaults: {
name: 'If', name: 'If',
color: '#408000', color: '#408000',
@ -35,7 +37,16 @@ export class IfV2 implements INodeType {
typeOptions: { typeOptions: {
filter: { filter: {
caseSensitive: '={{!$parameter.options.ignoreCase}}', 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, default: true,
}, },
{ {
displayName: 'Less Strict Type Validation', ...looseTypeValidationProperty,
description: 'Whether to try casting value types based on the selected operator', displayOptions: {
name: 'looseTypeValidation', show: {
type: 'boolean', '@version': [{ _cnd: { lt: 2.1 } }],
default: true, },
},
}, },
], ],
}, },
@ -82,7 +94,10 @@ export class IfV2 implements INodeType {
extractValue: true, extractValue: true,
}) as boolean; }) as boolean;
} catch (error) { } 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, 'description', ENABLE_LESS_STRICT_TYPE_VALIDATION);
} }
set(error, 'context.itemIndex', itemIndex); set(error, 'context.itemIndex', itemIndex);

View file

@ -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;
}
};
};

View file

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

View file

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

View file

@ -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[] = [ export const encodeDecodeOptions: INodePropertyOptions[] = [
{ {
name: 'armscii8', name: 'armscii8',

View file

@ -94,14 +94,61 @@ function parseFilterConditionValues(
)} ${suffix}`; )} ${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) { if (!leftValid) {
return { return {
ok: false, ok: false,
error: new FilterError( error: new FilterError(
composeInvalidTypeMessage(operator.type, typeof condition.leftValue, leftValueString), composeInvalidTypeMessage(operator.type, typeof condition.leftValue, leftValueString),
invalidTypeDescription, composeInvalidTypeDescription(operator.type, typeof condition.leftValue, 'first'),
), ),
}; };
} }
@ -111,7 +158,7 @@ function parseFilterConditionValues(
ok: false, ok: false,
error: new FilterError( error: new FilterError(
composeInvalidTypeMessage(rightType, typeof condition.rightValue, rightValueString), composeInvalidTypeMessage(rightType, typeof condition.rightValue, rightValueString),
invalidTypeDescription, composeInvalidTypeDescription(rightType, typeof condition.rightValue, 'second'),
), ),
}; };
} }