mudhorn/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2024-11-10 06:34:05 -08:00
Code Issues Actions Packages Projects Releases Wiki Activity

feat(core): Introduce simplified node versioning (#3205)

Browse source 
* ✨ Introduce simple node versioning

* ⚡ Add example how to read version in node-code for custom logic

* 🐛 Fix setting of parameters

* 🐛 Fix another instance where it sets the wrong parameter

* ⚡ Remove unnecessary TOODs

* ⚡ Revert Set Node example changes

* ;rotating_light: Add test
This commit is contained in:
Jan Oberhauser 2022-04-28 19:04:09 +02:00 committed by GitHub
parent 5e2589e626
commit d5b9b0cb95
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 277 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
packages/cli/src/CredentialsHelper.ts
View file

@ -342,6 +342,7 @@ export class CredentialsHelper extends ICredentialsHelper {
decryptedDataOriginal as INodeParameters,
true,
false,
null,
) as ICredentialDataDecryptedObject;
if (decryptedDataOriginal.oauthTokenData !== undefined) {
@ -436,8 +437,6 @@ export class CredentialsHelper extends ICredentialsHelper {
// Add special database related data
newCredentialsData.updatedAt = new Date();
// TODO: also add user automatically depending on who is logged in, if anybody is logged in
// Save the credentials in DB
const findQuery = {
id: credentials.id,
@ -562,7 +561,9 @@ export class CredentialsHelper extends ICredentialsHelper {
parameters: {},
name: 'Temp-Node',
type: nodeType.description.name,
typeVersion: nodeType.description.version,
typeVersion: Array.isArray(nodeType.description.version)
? nodeType.description.version.slice(-1)[0]
: nodeType.description.version,
position: [0, 0],
};

1
packages/core/src/NodeExecuteFunctions.ts
View file

@ -1298,6 +1298,7 @@ export async function getCredentials(
!NodeHelpers.displayParameter(
additionalData.currentNodeParameters || node.parameters,
nodeCredentialDescription,
node,
node.parameters,
)
) {

59
packages/core/test/Helpers.ts
View file

@ -517,6 +517,65 @@ class NodeTypesClass implements INodeTypes {
},
},
},
'n8n-nodes-base.versionTest': {
sourcePath: '',
type: {
description: {
displayName: 'Version Test',
name: 'versionTest',
group: ['input'],
version: 1,
description: 'Tests if versioning works',
defaults: {
name: 'Version Test',
color: '#0000FF',
},
inputs: ['main'],
outputs: ['main'],
properties: [
{
displayName: 'Display V1',
name: 'versionTest',
type: 'number',
displayOptions: {
show: {
'@version': [1],
},
},
default: 1,
},
{
displayName: 'Display V2',
name: 'versionTest',
type: 'number',
displayOptions: {
show: {
'@version': [2],
},
},
default: 2,
},
],
},
execute(this: IExecuteFunctions): Promise<INodeExecutionData[][]> {
const items = this.getInputData();
const returnData: INodeExecutionData[] = [];
for (let itemIndex = 0; itemIndex < items.length; itemIndex++) {
const newItem: INodeExecutionData = {
json: {
versionFromParameter: this.getNodeParameter('versionTest', itemIndex),
versionFromNode: this.getNode().typeVersion,
},
};
returnData.push(newItem);
}
return this.prepareOutputData(returnData);
},
},
},
'n8n-nodes-base.set': {
sourcePath: '',
type: {

139
packages/core/test/WorkflowExecute.test.ts
View file

@ -1169,6 +1169,145 @@ describe('WorkflowExecute', () => {
},
},
},
{
description:
'should display the correct parameters and so correct data when simplified node-versioning is used',
input: {
workflowData: {
nodes: [
{
parameters: {},
name: 'Start',
type: 'n8n-nodes-base.start',
typeVersion: 1,
position: [240, 300],
},
{
parameters: {},
name: 'VersionTest1a',
type: 'n8n-nodes-base.versionTest',
typeVersion: 1,
position: [460, 300],
},
{
parameters: {
versionTest: 11,
},
name: 'VersionTest1b',
type: 'n8n-nodes-base.versionTest',
typeVersion: 1,
position: [680, 300],
},
{
parameters: {},
name: 'VersionTest2a',
type: 'n8n-nodes-base.versionTest',
typeVersion: 2,
position: [880, 300],
},
{
parameters: {
versionTest: 22,
},
name: 'VersionTest2b',
type: 'n8n-nodes-base.versionTest',
typeVersion: 2,
position: [1080, 300],
},
],
connections: {
Start: {
main: [
[
{
node: 'VersionTest1a',
type: 'main',
index: 0,
},
],
],
},
VersionTest1a: {
main: [
[
{
node: 'VersionTest1b',
type: 'main',
index: 0,
},
],
],
},
VersionTest1b: {
main: [
[
{
node: 'VersionTest2a',
type: 'main',
index: 0,
},
],
],
},
VersionTest2a: {
main: [
[
{
node: 'VersionTest2b',
type: 'main',
index: 0,
},
],
],
},
},
},
},
output: {
nodeExecutionOrder: [
'Start',
'VersionTest1a',
'VersionTest1b',
'VersionTest2a',
'VersionTest2b',
],
nodeData: {
VersionTest1a: [
[
{
versionFromNode: 1,
versionFromParameter: 1,
},
],
],
VersionTest1b: [
[
{
versionFromNode: 1,
versionFromParameter: 11,
},
],
],
VersionTest2a: [
[
{
versionFromNode: 2,
versionFromParameter: 2,
},
],
],
VersionTest2b: [
[
{
versionFromNode: 2,
versionFromParameter: 22,
},
],
],
},
},
},
];
const fakeLogger = {

6
packages/editor-ui/src/components/CollectionParameter.vue
View file

@ -32,6 +32,7 @@
<script lang="ts">
import {
INodeUi,
IUpdateInformation,
} from '@/Interface';
@ -87,6 +88,9 @@ export default mixins(
return this.displayNodeParameter(option as INodeProperties);
});
},
node (): INodeUi {
return this.$store.getters.activeNode;
},
// Returns all the options which did not get added already
parameterOptions (): Array<INodePropertyOptions | INodeProperties> {
return (this.filteredOptions as Array<INodePropertyOptions | INodeProperties>).filter((option) => {
@ -127,7 +131,7 @@ export default mixins(
// If it is not defined no need to do a proper check
return true;
}
return this.displayParameter(this.nodeValues, parameter, this.path);
return this.displayParameter(this.nodeValues, parameter, this.path, this.node);
},
optionSelected (optionName: string) {
const options = this.getOptionProperties(optionName);

2
packages/editor-ui/src/components/CredentialEdit/CredentialEdit.vue
View file

@ -395,6 +395,7 @@ export default mixins(showMessage, nodeHelpers).extend({
this.credentialData as INodeParameters,
parameter,
'',
null,
);
},
getCredentialProperties(name: string): INodeProperties[] {
@ -598,6 +599,7 @@ export default mixins(showMessage, nodeHelpers).extend({
this.credentialData as INodeParameters,
false,
false,
null,
);
const credentialDetails: ICredentialsDecrypted = {

2
packages/editor-ui/src/components/NodeCredentials.vue
View file

@ -251,7 +251,7 @@ export default mixins(
// If it is not defined no need to do a proper check
return true;
}
return this.displayParameter(this.node.parameters, credentialTypeDescription, '');
return this.displayParameter(this.node.parameters, credentialTypeDescription, '', this.node);
},
getIssues (credentialTypeName: string): string[] {

4
packages/editor-ui/src/components/NodeSettings.vue
View file

@ -379,7 +379,7 @@ export default mixins(
}
// Get only the parameters which are different to the defaults
let nodeParameters = NodeHelpers.getNodeParameters(nodeType.properties, node.parameters, false, false);
let nodeParameters = NodeHelpers.getNodeParameters(nodeType.properties, node.parameters, false, false, node);
const oldNodeParameters = Object.assign({}, nodeParameters);
// Copy the data because it is the data of vuex so make sure that
@ -415,7 +415,7 @@ export default mixins(
// Get the parameters with the now new defaults according to the
// from the user actually defined parameters
nodeParameters = NodeHelpers.getNodeParameters(nodeType.properties, nodeParameters as INodeParameters, true, false);
nodeParameters = NodeHelpers.getNodeParameters(nodeType.properties, nodeParameters as INodeParameters, true, false, node);
for (const key of Object.keys(nodeParameters as object)) {
if (nodeParameters && nodeParameters[key] !== null && nodeParameters[key] !== undefined) {

2
packages/editor-ui/src/components/ParameterInput.vue
View file

@ -468,7 +468,7 @@ export default mixins(
const newPath = this.shortPath.split('.');
newPath.pop();
const issues = NodeHelpers.getParameterIssues(this.parameter, this.node.parameters, newPath.join('.'));
const issues = NodeHelpers.getParameterIssues(this.parameter, this.node.parameters, newPath.join('.'), this.node);
if (['options', 'multiOptions'].includes(this.parameter.type) && this.remoteParameterOptionsLoading === false && this.remoteParameterOptionsLoadingIssues === null) {
// Check if the value resolves to a valid option

11
packages/editor-ui/src/components/ParameterInputList.vue
View file

@ -89,7 +89,7 @@ import {
NodeParameterValue,
} from 'n8n-workflow';
import { IUpdateInformation } from '@/Interface';
import { INodeUi, IUpdateInformation } from '@/Interface';
import MultipleParameter from '@/components/MultipleParameter.vue';
import { genericHelpers } from '@/components/mixins/genericHelpers';
@ -124,6 +124,9 @@ export default mixins(
filteredParameterNames (): string[] {
return this.filteredParameters.map(parameter => parameter.name);
},
node (): INodeUi {
return this.$store.getters.activeNode;
},
},
methods: {
multipleValues (parameter: INodeProperties): boolean {
@ -213,13 +216,13 @@ export default mixins(
if (this.path) {
rawValues = JSON.parse(JSON.stringify(this.nodeValues));
set(rawValues, this.path, nodeValues);
return this.displayParameter(rawValues, parameter, this.path);
return this.displayParameter(rawValues, parameter, this.path, this.node);
} else {
return this.displayParameter(nodeValues, parameter, '');
return this.displayParameter(nodeValues, parameter, '', this.node);
}
}
return this.displayParameter(this.nodeValues, parameter, this.path);
return this.displayParameter(this.nodeValues, parameter, this.path, this.node);
},
valueChanged (parameterData: IUpdateInformation): void {
this.$emit('valueChanged', parameterData);

6
packages/editor-ui/src/components/mixins/nodeHelpers.ts
View file

@ -48,8 +48,8 @@ export const nodeHelpers = mixins(
},
// Returns if the given parameter should be displayed or not
displayParameter (nodeValues: INodeParameters, parameter: INodeProperties | INodeCredentialDescription, path: string) {
return NodeHelpers.displayParameterPath(nodeValues, parameter, path);
displayParameter (nodeValues: INodeParameters, parameter: INodeProperties | INodeCredentialDescription, path: string, node: INodeUi | null) {
return NodeHelpers.displayParameterPath(nodeValues, parameter, path, node);
},
// Returns all the issues of the node
@ -200,7 +200,7 @@ export const nodeHelpers = mixins(
let selectedCredentials: INodeCredentialsDetails;
for (const credentialTypeDescription of nodeType!.credentials!) {
// Check if credentials should be displayed else ignore
if (this.displayParameter(node.parameters, credentialTypeDescription, '') !== true) {
if (this.displayParameter(node.parameters, credentialTypeDescription, '', node) !== true) {
continue;
}

4
packages/editor-ui/src/components/mixins/workflowHelpers.ts
View file

@ -323,7 +323,7 @@ export const workflowHelpers = mixins(
if (nodeType !== null) {
// Node-Type is known so we can save the parameters correctly
const nodeParameters = NodeHelpers.getNodeParameters(nodeType.properties, node.parameters, false, false);
const nodeParameters = NodeHelpers.getNodeParameters(nodeType.properties, node.parameters, false, false, node);
nodeData.parameters = nodeParameters !== null ? nodeParameters : {};
// Add the node credentials if there are some set and if they should be displayed
@ -338,7 +338,7 @@ export const workflowHelpers = mixins(
continue;
}
if (this.displayParameter(node.parameters, credentialTypeDescription, '') === false) {
if (this.displayParameter(node.parameters, credentialTypeDescription, '', node) === false) {
// Credential should not be displayed so do also not save
continue;
}

8
packages/editor-ui/src/store.ts
View file

@ -645,7 +645,7 @@ export const store = new Vuex.Store({
},
updateNodeTypes (state, nodeTypes: INodeTypeDescription[]) {
const oldNodesToKeep = state.nodeTypes.filter(node => !nodeTypes.find(n => n.name === node.name && n.version === node.version));
const oldNodesToKeep = state.nodeTypes.filter(node => !nodeTypes.find(n => n.name === node.name && n.version.toString() === node.version.toString()));
const newNodesState = [...oldNodesToKeep, ...nodeTypes];
Vue.set(state, 'nodeTypes', newNodesState);
state.nodeTypes = newNodesState;
@ -852,7 +852,11 @@ export const store = new Vuex.Store({
nodeType: (state, getters) => (nodeType: string, typeVersion?: number): INodeTypeDescription | null => {
const foundType = state.nodeTypes.find(typeData => {
return typeData.name === nodeType && typeData.version === (typeVersion || typeData.defaultVersion || DEFAULT_NODETYPE_VERSION);
const typeVersion = Array.isArray(typeData.version)
? typeData.version
: [typeData.version];
return typeData.name === nodeType && typeVersion.some(versions => [typeVersion, typeData.defaultVersion, DEFAULT_NODETYPE_VERSION].includes(versions));
});
if (foundType === undefined) {

13
packages/editor-ui/src/views/NodeView.vue
View file

@ -1356,7 +1356,9 @@ export default mixins(
const newNodeData: INodeUi = {
name: nodeTypeData.defaults.name as string,
type: nodeTypeData.name,
typeVersion: nodeTypeData.version,
typeVersion: Array.isArray(nodeTypeData.version)
? nodeTypeData.version.slice(-1)[0]
: nodeTypeData.version,
position: [0, 0],
parameters: {},
};
@ -2445,7 +2447,7 @@ export default mixins(
if (nodeType !== null) {
let nodeParameters = null;
try {
nodeParameters = NodeHelpers.getNodeParameters(nodeType.properties, node.parameters, true, false);
nodeParameters = NodeHelpers.getNodeParameters(nodeType.properties, node.parameters, true, false, node);
} catch (e) {
console.error(this.$locale.baseText('nodeView.thereWasAProblemLoadingTheNodeParametersOfNode') + `: "${node.name}"`); // eslint-disable-line no-console
console.error(e); // eslint-disable-line no-console
@ -2753,10 +2755,13 @@ export default mixins(
const nodesToBeFetched:INodeTypeNameVersion[] = [];
allNodes.forEach(node => {
if(!!nodeInfos.find(n => n.name === node.name && n.version === node.version) && !node.hasOwnProperty('properties')) {
const nodeVersions = Array.isArray(node.version) ? node.version : [node.version];
if(!!nodeInfos.find(n => n.name === node.name && nodeVersions.includes(n.version)) && !node.hasOwnProperty('properties')) {
nodesToBeFetched.push({
name: node.name,
version: node.version,
version: Array.isArray(node.version)
? node.version.slice(-1)[0]
: node.version,
});
}
});

2
packages/workflow/src/Interfaces.ts
View file

@ -1113,7 +1113,7 @@ export interface IRequestOptionsFromParameters {
}
export interface INodeTypeDescription extends INodeTypeBaseDescription {
version: number;
version: number | number[];
defaults: INodeParameters;
eventTriggerDescription?: string;
activationMessage?: string;

28
packages/workflow/src/NodeHelpers.ts
View file

@ -265,6 +265,7 @@ export function getSpecialNodeParameters(nodeType: INodeType): INodeProperties[]
export function displayParameter(
nodeValues: INodeParameters,
parameter: INodeProperties | INodeCredentialDescription,
node: INode | null, // Allow null as it does also get used by credentials and they do not have versioning yet
nodeValuesRoot?: INodeParameters,
) {
if (!parameter.displayOptions) {
@ -282,6 +283,8 @@ export function displayParameter(
if (propertyName.charAt(0) === '/') {
// Get the value from the root of the node
value = get(nodeValuesRoot, propertyName.slice(1));
} else if (propertyName === '@version') {
value = node?.typeVersion || 0;
} else {
// Get the value from current level
value = get(nodeValues, propertyName);
@ -313,6 +316,8 @@ export function displayParameter(
if (propertyName.charAt(0) === '/') {
// Get the value from the root of the node
value = get(nodeValuesRoot, propertyName.slice(1));
} else if (propertyName === '@version') {
value = node?.typeVersion || 0;
} else {
// Get the value from current level
value = get(nodeValues, propertyName);
@ -352,6 +357,7 @@ export function displayParameterPath(
nodeValues: INodeParameters,
parameter: INodeProperties | INodeCredentialDescription,
path: string,
node: INode | null,
) {
let resolvedNodeValues = nodeValues;
if (path !== '') {
@ -364,7 +370,7 @@ export function displayParameterPath(
nodeValuesRoot = get(nodeValues, 'parameters') as INodeParameters;
}
return displayParameter(resolvedNodeValues, parameter, nodeValuesRoot);
return displayParameter(resolvedNodeValues, parameter, node, nodeValuesRoot);
}
/**
@ -433,6 +439,10 @@ export function getParamterDependencies(
// @ts-ignore
for (parameterName of Object.keys(nodeProperties.displayOptions[displayRule])) {
if (!dependencies[nodeProperties.name].includes(parameterName)) {
if (parameterName.charAt(0) === '@') {
// Is a special parameter so can be skipped
continue;
}
dependencies[nodeProperties.name].push(parameterName);
}
}
@ -532,6 +542,7 @@ export function getNodeParameters(
nodeValues: INodeParameters,
returnDefaults: boolean,
returnNoneDisplayed: boolean,
node: INode | null,
onlySimpleTypes = false,
dataIsResolved = false,
nodeValuesRoot?: INodeParameters,
@ -566,6 +577,7 @@ export function getNodeParameters(
nodeValues,
true,
true,
node,
true,
true,
nodeValuesRoot,
@ -594,7 +606,7 @@ export function getNodeParameters(
if (
!returnNoneDisplayed &&
!displayParameter(nodeValuesDisplayCheck, nodeProperties, nodeValuesRoot)
!displayParameter(nodeValuesDisplayCheck, nodeProperties, node, nodeValuesRoot)
) {
if (!returnNoneDisplayed || !returnDefaults) {
continue;
@ -605,7 +617,7 @@ export function getNodeParameters(
// Is a simple property so can be set as it is
if (duplicateParameterNames.includes(nodeProperties.name)) {
if (!displayParameter(nodeValuesDisplayCheck, nodeProperties, nodeValuesRoot)) {
if (!displayParameter(nodeValuesDisplayCheck, nodeProperties, node, nodeValuesRoot)) {
continue;
}
}
@ -677,6 +689,7 @@ export function getNodeParameters(
nodeValues[nodeProperties.name] as INodeParameters,
returnDefaults,
returnNoneDisplayed,
node,
false,
false,
nodeValuesRoot,
@ -737,6 +750,7 @@ export function getNodeParameters(
nodeValue,
returnDefaults,
returnNoneDisplayed,
node,
false,
false,
nodeValuesRoot,
@ -764,6 +778,7 @@ export function getNodeParameters(
(nodeValues[nodeProperties.name] as INodeParameters)[itemName] as INodeParameters,
returnDefaults,
returnNoneDisplayed,
node,
false,
false,
nodeValuesRoot,
@ -1021,7 +1036,7 @@ export function getNodeParametersIssues(
}
for (const nodeProperty of nodePropertiesArray) {
propertyIssues = getParameterIssues(nodeProperty, node.parameters, '');
propertyIssues = getParameterIssues(nodeProperty, node.parameters, '', node);
mergeIssues(foundIssues, propertyIssues);
}
@ -1135,12 +1150,13 @@ export function getParameterIssues(
nodeProperties: INodeProperties,
nodeValues: INodeParameters,
path: string,
node: INode,
): INodeIssues {
const foundIssues: INodeIssues = {};
let value;
if (nodeProperties.required === true) {
if (displayParameterPath(nodeValues, nodeProperties, path)) {
if (displayParameterPath(nodeValues, nodeProperties, path, node)) {
value = getParameterValueByPath(nodeValues, nodeProperties.name, path);
if (
@ -1235,7 +1251,7 @@ export function getParameterIssues(
let propertyIssues;
for (const optionData of checkChildNodeProperties) {
propertyIssues = getParameterIssues(optionData.data, nodeValues, optionData.basePath);
propertyIssues = getParameterIssues(optionData.data, nodeValues, optionData.basePath, node);
mergeIssues(foundIssues, propertyIssues);
}

9
packages/workflow/src/RoutingNode.ts
View file

@ -586,7 +586,14 @@ export class RoutingNode {
};
let basePath = path ? `${path}.` : '';
if (!NodeHelpers.displayParameter(this.node.parameters, nodeProperties, this.node.parameters)) {
if (
!NodeHelpers.displayParameter(
this.node.parameters,
nodeProperties,
this.node,
this.node.parameters,
)
) {
return undefined;
}
if (nodeProperties.routing) {

1
packages/workflow/src/Workflow.ts
View file

@ -110,6 +110,7 @@ export class Workflow {
node.parameters,
true,
false,
node,
);
node.parameters = nodeParameters !== null ? nodeParameters : {};
}

4
packages/workflow/test/NodeHelpers.test.ts
View file

@ -3018,6 +3018,7 @@ describe('Workflow', () => {
testData.input.nodeValues,
false,
false,
null,
);
expect(result).toEqual(testData.output.noneDisplayedFalse.defaultsFalse);
@ -3027,6 +3028,7 @@ describe('Workflow', () => {
testData.input.nodeValues,
true,
false,
null,
);
expect(result).toEqual(testData.output.noneDisplayedFalse.defaultsTrue);
@ -3036,6 +3038,7 @@ describe('Workflow', () => {
testData.input.nodeValues,
false,
true,
null,
);
expect(result).toEqual(testData.output.noneDisplayedTrue.defaultsFalse);
@ -3045,6 +3048,7 @@ describe('Workflow', () => {
testData.input.nodeValues,
true,
true,
null,
);
expect(result).toEqual(testData.output.noneDisplayedTrue.defaultsTrue);
});