fix(editor): Enable pinning main output with error and always allow unpinning (#11452)

Co-authored-by: Mutasem Aldmour <4711238+mutdmour@users.noreply.github.com>

packages/editor-ui/src/components/RunData.vue

@@ -289,7 +289,7 @@ export default defineComponent({
 				return false;
 			}
 
-			const canPinNode = usePinnedData(this.node).canPinNode(false);
+			const canPinNode = usePinnedData(this.node).canPinNode(false, this.currentOutputIndex);
 
 			return (
 				canPinNode &&
@@ -1214,9 +1214,7 @@ export default defineComponent({
 <template>
 	<div :class="['run-data', $style.container]" @mouseover="activatePane">
 		<n8n-callout
-			v-if="
+			v-if="pinnedData.hasData.value && !editMode.enabled && !isProductionExecutionPreview"
-				canPinData && pinnedData.hasData.value && !editMode.enabled && !isProductionExecutionPreview
-			"
 			theme="secondary"
 			icon="thumbtack"
 			:class="$style.pinnedDataCallout"

packages/editor-ui/src/composables/usePinnedData.test.ts

@@ -3,9 +3,10 @@ import { setActivePinia, createPinia } from 'pinia';
 import { ref } from 'vue';
 import { usePinnedData } from '@/composables/usePinnedData';
 import type { INodeUi } from '@/Interface';
-import { MAX_PINNED_DATA_SIZE } from '@/constants';
+import { HTTP_REQUEST_NODE_TYPE, IF_NODE_TYPE, MAX_PINNED_DATA_SIZE } from '@/constants';
 import { useWorkflowsStore } from '@/stores/workflows.store';
 import { useTelemetry } from '@/composables/useTelemetry';
+import { NodeConnectionType, STICKY_NODE_TYPE, type INodeTypeDescription } from 'n8n-workflow';
 
 vi.mock('@/composables/useToast', () => ({ useToast: vi.fn(() => ({ showError: vi.fn() })) }));
 vi.mock('@/composables/useI18n', () => ({
@@ -17,6 +18,13 @@ vi.mock('@/composables/useExternalHooks', () => ({
 	})),
 }));
 
+const getNodeType = vi.fn();
+vi.mock('@/stores/nodeTypes.store', () => ({
+	useNodeTypesStore: vi.fn(() => ({
+		getNodeType,
+	})),
+}));
+
 describe('usePinnedData', () => {
 	beforeEach(() => {
 		setActivePinia(createPinia());
@@ -133,4 +141,127 @@ describe('usePinnedData', () => {
 			expect(spy).toHaveBeenCalled();
 		});
 	});
+
+	describe('canPinData()', () => {
+		afterEach(() => {
+			vi.clearAllMocks();
+		});
+
+		it('allows pin on single output', async () => {
+			const node = ref({
+				name: 'single output node',
+				typeVersion: 1,
+				type: HTTP_REQUEST_NODE_TYPE,
+
+				parameters: {},
+				onError: 'stopWorkflow',
+			} as INodeUi);
+			getNodeType.mockReturnValue(makeNodeType([NodeConnectionType.Main], HTTP_REQUEST_NODE_TYPE));
+
+			const { canPinNode } = usePinnedData(node);
+
+			expect(canPinNode()).toBe(true);
+			expect(canPinNode(false, 0)).toBe(true);
+			// validate out of range index
+			expect(canPinNode(false, 1)).toBe(false);
+			expect(canPinNode(false, -1)).toBe(false);
+		});
+
+		it('allows pin on one main and one error output', async () => {
+			const node = ref({
+				name: 'single output node',
+				typeVersion: 1,
+				type: HTTP_REQUEST_NODE_TYPE,
+				parameters: {},
+				onError: 'continueErrorOutput',
+			} as INodeUi);
+			getNodeType.mockReturnValue(makeNodeType([NodeConnectionType.Main], HTTP_REQUEST_NODE_TYPE));
+
+			const { canPinNode } = usePinnedData(node);
+
+			expect(canPinNode()).toBe(true);
+			expect(canPinNode(false, 0)).toBe(true);
+			expect(canPinNode(false, 1)).toBe(false);
+			// validate out of range index
+			expect(canPinNode(false, 2)).toBe(false);
+			expect(canPinNode(false, -1)).toBe(false);
+		});
+
+		it('does not allow pin on two main outputs', async () => {
+			const node = ref({
+				name: 'single output node',
+				typeVersion: 1,
+				type: IF_NODE_TYPE,
+				parameters: {},
+				onError: 'stopWorkflow',
+			} as INodeUi);
+			getNodeType.mockReturnValue(
+				makeNodeType([NodeConnectionType.Main, NodeConnectionType.Main], IF_NODE_TYPE),
+			);
+
+			const { canPinNode } = usePinnedData(node);
+
+			expect(canPinNode()).toBe(false);
+			expect(canPinNode(false, 0)).toBe(false);
+			expect(canPinNode(false, 1)).toBe(false);
+			// validate out of range index
+			expect(canPinNode(false, 2)).toBe(false);
+			expect(canPinNode(false, -1)).toBe(false);
+		});
+
+		it('does not allow pin on denylisted node', async () => {
+			const node = ref({
+				name: 'single output node',
+				typeVersion: 1,
+				type: STICKY_NODE_TYPE,
+			} as INodeUi);
+			const { canPinNode } = usePinnedData(node);
+
+			expect(canPinNode()).toBe(false);
+			expect(canPinNode(false, 0)).toBe(false);
+		});
+
+		it('does not allow pin with checkDataEmpty and no pin', async () => {
+			const node = ref({
+				name: 'single output node',
+				typeVersion: 1,
+				type: HTTP_REQUEST_NODE_TYPE,
+			} as INodeUi);
+			getNodeType.mockReturnValue(makeNodeType([NodeConnectionType.Main], HTTP_REQUEST_NODE_TYPE));
+
+			const { canPinNode } = usePinnedData(node);
+
+			expect(canPinNode(true)).toBe(false);
+			expect(canPinNode(true, 0)).toBe(false);
+		});
+
+		it('does not allow pin without output', async () => {
+			const node = ref({
+				name: 'zero output node',
+				typeVersion: 1,
+				type: 'n8n-nodes-base.stopAndError',
+			} as INodeUi);
+			getNodeType.mockReturnValue(makeNodeType([], 'n8n-nodes-base.stopAndError'));
+
+			const { canPinNode } = usePinnedData(node);
+
+			expect(canPinNode()).toBe(false);
+			expect(canPinNode(false, 0)).toBe(false);
+			expect(canPinNode(false, -1)).toBe(false);
+			expect(canPinNode(false, 1)).toBe(false);
+		});
+	});
 });
+
+const makeNodeType = (outputs: NodeConnectionType[], name: string) =>
+	({
+		displayName: name,
+		name,
+		version: [1],
+		inputs: [],
+		outputs,
+		properties: [],
+		defaults: { color: '', name: '' },
+		group: [],
+		description: '',
+	}) as INodeTypeDescription;

packages/editor-ui/src/composables/usePinnedData.ts

@@ -75,9 +75,9 @@ export function usePinnedData(
 		);
 	});
 
-	function canPinNode(checkDataEmpty = false) {
+	function canPinNode(checkDataEmpty = false, outputIndex?: number) {
 		const targetNode = unref(node);
-		if (targetNode === null) return false;
+		if (targetNode === null || PIN_DATA_NODE_TYPES_DENYLIST.includes(targetNode.type)) return false;
 
 		const nodeType = useNodeTypesStore().getNodeType(targetNode.type, targetNode.typeVersion);
 		const dataToPin = getInputDataWithPinned(targetNode);
@@ -85,14 +85,25 @@ export function usePinnedData(
 		if (!nodeType || (checkDataEmpty && dataToPin.length === 0)) return false;
 
 		const workflow = workflowsStore.getCurrentWorkflow();
-		const outputs = NodeHelpers.getNodeOutputs(workflow, targetNode, nodeType);
+		const outputs = NodeHelpers.getNodeOutputs(workflow, targetNode, nodeType).map((output) =>
-		const mainOutputs = outputs.filter((output) =>
+			typeof output === 'string' ? { type: output } : output,
-			typeof output === 'string'
-				? output === NodeConnectionType.Main
-				: output.type === NodeConnectionType.Main,
 		);
 
-		return mainOutputs.length === 1 && !PIN_DATA_NODE_TYPES_DENYLIST.includes(targetNode.type);
+		const mainOutputs = outputs.filter(
+			(output) => output.type === NodeConnectionType.Main && output.category !== 'error',
+		);
+
+		let indexAcceptable = true;
+
+		if (outputIndex !== undefined) {
+			const output = outputs[outputIndex];
+
+			if (outputs[outputIndex] === undefined) return false;
+
+			indexAcceptable = output.type === NodeConnectionType.Main && output.category !== 'error';
+		}
+
+		return mainOutputs.length === 1 && indexAcceptable;
 	}
 
 	function isValidJSON(data: string): boolean {