feat(core): Node hints(warnings) system (#8954)

This commit is contained in:
Michael Kret 2024-05-13 15:46:02 +03:00 committed by GitHub
parent 4d2115c163
commit da6088d0bb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 265 additions and 6 deletions

View file

@ -34,6 +34,7 @@ import type {
WorkflowExecuteMode,
CloseFunction,
StartNodeData,
NodeExecutionHint,
} from 'n8n-workflow';
import {
LoggerProxy as Logger,
@ -41,6 +42,7 @@ import {
NodeHelpers,
NodeConnectionType,
ApplicationError,
NodeExecutionOutput,
} from 'n8n-workflow';
import get from 'lodash/get';
import * as NodeExecuteFunctions from './NodeExecuteFunctions';
@ -807,6 +809,7 @@ export class WorkflowExecute {
// Variables which hold temporary data for each node-execution
let executionData: IExecuteData;
let executionError: ExecutionBaseError | undefined;
let executionHints: NodeExecutionHint[] = [];
let executionNode: INode;
let nodeSuccessData: INodeExecutionData[][] | null | undefined;
let runIndex: number;
@ -828,7 +831,7 @@ export class WorkflowExecute {
let lastExecutionTry = '';
let closeFunction: Promise<void> | undefined;
return new PCancelable(async (resolve, reject, onCancel) => {
return new PCancelable(async (resolve, _reject, onCancel) => {
// Let as many nodes listen to the abort signal, without getting the MaxListenersExceededWarning
setMaxListeners(Infinity, this.abortController.signal);
@ -896,6 +899,7 @@ export class WorkflowExecute {
nodeSuccessData = null;
executionError = undefined;
executionHints = [];
executionData =
this.runExecutionData.executionData!.nodeExecutionStack.shift() as IExecuteData;
executionNode = executionData.node;
@ -1059,8 +1063,16 @@ export class WorkflowExecute {
this.mode,
this.abortController.signal,
);
nodeSuccessData = runNodeData.data;
if (nodeSuccessData instanceof NodeExecutionOutput) {
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
const hints: NodeExecutionHint[] = nodeSuccessData.getHints();
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
executionHints.push(...hints);
}
if (nodeSuccessData && executionData.node.onError === 'continueErrorOutput') {
// If errorOutput is activated check all the output items for error data.
// If any is found, route them to the last output as that will be the
@ -1244,7 +1256,7 @@ export class WorkflowExecute {
if (!inputData) {
return;
}
inputData.forEach((item, itemIndex) => {
inputData.forEach((_item, itemIndex) => {
pairedItem.push({
item: itemIndex,
input: inputIndex,
@ -1296,6 +1308,7 @@ export class WorkflowExecute {
}
taskData = {
hints: executionHints,
startTime,
executionTime: new Date().getTime() - startTime,
source: !executionData.source ? [] : executionData.source.main,

View file

@ -1,5 +1,10 @@
import type { IRun, WorkflowTestData } from 'n8n-workflow';
import { ApplicationError, createDeferredPromise, Workflow } from 'n8n-workflow';
import {
ApplicationError,
createDeferredPromise,
NodeExecutionOutput,
Workflow,
} from 'n8n-workflow';
import { WorkflowExecute } from '@/WorkflowExecute';
import * as Helpers from './helpers';
@ -192,4 +197,16 @@ describe('WorkflowExecute', () => {
});
}
});
describe('WorkflowExecute, NodeExecutionOutput type test', () => {
//TODO Add more tests here when execution hints are added to some node types
const nodeExecutionOutput = new NodeExecutionOutput(
[[{ json: { data: 123 } }]],
[{ message: 'TEXT HINT' }],
);
expect(nodeExecutionOutput).toBeInstanceOf(NodeExecutionOutput);
expect(nodeExecutionOutput[0][0].json.data).toEqual(123);
expect(nodeExecutionOutput.getHints()[0].message).toEqual('TEXT HINT');
});
});

View file

@ -152,6 +152,15 @@
</div>
<slot name="before-data" />
<n8n-callout
v-for="hint in getNodeHints()"
:key="hint.message"
:class="$style.hintCallout"
:theme="hint.type || 'info'"
>
<n8n-text v-html="hint.message" size="small"></n8n-text>
</n8n-callout>
<div
v-if="maxOutputIndex > 0 && branches.length > 1"
:class="$style.tabs"
@ -557,6 +566,7 @@ import type {
INodeTypeDescription,
IRunData,
IRunExecutionData,
NodeHint,
} from 'n8n-workflow';
import { NodeHelpers, NodeConnectionType } from 'n8n-workflow';
@ -824,6 +834,15 @@ export default defineComponent({
hasRunError(): boolean {
return Boolean(this.node && this.workflowRunData?.[this.node.name]?.[this.runIndex]?.error);
},
executionHints(): NodeHint[] {
if (this.hasNodeRun) {
const hints = this.node && this.workflowRunData?.[this.node.name]?.[this.runIndex]?.hints;
if (hints) return hints;
}
return [];
},
workflowExecution(): IExecutionResponse | null {
return this.workflowsStore.getWorkflowExecution;
},
@ -1083,6 +1102,46 @@ export default defineComponent({
}
return [];
},
shouldHintBeDisplayed(hint: NodeHint): boolean {
const { location, whenToDisplay } = hint;
if (location) {
if (location === 'ndv') {
return true;
}
if (location === 'inputPane' && this.paneType === 'input') {
return true;
}
if (location === 'outputPane' && this.paneType === 'output') {
return true;
}
return false;
}
if (whenToDisplay === 'afterExecution' && !this.hasNodeRun) {
return false;
}
if (whenToDisplay === 'beforeExecution' && this.hasNodeRun) {
return false;
}
return true;
},
getNodeHints(): NodeHint[] {
if (this.node && this.nodeType) {
const workflow = this.workflowsStore.getCurrentWorkflow();
const workflowNode = workflow.getNode(this.node.name);
if (workflowNode) {
const executionHints = this.executionHints;
const nodeHints = NodeHelpers.getNodeHints(workflow, workflowNode, this.nodeType);
return executionHints.concat(nodeHints).filter(this.shouldHintBeDisplayed);
}
}
return [];
},
onItemHover(itemIndex: number | null) {
if (itemIndex === null) {
this.$emit('itemHover', null);
@ -1788,6 +1847,12 @@ export default defineComponent({
border-top-left-radius: 0;
border-bottom-left-radius: 0;
}
.hintCallout {
margin-bottom: var(--spacing-xs);
margin-left: var(--spacing-s);
margin-right: var(--spacing-s);
}
</style>
<style lang="scss" scoped>

View file

@ -0,0 +1,2 @@
export const NODE_RAN_MULTIPLE_TIMES_WARNING =
"This node ran multiple times - once for each input item. You can change this by setting 'execute once' in the node settings. <a href='https://docs.n8n.io/flow-logic/looping/#executing-nodes-once' target='_blank'>More Info</a>";

View file

@ -392,7 +392,7 @@ export interface IGetExecuteTriggerFunctions {
}
export interface IRunNodeResponse {
data: INodeExecutionData[][] | null | undefined;
data: INodeExecutionData[][] | NodeExecutionOutput | null | undefined;
closeFunction?: CloseFunction;
}
export interface IGetExecuteFunctions {
@ -1423,6 +1423,20 @@ export interface SupplyData {
closeFunction?: CloseFunction;
}
export class NodeExecutionOutput extends Array {
private hints: NodeExecutionHint[];
constructor(data: INodeExecutionData[][], hints: NodeExecutionHint[] = []) {
super();
this.push(...data);
this.hints = hints;
}
public getHints(): NodeExecutionHint[] {
return this.hints;
}
}
export interface INodeType {
description: INodeTypeDescription;
supplyData?(this: IAllExecuteFunctions, itemIndex: number): Promise<SupplyData>;
@ -1745,9 +1759,20 @@ export interface INodeTypeDescription extends INodeTypeBaseDescription {
}
| boolean;
extendsCredential?: string;
hints?: NodeHint[];
__loadOptionsMethods?: string[]; // only for validation during build
}
export type NodeHint = {
message: string;
type?: 'info' | 'warning' | 'danger';
location?: 'outputPane' | 'inputPane' | 'ndv';
displayCondition?: string;
whenToDisplay?: 'always' | 'beforeExecution' | 'afterExecution';
};
export type NodeExecutionHint = Omit<NodeHint, 'whenToDisplay' | 'displayCondition'>;
export interface INodeHookDescription {
method: string;
}
@ -1929,6 +1954,7 @@ export interface ITaskData {
data?: ITaskDataConnections;
inputOverride?: ITaskDataConnections;
error?: ExecutionError;
hints?: NodeExecutionHint[];
source: Array<ISourceData | null>; // Is an array as nodes have multiple inputs
metadata?: ITaskMetadata;
}

View file

@ -42,6 +42,7 @@ import type {
INodeInputConfiguration,
GenericValue,
DisplayCondition,
NodeHint,
} from './Interfaces';
import {
isFilterValue,
@ -1120,6 +1121,50 @@ export function getNodeInputs(
}
}
export function getNodeHints(
workflow: Workflow,
node: INode,
nodeTypeData: INodeTypeDescription,
): NodeHint[] {
const hints: NodeHint[] = [];
if (nodeTypeData?.hints?.length) {
for (const hint of nodeTypeData.hints) {
if (hint.displayCondition) {
try {
const display = (workflow.expression.getSimpleParameterValue(
node,
hint.displayCondition,
'internal',
{},
) || false) as boolean;
if (typeof display !== 'boolean') {
console.warn(
`Condition was not resolved as boolean in '${node.name}' node for hint: `,
hint.message,
);
continue;
}
if (display) {
hints.push(hint);
}
} catch (e) {
console.warn(
`Could not calculate display condition in '${node.name}' node for hint: `,
hint.message,
);
}
} else {
hints.push(hint);
}
}
}
return hints;
}
export function getNodeOutputs(
workflow: Workflow,
node: INode,

View file

@ -1,5 +1,7 @@
import type { INodeParameters, INodeProperties } from '@/Interfaces';
import { getNodeParameters } from '@/NodeHelpers';
import type { INode, INodeParameters, INodeProperties, INodeTypeDescription } from '@/Interfaces';
import type { Workflow } from '../src';
import { getNodeParameters, getNodeHints } from '@/NodeHelpers';
describe('NodeHelpers', () => {
describe('getNodeParameters', () => {
@ -3437,4 +3439,93 @@ describe('NodeHelpers', () => {
});
}
});
describe('getNodeHints', () => {
//TODO: Add more tests here when hints are added to some node types
test('should return node hints if present in node type', () => {
const testType = {
hints: [
{
message: 'TEST HINT',
},
],
} as INodeTypeDescription;
const workflow = {} as unknown as Workflow;
const node: INode = {
name: 'Test Node Hints',
} as INode;
const nodeType = testType;
const hints = getNodeHints(workflow, node, nodeType);
expect(hints).toHaveLength(1);
expect(hints[0].message).toEqual('TEST HINT');
});
test('should not include hint if displayCondition is false', () => {
const testType = {
hints: [
{
message: 'TEST HINT',
displayCondition: 'FALSE DISPLAY CONDITION EXPESSION',
},
],
} as INodeTypeDescription;
const workflow = {
expression: {
getSimpleParameterValue(
_node: string,
_parameter: string,
_mode: string,
_additionalData = {},
) {
return false;
},
},
} as unknown as Workflow;
const node: INode = {
name: 'Test Node Hints',
} as INode;
const nodeType = testType;
const hints = getNodeHints(workflow, node, nodeType);
expect(hints).toHaveLength(0);
});
test('should include hint if displayCondition is true', () => {
const testType = {
hints: [
{
message: 'TEST HINT',
displayCondition: 'TRUE DISPLAY CONDITION EXPESSION',
},
],
} as INodeTypeDescription;
const workflow = {
expression: {
getSimpleParameterValue(
_node: string,
_parameter: string,
_mode: string,
_additionalData = {},
) {
return true;
},
},
} as unknown as Workflow;
const node: INode = {
name: 'Test Node Hints',
} as INode;
const nodeType = testType;
const hints = getNodeHints(workflow, node, nodeType);
expect(hints).toHaveLength(1);
});
});
});