Skip to content

Commit

Permalink
feat: More hints to nodes (n8n-io#10565)
Browse files Browse the repository at this point in the history
Co-authored-by: Giulio Andreini <[email protected]>
Co-authored-by: Shireen Missi <[email protected]>
  • Loading branch information
3 people authored Sep 4, 2024
1 parent cd0891e commit 66ddb4a
Show file tree
Hide file tree
Showing 6 changed files with 325 additions and 3 deletions.
24 changes: 23 additions & 1 deletion packages/editor-ui/src/components/RunData.vue
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import { useExternalHooks } from '@/composables/useExternalHooks';
import { useSourceControlStore } from '@/stores/sourceControl.store';
import { useRootStore } from '@/stores/root.store';
import RunDataPinButton from '@/components/RunDataPinButton.vue';
import { getGenericHints } from '@/utils/nodeViewUtils';
const LazyRunDataTable = defineAsyncComponent(
async () => await import('@/components/RunDataTable.vue'),
Expand Down Expand Up @@ -516,6 +517,10 @@ export default defineComponent({
return parentNodeData;
},
parentNodePinnedData(): INodeExecutionData[] {
const parentNode = this.workflow.getParentNodesByDepth(this.node?.name ?? '')[0];
return this.workflow.pinData?.[parentNode?.name || ''] || [];
},
},
watch: {
node(newNode: INodeUi, prevNode: INodeUi) {
Expand Down Expand Up @@ -645,13 +650,30 @@ export default defineComponent({
if (workflowNode) {
const executionHints = this.executionHints;
const nodeHints = NodeHelpers.getNodeHints(this.workflow, workflowNode, this.nodeType, {
runExecutionData: this.workflowExecution?.data ?? null,
runIndex: this.runIndex,
connectionInputData: this.parentNodeOutputData,
});
return executionHints.concat(nodeHints).filter(this.shouldHintBeDisplayed);
const hasMultipleInputItems =
this.parentNodeOutputData.length > 1 || this.parentNodePinnedData.length > 1;
const nodeOutputData =
this.workflowRunData?.[this.node.name]?.[this.runIndex]?.data?.main[0] || [];
const genericHints = getGenericHints({
workflowNode,
node: this.node,
nodeType: this.nodeType,
nodeOutputData,
workflow: this.workflow,
hasNodeRun: this.hasNodeRun,
hasMultipleInputItems,
});
return executionHints.concat(nodeHints, genericHints).filter(this.shouldHintBeDisplayed);
}
}
return [];
Expand Down
2 changes: 2 additions & 0 deletions packages/editor-ui/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,8 @@ export const OPEN_URL_PANEL_TRIGGER_NODE_TYPES = [
CHAT_TRIGGER_NODE_TYPE,
];

export const LIST_LIKE_NODE_OPERATIONS = ['getAll', 'getMany', 'read', 'search'];

export const PRODUCTION_ONLY_TRIGGER_NODE_TYPES = [CHAT_TRIGGER_NODE_TYPE];

// Node creator
Expand Down
139 changes: 139 additions & 0 deletions packages/editor-ui/src/utils/__tests__/nodeViewUtils.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
import { getGenericHints } from '../nodeViewUtils';
import type { INode, INodeTypeDescription, INodeExecutionData, Workflow } from 'n8n-workflow';
import type { INodeUi } from '@/Interface';
import { NodeHelpers } from 'n8n-workflow';

import { describe, it, expect, beforeEach } from 'vitest';
import { mock, type MockProxy } from 'vitest-mock-extended';

describe('getGenericHints', () => {
let mockWorkflowNode: MockProxy<INode>;
let mockNode: MockProxy<INodeUi>;
let mockNodeType: MockProxy<INodeTypeDescription>;
let mockNodeOutputData: INodeExecutionData[];
let mockWorkflow: MockProxy<Workflow>;
let hasMultipleInputItems: boolean;
let hasNodeRun: boolean;

beforeEach(() => {
mockWorkflowNode = mock<INode>();
mockNode = mock<INodeUi>();
mockNodeType = mock<INodeTypeDescription>();
mockNodeOutputData = [];
mockWorkflow = mock<Workflow>();

hasMultipleInputItems = false;
hasNodeRun = false;
});

it('should return a limit reached hint if node output data reaches the limit', () => {
mockWorkflowNode.parameters.limit = 5;
mockNodeOutputData = Array(5).fill({ json: {} });
hasNodeRun = true;

const hints = getGenericHints({
workflowNode: mockWorkflowNode,
node: mockNode,
nodeType: mockNodeType,
nodeOutputData: mockNodeOutputData,
hasMultipleInputItems,
workflow: mockWorkflow,
hasNodeRun,
});

expect(hints).toEqual([
{
message:
"Limit of 5 items reached. There may be more items that aren't being returned. Tweak the 'Return All' or 'Limit' parameters to access more items.",
location: 'outputPane',
whenToDisplay: 'afterExecution',
},
]);
});

it('should return an Execute Once hint if operation is list-like and Execute Once is not set', () => {
mockWorkflowNode.parameters.operation = 'getAll';
hasMultipleInputItems = true;
mockWorkflow.getNode.mockReturnValue({ executeOnce: false } as unknown as INode);

const hints = getGenericHints({
workflowNode: mockWorkflowNode,
node: mockNode,
nodeType: mockNodeType,
nodeOutputData: mockNodeOutputData,
hasMultipleInputItems,
workflow: mockWorkflow,
hasNodeRun,
});

expect(hints).toEqual([
{
message:
"The operation is performed for each input item. Use the 'Execute Once' setting to execute it only once for the first input item.",
location: 'outputPane',
},
]);
});

it('should return a hint for expression in field name if found in Set node', () => {
mockNode.type = 'n8n-nodes-base.set';
mockNode.parameters.mode = 'manual';
mockNodeType.properties = [];

vi.spyOn(NodeHelpers, 'getNodeParameters').mockReturnValue({
assignments: {
assignments: [
{
id: 'xxxxx',
name: '=',
value: '',
type: 'string',
},
],
},
options: {},
});

const hints = getGenericHints({
workflowNode: mockWorkflowNode,
node: mockNode,
nodeType: mockNodeType,
nodeOutputData: mockNodeOutputData,
hasMultipleInputItems,
workflow: mockWorkflow,
hasNodeRun,
});

expect(hints).toEqual([
{
message:
"An expression is used in 'Fields to Set' in field 1, did you mean to use it in the value instead?",
whenToDisplay: 'beforeExecution',
location: 'outputPane',
},
]);
});

it('should return Split In Batches setup hints if loop is not set up correctly', () => {
mockNode.type = 'n8n-nodes-base.splitInBatches';
mockWorkflow.getChildNodes.mockReturnValue([]);

const hints = getGenericHints({
workflowNode: mockWorkflowNode,
node: mockNode,
nodeType: mockNodeType,
nodeOutputData: mockNodeOutputData,
hasMultipleInputItems,
workflow: mockWorkflow,
hasNodeRun,
});

expect(hints).toEqual([
{
message: "No nodes connected to the 'loop' output of this node",
whenToDisplay: 'beforeExecution',
location: 'outputPane',
},
]);
});
});
120 changes: 118 additions & 2 deletions packages/editor-ui/src/utils/nodeViewUtils.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
import { isNumber, isValidNodeConnectionType } from '@/utils/typeGuards';
import { NODE_OUTPUT_DEFAULT_KEY, STICKY_NODE_TYPE } from '@/constants';
import {
LIST_LIKE_NODE_OPERATIONS,
NODE_OUTPUT_DEFAULT_KEY,
SET_NODE_TYPE,
SPLIT_IN_BATCHES_NODE_TYPE,
STICKY_NODE_TYPE,
} from '@/constants';
import type { EndpointStyle, IBounds, INodeUi, XYPosition } from '@/Interface';
import type { ArrayAnchorSpec, ConnectorSpec, OverlaySpec, PaintStyle } from '@jsplumb/common';
import type { Connection, Endpoint, SelectOptions } from '@jsplumb/core';
import { N8nConnector } from '@/plugins/connectors/N8nCustomConnector';
import type {
AssignmentCollectionValue,
IConnection,
INode,
INodeExecutionData,
INodeTypeDescription,
ITaskData,
NodeHint,
NodeInputConnections,
Workflow,
} from 'n8n-workflow';
import { NodeConnectionType } from 'n8n-workflow';
import { NodeConnectionType, NodeHelpers } from 'n8n-workflow';
import type { BrowserJsPlumbInstance } from '@jsplumb/browser-ui';
import { EVENT_CONNECTION_MOUSEOUT, EVENT_CONNECTION_MOUSEOVER } from '@jsplumb/browser-ui';
import { useUIStore } from '@/stores/ui.store';
Expand Down Expand Up @@ -1173,3 +1183,109 @@ export const getJSPlumbConnection = (
return uuids[0] === sourceEndpoint && uuids[1] === targetEndpoint;
});
};

export function getGenericHints({
workflowNode,
node,
nodeType,
nodeOutputData,
hasMultipleInputItems,
workflow,
hasNodeRun,
}: {
workflowNode: INode;
node: INodeUi;
nodeType: INodeTypeDescription;
nodeOutputData: INodeExecutionData[];
hasMultipleInputItems: boolean;
workflow: Workflow;
hasNodeRun: boolean;
}) {
const nodeHints: NodeHint[] = [];

// add limit reached hint
if (hasNodeRun && workflowNode.parameters.limit) {
if (nodeOutputData.length === workflowNode.parameters.limit) {
nodeHints.push({
message: `Limit of ${workflowNode.parameters.limit as number} items reached. There may be more items that aren't being returned. Tweak the 'Return All' or 'Limit' parameters to access more items.`,
location: 'outputPane',
whenToDisplay: 'afterExecution',
});
}
}

// add Execute Once hint
if (
hasMultipleInputItems &&
LIST_LIKE_NODE_OPERATIONS.includes((workflowNode.parameters.operation as string) || '')
) {
const executeOnce = workflow.getNode(node.name)?.executeOnce;
if (!executeOnce) {
nodeHints.push({
message:
"The operation is performed for each input item. Use the 'Execute Once' setting to execute it only once for the first input item.",
location: 'outputPane',
});
}
}

// add expression in field name hint for Set node
if (node.type === SET_NODE_TYPE && node.parameters.mode === 'manual') {
const rawParameters = NodeHelpers.getNodeParameters(
nodeType.properties,
node.parameters,
true,
false,
node,
undefined,
false,
);

const assignments =
((rawParameters?.assignments as AssignmentCollectionValue) || {})?.assignments || [];
const expressionInFieldName: number[] = [];

for (const [index, assignment] of assignments.entries()) {
if (assignment.name.startsWith('=')) {
expressionInFieldName.push(index + 1);
}
}

if (expressionInFieldName.length > 0) {
nodeHints.push({
message: `An expression is used in 'Fields to Set' in ${expressionInFieldName.length === 1 ? 'field' : 'fields'} ${expressionInFieldName.join(', ')}, did you mean to use it in the value instead?`,
whenToDisplay: 'beforeExecution',
location: 'outputPane',
});
}
}

// Split In Batches setup hints
if (node.type === SPLIT_IN_BATCHES_NODE_TYPE) {
const { connectionsBySourceNode } = workflow;

const firstNodesInLoop = connectionsBySourceNode[node.name]?.main[1] || [];

if (!firstNodesInLoop.length) {
nodeHints.push({
message: "No nodes connected to the 'loop' output of this node",
whenToDisplay: 'beforeExecution',
location: 'outputPane',
});
} else {
for (const nodeInConnection of firstNodesInLoop || []) {
const nodeChilds = workflow.getChildNodes(nodeInConnection.node) || [];
if (!nodeChilds.includes(node.name)) {
nodeHints.push({
message:
"The last node in the branch of the 'loop' output must be connected back to the input of this node to loop correctly",
whenToDisplay: 'beforeExecution',
location: 'outputPane',
});
}
}
}
}

return nodeHints;
}
18 changes: 18 additions & 0 deletions packages/nodes-base/nodes/HttpRequest/V3/HttpRequestV3.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
import {
BINARY_ENCODING,
NodeApiError,
NodeExecutionOutput,
NodeConnectionType,
NodeOperationError,
jsonParse,
Expand Down Expand Up @@ -2138,6 +2139,23 @@ export class HttpRequestV3 implements INodeType {

returnItems = returnItems.map(replaceNullValues);

if (
returnItems.length === 1 &&
returnItems[0].json.data &&
Array.isArray(returnItems[0].json.data)
) {
return new NodeExecutionOutput(
[returnItems],
[
{
message:
"The result has a 'data' property which contains an array of items, you can split this array into separate items by using the 'Split Out' node",
location: 'outputPane',
},
],
);
}

return [returnItems];
}
}
Loading

0 comments on commit 66ddb4a

Please sign in to comment.