Skip to content

Commit

Permalink
Merge branch 'master' into ADO-2365-update-design-system
Browse files Browse the repository at this point in the history
* master:
  refactor(editor): Add types to ndv event bus (no-changelog) (#10451)
  fix: Better errors in Switch, If and Filter nodes (#10457)
  fix(Google Sheets Node): Update to returnAllMatches option (#10440)
  fix(Google Sheets Node): Better error when column to match on is empty (#10442)
  fix(OpenAI Node): Throw node operations error in case of openAi client error (#10448)
  feat(editor): Update canvas control buttons design and behaviour in new canvas (no-changelog) (#10435)
  fix(editor): Fix flaky mapping tests (#10453)
  • Loading branch information
MiloradFilipovic committed Aug 19, 2024
2 parents cea9d03 + ab9faf1 commit 9007924
Show file tree
Hide file tree
Showing 27 changed files with 372 additions and 104 deletions.
22 changes: 16 additions & 6 deletions cypress/e2e/14-mapping.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ describe('Data mapping', () => {

ndv.actions.mapDataFromHeader(1, 'value');
ndv.getters.inlineExpressionEditorInput().should('have.text', '{{ $json.timestamp }}');
ndv.getters.inlineExpressionEditorInput().type('{esc}');
ndv.getters.parameterExpressionPreview('value').should('include.text', '2024');

ndv.actions.mapDataFromHeader(2, 'value');
ndv.getters
Expand Down Expand Up @@ -133,6 +135,7 @@ describe('Data mapping', () => {

ndv.actions.mapToParameter('value');
ndv.getters.inlineExpressionEditorInput().should('have.text', '{{ $json.input[0].count }}');
ndv.getters.inlineExpressionEditorInput().type('{esc}');
ndv.getters.parameterExpressionPreview('value').should('include.text', '0');

ndv.getters
Expand All @@ -146,6 +149,7 @@ describe('Data mapping', () => {
ndv.getters
.inlineExpressionEditorInput()
.should('have.text', '{{ $json.input }}{{ $json.input[0].count }}');
ndv.getters.inlineExpressionEditorInput().type('{esc}');
ndv.actions.validateExpressionPreview('value', '[object Object]0');
});

Expand All @@ -163,6 +167,7 @@ describe('Data mapping', () => {

ndv.actions.mapToParameter('value');
ndv.getters.inlineExpressionEditorInput().should('have.text', '{{ $json.input[0].count }}');
ndv.getters.inlineExpressionEditorInput().type('{esc}');
ndv.actions.validateExpressionPreview('value', '0');

ndv.getters.inputDataContainer().find('span').contains('input').realMouseDown();
Expand Down Expand Up @@ -192,6 +197,7 @@ describe('Data mapping', () => {
ndv.getters
.inlineExpressionEditorInput()
.should('have.text', `{{ $('${SCHEDULE_TRIGGER_NODE_NAME}').item.json.input[0].count }}`);
ndv.getters.inlineExpressionEditorInput().type('{esc}');

ndv.actions.switchInputMode('Table');
ndv.actions.selectInputNode(SCHEDULE_TRIGGER_NODE_NAME);
Expand Down Expand Up @@ -271,12 +277,12 @@ describe('Data mapping', () => {

ndv.actions.typeIntoParameterInput('value', 'fun');
ndv.actions.clearParameterInput('value'); // keep focus on param
cy.wait(300);

ndv.getters.inputDataContainer().should('exist').find('span').contains('count').realMouseDown();

ndv.actions.mapToParameter('value');
ndv.getters.inlineExpressionEditorInput().should('have.text', '{{ $json.input[0].count }}');
ndv.getters.inlineExpressionEditorInput().type('{esc}');
ndv.actions.validateExpressionPreview('value', '0');

ndv.getters.inputDataContainer().find('span').contains('input').realMouseDown();
Expand Down Expand Up @@ -350,19 +356,23 @@ describe('Data mapping', () => {
workflowPage.actions.zoomToFit();

workflowPage.actions.openNode('Set');
ndv.actions.clearParameterInput('value');
ndv.actions.typeIntoParameterInput('value', '=');
ndv.actions.typeIntoParameterInput('value', 'hello world{enter}{enter}newline');
ndv.getters.inlineExpressionEditorInput().find('.cm-content').paste('hello world\n\nnewline');
ndv.getters.inlineExpressionEditorInput().type('{esc}');

ndv.getters.inputDataContainer().should('exist').find('span').contains('count').realMouseDown();

ndv.actions.mapToParameter('value');
ndv.getters
.inlineExpressionEditorInput()
.should('have.text', '{{ $json.input[0].count }}hello worldnewline');
ndv.getters.inlineExpressionEditorInput().type('{esc}');
ndv.actions.validateExpressionPreview('value', '0hello world\n\nnewline');

ndv.getters.inputDataContainer().find('span').contains('input').realMouseDown();
ndv.actions.mapToParameter('value', 'bottom');
ndv.actions.mapToParameter('value', 'center');

ndv.getters
.inlineExpressionEditorInput()
.should('have.text', '{{ $json.input[0].count }}hello worldnewline{{ $json.input }}');
.should('have.text', '{{ $json.input[0].count }}hello world{{ $json.input }}newline');
});
});
4 changes: 2 additions & 2 deletions cypress/e2e/33-settings-personal.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,13 @@ describe('Personal Settings', () => {
});
});
// eslint-disable-next-line n8n-local-rules/no-skipped-tests
it.skip('not allow malicious values for personal data', () => {
it('not allow malicious values for personal data', () => {
cy.visit('/settings/personal');
INVALID_NAMES.forEach((name) => {
cy.getByTestId('personal-data-form').find('input[name="firstName"]').clear().type(name);
cy.getByTestId('personal-data-form').find('input[name="lastName"]').clear().type(name);
cy.getByTestId('save-settings-button').click();
errorToast().should('contain', 'Malicious firstName | Malicious lastName');
errorToast().should('contain', 'Potentially malicious string | Potentially malicious string');
errorToast().find('.el-notification__closeBtn').click();
});
});
Expand Down
2 changes: 1 addition & 1 deletion cypress/pages/ndv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ export class NDV extends BasePage {
this.getters.editPinnedDataButton().click();

this.getters.pinnedDataEditor().click();
this.getters.pinnedDataEditor().type('{selectall}{backspace}').paste(JSON.stringify(data));
this.getters.pinnedDataEditor().invoke('text', '').paste(JSON.stringify(data));

this.actions.savePinnedData();
},
Expand Down
2 changes: 2 additions & 0 deletions cypress/support/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ beforeEach(() => {

cy.window().then((win): void => {
win.localStorage.setItem('N8N_THEME', 'light');
win.localStorage.setItem('N8N_AUTOCOMPLETE_ONBOARDED', 'true');
win.localStorage.setItem('N8N_MAPPING_ONBOARDED', 'true');
});

cy.intercept('GET', '/rest/settings', (req) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import { OpenAIAssistantRunnable } from 'langchain/experimental/openai_assistant
import type { OpenAIToolType } from 'langchain/dist/experimental/openai_assistant/schema';
import { OpenAI as OpenAIClient } from 'openai';

import { NodeConnectionType, NodeOperationError, updateDisplayOptions } from 'n8n-workflow';
import {
ApplicationError,
NodeConnectionType,
NodeOperationError,
updateDisplayOptions,
} from 'n8n-workflow';
import type {
IDataObject,
IExecuteFunctions,
Expand Down Expand Up @@ -228,25 +233,36 @@ export async function execute(this: IExecuteFunctions, i: number): Promise<INode
}
}

const response = await agentExecutor.withConfig(getTracingConfig(this)).invoke(chainValues);
if (memory) {
await memory.saveContext({ input }, { output: response.output });
let filteredResponse: IDataObject = {};
try {
const response = await agentExecutor.withConfig(getTracingConfig(this)).invoke(chainValues);
if (memory) {
await memory.saveContext({ input }, { output: response.output });

if (response.threadId && response.runId) {
const threadRun = await client.beta.threads.runs.retrieve(response.threadId, response.runId);
response.usage = threadRun.usage;
if (response.threadId && response.runId) {
const threadRun = await client.beta.threads.runs.retrieve(
response.threadId,
response.runId,
);
response.usage = threadRun.usage;
}
}
}

if (
options.preserveOriginalTools !== false &&
nodeVersion >= 1.3 &&
(assistantTools ?? [])?.length
) {
await client.beta.assistants.update(assistantId, {
tools: assistantTools,
});
if (
options.preserveOriginalTools !== false &&
nodeVersion >= 1.3 &&
(assistantTools ?? [])?.length
) {
await client.beta.assistants.update(assistantId, {
tools: assistantTools,
});
}
filteredResponse = omit(response, ['signal', 'timeout']) as IDataObject;
} catch (error) {
if (!(error instanceof ApplicationError)) {
throw new NodeOperationError(this.getNode(), error.message, { itemIndex: i });
}
}
const filteredResponse = omit(response, ['signal', 'timeout']);

return [{ json: filteredResponse, pairedItem: { item: i } }];
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('NoUrl', () => {
const entity = new Entity();

describe('URLs', () => {
const URLS = ['http://google.com', 'www.domain.tld'];
const URLS = ['http://google.com', 'www.domain.tld', 'n8n.io'];

for (const str of URLS) {
test(`should block ${str}`, async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/validators/no-url.validator.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ValidationOptions, ValidatorConstraintInterface } from 'class-validator';
import { registerDecorator, ValidatorConstraint } from 'class-validator';

const URL_REGEX = /^(https?:\/\/|www\.)/i;
const URL_REGEX = /^(https?:\/\/|www\.)|(\.[\p{L}\d-]+)/iu;

@ValidatorConstraint({ name: 'NoUrl', async: false })
class NoUrlConstraint implements ValidatorConstraintInterface {
Expand Down
1 change: 0 additions & 1 deletion packages/editor-ui/src/components/NodeSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,6 @@ export default defineComponent({
if (this.node) {
this.historyStore.pushCommandToUndo(new RenameNodeCommand(this.node.name, name));
}
// @ts-ignore
this.valueChanged({
value: name,
name: 'name',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ const fieldDescription = computed<string>(() => {
resourceMapperTypeOptions.value?.multiKeyMatch === true
? `${pluralFieldWord.value}`
: `${singularFieldWord.value}`,
nodeDisplayName: props.serviceName,
},
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,13 +182,18 @@ describe('ResourceMapper.vue', () => {
},
{ merge: true },
);

await waitAllPromises();
expect(getByText('Set the value for each foo')).toBeInTheDocument();
expect(
getByText('Look for incoming data that matches the foos in the service'),
).toBeInTheDocument();
expect(getByText('Foos to Match On')).toBeInTheDocument();
expect(getByText('The foos that identify the row(s) to modify')).toBeInTheDocument();
expect(
getByText(
'The foos to use when matching rows in the service to the input items of this node. Usually an ID.',
),
).toBeInTheDocument();
});

it('should render correct fields based on saved schema', async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/editor-ui/src/components/canvas/Canvas.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ import { createCanvasConnection, createCanvasNodeElement } from '@/__tests__/dat
import { NodeConnectionType } from 'n8n-workflow';
import type { useDeviceSupport } from 'n8n-design-system';

const matchMedia = global.window.matchMedia;
// @ts-expect-error Initialize window object
global.window = jsdom.window as unknown as Window & typeof globalThis;
global.window.matchMedia = matchMedia;

vi.mock('n8n-design-system', async (importOriginal) => {
const actual = await importOriginal<typeof useDeviceSupport>();
return { ...actual, useDeviceSupport: vi.fn(() => ({ isCtrlKeyPressed: vi.fn() })) };
});

vi.mock('@/composables/useDeviceSupport');

let renderComponent: ReturnType<typeof createComponentRenderer>;
beforeEach(() => {
const pinia = createPinia();
Expand Down
Loading

0 comments on commit 9007924

Please # to comment.