From 7073ec6fe5384cc8c50dcb242212999a1fbc9041 Mon Sep 17 00:00:00 2001 From: Elias Meire Date: Fri, 27 Sep 2024 12:04:00 +0200 Subject: [PATCH] fix(editor): Fix performance issue in credentials list (#10988) --- packages/core/bin/generate-ui-types | 42 ++++-- .../src/components/CredentialIcon.vue | 84 +++++------ .../composables/useActionsGeneration.ts | 1 + .../__tests__/CredentialIcon.test.ts | 131 ++++++++++++------ .../editor-ui/src/stores/credentials.store.ts | 40 +++--- packages/workflow/src/Interfaces.ts | 6 +- 6 files changed, 183 insertions(+), 121 deletions(-) diff --git a/packages/core/bin/generate-ui-types b/packages/core/bin/generate-ui-types index 8cecb6b054d74..f73ca87a15eca 100755 --- a/packages/core/bin/generate-ui-types +++ b/packages/core/bin/generate-ui-types @@ -30,18 +30,6 @@ function findReferencedMethods(obj, refs = {}, latestName = '') { const loader = new PackageDirectoryLoader(packageDir); await loader.loadAll(); - const knownCredentials = loader.known.credentials; - const credentialTypes = Object.values(loader.credentialTypes).map((data) => { - const credentialType = data.type; - if ( - knownCredentials[credentialType.name].supportedNodes?.length > 0 && - credentialType.httpRequestNode - ) { - credentialType.httpRequestNode.hidden = true; - } - return credentialType; - }); - const loaderNodeTypes = Object.values(loader.nodeTypes); const definedMethods = loaderNodeTypes.reduce((acc, cur) => { @@ -76,6 +64,36 @@ function findReferencedMethods(obj, refs = {}, latestName = '') { }), ); + const knownCredentials = loader.known.credentials; + const credentialTypes = Object.values(loader.credentialTypes).map((data) => { + const credentialType = data.type; + const supportedNodes = knownCredentials[credentialType.name].supportedNodes ?? []; + if (supportedNodes.length > 0 && credentialType.httpRequestNode) { + credentialType.httpRequestNode.hidden = true; + } + + credentialType.supportedNodes = supportedNodes; + + if (!credentialType.iconUrl && !credentialType.icon) { + for (const supportedNode of supportedNodes) { + const nodeType = loader.nodeTypes[supportedNode]?.type.description; + + if (!nodeType) continue; + if (nodeType.icon) { + credentialType.icon = nodeType.icon; + credentialType.iconColor = nodeType.iconColor; + break; + } + if (nodeType.iconUrl) { + credentialType.iconUrl = nodeType.iconUrl; + break; + } + } + } + + return credentialType; + }); + const referencedMethods = findReferencedMethods(nodeTypes); await Promise.all([ diff --git a/packages/editor-ui/src/components/CredentialIcon.vue b/packages/editor-ui/src/components/CredentialIcon.vue index 1da011d932fd3..07ca323e17882 100644 --- a/packages/editor-ui/src/components/CredentialIcon.vue +++ b/packages/editor-ui/src/components/CredentialIcon.vue @@ -1,50 +1,59 @@ diff --git a/packages/editor-ui/src/components/Node/NodeCreator/composables/useActionsGeneration.ts b/packages/editor-ui/src/components/Node/NodeCreator/composables/useActionsGeneration.ts index 85c306cdf6c5f..8ada34e30c624 100644 --- a/packages/editor-ui/src/components/Node/NodeCreator/composables/useActionsGeneration.ts +++ b/packages/editor-ui/src/components/Node/NodeCreator/composables/useActionsGeneration.ts @@ -60,6 +60,7 @@ function getNodeTypeBase(nodeTypeDescription: INodeTypeDescription, label?: stri categories: [category], }, iconUrl: nodeTypeDescription.iconUrl, + iconColor: nodeTypeDescription.iconColor, outputs: nodeTypeDescription.outputs, icon: nodeTypeDescription.icon, defaults: nodeTypeDescription.defaults, diff --git a/packages/editor-ui/src/components/__tests__/CredentialIcon.test.ts b/packages/editor-ui/src/components/__tests__/CredentialIcon.test.ts index 0eb04df3968ec..79bcc4b88265f 100644 --- a/packages/editor-ui/src/components/__tests__/CredentialIcon.test.ts +++ b/packages/editor-ui/src/components/__tests__/CredentialIcon.test.ts @@ -1,66 +1,111 @@ -import { createTestingPinia } from '@pinia/testing'; +import { createTestingPinia, type TestingPinia } from '@pinia/testing'; +import type { ICredentialType, INodeTypeDescription } from 'n8n-workflow'; import { mock } from 'vitest-mock-extended'; -import type { INodeTypeDescription } from 'n8n-workflow'; import CredentialIcon from '@/components/CredentialIcon.vue'; -import { STORES } from '@/constants'; -import { groupNodeTypesByNameAndType } from '@/utils/nodeTypes/nodeTypeTransforms'; import { createComponentRenderer } from '@/__tests__/render'; +import { useCredentialsStore } from '@/stores/credentials.store'; +import { useRootStore } from '@/stores/root.store'; +import { useNodeTypesStore } from '../../stores/nodeTypes.store'; -const twitterV1 = mock({ - version: 1, - credentials: [{ name: 'twitterOAuth1Api', required: true }], - iconUrl: 'icons/n8n-nodes-base/dist/nodes/Twitter/x.svg', -}); +describe('CredentialIcon', () => { + const renderComponent = createComponentRenderer(CredentialIcon, { + pinia: createTestingPinia(), + global: { + stubs: ['n8n-tooltip'], + }, + }); + let pinia: TestingPinia; -const twitterV2 = mock({ - version: 2, - credentials: [{ name: 'twitterOAuth2Api', required: true }], - iconUrl: 'icons/n8n-nodes-base/dist/nodes/Twitter/x.svg', -}); + beforeEach(() => { + pinia = createTestingPinia({ stubActions: false }); + }); -const nodeTypes = groupNodeTypesByNameAndType([twitterV1, twitterV2]); -const initialState = { - [STORES.CREDENTIALS]: {}, - [STORES.NODE_TYPES]: { nodeTypes }, -}; - -const renderComponent = createComponentRenderer(CredentialIcon, { - pinia: createTestingPinia({ initialState }), - global: { - stubs: ['n8n-tooltip'], - }, -}); + it('shows correct icon when iconUrl is set on credential', () => { + const testIconUrl = 'icons/n8n-nodes-base/dist/nodes/Test/test.svg'; + useCredentialsStore().setCredentialTypes([ + mock({ + name: 'test', + iconUrl: testIconUrl, + }), + ]); -describe('CredentialIcon', () => { - const findIcon = (baseElement: Element) => baseElement.querySelector('img'); + const { getByRole } = renderComponent({ + pinia, + props: { + credentialTypeName: 'test', + }, + }); - it('shows correct icon for credential type that is for the latest node type version', () => { - const { baseElement } = renderComponent({ - pinia: createTestingPinia({ initialState }), + expect(getByRole('img')).toHaveAttribute('src', useRootStore().baseUrl + testIconUrl); + }); + + it('shows correct icon when icon is set on credential', () => { + useCredentialsStore().setCredentialTypes([ + mock({ + name: 'test', + icon: 'fa:clock', + iconColor: 'azure', + }), + ]); + + const { getByRole } = renderComponent({ + pinia, props: { - credentialTypeName: 'twitterOAuth2Api', + credentialTypeName: 'test', }, }); - expect(findIcon(baseElement)).toHaveAttribute( - 'src', - '/icons/n8n-nodes-base/dist/nodes/Twitter/x.svg', - ); + const icon = getByRole('img', { hidden: true }); + expect(icon.tagName).toBe('svg'); + expect(icon).toHaveClass('fa-clock'); }); - it('shows correct icon for credential type that is for an older node type version', () => { + it('shows correct icon when credential has an icon with node: prefix', () => { + const testIconUrl = 'icons/n8n-nodes-base/dist/nodes/Test/test.svg'; + useCredentialsStore().setCredentialTypes([ + mock({ + name: 'test', + icon: 'node:n8n-nodes-base.test', + iconColor: 'azure', + }), + ]); + + useNodeTypesStore().setNodeTypes([ + mock({ + version: 1, + name: 'n8n-nodes-base.test', + iconUrl: testIconUrl, + }), + ]); + + const { getByRole } = renderComponent({ + pinia, + props: { + credentialTypeName: 'test', + }, + }); + + expect(getByRole('img')).toHaveAttribute('src', useRootStore().baseUrl + testIconUrl); + }); + + it('shows fallback icon when icon is not found', () => { + useCredentialsStore().setCredentialTypes([ + mock({ + name: 'test', + icon: 'node:n8n-nodes-base.test', + iconColor: 'azure', + }), + ]); + const { baseElement } = renderComponent({ - pinia: createTestingPinia({ initialState }), + pinia, props: { - credentialTypeName: 'twitterOAuth1Api', + credentialTypeName: 'test', }, }); - expect(findIcon(baseElement)).toHaveAttribute( - 'src', - '/icons/n8n-nodes-base/dist/nodes/Twitter/x.svg', - ); + expect(baseElement.querySelector('.nodeIconPlaceholder')).toBeInTheDocument(); }); }); diff --git a/packages/editor-ui/src/stores/credentials.store.ts b/packages/editor-ui/src/stores/credentials.store.ts index ec71aa5b29993..4e87eaaf98567 100644 --- a/packages/editor-ui/src/stores/credentials.store.ts +++ b/packages/editor-ui/src/stores/credentials.store.ts @@ -1,32 +1,31 @@ import type { - INodeUi, - IUsedCredential, ICredentialMap, ICredentialsDecryptedResponse, ICredentialsResponse, ICredentialsState, ICredentialTypeMap, + INodeUi, + IUsedCredential, } from '@/Interface'; import * as credentialsApi from '@/api/credentials'; import * as credentialsEeApi from '@/api/credentials.ee'; -import { makeRestApiRequest } from '@/utils/apiUtils'; -import { getAppNameFromCredType } from '@/utils/nodeTypesUtils'; import { EnterpriseEditionFeature, STORES } from '@/constants'; import { i18n } from '@/plugins/i18n'; +import type { ProjectSharingData } from '@/types/projects.types'; +import { makeRestApiRequest } from '@/utils/apiUtils'; +import { getAppNameFromCredType } from '@/utils/nodeTypesUtils'; +import { splitName } from '@/utils/projects.utils'; +import { isEmpty, isPresent } from '@/utils/typesUtils'; import type { ICredentialsDecrypted, ICredentialType, INodeCredentialTestResult, - INodeTypeDescription, } from 'n8n-workflow'; import { defineStore } from 'pinia'; -import { useRootStore } from './root.store'; +import { computed, ref } from 'vue'; import { useNodeTypesStore } from './nodeTypes.store'; +import { useRootStore } from './root.store'; import { useSettingsStore } from './settings.store'; -import { isEmpty } from '@/utils/typesUtils'; -import type { ProjectSharingData } from '@/types/projects.types'; -import { splitName } from '@/utils/projects.utils'; -import { computed, ref } from 'vue'; const DEFAULT_CREDENTIAL_NAME = 'Unnamed credential'; const DEFAULT_CREDENTIAL_POSTFIX = 'account'; @@ -131,22 +130,15 @@ export const useCredentialsStore = defineStore(STORES.CREDENTIALS, () => { const getNodesWithAccess = computed(() => { return (credentialTypeName: string) => { + const credentialType = getCredentialTypeByName.value(credentialTypeName); + if (!credentialType) { + return []; + } const nodeTypesStore = useNodeTypesStore(); - const allNodeTypes: INodeTypeDescription[] = nodeTypesStore.allNodeTypes; - - return allNodeTypes.filter((nodeType: INodeTypeDescription) => { - if (!nodeType.credentials) { - return false; - } - for (const credentialTypeDescription of nodeType.credentials) { - if (credentialTypeDescription.name === credentialTypeName) { - return true; - } - } - - return false; - }); + return (credentialType.supportedNodes ?? []) + .map((nodeType) => nodeTypesStore.getNodeType(nodeType)) + .filter(isPresent); }; }); diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 0d56f79009dfd..62cf8c5bed186 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -314,6 +314,7 @@ export interface ICredentialType { name: string; displayName: string; icon?: Icon; + iconColor?: ThemeIconColor; iconUrl?: Themed; extends?: string[]; properties: INodeProperties[]; @@ -327,6 +328,7 @@ export interface ICredentialType { test?: ICredentialTestRequest; genericAuth?: boolean; httpRequestNode?: ICredentialHttpRequestNode; + supportedNodes?: string[]; } export interface ICredentialTypes { @@ -1617,7 +1619,7 @@ export interface IWorkflowIssues { [key: string]: INodeIssues; } -export type NodeIconColor = +export type ThemeIconColor = | 'gray' | 'black' | 'blue' @@ -1642,7 +1644,7 @@ export interface INodeTypeBaseDescription { displayName: string; name: string; icon?: Icon; - iconColor?: NodeIconColor; + iconColor?: ThemeIconColor; iconUrl?: Themed; badgeIconUrl?: Themed; group: string[];