fix(Slack Node): Normalize multiOptions values when expression returns a string (#31269)

This commit is contained in:
Alexander Gekov 2026-06-03 11:15:04 +03:00 committed by GitHub
parent 21d7daaa82
commit 95e4ee7ee2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 537 additions and 13 deletions

View File

@ -10,6 +10,26 @@ import type {
} from 'n8n-workflow';
import { NodeApiError, NodeOperationError } from 'n8n-workflow';
// When an expression is wrapped in surrounding text/whitespace, n8n switches to
// string interpolation and a multiOptions array is coerced to a comma-joined
// string. Accept both shapes so the Slack node degrades gracefully.
export function toMultiOptionsCsv(value: unknown): string {
if (Array.isArray(value)) {
return value
.map((entry) => String(entry).trim())
.filter((entry) => entry.length > 0)
.join(',');
}
if (typeof value === 'string') {
return value
.split(',')
.map((entry) => entry.trim())
.filter((entry) => entry.length > 0)
.join(',');
}
return '';
}
export async function slackApiRequest(
this: IExecuteFunctions | ILoadOptionsFunctions,
method: IHttpRequestMethods,

View File

@ -16,7 +16,12 @@ import { oldVersionNotice } from '@utils/descriptions';
import { channelFields, channelOperations } from './ChannelDescription';
import { fileFields, fileOperations } from './FileDescription';
import { slackApiRequest, slackApiRequestAllItems, validateJSON } from './GenericFunctions';
import {
slackApiRequest,
slackApiRequestAllItems,
toMultiOptionsCsv,
validateJSON,
} from './GenericFunctions';
import { messageFields, messageOperations } from './MessageDescription';
import type { IAttachment } from './MessageInterface';
import { reactionFields, reactionOperations } from './ReactionDescription';
@ -367,7 +372,7 @@ export class SlackV1 implements INodeType {
const returnAll = this.getNodeParameter('returnAll', i);
const filters = this.getNodeParameter('filters', i);
if (filters.types) {
qs.types = (filters.types as string[]).join(',');
qs.types = toMultiOptionsCsv(filters.types);
}
if (filters.excludeArchived) {
qs.exclude_archived = filters.excludeArchived as boolean;
@ -426,7 +431,7 @@ export class SlackV1 implements INodeType {
//https://api.slack.com/methods/conversations.invite
if (operation === 'invite') {
const channel = this.getNodeParameter('channelId', i) as string;
const userIds = (this.getNodeParameter('userIds', i) as string[]).join(',');
const userIds = toMultiOptionsCsv(this.getNodeParameter('userIds', i));
const body: IDataObject = {
channel,
users: userIds,
@ -507,7 +512,7 @@ export class SlackV1 implements INodeType {
body.return_im = options.returnIm as boolean;
}
if (options.users) {
body.users = (options.users as string[]).join(',');
body.users = toMultiOptionsCsv(options.users);
}
responseData = await slackApiRequest.call(
this,
@ -1078,7 +1083,7 @@ export class SlackV1 implements INodeType {
const options = this.getNodeParameter('options', i);
const body: IDataObject = {};
if (options.channelIds) {
body.channels = (options.channelIds as string[]).join(',');
body.channels = toMultiOptionsCsv(options.channelIds);
}
if (options.fileName) {
body.filename = options.fileName as string;
@ -1148,7 +1153,7 @@ export class SlackV1 implements INodeType {
qs.ts_to = filters.tsTo as string;
}
if (filters.types) {
qs.types = (filters.types as string[]).join(',');
qs.types = toMultiOptionsCsv(filters.types);
}
if (filters.userId) {
qs.user = filters.userId as string;

View File

@ -35,6 +35,26 @@ function isDefined<T>(value: T | undefined | null | ''): value is NonNullable<T>
return value !== undefined && value !== null && value !== '';
}
// When an expression is wrapped in surrounding text/whitespace, n8n switches to
// string interpolation and a multiOptions array is coerced to a comma-joined
// string. Accept both shapes so the Slack node degrades gracefully.
export function toMultiOptionsCsv(value: unknown): string {
if (Array.isArray(value)) {
return value
.map((entry) => String(entry).trim())
.filter((entry) => entry.length > 0)
.join(',');
}
if (typeof value === 'string') {
return value
.split(',')
.map((entry) => entry.trim())
.filter((entry) => entry.length > 0)
.join(',');
}
return '';
}
export async function slackApiRequest(
this: IExecuteFunctions | ILoadOptionsFunctions | IWebhookFunctions,
method: IHttpRequestMethods,

View File

@ -31,6 +31,7 @@ import {
createSendAndWaitMessageBody,
processThreadOptions,
slackApiRequestAllItemsWithRateLimit,
toMultiOptionsCsv,
} from './GenericFunctions';
import {
channelRLC,
@ -526,7 +527,7 @@ export class SlackV2 implements INodeType {
const returnAll = this.getNodeParameter('returnAll', i);
const filters = this.getNodeParameter('filters', i);
if (filters.types) {
qs.types = (filters.types as string[]).join(',');
qs.types = toMultiOptionsCsv(filters.types);
}
if (filters.excludeArchived) {
qs.exclude_archived = filters.excludeArchived as boolean;
@ -601,7 +602,7 @@ export class SlackV2 implements INodeType {
{},
{ extractValue: true },
) as string;
const userIds = (this.getNodeParameter('userIds', i) as string[]).join(',');
const userIds = toMultiOptionsCsv(this.getNodeParameter('userIds', i));
const body: IDataObject = {
channel,
users: userIds,
@ -692,7 +693,7 @@ export class SlackV2 implements INodeType {
body.return_im = options.returnIm as boolean;
}
if (options.users) {
body.users = (options.users as string[]).join(',');
body.users = toMultiOptionsCsv(options.users);
}
responseData = await slackApiRequest.call(
this,
@ -1100,7 +1101,7 @@ export class SlackV2 implements INodeType {
const fileBody: IDataObject = {};
if (options.channelIds) {
body.channels = (options.channelIds as string[]).join(',');
body.channels = toMultiOptionsCsv(options.channelIds);
}
if (options.channelId) {
body.channel_id = options.channelId as string;
@ -1225,7 +1226,7 @@ export class SlackV2 implements INodeType {
qs.ts_to = filters.tsTo as string;
}
if (filters.types) {
qs.types = (filters.types as string[]).join(',');
qs.types = toMultiOptionsCsv(filters.types);
}
if (filters.userId) {
qs.user = filters.userId as string;

View File

@ -1,6 +1,11 @@
import type { IExecuteFunctions } from 'n8n-workflow';
import { slackApiRequest, slackApiRequestAllItems, validateJSON } from '../../V1/GenericFunctions';
import {
slackApiRequest,
slackApiRequestAllItems,
toMultiOptionsCsv,
validateJSON,
} from '../../V1/GenericFunctions';
jest.mock('n8n-workflow', () => ({
...jest.requireActual('n8n-workflow'),
@ -147,6 +152,31 @@ describe('Slack V1 > GenericFunctions', () => {
});
});
describe('toMultiOptionsCsv', () => {
it('joins array values', () => {
expect(toMultiOptionsCsv(['U123', 'U456'])).toBe('U123,U456');
});
it('accepts a comma-joined string and trims trailing whitespace', () => {
expect(toMultiOptionsCsv('U123,U456 ')).toBe('U123,U456');
});
it('trims whitespace around each entry in a comma-string', () => {
expect(toMultiOptionsCsv(' U123 , U456 ')).toBe('U123,U456');
});
it('drops empty entries', () => {
expect(toMultiOptionsCsv(['U123', '', ' ', 'U456'])).toBe('U123,U456');
});
it('returns empty string for undefined/null/empty', () => {
expect(toMultiOptionsCsv(undefined)).toBe('');
expect(toMultiOptionsCsv(null)).toBe('');
expect(toMultiOptionsCsv('')).toBe('');
expect(toMultiOptionsCsv([])).toBe('');
});
});
describe('validateJSON', () => {
it('should return undefined for invalid JSON', () => {
const result = validateJSON('{invalid:json}');

View File

@ -0,0 +1,281 @@
import { mockDeep } from 'jest-mock-extended';
import type { IExecuteFunctions, INode } from 'n8n-workflow';
import * as GenericFunctions from '../../V1/GenericFunctions';
import { SlackV1 } from '../../V1/SlackV1.node';
describe('SlackV1 — multiOptions parameter normalization', () => {
let node: SlackV1;
let mockExecuteFunctions: jest.Mocked<IExecuteFunctions>;
let slackApiRequestSpy: jest.SpyInstance;
let slackApiRequestAllItemsSpy: jest.SpyInstance;
const mockNode: INode = {
id: 'test-node-id',
name: 'Slack Test',
type: 'n8n-nodes-base.slack',
typeVersion: 1,
position: [0, 0],
parameters: {},
};
beforeEach(() => {
node = new SlackV1({
name: 'Slack',
displayName: 'Slack',
description: 'Slack V1 node test',
group: ['input'],
});
mockExecuteFunctions = mockDeep<IExecuteFunctions>();
slackApiRequestSpy = jest.spyOn(GenericFunctions, 'slackApiRequest');
slackApiRequestAllItemsSpy = jest.spyOn(GenericFunctions, 'slackApiRequestAllItems');
jest.clearAllMocks();
mockExecuteFunctions.getInputData.mockReturnValue([{ json: {} }]);
mockExecuteFunctions.getNode.mockReturnValue(mockNode);
mockExecuteFunctions.continueOnFail.mockReturnValue(false);
(mockExecuteFunctions.helpers.constructExecutionMetaData as jest.Mock).mockImplementation(
(data: any, options: any) => {
return data.map((item: any, index: number) => ({
...item,
pairedItem: { item: options?.itemData?.item ?? index },
}));
},
);
(mockExecuteFunctions.helpers.returnJsonArray as jest.Mock).mockImplementation((data: any) => {
return [{ json: data }];
});
});
afterEach(() => {
jest.resetAllMocks();
});
const mockParams = (params: Record<string, any>) => {
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {
return params[paramName];
});
};
describe('channel: getAll — filters.types', () => {
it('joins an array of types', async () => {
mockParams({
authentication: 'accessToken',
resource: 'channel',
operation: 'getAll',
returnAll: false,
limit: 10,
filters: { types: ['public_channel', 'private_channel'] },
});
slackApiRequestSpy.mockResolvedValue({ channels: [] });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'GET',
'/conversations.list',
{},
expect.objectContaining({ types: 'public_channel,private_channel' }),
);
});
it('normalizes a comma-joined string with trailing whitespace', async () => {
mockParams({
authentication: 'accessToken',
resource: 'channel',
operation: 'getAll',
returnAll: false,
limit: 10,
filters: { types: 'public_channel,private_channel ' },
});
slackApiRequestSpy.mockResolvedValue({ channels: [] });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'GET',
'/conversations.list',
{},
expect.objectContaining({ types: 'public_channel,private_channel' }),
);
});
});
describe('channel: invite — userIds', () => {
it('joins an array of user IDs', async () => {
mockParams({
authentication: 'accessToken',
resource: 'channel',
operation: 'invite',
channelId: 'C123456789',
userIds: ['U111111111', 'U222222222'],
});
slackApiRequestSpy.mockResolvedValue({ channel: { id: 'C123456789' } });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'POST',
'/conversations.invite',
{ channel: 'C123456789', users: 'U111111111,U222222222' },
{},
);
});
it('normalizes a comma-joined string with trailing whitespace (whitespace-coerced expression)', async () => {
mockParams({
authentication: 'accessToken',
resource: 'channel',
operation: 'invite',
channelId: 'C123456789',
userIds: 'U111111111,U222222222 ',
});
slackApiRequestSpy.mockResolvedValue({ channel: { id: 'C123456789' } });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'POST',
'/conversations.invite',
{ channel: 'C123456789', users: 'U111111111,U222222222' },
{},
);
});
});
describe('channel: open — options.users', () => {
it('joins an array of users', async () => {
mockParams({
authentication: 'accessToken',
resource: 'channel',
operation: 'open',
options: { users: ['U111111111', 'U222222222'], returnIm: true },
});
slackApiRequestSpy.mockResolvedValue({ channel: { id: 'D1' } });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'POST',
'/conversations.open',
{ users: 'U111111111,U222222222', return_im: true },
{},
);
});
it('normalizes a comma-joined string for users option', async () => {
mockParams({
authentication: 'accessToken',
resource: 'channel',
operation: 'open',
options: { users: 'U111111111,U222222222 ' },
});
slackApiRequestSpy.mockResolvedValue({ channel: { id: 'D1' } });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'POST',
'/conversations.open',
{ users: 'U111111111,U222222222' },
{},
);
});
});
describe('file: upload — options.channelIds', () => {
it('joins an array of channel IDs', async () => {
mockParams({
authentication: 'accessToken',
resource: 'file',
operation: 'upload',
binaryData: false,
fileContent: 'hello',
options: { channelIds: ['C111111111', 'C222222222'] },
});
slackApiRequestSpy.mockResolvedValue({ file: { id: 'F1' } });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'POST',
'/files.upload',
expect.objectContaining({ channels: 'C111111111,C222222222' }),
{},
{ 'Content-Type': 'application/x-www-form-urlencoded' },
expect.objectContaining({
form: expect.objectContaining({ channels: 'C111111111,C222222222' }),
}),
);
});
it('normalizes a comma-joined string for channelIds option', async () => {
mockParams({
authentication: 'accessToken',
resource: 'file',
operation: 'upload',
binaryData: false,
fileContent: 'hello',
options: { channelIds: 'C111111111,C222222222 ' },
});
slackApiRequestSpy.mockResolvedValue({ file: { id: 'F1' } });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'POST',
'/files.upload',
expect.objectContaining({ channels: 'C111111111,C222222222' }),
{},
{ 'Content-Type': 'application/x-www-form-urlencoded' },
expect.anything(),
);
});
});
describe('file: getAll — filters.types', () => {
it('joins an array of file types', async () => {
mockParams({
authentication: 'accessToken',
resource: 'file',
operation: 'getAll',
returnAll: true,
filters: { types: ['images', 'pdfs'] },
});
slackApiRequestAllItemsSpy.mockResolvedValue([]);
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestAllItemsSpy).toHaveBeenCalledWith(
'files',
'GET',
'/files.list',
{},
expect.objectContaining({ types: 'images,pdfs' }),
);
});
it('normalizes a comma-joined string for file types', async () => {
mockParams({
authentication: 'accessToken',
resource: 'file',
operation: 'getAll',
returnAll: true,
filters: { types: 'images,pdfs ' },
});
slackApiRequestAllItemsSpy.mockResolvedValue([]);
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestAllItemsSpy).toHaveBeenCalledWith(
'files',
'GET',
'/files.list',
{},
expect.objectContaining({ types: 'images,pdfs' }),
);
});
});
});

View File

@ -151,6 +151,56 @@ describe('SlackV2', () => {
});
});
describe('Channel List Operations - multiOptions types filter', () => {
it('should send types as CSV when filters.types is an array', async () => {
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {
const params: Record<string, any> = {
resource: 'channel',
operation: 'getAll',
returnAll: false,
limit: 50,
filters: { types: ['public_channel', 'private_channel'] },
};
return params[paramName];
});
slackApiRequestSpy.mockResolvedValue({ channels: [] });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'GET',
'/conversations.list',
{},
expect.objectContaining({ types: 'public_channel,private_channel' }),
);
});
it('should normalize types when filters.types is a comma-joined string', async () => {
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {
const params: Record<string, any> = {
resource: 'channel',
operation: 'getAll',
returnAll: false,
limit: 50,
filters: { types: 'public_channel,private_channel ' },
};
return params[paramName];
});
slackApiRequestSpy.mockResolvedValue({ channels: [] });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'GET',
'/conversations.list',
{},
expect.objectContaining({ types: 'public_channel,private_channel' }),
);
});
});
describe('Channel Operations - History, Invite, Leave, Members, etc.', () => {
it('should get channel history with pagination', async () => {
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {
@ -318,6 +368,32 @@ describe('SlackV2', () => {
]);
});
it('should invite users when userIds is a comma-joined string (whitespace-coerced expression)', async () => {
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {
const params: Record<string, any> = {
resource: 'channel',
operation: 'invite',
channelId: 'C123456789',
userIds: 'U111111111,U222222222 ',
};
return params[paramName];
});
slackApiRequestSpy.mockResolvedValue({ channel: { id: 'C123456789' } });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'POST',
'/conversations.invite',
{
channel: 'C123456789',
users: 'U111111111,U222222222',
},
{},
);
});
it('should leave channel', async () => {
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {
const params: Record<string, any> = {
@ -1416,6 +1492,56 @@ describe('SlackV2', () => {
});
});
describe('File List Operations - multiOptions types filter', () => {
it('should send types as CSV when filters.types is an array', async () => {
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {
const params: Record<string, any> = {
resource: 'file',
operation: 'getAll',
returnAll: false,
limit: 50,
filters: { types: ['images', 'pdfs'] },
};
return params[paramName];
});
slackApiRequestSpy.mockResolvedValue({ files: [{ id: 'F1' }] });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'GET',
'/files.list',
{},
expect.objectContaining({ types: 'images,pdfs' }),
);
});
it('should normalize types when filters.types is a comma-joined string with trailing whitespace', async () => {
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {
const params: Record<string, any> = {
resource: 'file',
operation: 'getAll',
returnAll: false,
limit: 50,
filters: { types: 'images,pdfs ' },
};
return params[paramName];
});
slackApiRequestSpy.mockResolvedValue({ files: [] });
await node.execute.call(mockExecuteFunctions);
expect(slackApiRequestSpy).toHaveBeenCalledWith(
'GET',
'/files.list',
{},
expect.objectContaining({ types: 'images,pdfs' }),
);
});
});
describe('Error Handling with continueOnFail', () => {
it('should continue execution on API error when continueOnFail is true', async () => {
mockExecuteFunctions.getNodeParameter.mockImplementation((paramName: string) => {

View File

@ -1,7 +1,11 @@
import { type MockProxy, mock } from 'jest-mock-extended';
import type { IExecuteFunctions } from 'n8n-workflow';
import { getTarget, createSendAndWaitMessageBody } from '../../V2/GenericFunctions';
import {
getTarget,
createSendAndWaitMessageBody,
toMultiOptionsCsv,
} from '../../V2/GenericFunctions';
describe('Slack Utility Functions', () => {
let mockExecuteFunctions: MockProxy<IExecuteFunctions>;
@ -18,6 +22,43 @@ describe('Slack Utility Functions', () => {
jest.clearAllMocks();
});
describe('toMultiOptionsCsv', () => {
it('joins array values', () => {
expect(toMultiOptionsCsv(['U123', 'U456'])).toBe('U123,U456');
});
it('trims entries inside an array (interpolated array elements)', () => {
expect(toMultiOptionsCsv(['U123 ', ' U456'])).toBe('U123,U456');
});
it('drops empty array entries', () => {
expect(toMultiOptionsCsv(['U123', '', ' ', 'U456'])).toBe('U123,U456');
});
it('coerces non-string array entries via String()', () => {
expect(toMultiOptionsCsv([1, 2, 3])).toBe('1,2,3');
});
it('accepts a comma-joined string (the whitespace-expression coercion case)', () => {
expect(toMultiOptionsCsv('U123,U456')).toBe('U123,U456');
});
it('trims surrounding whitespace on a string value (trailing-space expression bug)', () => {
expect(toMultiOptionsCsv('U123,U456 ')).toBe('U123,U456');
});
it('trims whitespace around each entry in a comma-string', () => {
expect(toMultiOptionsCsv(' U123 , U456 ')).toBe('U123,U456');
});
it('returns empty string for undefined/null/empty', () => {
expect(toMultiOptionsCsv(undefined)).toBe('');
expect(toMultiOptionsCsv(null)).toBe('');
expect(toMultiOptionsCsv('')).toBe('');
expect(toMultiOptionsCsv([])).toBe('');
});
});
describe('getTarget', () => {
it('should return correct target id', () => {
mockExecuteFunctions.getNodeParameter.mockImplementation((parameterName: string) => {