🔀 Merge branch 'master' into user-management-p2

packages/cli/commands/import/credentials.ts

@@ -95,7 +95,7 @@ export class ImportCredentialsCommand extends Command {
 
 				inputPath = inputPath.replace(/\/$/g, '');
 
-				const files = await glob(`${inputPath}/*.json`);
+				const files = await glob('*.json', { cwd: inputPath });
 
 				totalImported = files.length;
 

packages/cli/commands/import/workflow.ts

@@ -117,7 +117,7 @@ export class ImportWorkflowsCommand extends Command {
 
 				inputPath = inputPath.replace(/\/$/g, '');
 
-				const files = await glob(`${inputPath}/*.json`);
+				const files = await glob('*.json', { cwd: inputPath });
 
 				totalImported = files.length;
 

packages/cli/src/CommunityNodes/helpers.ts

@@ -238,6 +238,9 @@ export function isNpmError(error: unknown): error is { code: number; stdout: str
 
 const context = createContext({ require });
 export const loadClassInIsolation = (filePath: string, className: string) => {
+	if (process.platform === 'win32') {
+		filePath = filePath.replace(/\\/g, '/');
+	}
 	const script = new Script(`new (require('${filePath}').${className})()`);
 	// eslint-disable-next-line @typescript-eslint/no-unsafe-return
 	return script.runInContext(context);

packages/cli/src/LoadNodesAndCredentials.ts

@@ -493,7 +493,9 @@ class LoadNodesAndCredentialsClass {
 	 * @returns {Promise<void>}
 	 */
 	async loadDataFromDirectory(setPackageName: string, directory: string): Promise<void> {
-		const files = await glob(path.join(directory, '**/*.@(node|credentials).js'));
+		const files = await glob('**/*.@(node|credentials).js', {
+			cwd: directory,
+		});
 
 		for (const filePath of files) {
 			const [fileName, type] = path.parse(filePath).name.split('.');

packages/cli/test/integration/users.api.test.ts

@@ -19,6 +19,9 @@ import * as testDb from './shared/testDb';
 import type { AuthAgent } from './shared/types';
 import * as utils from './shared/utils';
 
+import * as UserManagementMailer from '../../src/UserManagement/email/UserManagementMailer';
+import { NodeMailer } from '../../src/UserManagement/email/NodeMailer';
+
 jest.mock('../../src/telemetry');
 jest.mock('../../src/UserManagement/email/NodeMailer');
 
@@ -536,19 +539,45 @@ test('POST /users should ignore an empty payload', async () => {
 	expect(users.length).toBe(1);
 });
 
-// TODO: /users/:id/reinvite route tests missing
+test('POST /users/:id/reinvite should send reinvite, but fail if user already accepted invite', async () => {
+	const owner = await testDb.createUser({ globalRole: globalOwnerRole });
+	const authOwnerAgent = utils.createAgent(app, { auth: true, user: owner });
+
+	config.set('userManagement.emails.mode', 'smtp');
+
+	// those configs are needed to make sure the reinvite email is sent,because of this check isEmailSetUp()
+	config.set('userManagement.emails.smtp.host', 'host');
+	config.set('userManagement.emails.smtp.auth.user', 'user');
+	config.set('userManagement.emails.smtp.auth.pass', 'pass');
 
-// TODO: UserManagementMailer is a singleton - cannot reinstantiate with wrong creds
-// test('POST /users should error for wrong SMTP config', async () => {
-// 	const owner = await Db.collections.User.findOneOrFail();
-// 	const authOwnerAgent = utils.createAgent(app, { auth: true, user: owner });
+	const email = randomEmail();
+	const payload = [{ email }];
+	const response = await authOwnerAgent.post('/users').send(payload);
 
-// 	config.set('userManagement.emails.mode', 'smtp');
-// 	config.set('userManagement.emails.smtp.host', 'XYZ'); // break SMTP config
+	expect(response.statusCode).toBe(200);
 
-// 	const payload = TEST_EMAILS_TO_CREATE_USER_SHELLS.map((e) => ({ email: e }));
+	const { data } = response.body;
+	const invitedUserId = data[0].user.id;
+	const reinviteResponse = await authOwnerAgent.post(`/users/${invitedUserId}/reinvite`);
 
-// 	const response = await authOwnerAgent.post('/users').send(payload);
+	expect(reinviteResponse.statusCode).toBe(200);
 
-// 	expect(response.statusCode).toBe(500);
-// });
+	const member = await testDb.createUser({ globalRole: globalMemberRole });
+	const reinviteMemberResponse = await authOwnerAgent.post(`/users/${member.id}/reinvite`);
+
+	expect(reinviteMemberResponse.statusCode).toBe(400);
+});
+
+test('UserManagementMailer expect NodeMailer.verifyConnection have been called', async () => {
+	jest.spyOn(NodeMailer.prototype, 'verifyConnection').mockImplementation(async () => {});
+
+	// NodeMailer.verifyConnection called 1 time
+	const userManagementMailer = UserManagementMailer.getInstance();
+	// NodeMailer.verifyConnection called 2 time
+	(await userManagementMailer).verifyConnection();
+
+	expect(NodeMailer.prototype.verifyConnection).toHaveBeenCalledTimes(2);
+
+	// @ts-ignore
+	NodeMailer.prototype.verifyConnection.mockRestore();
+});

packages/cli/test/unit/ActiveExecutions.test.ts

@@ -2,7 +2,7 @@ import { ActiveExecutions, IWorkflowExecutionDataProcess, Db } from '../../src';
 import { mocked } from 'jest-mock';
 import PCancelable from 'p-cancelable';
 import { v4 as uuid } from 'uuid';
-import type { IRun } from 'n8n-workflow';
+import { createDeferredPromise, IDeferredPromise, IExecuteResponsePromiseData, IRun } from 'n8n-workflow';
 
 const FAKE_EXECUTION_ID = '15';
 const FAKE_SECOND_EXECUTION_ID = '20';
@@ -37,6 +37,7 @@ describe('ActiveExecutions', () => {
 	test('Should add execution to active execution list', async () => {
 		const newExecution = mockExecutionData();
 		const executionId = await activeExecutions.add(newExecution);
+
 		expect(executionId).toBe(FAKE_EXECUTION_ID);
 		expect(activeExecutions.getActiveExecutions().length).toBe(1);
 		expect(mocked(Db.collections.Execution.save)).toHaveBeenCalledTimes(1);
@@ -46,6 +47,7 @@ describe('ActiveExecutions', () => {
 	test('Should update execution if add is called with execution ID', async () => {
 		const newExecution = mockExecutionData();
 		const executionId = await activeExecutions.add(newExecution, undefined, FAKE_SECOND_EXECUTION_ID);
+
 		expect(executionId).toBe(FAKE_SECOND_EXECUTION_ID);
 		expect(activeExecutions.getActiveExecutions().length).toBe(1);
 		expect(mocked(Db.collections.Execution.save)).toHaveBeenCalledTimes(0);
@@ -54,6 +56,7 @@ describe('ActiveExecutions', () => {
 
 	test('Should fail attaching execution to invalid executionId', async () => {
 		const deferredPromise = mockCancelablePromise();
+
 		expect(() => {
 			activeExecutions.attachWorkflowExecution(FAKE_EXECUTION_ID, deferredPromise);
 		}).toThrow();
@@ -63,7 +66,59 @@ describe('ActiveExecutions', () => {
 		const newExecution = mockExecutionData();
 		await activeExecutions.add(newExecution, undefined, FAKE_EXECUTION_ID);
 		const deferredPromise = mockCancelablePromise();
-		activeExecutions.attachWorkflowExecution(FAKE_EXECUTION_ID, deferredPromise);
+
+		expect(() => activeExecutions.attachWorkflowExecution(FAKE_EXECUTION_ID, deferredPromise)).not.toThrow();
+	});
+
+	test('Should attach and resolve response promise to existing execution', async () => {
+		const newExecution = mockExecutionData();
+		await activeExecutions.add(newExecution, undefined, FAKE_EXECUTION_ID);
+		const deferredPromise = await mockDeferredPromise();
+		activeExecutions.attachResponsePromise(FAKE_EXECUTION_ID, deferredPromise);
+		const fakeResponse = {data: {resultData: {runData: {}}}};
+		activeExecutions.resolveResponsePromise(FAKE_EXECUTION_ID, fakeResponse);
+
+		expect(deferredPromise.promise()).resolves.toEqual(fakeResponse);
+	});
+
+	test('Should remove an existing execution', async () => {
+		const newExecution = mockExecutionData();
+		const executionId = await activeExecutions.add(newExecution);
+		activeExecutions.remove(executionId);
+
+		expect(activeExecutions.getActiveExecutions().length).toBe(0);
+	});
+
+	test('Should resolve post execute promise on removal', async () => {
+		const newExecution = mockExecutionData();
+		const executionId = await activeExecutions.add(newExecution);
+		const postExecutePromise = activeExecutions.getPostExecutePromise(executionId);
+		// Force the above to be executed since we cannot await it
+		await new Promise((res) => {
+			setTimeout(res, 100);
+		});
+		const fakeOutput = mockFullRunData();
+		activeExecutions.remove(executionId, fakeOutput);
+
+		expect(postExecutePromise).resolves.toEqual(fakeOutput);
+	});
+
+	test('Should throw error when trying to create a promise with invalid execution', async() => {
+		expect(
+				activeExecutions.getPostExecutePromise(FAKE_EXECUTION_ID)
+		).rejects.toThrow();
+	});
+
+	test('Should call function to cancel execution when asked to stop', async () => {
+		const newExecution = mockExecutionData();
+		const executionId = await activeExecutions.add(newExecution);
+		const cancelExecution = jest.fn();
+		const cancellablePromise = mockCancelablePromise();
+		cancellablePromise.cancel = cancelExecution;
+		activeExecutions.attachWorkflowExecution(executionId, cancellablePromise);
+		activeExecutions.stopExecution(executionId);
+
+		expect(cancelExecution).toHaveBeenCalledTimes(1);
 	});
 
 });
@@ -83,9 +138,25 @@ function mockExecutionData(): IWorkflowExecutionDataProcess {
 	}
 }
 
+function mockFullRunData(): IRun {
+	return {
+		data: {
+			resultData: {
+				runData: {}
+			}
+		},
+		mode: 'manual',
+		startedAt: new Date(),
+	};
+}
+
 function mockCancelablePromise(): PCancelable<IRun> {
 	return new PCancelable(async (resolve) => {
 		resolve();
 	});
 }
 
+function mockDeferredPromise(): Promise<IDeferredPromise<IExecuteResponsePromiseData>> {
+	return createDeferredPromise<IExecuteResponsePromiseData>();
+}
+

packages/core/src/NodeExecuteFunctions.ts

@@ -1327,7 +1327,7 @@ export function returnJsonArray(jsonData: IDataObject | IDataObject[]): INodeExe
 	}
 
 	jsonData.forEach((data: IDataObject & { json?: IDataObject }) => {
-		if (data.json) {
+		if (data?.json) {
 			// We already have the JSON key so avoid double wrapping
 			returnData.push({ ...data, json: data.json });
 		} else {

packages/nodes-base/nodes/Function/Function.node.ts

@@ -95,7 +95,7 @@ return items;`,
 				if (item?.binary && item?.index !== undefined && item?.index !== null) {
 					for (const binaryPropertyName of Object.keys(item.binary)) {
 						item.binary[binaryPropertyName].data = (
-							await this.helpers.getBinaryDataBuffer(item.index, binaryPropertyName)
+							await this.helpers.getBinaryDataBuffer(item.index as number, binaryPropertyName)
 						)?.toString('base64');
 					}
 				}

packages/nodes-base/nodes/FunctionItem/FunctionItem.node.ts

@@ -113,7 +113,7 @@ return item;`,
 						if (item?.binary && item?.index !== undefined && item?.index !== null) {
 							for (const binaryPropertyName of Object.keys(item.binary)) {
 								item.binary[binaryPropertyName].data = (
-									await this.helpers.getBinaryDataBuffer(item.index, binaryPropertyName)
+									await this.helpers.getBinaryDataBuffer(item.index as number, binaryPropertyName)
 								)?.toString('base64');
 							}
 						}

packages/nodes-base/nodes/ItemLists/ItemLists.node.ts

@@ -150,6 +150,9 @@ export class ItemLists implements INodeType {
 								type: 'string',
 								default: '',
 								description: 'A field in the input items to aggregate together',
+								// eslint-disable-next-line n8n-nodes-base/node-param-placeholder-miscased-id
+								placeholder: 'e.g. id',
+								hint: ' Enter the field name as text',
 							},
 						],
 					},
@@ -206,6 +209,9 @@ export class ItemLists implements INodeType {
 								type: 'string',
 								default: '',
 								description: 'The name of a field in the input items to aggregate together',
+								// eslint-disable-next-line n8n-nodes-base/node-param-placeholder-miscased-id
+								placeholder: 'e.g. id',
+								hint: ' Enter the field name as text',
 							},
 							{
 								displayName: 'Rename Field',
@@ -293,6 +299,9 @@ export class ItemLists implements INodeType {
 								type: 'string',
 								default: '',
 								description: 'A field in the input to exclude from the object in output array',
+								// eslint-disable-next-line n8n-nodes-base/node-param-placeholder-miscased-id
+								placeholder: 'e.g. id',
+								hint: ' Enter the field name as text',
 							},
 						],
 					},
@@ -326,6 +335,9 @@ export class ItemLists implements INodeType {
 								type: 'string',
 								default: '',
 								description: 'Specify fields that will be included in output array',
+								// eslint-disable-next-line n8n-nodes-base/node-param-placeholder-miscased-id
+								placeholder: 'e.g. id',
+								hint: ' Enter the field name as text',
 							},
 						],
 					},
@@ -394,6 +406,9 @@ export class ItemLists implements INodeType {
 								type: 'string',
 								default: '',
 								description: 'A field in the input to exclude from the comparison',
+								// eslint-disable-next-line n8n-nodes-base/node-param-placeholder-miscased-id
+								placeholder: 'e.g. id',
+								hint: ' Enter the field name as text',
 							},
 						],
 					},
@@ -426,6 +441,9 @@ export class ItemLists implements INodeType {
 								type: 'string',
 								default: '',
 								description: 'A field in the input to add to the comparison',
+								// eslint-disable-next-line n8n-nodes-base/node-param-placeholder-miscased-id
+								placeholder: 'e.g. id',
+								hint: ' Enter the field name as text',
 							},
 						],
 					},
@@ -479,6 +497,9 @@ export class ItemLists implements INodeType {
 								required: true,
 								default: '',
 								description: 'The field to sort by',
+								// eslint-disable-next-line n8n-nodes-base/node-param-placeholder-miscased-id
+								placeholder: 'e.g. id',
+								hint: ' Enter the field name as text',
 							},
 							{
 								displayName: 'Order',

packages/nodes-base/nodes/Merge/Merge.node.json

@@ -2,9 +2,7 @@
 	"node": "n8n-nodes-base.merge",
 	"nodeVersion": "1.0",
 	"codexVersion": "1.0",
-	"categories": [
-		"Core Nodes"
-	],
+	"categories": ["Core Nodes"],
 	"resources": {
 		"primaryDocumentation": [
 			{
@@ -43,14 +41,8 @@
 			}
 		]
 	},
-	"alias": [
-		"Join",
-		"Concatenate",
-		"Wait"
-	],
+	"alias": ["Join", "Concatenate", "Wait"],
 	"subcategories": {
-		"Core Nodes": [
-			"Flow"
-		]
+		"Core Nodes": ["Flow"]
 	}
 }