fix(core): Fix populating of node custom api call options (#5347)

* feat(core): Fix populating of node custom api call options

* lint fixes

* Adress PR comments

* Add e2e test and only inject custom API options for latest version

* Make sure to injectCustomApiCallOption for the latest version of node

* feat(cli): Move apiCallOption injection to LoadNodesAndCredentials and add e2e tests to check for custom nodes credentials

* Load nodes and credentials fixtures from a single place

* Console warning if credential is invalid during customApiOptions injection
This commit is contained in:
OlegIvaniv 2023-02-03 13:14:59 +01:00 committed by GitHub
parent 4dab2fec49
commit 6985500a7d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 269 additions and 104 deletions

View file

@ -1,4 +1,3 @@
import { HTTP_REQUEST_NODE_TYPE } from './../../packages/editor-ui/src/constants';
import {
NEW_NOTION_ACCOUNT_NAME,
NOTION_NODE_NAME,
@ -6,7 +5,6 @@ import {
HTTP_REQUEST_NODE_NAME,
NEW_QUERY_AUTH_ACCOUNT_NAME,
} from './../constants';
import { visit } from 'recast';
import {
DEFAULT_USER_EMAIL,
DEFAULT_USER_PASSWORD,
@ -252,4 +250,24 @@ describe('Credentials', () => {
credentialsModal.actions.fillCredentialsForm();
workflowPage.getters.nodeCredentialsSelect().should('contain', NEW_QUERY_AUTH_ACCOUNT_NAME);
});
it('should render custom node with n8n credential', () => {
workflowPage.actions.visit();
workflowPage.actions.addNodeToCanvas('Manual Trigger');
workflowPage.actions.addNodeToCanvas('E2E Node with native n8n credential', true, true);
workflowPage.getters.nodeCredentialsLabel().click();
cy.contains('Create New Credential').click();
credentialsModal.getters.editCredentialModal().should('be.visible');
credentialsModal.getters.editCredentialModal().should('contain.text', 'Notion API');
})
it('should render custom node with custom credential', () => {
workflowPage.actions.visit();
workflowPage.actions.addNodeToCanvas('Manual Trigger');
workflowPage.actions.addNodeToCanvas('E2E Node with custom credential', true, true);
workflowPage.getters.nodeCredentialsLabel().click();
cy.contains('Create New Credential').click();
credentialsModal.getters.editCredentialModal().should('be.visible');
credentialsModal.getters.editCredentialModal().should('contain.text', 'Custom E2E Credential');
})
});

View file

@ -1,6 +1,5 @@
import { NodeCreator } from '../pages/features/node-creator';
import { INodeTypeDescription } from 'n8n-workflow';
import CustomNodeFixture from '../fixtures/Custom_node.json';
import { DEFAULT_USER_EMAIL, DEFAULT_USER_PASSWORD } from '../constants';
import { randFirstName, randLastName } from '@ngneat/falso';
@ -19,20 +18,6 @@ describe('Node Creator', () => {
beforeEach(() => {
cy.signin({ email, password });
cy.intercept('GET', '/types/nodes.json', (req) => {
// Delete caching headers so that we can intercept the request
['etag', 'if-none-match', 'if-modified-since'].forEach((header) => {
delete req.headers[header];
});
req.continue((res) => {
const nodes = res.body as INodeTypeDescription[];
nodes.push(CustomNodeFixture as INodeTypeDescription);
res.send(nodes);
});
}).as('nodesIntercept');
cy.visit(nodeCreatorFeature.url);
cy.waitForLoad();
});
@ -153,6 +138,7 @@ describe('Node Creator', () => {
});
it('should render and select community node', () => {
cy.intercept('GET', '/types/nodes.json').as('nodesIntercept');
cy.wait('@nodesIntercept').then(() => {
const customCategory = 'Custom Category';
const customNode = 'E2E Node';

View file

@ -9,11 +9,13 @@ describe('NDV', () => {
beforeEach(() => {
cy.resetAll();
cy.skipSetup();
workflowsPage.actions.createWorkflowFromCard();
workflowPage.actions.renameWorkflow(uuid());
workflowPage.actions.saveWorkflowOnButtonClick();
});
it('should show up when double clicked on a node and close when Back to canvas clicked', () => {
workflowPage.actions.addInitialNodeToCanvas('Manual Trigger');
workflowPage.getters.canvasNodes().first().dblclick();

View file

@ -0,0 +1,19 @@
{
"name": "customE2eCredential",
"displayName": "Custom E2E Credential",
"properties": [{
"displayName": "API Key",
"name": "apiKey",
"type": "string",
"default": "",
"required": false
}],
"authenticate": {
"type": "generic",
"properties": {
"qs": {
"auth": "={{$credentials.apiKey}}"
}
}
}
}

View file

@ -0,0 +1,57 @@
{
"properties": [
{
"displayName": "Test property",
"name": "testProp",
"type": "string",
"required": true,
"noDataExpression": false,
"default": "Some default"
},
{
"displayName": "Resource",
"name": "resource",
"type": "options",
"noDataExpression": true,
"options": [
{
"name": "option1",
"value": "option1"
},
{
"name": "option2",
"value": "option2"
},
{
"name": "option3",
"value": "option3"
},
{
"name": "option4",
"value": "option4"
}
],
"default": "option2"
}
],
"displayName": "E2E Node with custom credential",
"name": "@e2e/n8n-nodes-e2e-custom-credential",
"group": ["transform"],
"codex": {
"categories": ["Custom Category"]
},
"version": 1,
"description": "Demonstrate rendering of node with custom credential",
"defaults": {
"name": "E2E Node with custom credential"
},
"inputs": ["main"],
"outputs": ["main"],
"icon": "fa:network-wired",
"credentials": [
{
"name": "customE2eCredential",
"required": true
}
]
}

View file

@ -0,0 +1,57 @@
{
"properties": [
{
"displayName": "Test property",
"name": "testProp",
"type": "string",
"required": true,
"noDataExpression": false,
"default": "Some default"
},
{
"displayName": "Resource",
"name": "resource",
"type": "options",
"noDataExpression": true,
"options": [
{
"name": "option1",
"value": "option1"
},
{
"name": "option2",
"value": "option2"
},
{
"name": "option3",
"value": "option3"
},
{
"name": "option4",
"value": "option4"
}
],
"default": "option2"
}
],
"displayName": "E2E Node with native n8n credential",
"name": "@e2e/n8n-nodes-e2e-credential",
"group": ["transform"],
"codex": {
"categories": ["Custom Category"]
},
"version": 1,
"description": "Demonstrate rendering of node with native credential",
"defaults": {
"name": "E2E Node with native n8n credential"
},
"inputs": ["main"],
"outputs": ["main"],
"icon": "fa:network-wired",
"credentials": [
{
"name": "notionApi",
"required": true
}
]
}

View file

@ -27,6 +27,7 @@ export class NDV extends BasePage {
parameterInput: (parameterName: string) => cy.getByTestId(`parameter-input-${parameterName}`),
nodeNameContainer: () => cy.getByTestId('node-title-container'),
nodeRenameInput: () => cy.getByTestId('node-rename-input'),
httpRequestNotice: () => cy.getByTestId('node-parameters-http-notice'),
};
actions = {

View file

@ -57,8 +57,8 @@ Cypress.Commands.add(
);
Cypress.Commands.add('waitForLoad', () => {
cy.getByTestId('node-view-loader').should('not.exist', { timeout: 10000 });
cy.get('.el-loading-mask').should('not.exist', { timeout: 10000 });
cy.getByTestId('node-view-loader', { timeout: 10000 }).should('not.exist');
cy.get('.el-loading-mask', { timeout: 10000 }).should('not.exist');
});
Cypress.Commands.add('signin', ({ email, password }) => {

View file

@ -14,3 +14,30 @@
// ***********************************************************
import './commands';
import CustomNodeFixture from '../fixtures/Custom_node.json';
import CustomNodeWithN8nCredentialFixture from '../fixtures/Custom_node_n8n_credential.json';
import CustomNodeWithCustomCredentialFixture from '../fixtures/Custom_node_custom_credential.json';
import CustomCredential from '../fixtures/Custom_credential.json';
// Load custom nodes and credentials fixtures
beforeEach(() => {
cy.intercept('GET', '/types/nodes.json', (req) => {
req.continue((res) => {
const nodes = res.body;
res.headers['cache-control'] = 'no-cache, no-store';
nodes.push(CustomNodeFixture, CustomNodeWithN8nCredentialFixture, CustomNodeWithCustomCredentialFixture);
res.send(nodes);
});
}).as('nodesIntercept');
cy.intercept('GET', '/types/credentials.json', (req) => {
req.continue((res) => {
const credentials = res.body;
res.headers['cache-control'] = 'no-cache, no-store';
credentials.push(CustomCredential);
res.send(credentials);
});
}).as('credentialsIntercept');
})

View file

@ -12,6 +12,7 @@ import type {
ILogger,
INodesAndCredentials,
KnownNodesAndCredentials,
INodeTypeDescription,
LoadedNodesAndCredentials,
} from 'n8n-workflow';
import { LoggerProxy, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow';
@ -29,7 +30,13 @@ import config from '@/config';
import type { InstalledPackages } from '@db/entities/InstalledPackages';
import type { InstalledNodes } from '@db/entities/InstalledNodes';
import { executeCommand } from '@/CommunityNodes/helpers';
import { CLI_DIR, GENERATED_STATIC_DIR, RESPONSE_ERROR_MESSAGES } from '@/constants';
import {
CLI_DIR,
GENERATED_STATIC_DIR,
RESPONSE_ERROR_MESSAGES,
CUSTOM_API_CALL_KEY,
CUSTOM_API_CALL_NAME,
} from '@/constants';
import {
persistInstalledPackageData,
removePackageFromDatabase,
@ -66,6 +73,7 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
await this.loadNodesFromBasePackages();
await this.loadNodesFromDownloadedPackages();
await this.loadNodesFromCustomDirectories();
this.injectCustomApiCallOptions();
}
async generateTypesForFrontend() {
@ -307,6 +315,60 @@ export class LoadNodesAndCredentialsClass implements INodesAndCredentials {
}
}
/**
* Whether any of the node's credential types may be used to
* make a request from a node other than itself.
*/
private supportsProxyAuth(description: INodeTypeDescription) {
if (!description.credentials) return false;
return description.credentials.some(({ name }) => {
const credType = this.types.credentials.find((t) => t.name === name);
if (!credType) {
LoggerProxy.warn(
`Failed to load Custom API options for the node "${description.name}": Unknown credential name "${name}"`,
);
return false;
}
if (credType.authenticate !== undefined) return true;
return (
Array.isArray(credType.extends) &&
credType.extends.some((parentType) =>
['oAuth2Api', 'googleOAuth2Api', 'oAuth1Api'].includes(parentType),
)
);
});
}
/**
* Inject a `Custom API Call` option into `resource` and `operation`
* parameters in a latest-version node that supports proxy auth.
*/
private injectCustomApiCallOptions() {
this.types.nodes.forEach((node: INodeTypeDescription) => {
const isLatestVersion =
node.defaultVersion === undefined || node.defaultVersion === node.version;
if (isLatestVersion) {
if (!this.supportsProxyAuth(node)) return;
node.properties.forEach((p) => {
if (
['resource', 'operation'].includes(p.name) &&
Array.isArray(p.options) &&
p.options[p.options.length - 1].name !== CUSTOM_API_CALL_NAME
) {
p.options.push({
name: CUSTOM_API_CALL_NAME,
value: CUSTOM_API_CALL_KEY,
});
}
});
}
});
}
private unloadNodes(installedNodes: InstalledNodes[]): void {
installedNodes.forEach((installedNode) => {
delete this.loaded.nodes[installedNode.type];

View file

@ -2,69 +2,13 @@ import express from 'express';
import { readFile } from 'fs/promises';
import get from 'lodash.get';
import type { ICredentialType, INodeTypeDescription, INodeTypeNameVersion } from 'n8n-workflow';
import type { INodeTypeDescription, INodeTypeNameVersion } from 'n8n-workflow';
import { CredentialTypes } from '@/CredentialTypes';
import config from '@/config';
import { NodeTypes } from '@/NodeTypes';
import * as ResponseHelper from '@/ResponseHelper';
import { getNodeTranslationPath } from '@/TranslationHelpers';
function isOAuth(credType: ICredentialType) {
return (
Array.isArray(credType.extends) &&
credType.extends.some((parentType) =>
['oAuth2Api', 'googleOAuth2Api', 'oAuth1Api'].includes(parentType),
)
);
}
/**
* Whether any of the node's credential types may be used to
* make a request from a node other than itself.
*/
function supportsProxyAuth(description: INodeTypeDescription) {
if (!description.credentials) return false;
const credentialTypes = CredentialTypes();
return description.credentials.some(({ name }) => {
const credType = credentialTypes.getByName(name);
if (credType.authenticate !== undefined) return true;
return isOAuth(credType);
});
}
const CUSTOM_API_CALL_NAME = 'Custom API Call';
const CUSTOM_API_CALL_KEY = '__CUSTOM_API_CALL__';
/**
* Inject a `Custom API Call` option into `resource` and `operation`
* parameters in a node that supports proxy auth.
*/
function injectCustomApiCallOption(description: INodeTypeDescription) {
if (!supportsProxyAuth(description)) return description;
description.properties.forEach((p) => {
if (
['resource', 'operation'].includes(p.name) &&
Array.isArray(p.options) &&
p.options[p.options.length - 1].name !== CUSTOM_API_CALL_NAME
) {
p.options.push({
name: CUSTOM_API_CALL_NAME,
value: CUSTOM_API_CALL_KEY,
});
}
return p;
});
return description;
}
export const nodeTypesController = express.Router();
// Returns node information based on node names and versions
@ -78,7 +22,7 @@ nodeTypesController.post(
if (defaultLocale === 'en') {
return nodeInfos.reduce<INodeTypeDescription[]>((acc, { name, version }) => {
const { description } = NodeTypes().getByNameAndVersion(name, version);
acc.push(injectCustomApiCallOption(description));
acc.push(description);
return acc;
}, []);
}
@ -103,7 +47,7 @@ nodeTypesController.post(
// ignore - no translation exists at path
}
nodeTypes.push(injectCustomApiCallOption(description));
nodeTypes.push(description);
}
const nodeTypes: INodeTypeDescription[] = [];

View file

@ -12,6 +12,8 @@ export const inProduction = NODE_ENV === 'production';
export const inDevelopment = !NODE_ENV || NODE_ENV === 'development';
export const inTest = NODE_ENV === 'test';
export const inE2ETests = E2E_TESTS === 'true';
export const CUSTOM_API_CALL_NAME = 'Custom API Call';
export const CUSTOM_API_CALL_KEY = '__CUSTOM_API_CALL__';
export const CLI_DIR = resolve(__dirname, '..');
export const TEMPLATES_DIR = join(CLI_DIR, 'templates');

View file

@ -99,10 +99,10 @@ export abstract class DirectoryLoader {
}
} else {
// Short renaming to avoid type issues
const tmpNode = tempNode;
nodeVersion = Array.isArray(tmpNode.description.version)
? tmpNode.description.version.slice(-1)[0]
: tmpNode.description.version;
nodeVersion = Array.isArray(tempNode.description.version)
? tempNode.description.version.slice(-1)[0]
: tempNode.description.version;
}
this.known.nodes[fullNodeName] = {

View file

@ -233,7 +233,7 @@ const telemetry = instance?.proxy.$telemetry;
const { categorizedItems: allNodes, isTriggerNode } = useNodeTypesStore();
const containsAPIAction = computed(
() =>
state.latestNodeData?.properties.some((p) =>
activeNodeActions.value?.properties.some((p) =>
p.options?.find((o) => o.name === CUSTOM_API_CALL_NAME),
) === true,
);
@ -338,27 +338,10 @@ function getCustomAPICallHintLocale(key: string) {
interpolate: { nodeNameTitle },
});
}
// The nodes.json doesn't contain API CALL option so we need to fetch the node detail
// to determine if need to render the API CALL hint
async function fetchNodeDetails() {
if (!state.activeNodeActions) return;
const { getNodesInformation } = useNodeTypesStore();
const { version, name } = state.activeNodeActions;
const payload = {
name,
version: Array.isArray(version) ? version?.slice(-1)[0] : version,
} as INodeTypeNameVersion;
const nodesInfo = await getNodesInformation([payload], false);
state.latestNodeData = nodesInfo[0];
}
function setActiveActionsNodeType(nodeType: INodeTypeDescription | null) {
state.activeNodeActions = nodeType;
setShowTabs(false);
fetchNodeDetails();
if (nodeType) trackActionsView();
}

View file

@ -116,7 +116,11 @@
</n8n-text>
</div>
<div v-if="isCustomApiCallSelected(nodeValues)" class="parameter-item parameter-notice">
<div
v-if="isCustomApiCallSelected(nodeValues)"
class="parameter-item parameter-notice"
data-test-id="node-parameters-http-notice"
>
<n8n-notice
:content="
$locale.baseText('nodeSettings.useTheHttpRequestNode', {

View file

@ -56,9 +56,12 @@ const customNodeActionsParsers: {
},
};
function filterSinglePlaceholderAction(actions: INodeActionTypeDescription[]) {
function filterActions(actions: INodeActionTypeDescription[]) {
return actions.filter(
(action: INodeActionTypeDescription, _: number, arr: INodeActionTypeDescription[]) => {
const isApiCall = action.actionKey === CUSTOM_API_CALL_KEY;
if (isApiCall) return false;
const isPlaceholderTriggerAction = action.actionKey === PLACEHOLDER_RECOMMENDED_ACTION_KEY;
return !isPlaceholderTriggerAction || (isPlaceholderTriggerAction && arr.length > 1);
},
@ -339,7 +342,7 @@ export const useNodeCreatorStore = defineStore(STORES.NODE_CREATOR, {
const filteredNodes = Object.values(mergedNodes).map((node) => ({
...node,
actions: filterSinglePlaceholderAction(node.actions || []),
actions: filterActions(node.actions || []),
}));
return filteredNodes;