diff --git a/cypress/e2e/26-resource-locator.cy.ts b/cypress/e2e/26-resource-locator.cy.ts index 124e322c2b..5484febc5b 100644 --- a/cypress/e2e/26-resource-locator.cy.ts +++ b/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', () => { diff --git a/packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.test.constants.ts b/packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.test.constants.ts new file mode 100644 index 0000000000..57d161b752 --- /dev/null +++ b/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: {}, + }, +}; diff --git a/packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.test.ts b/packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.test.ts new file mode 100644 index 0000000000..966093e2b7 --- /dev/null +++ b/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>; + +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(); + }); + }); +}); diff --git a/packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.vue b/packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.vue index 649272a12a..49a228b5e5 100644 --- a/packages/frontend/editor-ui/src/components/ResourceLocator/ResourceLocator.vue +++ b/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" >