Skip to content

Commit

Permalink
fix(Microsoft Excel 365 Node): Better error and description on unsupp…
Browse files Browse the repository at this point in the history
…orted range in upsert, update, getRange operations (#8452)
  • Loading branch information
michael-radency committed Jan 26, 2024
1 parent c70fa66 commit 8a595d1
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { mock } from 'jest-mock-extended';
import type { IDataObject, IExecuteFunctions, IGetNodeParameterOptions, INode } from 'n8n-workflow';
import { constructExecutionMetaData } from 'n8n-core';
import {
checkRange,
prepareOutput,
updateByAutoMaping,
updateByDefinedValues,
Expand Down Expand Up @@ -491,7 +492,7 @@ describe('Test MicrosoftExcelV2, updateByAutoMaping', () => {
expect(updateSummary.updatedData[4][1]).toEqual('Ismael'); // updated value
});

it('should update all occurances', () => {
it('should update all occurrences', () => {
const items = [
{
json: {
Expand Down Expand Up @@ -556,3 +557,19 @@ describe('Test MicrosoftExcelV2, updateByAutoMaping', () => {
expect(updateSummary.appendData[1]).toEqual({ id: 5, name: 'Victor', age: 67, data: 'data 5' });
});
});

describe('Test MicrosoftExcelV2, checkRange', () => {
it('should not throw error', () => {
const range = 'A1:D4';
expect(() => {
checkRange(node, range);
}).not.toThrow();
});

it('should throw error', () => {
const range = 'A:D';
expect(() => {
checkRange(node, range);
}).toThrow();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type {
INodeProperties,
} from 'n8n-workflow';
import type { ExcelResponse } from '../../helpers/interfaces';
import { prepareOutput } from '../../helpers/utils';
import { checkRange, prepareOutput } from '../../helpers/utils';
import { microsoftApiRequest } from '../../transport';
import { workbookRLC, worksheetRLC } from '../common.descriptions';
import { updateDisplayOptions } from '@utils/utilities';
Expand All @@ -25,7 +25,8 @@ const properties: INodeProperties[] = [
type: 'string',
placeholder: 'e.g. A1:B2',
default: '',
description: 'The sheet range to read the data from specified using a A1-style notation',
description:
'The sheet range to read the data from specified using a A1-style notation, has to be specific e.g A1:B5, generic ranges like A:B are not supported',
hint: 'Leave blank to return entire sheet',
displayOptions: {
show: {
Expand Down Expand Up @@ -140,6 +141,7 @@ export async function execute(
const options = this.getNodeParameter('options', i, {});

const range = this.getNodeParameter('range', i, '') as string;
checkRange(this.getNode(), range);

const rawData = (options.rawData as boolean) || false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ import type {
} from 'n8n-workflow';
import { NodeOperationError } from 'n8n-workflow';
import type { ExcelResponse, UpdateSummary } from '../../helpers/interfaces';
import { prepareOutput, updateByAutoMaping, updateByDefinedValues } from '../../helpers/utils';
import {
checkRange,
prepareOutput,
updateByAutoMaping,
updateByDefinedValues,
} from '../../helpers/utils';
import { microsoftApiRequest } from '../../transport';
import { workbookRLC, worksheetRLC } from '../common.descriptions';
import { generatePairedItemData, processJsonInput, updateDisplayOptions } from '@utils/utilities';
Expand All @@ -33,7 +38,7 @@ const properties: INodeProperties[] = [
placeholder: 'e.g. A1:B2',
default: '',
description:
'The sheet range to read the data from specified using a A1-style notation. Leave blank to use whole used range in the sheet.',
'The sheet range to read the data from specified using a A1-style notation, has to be specific e.g A1:B5, generic ranges like A:B are not supported. Leave blank to use whole used range in the sheet.',
hint: 'First row must contain column names',
},
{
Expand Down Expand Up @@ -254,6 +259,8 @@ export async function execute(
}) as string;

let range = this.getNodeParameter('range', 0, '') as string;
checkRange(this.getNode(), range);

const dataMode = this.getNodeParameter('dataMode', 0) as string;

let worksheetData: IDataObject = {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,12 @@ import type {
} from 'n8n-workflow';
import { NodeOperationError } from 'n8n-workflow';
import type { ExcelResponse, UpdateSummary } from '../../helpers/interfaces';
import { prepareOutput, updateByAutoMaping, updateByDefinedValues } from '../../helpers/utils';
import {
checkRange,
prepareOutput,
updateByAutoMaping,
updateByDefinedValues,
} from '../../helpers/utils';
import { microsoftApiRequest } from '../../transport';
import { workbookRLC, worksheetRLC } from '../common.descriptions';
import { generatePairedItemData, processJsonInput, updateDisplayOptions } from '@utils/utilities';
Expand All @@ -33,7 +38,7 @@ const properties: INodeProperties[] = [
placeholder: 'e.g. A1:B2',
default: '',
description:
'The sheet range to read the data from specified using a A1-style notation. Leave blank to use whole used range in the sheet.',
'The sheet range to read the data from specified using a A1-style notation, has to be specific e.g A1:B5, generic ranges like A:B are not supported. Leave blank to use whole used range in the sheet.',
hint: 'First row must contain column names',
},
{
Expand Down Expand Up @@ -191,6 +196,8 @@ export async function execute(
}) as string;

let range = this.getNodeParameter('range', 0, '') as string;
checkRange(this.getNode(), range);

const dataMode = this.getNodeParameter('dataMode', 0) as string;

let worksheetData: IDataObject = {};
Expand Down
11 changes: 11 additions & 0 deletions packages/nodes-base/nodes/Microsoft/Excel/v2/helpers/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,3 +206,14 @@ export function updateByAutoMaping(

return summary;
}

export const checkRange = (node: INode, range: string) => {
const rangeRegex = /^[A-Z]+:[A-Z]+$/i;

if (rangeRegex.test(range)) {
throw new NodeOperationError(
node,
`Specify the range more precisely e.g. A1:B5, generic ranges like ${range} are not supported`,
);
}
};

0 comments on commit 8a595d1

Please sign in to comment.