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>

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<ILoadOptionsFunctions>;
+
+	beforeEach(() => {
+		loadOptionsFunctions = mock<ILoadOptionsFunctions>();
+	});
+
+	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');
+	});
+});

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', () => {

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 === ''));
+	const emptyRowIndex = data.findIndex((row) => row.slice(baseLength).every((cell) => cell === ''));
-	if (emtyRowIndex === -1) {
+	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);

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 !== '');