From 3968a60bf74dbe7b72996fda53a5b5eaaed272a4 Mon Sep 17 00:00:00 2001 From: Temo <72650579+TemoSulava@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:51:43 +0000 Subject: [PATCH] fix(ERPNext Node): Return each row once and tolerate empty Get Many options (#36860) --- .../nodes-base/nodes/ERPNext/ERPNext.node.ts | 13 +- .../nodes/ERPNext/GenericFunctions.ts | 2 +- .../nodes/ERPNext/test/ERPNext.node.test.ts | 137 ++++++++++++++++++ .../ERPNext/test/GenericFunctions.test.ts | 49 +++++++ 4 files changed, 193 insertions(+), 8 deletions(-) create mode 100644 packages/nodes-base/nodes/ERPNext/test/ERPNext.node.test.ts create mode 100644 packages/nodes-base/nodes/ERPNext/test/GenericFunctions.test.ts diff --git a/packages/nodes-base/nodes/ERPNext/ERPNext.node.ts b/packages/nodes-base/nodes/ERPNext/ERPNext.node.ts index 539c78765c3..27bd2ad07ca 100644 --- a/packages/nodes-base/nodes/ERPNext/ERPNext.node.ts +++ b/packages/nodes-base/nodes/ERPNext/ERPNext.node.ts @@ -117,7 +117,6 @@ export class ERPNext implements INodeType { let responseData; const body: IDataObject = {}; - const qs: IDataObject = {}; const resource = this.getNodeParameter('resource', 0); const operation = this.getNodeParameter('operation', 0); @@ -158,16 +157,17 @@ export class ERPNext implements INodeType { const docType = this.getNodeParameter('docType', i) as string; const endpoint = `/api/resource/${docType}`; + const qs: IDataObject = {}; const { fields, filters } = this.getNodeParameter('options', i) as { - fields: string[]; - filters: { - customProperty: Array<{ field: string; operator: string; value: string }>; + fields?: string[]; + filters?: { + customProperty?: Array<{ field: string; operator: string; value: string }>; }; }; // fields=["test", "example", "hi"] - if (fields) { + if (fields?.length) { if (fields.includes('*')) { qs.fields = JSON.stringify(['*']); } else { @@ -175,8 +175,7 @@ export class ERPNext implements INodeType { } } // filters=[["Person","first_name","=","Jane"]] - // TODO: filters not working - if (filters) { + if (filters?.customProperty?.length) { qs.filters = JSON.stringify( filters.customProperty.map((filter) => { return [docType, filter.field, toSQL(filter.operator), filter.value]; diff --git a/packages/nodes-base/nodes/ERPNext/GenericFunctions.ts b/packages/nodes-base/nodes/ERPNext/GenericFunctions.ts index a9286d18f87..cddcb77ee87 100644 --- a/packages/nodes-base/nodes/ERPNext/GenericFunctions.ts +++ b/packages/nodes-base/nodes/ERPNext/GenericFunctions.ts @@ -82,7 +82,7 @@ export async function erpNextApiRequestAllItems( do { responseData = await erpNextApiRequest.call(this, method, resource, body, query); returnData.push.apply(returnData, responseData[propertyName] as IDataObject[]); - query.limit_start += query.limit_page_length - 1; + query.limit_start += query.limit_page_length; } while (responseData.data && responseData.data.length > 0); return returnData; diff --git a/packages/nodes-base/nodes/ERPNext/test/ERPNext.node.test.ts b/packages/nodes-base/nodes/ERPNext/test/ERPNext.node.test.ts new file mode 100644 index 00000000000..31c6a251a84 --- /dev/null +++ b/packages/nodes-base/nodes/ERPNext/test/ERPNext.node.test.ts @@ -0,0 +1,137 @@ +import type { IDataObject, IExecuteFunctions, INodeExecutionData } from 'n8n-workflow'; +import type { Mock } from 'vitest'; +import { mockDeep } from 'vitest-mock-extended'; + +import { ERPNext } from '../ERPNext.node'; +import * as GenericFunctions from '../GenericFunctions'; + +describe('ERPNext > document: getAll', () => { + let node: ERPNext; + let mockExecuteFunctions: ReturnType>; + + const mockParameters = (options: IDataObject | ((itemIndex: number) => IDataObject)) => { + mockExecuteFunctions.getNodeParameter.mockImplementation((parameter, itemIndex) => { + switch (parameter) { + case 'resource': + return 'document'; + case 'operation': + return 'getAll'; + case 'docType': + return 'Customer'; + case 'options': + return typeof options === 'function' ? options(itemIndex) : options; + case 'returnAll': + return false; + case 'limit': + return 10; + default: + return undefined; + } + }); + }; + + beforeEach(() => { + node = new ERPNext(); + mockExecuteFunctions = mockDeep(); + mockExecuteFunctions.getInputData.mockReturnValue([{ json: {} }]); + (mockExecuteFunctions.helpers.returnJsonArray as Mock).mockImplementation( + (data: IDataObject | IDataObject[]) => + (Array.isArray(data) ? data : [data]).map((json) => ({ json })), + ); + (mockExecuteFunctions.helpers.constructExecutionMetaData as Mock).mockImplementation( + (data: INodeExecutionData[]) => data, + ); + vi.spyOn(GenericFunctions, 'erpNextApiRequest').mockResolvedValue({ + data: [{ name: 'Customer A' }], + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should not send filters when the Filters collection is left empty', async () => { + mockParameters({ fields: ['name'], filters: {} }); + + const result = await node.execute.call(mockExecuteFunctions); + + expect(result[0]).toEqual([{ json: { name: 'Customer A' } }]); + expect(GenericFunctions.erpNextApiRequest).toHaveBeenCalledWith( + 'GET', + '/api/resource/Customer', + {}, + { fields: JSON.stringify(['name']), limit_page_length: 10, limit_start: 0 }, + ); + }); + + it('should not send fields or filters when both collections are left empty', async () => { + mockParameters({ fields: [], filters: { customProperty: [] } }); + + await node.execute.call(mockExecuteFunctions); + + expect(GenericFunctions.erpNextApiRequest).toHaveBeenCalledWith( + 'GET', + '/api/resource/Customer', + {}, + { limit_page_length: 10, limit_start: 0 }, + ); + }); + + it('should not carry query options over from a previous item', async () => { + mockExecuteFunctions.getInputData.mockReturnValue([{ json: {} }, { json: {} }]); + mockParameters((itemIndex) => (itemIndex === 0 ? { fields: ['name'] } : {})); + + await node.execute.call(mockExecuteFunctions); + + expect(GenericFunctions.erpNextApiRequest).toHaveBeenNthCalledWith( + 1, + 'GET', + '/api/resource/Customer', + {}, + { fields: JSON.stringify(['name']), limit_page_length: 10, limit_start: 0 }, + ); + expect(GenericFunctions.erpNextApiRequest).toHaveBeenNthCalledWith( + 2, + 'GET', + '/api/resource/Customer', + {}, + { limit_page_length: 10, limit_start: 0 }, + ); + }); + + it('should not carry filters over from a previous item', async () => { + mockExecuteFunctions.getInputData.mockReturnValue([{ json: {} }, { json: {} }]); + mockParameters((itemIndex) => + itemIndex === 0 + ? { filters: { customProperty: [{ field: 'first_name', operator: 'is', value: 'Jane' }] } } + : {}, + ); + + await node.execute.call(mockExecuteFunctions); + + expect(GenericFunctions.erpNextApiRequest).toHaveBeenNthCalledWith( + 2, + 'GET', + '/api/resource/Customer', + {}, + { limit_page_length: 10, limit_start: 0 }, + ); + }); + + it('should send filters when the Filters collection has entries', async () => { + mockParameters({ + filters: { customProperty: [{ field: 'first_name', operator: 'is', value: 'Jane' }] }, + }); + + await node.execute.call(mockExecuteFunctions); + + expect(GenericFunctions.erpNextApiRequest).toHaveBeenCalledWith( + 'GET', + '/api/resource/Customer', + {}, + expect.objectContaining({ + filters: JSON.stringify([['Customer', 'first_name', '=', 'Jane']]), + }), + ); + }); +}); diff --git a/packages/nodes-base/nodes/ERPNext/test/GenericFunctions.test.ts b/packages/nodes-base/nodes/ERPNext/test/GenericFunctions.test.ts new file mode 100644 index 00000000000..2fb5295707a --- /dev/null +++ b/packages/nodes-base/nodes/ERPNext/test/GenericFunctions.test.ts @@ -0,0 +1,49 @@ +import type { IExecuteFunctions, IDataObject } from 'n8n-workflow'; +import { mockDeep } from 'vitest-mock-extended'; + +import { erpNextApiRequestAllItems } from '../GenericFunctions'; + +describe('ERPNext > GenericFunctions', () => { + describe('erpNextApiRequestAllItems', () => { + let mockExecuteFunctions: ReturnType>; + + beforeEach(() => { + mockExecuteFunctions = mockDeep(); + mockExecuteFunctions.getCredentials.mockResolvedValue({ + environment: 'selfHosted', + domain: 'https://erpnext.local', + }); + }); + + it('should advance the offset by the page size and return every record once', async () => { + const allRecords: IDataObject[] = Array.from({ length: 1001 }, (_, index) => ({ + name: `doc-${index}`, + })); + const requestedOffsets: number[] = []; + + mockExecuteFunctions.helpers.requestWithAuthentication.mockImplementation( + async (_credentialsType, options) => { + const { limit_start: offset, limit_page_length: pageSize } = options.qs as { + limit_start: number; + limit_page_length: number; + }; + requestedOffsets.push(offset); + + return await Promise.resolve({ data: allRecords.slice(offset, offset + pageSize) }); + }, + ); + + const result = (await erpNextApiRequestAllItems.call( + mockExecuteFunctions, + 'data', + 'GET', + '/api/resource/Customer', + {}, + )) as IDataObject[]; + + expect(requestedOffsets).toEqual([0, 1000, 2000]); + expect(result).toHaveLength(1001); + expect(new Set(result.map(({ name }) => name)).size).toBe(1001); + }); + }); +});