fix: Validate loadOption parameter in GSheet and Notion nodes (#34695)

This commit is contained in:
yehorkardash
2026-07-24 09:16:52 +02:00
committed by GitHub
parent 6f3d2b9faa
commit 1f82beef47
8 changed files with 84 additions and 13 deletions
@@ -82,6 +82,21 @@ describe('Google Sheets Functions', () => {
expect(result).toEqual([]);
});
it('should return an empty array if sheetName is null', async () => {
mockLoadOptionsFunctions.getNodeParameter = vi
.fn()
.mockReturnValueOnce({ mode: 'mode', value: 'value' }) // documentId
.mockReturnValueOnce('Sheet1') // sheetName extracted value
.mockReturnValueOnce(undefined); // sheetName resource locator
const result = await getSheetHeaderRow.call(
mockLoadOptionsFunctions as ILoadOptionsFunctions,
);
expect(result).toEqual([]);
expect(mockGoogleSheetInstance.spreadsheetGetSheet).not.toHaveBeenCalled();
});
it('should throw an error if no data is returned', async () => {
mockGoogleSheetInstance.spreadsheetGetSheet.mockResolvedValue({
title: 'Sheet1',
@@ -112,4 +112,16 @@ describe('Google Sheets, getMappingColumns', () => {
expect(result.fields).toHaveLength(3);
expect(mockGoogleSheetInstance.getData).toHaveBeenCalledWith('Sheet1!10:10', 'FORMATTED_VALUE');
});
it('should return no fields when sheetName is null', async () => {
loadOptionsFunctions.getNodeParameter
.mockReturnValueOnce({ mode: 'id', value: 'spreadsheetId' }) // documentId
.mockReturnValueOnce('Sheet1') // sheetName extracted value
.mockReturnValueOnce(undefined); // sheetName resource locator
const result = await getMappingColumns.call(loadOptionsFunctions);
expect(result).toEqual({ fields: [] });
expect(mockGoogleSheetInstance.spreadsheetGetSheet).not.toHaveBeenCalled();
});
});
@@ -39,7 +39,7 @@ export async function getSheets(this: ILoadOptionsFunctions): Promise<INodePrope
export async function getSheetHeaderRow(
this: ILoadOptionsFunctions,
): Promise<INodePropertyOptions[]> {
const documentId = this.getNodeParameter('documentId', 0) as IDataObject | null;
const documentId = this.getNodeParameter('documentId', null) as IDataObject | null;
if (!documentId) return [];
@@ -51,10 +51,14 @@ export async function getSheetHeaderRow(
const sheetWithinDocument = this.getNodeParameter('sheetName', undefined, {
extractValue: true,
}) as string;
const { mode: sheetMode } = this.getNodeParameter('sheetName', 0) as {
mode: ResourceLocator;
const { mode: sheetMode } = (this.getNodeParameter('sheetName') ?? { mode: null }) as {
mode: ResourceLocator | null;
};
if (!sheetMode) {
return [];
}
const { title: sheetName } = await sheet.spreadsheetGetSheet(
this.getNode(),
sheetMode,
@@ -24,7 +24,13 @@ export async function getMappingColumns(
const sheetWithinDocument = this.getNodeParameter('sheetName', undefined, {
extractValue: true,
}) as string;
const { mode: sheetMode } = this.getNodeParameter('sheetName', 0) as { mode: ResourceLocator };
const { mode: sheetMode } = (this.getNodeParameter('sheetName') ?? { mode: null }) as {
mode: ResourceLocator | null;
};
if (!sheetMode) {
return { fields: [] };
}
const { title: sheetName } = await sheet.spreadsheetGetSheet(
this.getNode(),
@@ -0,0 +1,19 @@
import type { ILoadOptionsFunctions } from 'n8n-workflow';
import { getDatabaseOptionsFromPage } from '../../v2/methods/loadOptions';
function createLoadOptionsContext(parameters: Record<string, unknown>): ILoadOptionsFunctions {
return {
getCurrentNodeParameter: vi.fn((name: string) => parameters[name]),
} as unknown as ILoadOptionsFunctions;
}
describe('Notion V2 load options', () => {
it('returns no options when database options are requested without a page', async () => {
const context = createLoadOptionsContext({ pageId: null });
const result = await getDatabaseOptionsFromPage.call(context);
expect(result).toEqual([]);
});
});
@@ -19,9 +19,7 @@ const mockNotionApiRequest = Transport.notionApiRequestV3 as Mock;
function createLoadOptionsContext(parameters: Record<string, unknown>): ILoadOptionsFunctions {
return {
getCurrentNodeParameter: vi.fn(
(name: string, fallback?: unknown) => parameters[name] ?? fallback,
),
getCurrentNodeParameter: vi.fn((name: string) => parameters[name]),
} as unknown as ILoadOptionsFunctions;
}
@@ -46,6 +44,15 @@ describe('Notion V3 load options', () => {
expect(result).toEqual([]);
});
it('returns no options when page data source properties are requested without a page', async () => {
const context = createLoadOptionsContext({ pageId: null });
const result = await getDataSourcePropertiesFromPage.call(context);
expect(result).toEqual([]);
expect(mockNotionApiRequest).not.toHaveBeenCalled();
});
it('uses only the last key segment as the property type for selected data source options', async () => {
mockGetDataSourceProperties.mockResolvedValueOnce({
'Stage | Owner': {
@@ -166,9 +166,13 @@ export async function getDatabaseIdFromPage(
export async function getDatabaseOptionsFromPage(
this: ILoadOptionsFunctions,
): Promise<INodePropertyOptions[]> {
const pageId = extractPageId(
this.getCurrentNodeParameter('pageId', { extractValue: true }) as string,
);
const pageIdValue = this.getCurrentNodeParameter('pageId', { extractValue: true }) as
| string
| null;
const pageId = extractPageId(pageIdValue ?? '');
if (!pageId) {
return [];
}
const [name, type] = (this.getCurrentNodeParameter('&key') as string).split('|');
const {
parent: { database_id: databaseId },
@@ -123,9 +123,13 @@ export async function getUsers(this: ILoadOptionsFunctions): Promise<INodeProper
async function getParentDataSourceIdFromPage(
this: ILoadOptionsFunctions,
): Promise<string | undefined> {
const pageId = extractPageId(
this.getCurrentNodeParameter('pageId', { extractValue: true }) as string,
);
const pageIdValue = this.getCurrentNodeParameter('pageId', { extractValue: true }) as
| string
| null;
const pageId = extractPageId(pageIdValue ?? '');
if (!pageId) {
return undefined;
}
const page = await notionApiRequestV3.call(this, 'GET', `/pages/${pageId}`);
if (!isDataObject(page) || !isDataObject(page.parent)) {
return undefined;