Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: More hints to nodes #10565

Merged
merged 15 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading