Merge c09bfa0940 into d2dd1796a8

cypress/e2e/26-resource-locator.cy.ts

@@ -5,8 +5,8 @@ const workflowPage = new WorkflowPage();
 const ndv = new NDV();
 const credentialsModal = new CredentialsModal();
 
-const NO_CREDENTIALS_MESSAGE = 'Please add your credential';
-const INVALID_CREDENTIALS_MESSAGE = 'Please check your credential';
+const NO_CREDENTIALS_MESSAGE = 'Add your credential';
+const INVALID_CREDENTIALS_MESSAGE = 'Check your credential';
 const MODE_SELECTOR_LIST = 'From list';
 
 describe('Resource Locator', () => {
@@ -69,15 +69,6 @@ describe('Resource Locator', () => {
 		ndv.getters.resourceLocator('owner').should('be.visible');
 		ndv.getters.resourceLocatorInput('owner').click();
 		ndv.getters.resourceLocatorErrorMessage().should('contain', NO_CREDENTIALS_MESSAGE);
-
-		workflowPage.getters.nodeCredentialsSelect().click();
-		workflowPage.getters.nodeCredentialsCreateOption().click();
-		credentialsModal.getters.credentialsEditModal().should('be.visible');
-		credentialsModal.actions.fillCredentialsForm();
-
-		ndv.getters.resourceLocatorInput('owner').click();
-		ndv.getters.resourceLocatorSearch('owner').type('owner');
-		ndv.getters.resourceLocatorErrorMessage().should('contain', INVALID_CREDENTIALS_MESSAGE);
 	});
 
 	it('should reset resource locator when dependent field is changed', () => {

packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.test.constants.ts

@@ -0,0 +1,84 @@
+import type { INode, INodeParameterResourceLocator, INodeProperties } from 'n8n-workflow';
+
+export const TEST_MODEL_VALUE: INodeParameterResourceLocator = {
+	__rl: true,
+	value: 'test',
+	mode: 'list',
+	cachedResultName: 'table',
+	cachedResultUrl: 'https://test.com/test',
+};
+
+export const TEST_PARAMETER_MULTI_MODE: INodeProperties = {
+	displayName: 'Test Parameter',
+	name: 'testParamMultiMode',
+	type: 'resourceLocator',
+	default: { mode: 'list', value: '' },
+	required: true,
+	modes: [
+		{
+			displayName: 'From List',
+			name: 'list',
+			type: 'list',
+			typeOptions: { searchListMethod: 'testSearch', searchable: true },
+		},
+		{
+			displayName: 'By URL',
+			name: 'url',
+			type: 'string',
+			placeholder: 'https://test.com/test',
+		},
+		{
+			displayName: 'ID',
+			name: 'id',
+			type: 'string',
+			placeholder: 'id',
+		},
+	],
+};
+
+export const TEST_PARAMETER_SINGLE_MODE: INodeProperties = {
+	...TEST_PARAMETER_MULTI_MODE,
+	name: 'testParameterSingleMode',
+	modes: [
+		{
+			displayName: 'From List',
+			name: 'list',
+			type: 'list',
+			typeOptions: { searchListMethod: 'testSearch', searchable: true },
+		},
+	],
+};
+
+export const TEST_NODE_MULTI_MODE: INode = {
+	type: 'n8n-nodes-base.airtable',
+	typeVersion: 2.1,
+	position: [80, -260],
+	id: '377e4287-b1e0-44cc-ba0f-7bb3d676d60c',
+	name: 'Test Node - Multi Mode',
+	parameters: {
+		authentication: 'testAuth',
+		resource: 'test',
+		operation: 'get',
+		testParamMultiMode: TEST_MODEL_VALUE,
+		id: '',
+		options: {},
+	},
+	credentials: {
+		testAuth: {
+			id: '1234',
+			name: 'Test Account',
+		},
+	},
+};
+
+export const TEST_NODE_SINGLE_MODE: INode = {
+	...TEST_NODE_MULTI_MODE,
+	parameters: {
+		authentication: 'testAuth',
+		resource: 'test',
+		operation: 'get',
+		testParameterSingleMode: TEST_MODEL_VALUE,
+		id: '',
+		options: {},
+	},
+};

packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.test.ts

@@ -0,0 +1,232 @@
+import { createComponentRenderer } from '@/__tests__/render';
+import { useNodeTypesStore } from '@/stores/nodeTypes.store';
+import ResourceLocator from './ResourceLocator.vue';
+import { createTestingPinia } from '@pinia/testing';
+import userEvent from '@testing-library/user-event';
+import { screen, waitFor } from '@testing-library/vue';
+import { mockedStore } from '@/__tests__/utils';
+import {
+	TEST_MODEL_VALUE,
+	TEST_NODE_MULTI_MODE,
+	TEST_NODE_SINGLE_MODE,
+	TEST_PARAMETER_MULTI_MODE,
+	TEST_PARAMETER_SINGLE_MODE,
+} from './ResourceLocator.test.constants';
+
+vi.mock('vue-router', async () => {
+	const actual = await vi.importActual('vue-router');
+	const params = {};
+	const location = {};
+	return {
+		...actual,
+		useRouter: () => ({
+			push: vi.fn(),
+		}),
+		useRoute: () => ({
+			params,
+			location,
+		}),
+	};
+});
+vi.mock('@/composables/useWorkflowHelpers', () => {
+	return {
+		useWorkflowHelpers: vi.fn(() => ({
+			resolveExpression: vi.fn().mockImplementation((val) => val),
+			resolveRequiredParameters: vi.fn().mockImplementation((_, params) => params),
+		})),
+	};
+});
+vi.mock('@/composables/useTelemetry', () => ({
+	useTelemetry: () => ({ track: vi.fn() }),
+}));
+
+let nodeTypesStore: ReturnType<typeof mockedStore<typeof useNodeTypesStore>>;
+
+const renderComponent = createComponentRenderer(ResourceLocator, {
+	props: {
+		modelValue: TEST_MODEL_VALUE,
+		parameter: TEST_PARAMETER_MULTI_MODE,
+		path: `parameters.${TEST_PARAMETER_MULTI_MODE.name}`,
+		node: TEST_NODE_MULTI_MODE,
+		displayTitle: 'Test Resource Locator',
+		expressionComputedValue: '',
+		isValueExpression: false,
+	},
+	global: {
+		stubs: {
+			ResourceLocatorDropdown: false,
+			ExpressionParameterInput: true,
+			ParameterIssues: true,
+			N8nCallout: true,
+			'font-awesome-icon': true,
+			FromAiOverrideField: true,
+			FromAiOverrideButton: true,
+			ParameterOverrideSelectableList: true,
+		},
+	},
+});
+
+describe('ResourceLocator', () => {
+	beforeEach(() => {
+		createTestingPinia();
+		nodeTypesStore = mockedStore(useNodeTypesStore);
+		nodeTypesStore.getNodeType = vi.fn().mockReturnValue({ displayName: 'Test Node' });
+	});
+	afterEach(() => {
+		vi.clearAllMocks();
+	});
+
+	it('renders multi-mode correctly', async () => {
+		const { getByTestId } = renderComponent();
+		expect(getByTestId(`resource-locator-${TEST_PARAMETER_MULTI_MODE.name}`)).toBeInTheDocument();
+		// Should render mode selector with all available modes
+		expect(getByTestId('rlc-mode-selector')).toBeInTheDocument();
+		await userEvent.click(getByTestId('rlc-mode-selector'));
+		TEST_PARAMETER_MULTI_MODE.modes?.forEach((mode) => {
+			expect(screen.getByTestId(`mode-${mode.name}`)).toBeInTheDocument();
+		});
+	});
+
+	it('renders single mode correctly', async () => {
+		const { getByTestId, queryByTestId } = renderComponent({
+			props: {
+				modelValue: TEST_MODEL_VALUE,
+				parameter: TEST_PARAMETER_SINGLE_MODE,
+				path: `parameters.${TEST_PARAMETER_SINGLE_MODE.name}`,
+				node: TEST_NODE_SINGLE_MODE,
+				displayTitle: 'Test Resource Locator',
+				expressionComputedValue: '',
+			},
+		});
+		expect(getByTestId(`resource-locator-${TEST_PARAMETER_SINGLE_MODE.name}`)).toBeInTheDocument();
+		// Should not render mode selector
+		expect(queryByTestId('rlc-mode-selector')).not.toBeInTheDocument();
+	});
+
+	it('renders fetched resources correctly', async () => {
+		const TEST_ITEMS = [
+			{ name: 'Test Resource', value: 'test-resource', url: 'https://test.com/test-resource' },
+			{
+				name: 'Test Resource 2',
+				value: 'test-resource-2',
+				url: 'https://test.com/test-resource-2',
+			},
+		];
+		nodeTypesStore.getResourceLocatorResults.mockResolvedValue({
+			results: TEST_ITEMS,
+			paginationToken: null,
+		});
+		const { getByTestId, getByText, getAllByTestId } = renderComponent();
+
+		expect(getByTestId(`resource-locator-${TEST_PARAMETER_MULTI_MODE.name}`)).toBeInTheDocument();
+		// Click on the input to fetch resources
+		await userEvent.click(getByTestId('rlc-input'));
+		// Wait for the resources to be fetched
+		await waitFor(() => {
+			expect(nodeTypesStore.getResourceLocatorResults).toHaveBeenCalled();
+		});
+		// Expect the items to be rendered
+		expect(getAllByTestId('rlc-item')).toHaveLength(TEST_ITEMS.length);
+		// We should be getting one item for each result
+		TEST_ITEMS.forEach((item) => {
+			expect(getByText(item.name)).toBeInTheDocument();
+		});
+	});
+
+	// Testing error message deduplication
+	describe('ResourceLocator credentials error handling', () => {
+		it.each([
+			{
+				testName: 'period-separated credential message',
+				error: {
+					message: 'Authentication failed. Please check your credentials.',
+					httpCode: '401',
+					description: 'Authentication failed. Please check your credentials.',
+				},
+				expectedMessage: 'Authentication failed.',
+			},
+			{
+				testName: 'dash-separated credential message',
+				error: {
+					message: 'Authentication failed - Please check your credentials.',
+					httpCode: '401',
+					description: 'Authentication failed. Please check your credentials.',
+				},
+				expectedMessage: 'Authentication failed',
+			},
+			{
+				testName: 'credential message with "Perhaps" phrasing',
+				error: {
+					message: 'Authentication failed - Perhaps check your credentials?',
+					httpCode: '401',
+					description: 'Authentication failed. Please check your credentials.',
+				},
+				expectedMessage: 'Authentication failed',
+			},
+			{
+				testName: 'singular credential phrasing',
+				error: {
+					message: 'Authentication failed. You should check your credential.',
+					httpCode: '401',
+					description: 'Authentication failed.',
+				},
+				expectedMessage: 'Authentication failed.',
+			},
+			{
+				testName: 'verify credentials phrasing',
+				error: {
+					message: 'Authentication failed - Please verify your credentials.',
+					httpCode: '401',
+					description: 'Authentication failed.',
+				},
+				expectedMessage: 'Authentication failed',
+			},
+		])('$testName', async ({ error, expectedMessage }) => {
+			nodeTypesStore.getResourceLocatorResults.mockRejectedValue(error);
+
+			const { getByTestId, findByTestId } = renderComponent();
+
+			expect(getByTestId(`resource-locator-${TEST_PARAMETER_MULTI_MODE.name}`)).toBeInTheDocument();
+
+			await userEvent.click(getByTestId('rlc-input'));
+			await waitFor(() => {
+				expect(nodeTypesStore.getResourceLocatorResults).toHaveBeenCalled();
+			});
+
+			const errorContainer = await findByTestId('rlc-error-container');
+			expect(errorContainer).toBeInTheDocument();
+
+			expect(getByTestId('rlc-error-code')).toHaveTextContent(error.httpCode);
+			expect(getByTestId('rlc-error-message')).toHaveTextContent(expectedMessage);
+
+			expect(getByTestId('permission-error-link')).toBeInTheDocument();
+		});
+
+		it('renders generic error correctly', async () => {
+			const TEST_500_ERROR = {
+				message: 'Whoops',
+				httpCode: '500',
+				description: 'Something went wrong. Please try again later.',
+			};
+
+			nodeTypesStore.getResourceLocatorResults.mockRejectedValue(TEST_500_ERROR);
+			const { getByTestId, findByTestId, queryByTestId } = renderComponent();
+
+			expect(getByTestId(`resource-locator-${TEST_PARAMETER_MULTI_MODE.name}`)).toBeInTheDocument();
+
+			await userEvent.click(getByTestId('rlc-input'));
+
+			await waitFor(() => {
+				expect(nodeTypesStore.getResourceLocatorResults).toHaveBeenCalled();
+			});
+
+			const errorContainer = await findByTestId('rlc-error-container');
+			expect(errorContainer).toBeInTheDocument();
+
+			expect(errorContainer).toHaveTextContent(TEST_500_ERROR.httpCode);
+			expect(errorContainer).toHaveTextContent(TEST_500_ERROR.message);
+
+			expect(queryByTestId('permission-error-link')).not.toBeInTheDocument();
+		});
+	});
+});

packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.vue

@@ -53,11 +53,32 @@ import {
 	updateFromAIOverrideValues,
 	type FromAIOverride,
 } from '../../utils/fromAIOverrideUtils';
+import { N8nNotice } from '@n8n/design-system';
+
+/**
+ * Regular expression to check if the error message contains credential-related phrases.
+ */
+const CHECK_CREDENTIALS_REGEX = /check\s+(your\s+)?credentials?/i;
+/**
+ * Error codes and messages that indicate a permission error.
+ */
+const PERMISSION_ERROR_CODES = ['401', '403'];
+const NODE_API_AUTH_ERROR_MESSAGES = [
+	'NodeApiError: Authorization failed',
+	'NodeApiError: Unable to sign without access token',
+	'secretOrPrivateKey must be an asymmetric key when using RS256',
+];
 
 interface IResourceLocatorQuery {
 	results: INodeListSearchItems[];
 	nextPageToken: unknown;
 	error: boolean;
+	errorDetails?: {
+		message?: string;
+		description?: string;
+		httpCode?: string;
+		stackTrace?: string;
+	};
 	loading: boolean;
 }
 
@@ -148,12 +169,21 @@ const selectedMode = computed(() => {
 
 const isListMode = computed(() => selectedMode.value === 'list');
 
-const hasCredential = computed(() => {
-	const node = ndvStore.activeNode;
-	if (!node) {
-		return false;
-	}
-	return !!(node?.credentials && Object.keys(node.credentials).length === 1);
+/**
+ * Check if the current response contains an error that indicates a credential issue.
+ * We do this by checking error http code
+ * But, since out NodeApiErrors sometimes just return 500, we also check the message and stack trace
+ */
+const hasCredentialError = computed(() => {
+	const stackTraceContainsCredentialError = (currentResponse.value?.errorDetails?.stackTrace ?? '')
+		.split('\n')
+		.some((line) => NODE_API_AUTH_ERROR_MESSAGES.includes(line.trim()));
+
+	return (
+		PERMISSION_ERROR_CODES.includes(currentResponse.value?.errorDetails?.httpCode ?? '') ||
+		NODE_API_AUTH_ERROR_MESSAGES.includes(currentResponse.value?.errorDetails?.message ?? '') ||
+		stackTraceContainsCredentialError
+	);
 });
 
 const credentialsNotSet = computed(() => {
@@ -643,10 +673,41 @@ async function loadResources() {
 		setResponse(paramsKey, {
 			loading: false,
 			error: true,
+			errorDetails: {
+				message: removeDuplicateTextFromErrorMessage(e.message),
+				description: e.description,
+				httpCode: e.httpCode,
+				stackTrace: e.stacktrace,
+			},
 		});
 	}
 }
 
+/**
+ * Removes duplicate credential-related sentences from error messages.
+ * We are already showing a link to create/check the credentials, so we don't need to repeat the same message.
+ */
+function removeDuplicateTextFromErrorMessage(message: string): string {
+	let segments: string[] = [];
+
+	// Split message into sentences or segments
+	if (/[-–—]/.test(message)) {
+		// By various dash types
+		segments = message.split(/\s*[-–—]\s*/);
+	} else {
+		// By sentence boundaries
+		segments = message.split(/(?<=[.!?])\s+/);
+	}
+
+	// Filter out segments containing credential check phrases
+	const filteredSegments = segments.filter((segment: string) => {
+		if (!segment.trim()) return false;
+		return !CHECK_CREDENTIALS_REGEX.test(segment);
+	});
+
+	return filteredSegments.join(' ').trim();
+}
+
 function onInputFocus(): void {
 	if (!isListMode.value || resourceDropdownVisible.value) {
 		return;
@@ -777,19 +838,44 @@ function removeOverride() {
 			@load-more="loadResourcesDebounced"
 		>
 			<template #error>
-				<div :class="$style.error" data-test-id="rlc-error-container">
-					<n8n-text color="text-dark" align="center" tag="div">
+				<div :class="$style.errorContainer" data-test-id="rlc-error-container">
+					<n8n-text
+						v-if="credentialsNotSet || currentResponse.errorDetails"
+						color="text-dark"
+						align="center"
+						tag="div"
+					>
 						{{ i18n.baseText('resourceLocator.mode.list.error.title') }}
 					</n8n-text>
-					<n8n-text v-if="hasCredential || credentialsNotSet" size="small" color="text-base">
-						{{ i18n.baseText('resourceLocator.mode.list.error.description.part1') }}
-						<a v-if="credentialsNotSet" @click="createNewCredential">{{
-							i18n.baseText('resourceLocator.mode.list.error.description.part2.noCredentials')
-						}}</a>
-						<a v-else-if="hasCredential" @click="openCredential">{{
-							i18n.baseText('resourceLocator.mode.list.error.description.part2.hasCredentials')
-						}}</a>
+					<div v-if="currentResponse.errorDetails" :class="$style.errorDetails">
+						<n8n-text size="small">
+							<span v-if="currentResponse.errorDetails.httpCode" data-test-id="rlc-error-code">
+								{{ currentResponse.errorDetails.httpCode }} -
+							</span>
+							<span data-test-id="rlc-error-message">{{
+								currentResponse.errorDetails.message
+							}}</span>
 						</n8n-text>
+						<N8nNotice
+							v-if="currentResponse.errorDetails.description"
+							theme="warning"
+							:class="$style.errorDescription"
+						>
+							{{ currentResponse.errorDetails.description }}
+						</N8nNotice>
+					</div>
+					<div v-if="hasCredentialError || credentialsNotSet" data-test-id="permission-error-link">
+						<a
+							v-if="credentialsNotSet"
+							:class="$style['credential-link']"
+							@click="createNewCredential"
+						>
+							{{ i18n.baseText('resourceLocator.mode.list.error.description.noCredentials') }}
+						</a>
+						<a v-else :class="$style['credential-link']" @click="openCredential">
+							{{ i18n.baseText('resourceLocator.mode.list.error.description.checkCredentials') }}
+						</a>
+					</div>
 				</div>
 			</template>
 			<div
@@ -820,6 +906,7 @@ function removeOverride() {
 						<n8n-option
 							v-for="mode in parameter.modes"
 							:key="mode.name"
+							:data-test-id="`mode-${mode.name}`"
 							:value="mode.name"
 							:label="getModeLabel(mode)"
 							:disabled="isValueExpression && mode.name === 'list'"

packages/frontend/editor-ui/src/components/ResourceLocator/resourceLocator.scss

@@ -140,12 +140,29 @@ $--mode-selector-width: 92px;
 	cursor: pointer;
 }
 
-.error {
-	max-width: 170px;
-	word-break: normal;
+.errorContainer,
+.errorDetails {
+	display: flex;
+	gap: var(--spacing-s);
+	flex-direction: column;
+}
+
+.errorContainer {
+	width: 80%;
+	align-items: center;
 	text-align: center;
 }
 
+.errorDescription {
+	text-align: left;
+	margin: 0;
+	word-break: normal;
+}
+
+.credential-link {
+	font-size: var(--font-size-2xs);
+}
+
 .openResourceLink {
 	width: 25px !important;
 	padding-left: var(--spacing-2xs);

packages/frontend/editor-ui/src/plugins/i18n/locales/en.json

@@ -1590,9 +1590,8 @@
 	"resourceLocator.mode.list": "From list",
 	"resourceLocator.mode.list.disabled.title": "Change to Fixed mode to choose From List",
 	"resourceLocator.mode.list.error.title": "Could not load list",
-	"resourceLocator.mode.list.error.description.part1": "Please",
-	"resourceLocator.mode.list.error.description.part2.hasCredentials": "check your credential",
-	"resourceLocator.mode.list.error.description.part2.noCredentials": "add your credential",
+	"resourceLocator.mode.list.error.description.checkCredentials": "Check your credential",
+	"resourceLocator.mode.list.error.description.noCredentials": "Add your credential",
 	"resourceLocator.mode.list.noResults": "No results",
 	"resourceLocator.mode.list.openUrl": "Open URL",
 	"resourceLocator.mode.list.placeholder": "Choose...",