From 55cbc900a48c579b712dddfa74e133e1d9c11799 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Mon, 1 Jul 2024 14:48:39 +0300 Subject: [PATCH] fix(editor): Fix new node credential creation via Resource Locator Component (#9896) --- cypress/e2e/26-resource-locator.cy.ts | 10 ++++ cypress/pages/ndv.ts | 1 + .../src/components/NodeCredentials.vue | 50 ++++++++++++++----- .../ResourceLocator/ResourceLocator.vue | 11 +++- .../editor-ui/src/utils/nodeTypesUtils.ts | 36 +++++++++---- 5 files changed, 84 insertions(+), 24 deletions(-) diff --git a/cypress/e2e/26-resource-locator.cy.ts b/cypress/e2e/26-resource-locator.cy.ts index 5c8eef714a..6e431690ad 100644 --- a/cypress/e2e/26-resource-locator.cy.ts +++ b/cypress/e2e/26-resource-locator.cy.ts @@ -37,6 +37,16 @@ describe('Resource Locator', () => { ndv.getters.resourceLocatorErrorMessage().should('contain', NO_CREDENTIALS_MESSAGE); }); + it('should show create credentials modal when clicking "add your credential"', () => { + workflowPage.actions.addInitialNodeToCanvas('Manual'); + workflowPage.actions.addNodeToCanvas('Google Sheets', true, true, 'Update row in sheet'); + ndv.getters.resourceLocator('documentId').should('be.visible'); + ndv.getters.resourceLocatorInput('documentId').click(); + ndv.getters.resourceLocatorErrorMessage().should('contain', NO_CREDENTIALS_MESSAGE); + ndv.getters.resourceLocatorAddCredentials().click(); + credentialsModal.getters.credentialsEditModal().should('be.visible'); + }); + it('should show appropriate error when credentials are not valid', () => { workflowPage.actions.addInitialNodeToCanvas('Manual'); workflowPage.actions.addNodeToCanvas('Google Sheets', true, true, 'Update row in sheet'); diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts index 99f44d1a8b..018ec43a5d 100644 --- a/cypress/pages/ndv.ts +++ b/cypress/pages/ndv.ts @@ -77,6 +77,7 @@ export class NDV extends BasePage { resourceLocatorDropdown: (paramName: string) => this.getters.resourceLocator(paramName).find('[data-test-id="resource-locator-dropdown"]'), resourceLocatorErrorMessage: () => cy.getByTestId('rlc-error-container'), + resourceLocatorAddCredentials: () => this.getters.resourceLocatorErrorMessage().find('a'), resourceLocatorModeSelector: (paramName: string) => this.getters.resourceLocator(paramName).find('[data-test-id="rlc-mode-selector"]'), resourceLocatorSearch: (paramName: string) => diff --git a/packages/editor-ui/src/components/NodeCredentials.vue b/packages/editor-ui/src/components/NodeCredentials.vue index 193934f736..ad9b229541 100644 --- a/packages/editor-ui/src/components/NodeCredentials.vue +++ b/packages/editor-ui/src/components/NodeCredentials.vue @@ -147,6 +147,7 @@ import { isRequiredCredential, } from '@/utils/nodeTypesUtils'; import { assert } from '@/utils/assert'; +import { ndvEventBus } from '@/event-bus'; interface CredentialDropdownOption extends ICredentialsResponse { typeDisplayName: string; @@ -334,6 +335,11 @@ export default defineComponent({ } }); }); + + ndvEventBus.on('credential.createNew', this.onCreateAndAssignNewCredential); + }, + beforeUnmount() { + ndvEventBus.off('credential.createNew', this.onCreateAndAssignNewCredential); }, methods: { getAllRelatedCredentialTypes(credentialType: INodeCredentialDescription): string[] { @@ -416,25 +422,45 @@ export default defineComponent({ this.$emit('credentialSelected', updateInformation); }, - onCredentialSelected( + createNewCredential( credentialType: string, - credentialId: string | null | undefined, - requiredCredentials = false, + listenForAuthChange: boolean = false, + showAuthOptions = false, ) { - if (credentialId === this.NEW_CREDENTIALS_TEXT) { + if (listenForAuthChange) { // If new credential dialog is open, start listening for auth type change which should happen in the modal // this will be handled in this component's watcher which will set subscribed credential accordingly this.listeningForAuthChange = true; this.subscribedToCredentialType = credentialType; } - if (!credentialId || credentialId === this.NEW_CREDENTIALS_TEXT) { - this.uiStore.openNewCredential(credentialType, requiredCredentials); - this.$telemetry.track('User opened Credential modal', { - credential_type: credentialType, - source: 'node', - new_credential: true, - workflow_id: this.workflowsStore.workflowId, - }); + + this.uiStore.openNewCredential(credentialType, showAuthOptions); + this.$telemetry.track('User opened Credential modal', { + credential_type: credentialType, + source: 'node', + new_credential: true, + workflow_id: this.workflowsStore.workflowId, + }); + }, + + onCreateAndAssignNewCredential({ + type, + showAuthOptions, + }: { + type: string; + showAuthOptions: boolean; + }) { + this.createNewCredential(type, true, showAuthOptions); + }, + + onCredentialSelected( + credentialType: string, + credentialId: string | null | undefined, + showAuthOptions = false, + ) { + const newCredentialOptionSelected = credentialId === this.NEW_CREDENTIALS_TEXT; + if (!credentialId || newCredentialOptionSelected) { + this.createNewCredential(credentialType, newCredentialOptionSelected, showAuthOptions); return; } diff --git a/packages/editor-ui/src/components/ResourceLocator/ResourceLocator.vue b/packages/editor-ui/src/components/ResourceLocator/ResourceLocator.vue index 3581d8a0f8..0a7e1aa94d 100644 --- a/packages/editor-ui/src/components/ResourceLocator/ResourceLocator.vue +++ b/packages/editor-ui/src/components/ResourceLocator/ResourceLocator.vue @@ -175,6 +175,7 @@ import ResourceLocatorDropdown from './ResourceLocatorDropdown.vue'; import { useDebounce } from '@/composables/useDebounce'; import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers'; import { useRouter } from 'vue-router'; +import { ndvEventBus } from '@/event-bus'; interface IResourceLocatorQuery { results: INodeListSearchItems[]; @@ -566,12 +567,18 @@ export default defineComponent({ if (!nodeType) { return; } + + const defaultCredentialType = nodeType.credentials?.[0].name ?? ''; const mainAuthType = getMainAuthField(nodeType); - const showAuthSelector = + const showAuthOptions = mainAuthType !== null && Array.isArray(mainAuthType.options) && mainAuthType.options?.length > 0; - this.uiStore.openNewCredential('', showAuthSelector); + + ndvEventBus.emit('credential.createNew', { + type: defaultCredentialType, + showAuthOptions, + }); }, findModeByName(name: string): INodePropertyMode | null { if (this.parameter.modes) { diff --git a/packages/editor-ui/src/utils/nodeTypesUtils.ts b/packages/editor-ui/src/utils/nodeTypesUtils.ts index 7ca076a1d3..91fe812e4e 100644 --- a/packages/editor-ui/src/utils/nodeTypesUtils.ts +++ b/packages/editor-ui/src/utils/nodeTypesUtils.ts @@ -120,8 +120,10 @@ export const hasOnlyListMode = (parameter: INodeProperties): boolean => { ); }; -// A credential type is considered required if it has no dependencies -// or if it's only dependency is the main authentication fields +/** + * A credential type is considered required if it has no dependencies + * or if it's only dependency is the main authentication fields + */ export const isRequiredCredential = ( nodeType: INodeTypeDescription | null, credential: INodeCredentialDescription, @@ -129,19 +131,24 @@ export const isRequiredCredential = ( if (!credential.displayOptions?.show) { return true; } + const mainAuthField = getMainAuthField(nodeType); if (mainAuthField) { return mainAuthField.name in credential.displayOptions.show; } + return false; }; -// Finds the main authentication filed for the node type -// It's the field that node's required credential depend on +/** + * Find the main authentication field for the node type. + * It's the field that node's required credential depend on + */ export const getMainAuthField = (nodeType: INodeTypeDescription | null): INodeProperties | null => { if (!nodeType) { return null; } + const credentialDependencies = getNodeAuthFields(nodeType); const authenticationField = credentialDependencies.find( @@ -149,18 +156,21 @@ export const getMainAuthField = (nodeType: INodeTypeDescription | null): INodePr prop.name === MAIN_AUTH_FIELD_NAME && !prop.options?.find((option) => 'value' in option && option.value === 'none'), ) ?? null; + // If there is a field name `authentication`, use it // Otherwise, try to find alternative main auth field const mainAuthFiled = - authenticationField || findAlternativeAuthField(nodeType, credentialDependencies); + authenticationField ?? findAlternativeAuthField(nodeType, credentialDependencies); // Main authentication field has to be required const isFieldRequired = mainAuthFiled ? isNodeParameterRequired(nodeType, mainAuthFiled) : false; return mainAuthFiled && isFieldRequired ? mainAuthFiled : null; }; -// A field is considered main auth filed if: -// 1. It is a credential dependency -// 2. If all of it's possible values are used in credential's display options +/** + * A field is considered main auth filed if: + * 1. It is a credential dependency + * 2. If all of it's possible values are used in credential's display options + */ const findAlternativeAuthField = ( nodeType: INodeTypeDescription, fields: INodeProperties[], @@ -191,7 +201,9 @@ const findAlternativeAuthField = ( return alternativeAuthField || null; }; -// Gets all authentication types that a given node type supports +/** + * Gets all authentication types that a given node type supports + */ export const getNodeAuthOptions = ( nodeType: INodeTypeDescription | null, nodeVersion?: number, @@ -257,9 +269,10 @@ export const getAllNodeCredentialForAuthType = ( return ( nodeType.credentials?.filter( (cred) => cred.displayOptions?.show && authType in (cred.displayOptions.show || {}), - ) || [] + ) ?? [] ); } + return []; }; @@ -310,6 +323,9 @@ export const isAuthRelatedParameter = ( return isRelated; }; +/** + * Get all node type properties needed for determining whether to show authentication fields + */ export const getNodeAuthFields = ( nodeType: INodeTypeDescription | null, nodeVersion?: number,