From 5d05f7f436a32b98d35a7b87968990e845ec56bb Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Mon, 10 Feb 2025 11:30:33 +0200 Subject: [PATCH] fix(Google Sheets Node): RMC should correctly map columns if data location set in options (#13116) Co-authored-by: Shireen Missi <94372015+ShireenMissi@users.noreply.github.com> --- .../test/v2/methods/resourceMapping.test.ts | 113 ++++++++++++++++++ .../Google/Sheet/test/v2/utils/utils.test.ts | 21 ++++ .../Sheet/v2/helpers/GoogleSheets.utils.ts | 10 +- .../Sheet/v2/methods/resourceMapping.ts | 18 ++- 4 files changed, 157 insertions(+), 5 deletions(-) create mode 100644 packages/nodes-base/nodes/Google/Sheet/test/v2/methods/resourceMapping.test.ts diff --git a/packages/nodes-base/nodes/Google/Sheet/test/v2/methods/resourceMapping.test.ts b/packages/nodes-base/nodes/Google/Sheet/test/v2/methods/resourceMapping.test.ts new file mode 100644 index 0000000000..1ca732e446 --- /dev/null +++ b/packages/nodes-base/nodes/Google/Sheet/test/v2/methods/resourceMapping.test.ts @@ -0,0 +1,113 @@ +import type { MockProxy } from 'jest-mock-extended'; +import { mock } from 'jest-mock-extended'; +import type { ILoadOptionsFunctions, INode } from 'n8n-workflow'; + +import { getMappingColumns } from '../../../v2/methods/resourceMapping'; + +jest.mock('../../../v2/helpers/GoogleSheets.utils'); + +const mockGoogleSheetInstance = { + spreadsheetGetSheets: jest.fn(), + spreadsheetGetSheet: jest.fn(), + getData: jest.fn(), + testFilter: jest.fn(), +}; + +jest.mock('../../../v2/helpers/GoogleSheet', () => ({ + GoogleSheet: jest.fn().mockImplementation(() => mockGoogleSheetInstance), +})); + +describe('Google Sheets, getMappingColumns', () => { + let loadOptionsFunctions: MockProxy; + + beforeEach(() => { + loadOptionsFunctions = mock(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should map columns and add row number for update operation', async () => { + loadOptionsFunctions.getNode.mockReturnValue({} as INode); + loadOptionsFunctions.getNodeParameter + .mockReturnValueOnce({ mode: 'id', value: 'spreadsheetId' }) // documentId + .mockReturnValueOnce({ mode: 'name', value: 'Sheet1' }) // sheetName + .mockReturnValueOnce({ mode: 'name' }) // sheetName mode + .mockReturnValueOnce({}) // options.locationDefine.values + .mockReturnValueOnce('update'); // operation + + mockGoogleSheetInstance.spreadsheetGetSheet.mockResolvedValueOnce({ + title: 'Sheet1', + sheetId: 1, + }); + mockGoogleSheetInstance.getData.mockResolvedValueOnce([['id', 'name', 'email']]); + mockGoogleSheetInstance.testFilter.mockReturnValueOnce(['id', 'name', 'email']); + + const result = await getMappingColumns.call(loadOptionsFunctions); + + expect(result.fields).toHaveLength(4); + expect(result.fields).toEqual([ + { + canBeUsedToMatch: true, + defaultMatch: true, + display: true, + displayName: 'id', + id: 'id', + required: false, + type: 'string', + }, + { + canBeUsedToMatch: true, + defaultMatch: false, + display: true, + displayName: 'name', + id: 'name', + required: false, + type: 'string', + }, + { + canBeUsedToMatch: true, + defaultMatch: false, + display: true, + displayName: 'email', + id: 'email', + required: false, + type: 'string', + }, + { + canBeUsedToMatch: true, + defaultMatch: false, + display: true, + displayName: 'row_number', + id: 'row_number', + readOnly: true, + removed: true, + required: false, + type: 'string', + }, + ]); + }); + + it('should map columns and add row number for appendOrUpdate operation', async () => { + loadOptionsFunctions.getNode.mockReturnValue({} as INode); + loadOptionsFunctions.getNodeParameter + .mockReturnValueOnce({ mode: 'id', value: 'spreadsheetId' }) // documentId + .mockReturnValueOnce({ mode: 'name', value: 'Sheet1' }) // sheetName + .mockReturnValueOnce({ mode: 'name' }) // sheetName mode + .mockReturnValueOnce({ headerRow: 10 }) // options.locationDefine.values + .mockReturnValueOnce('appendOrUpdate'); // operation + + mockGoogleSheetInstance.spreadsheetGetSheet.mockResolvedValueOnce({ + title: 'Sheet1', + sheetId: 1, + }); + mockGoogleSheetInstance.getData.mockResolvedValueOnce([['id', 'name', 'email']]); + mockGoogleSheetInstance.testFilter.mockReturnValueOnce(['id', 'name', 'email']); + + const result = await getMappingColumns.call(loadOptionsFunctions); + + expect(result.fields).toHaveLength(3); + expect(mockGoogleSheetInstance.getData).toHaveBeenCalledWith('Sheet1!10:10', 'FORMATTED_VALUE'); + }); +}); diff --git a/packages/nodes-base/nodes/Google/Sheet/test/v2/utils/utils.test.ts b/packages/nodes-base/nodes/Google/Sheet/test/v2/utils/utils.test.ts index 4c3f9861be..4be13410dc 100644 --- a/packages/nodes-base/nodes/Google/Sheet/test/v2/utils/utils.test.ts +++ b/packages/nodes-base/nodes/Google/Sheet/test/v2/utils/utils.test.ts @@ -458,6 +458,27 @@ describe('Test Google Sheets, checkForSchemaChanges', () => { ] as ResourceMapperField[]), ).toThrow("Column names were updated after the node's setup"); }); + + it('should filter out empty columns without throwing an error', async () => { + const node: INode = { + id: '1', + name: 'Google Sheets', + typeVersion: 4.4, + type: 'n8n-nodes-base.googleSheets', + position: [60, 760], + parameters: { + operation: 'append', + }, + }; + + expect(() => + checkForSchemaChanges(node, ['', '', 'id', 'name', 'data'], [ + { id: 'id' }, + { id: 'name' }, + { id: 'data' }, + ] as ResourceMapperField[]), + ).not.toThrow(); + }); }); describe('Test Google Sheets, getSpreadsheetId', () => { diff --git a/packages/nodes-base/nodes/Google/Sheet/v2/helpers/GoogleSheets.utils.ts b/packages/nodes-base/nodes/Google/Sheet/v2/helpers/GoogleSheets.utils.ts index c717e5c8b6..e5b8c8ef55 100644 --- a/packages/nodes-base/nodes/Google/Sheet/v2/helpers/GoogleSheets.utils.ts +++ b/packages/nodes-base/nodes/Google/Sheet/v2/helpers/GoogleSheets.utils.ts @@ -105,11 +105,11 @@ export function addRowNumber(data: SheetRangeData, headerRow: number) { export function trimToFirstEmptyRow(data: SheetRangeData, includesRowNumber = true) { const baseLength = includesRowNumber ? 1 : 0; - const emtyRowIndex = data.findIndex((row) => row.slice(baseLength).every((cell) => cell === '')); - if (emtyRowIndex === -1) { + const emptyRowIndex = data.findIndex((row) => row.slice(baseLength).every((cell) => cell === '')); + if (emptyRowIndex === -1) { return data; } - return data.slice(0, emtyRowIndex); + return data.slice(0, emptyRowIndex); } export function removeEmptyRows(data: SheetRangeData, includesRowNumber = true) { @@ -343,8 +343,10 @@ export function checkForSchemaChanges( schema: ResourceMapperField[], ) { const updatedColumnNames: Array<{ oldName: string; newName: string }> = []; + // RMC filters out empty columns so do the same here + columnNames = columnNames.filter((col) => col !== ''); - //if sheet does not contain ROW_NUMBER ignore it as data come from read rows operation + // if sheet does not contain ROW_NUMBER ignore it as data come from read rows operation const schemaColumns = columnNames.includes(ROW_NUMBER) ? schema.map((s) => s.id) : schema.filter((s) => s.id !== ROW_NUMBER).map((s) => s.id); diff --git a/packages/nodes-base/nodes/Google/Sheet/v2/methods/resourceMapping.ts b/packages/nodes-base/nodes/Google/Sheet/v2/methods/resourceMapping.ts index 664a44d094..8268d54fbf 100644 --- a/packages/nodes-base/nodes/Google/Sheet/v2/methods/resourceMapping.ts +++ b/packages/nodes-base/nodes/Google/Sheet/v2/methods/resourceMapping.ts @@ -31,7 +31,23 @@ export async function getMappingColumns( sheetMode, sheetWithinDocument, ); - const sheetData = await sheet.getData(`${sheetName}!1:1`, 'FORMATTED_VALUE'); + + const locationDefine = this.getNodeParameter( + 'options.locationDefine.values', + 0, + {}, + ) as IDataObject; + + let columnNamesRow = 1; + + if (locationDefine.headerRow) { + columnNamesRow = locationDefine.headerRow as number; + } + + const sheetData = await sheet.getData( + `${sheetName}!${columnNamesRow}:${columnNamesRow}`, + 'FORMATTED_VALUE', + ); const columns = sheet.testFilter(sheetData || [], 0, 0).filter((col) => col !== '');