1
1
mirror of https://github.com/n8n-io/n8n.git synced 2024-09-11 13:15:28 +03:00

fix(editor): Fix new node credential creation via Resource Locator Component (#9896)

This commit is contained in:
Alex Grozav 2024-07-01 14:48:39 +03:00 committed by GitHub
parent 59c8bf1c44
commit 55cbc900a4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 84 additions and 24 deletions

View File

@ -37,6 +37,16 @@ describe('Resource Locator', () => {
ndv.getters.resourceLocatorErrorMessage().should('contain', NO_CREDENTIALS_MESSAGE); 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', () => { it('should show appropriate error when credentials are not valid', () => {
workflowPage.actions.addInitialNodeToCanvas('Manual'); workflowPage.actions.addInitialNodeToCanvas('Manual');
workflowPage.actions.addNodeToCanvas('Google Sheets', true, true, 'Update row in sheet'); workflowPage.actions.addNodeToCanvas('Google Sheets', true, true, 'Update row in sheet');

View File

@ -77,6 +77,7 @@ export class NDV extends BasePage {
resourceLocatorDropdown: (paramName: string) => resourceLocatorDropdown: (paramName: string) =>
this.getters.resourceLocator(paramName).find('[data-test-id="resource-locator-dropdown"]'), this.getters.resourceLocator(paramName).find('[data-test-id="resource-locator-dropdown"]'),
resourceLocatorErrorMessage: () => cy.getByTestId('rlc-error-container'), resourceLocatorErrorMessage: () => cy.getByTestId('rlc-error-container'),
resourceLocatorAddCredentials: () => this.getters.resourceLocatorErrorMessage().find('a'),
resourceLocatorModeSelector: (paramName: string) => resourceLocatorModeSelector: (paramName: string) =>
this.getters.resourceLocator(paramName).find('[data-test-id="rlc-mode-selector"]'), this.getters.resourceLocator(paramName).find('[data-test-id="rlc-mode-selector"]'),
resourceLocatorSearch: (paramName: string) => resourceLocatorSearch: (paramName: string) =>

View File

@ -147,6 +147,7 @@ import {
isRequiredCredential, isRequiredCredential,
} from '@/utils/nodeTypesUtils'; } from '@/utils/nodeTypesUtils';
import { assert } from '@/utils/assert'; import { assert } from '@/utils/assert';
import { ndvEventBus } from '@/event-bus';
interface CredentialDropdownOption extends ICredentialsResponse { interface CredentialDropdownOption extends ICredentialsResponse {
typeDisplayName: string; typeDisplayName: string;
@ -334,6 +335,11 @@ export default defineComponent({
} }
}); });
}); });
ndvEventBus.on('credential.createNew', this.onCreateAndAssignNewCredential);
},
beforeUnmount() {
ndvEventBus.off('credential.createNew', this.onCreateAndAssignNewCredential);
}, },
methods: { methods: {
getAllRelatedCredentialTypes(credentialType: INodeCredentialDescription): string[] { getAllRelatedCredentialTypes(credentialType: INodeCredentialDescription): string[] {
@ -416,25 +422,45 @@ export default defineComponent({
this.$emit('credentialSelected', updateInformation); this.$emit('credentialSelected', updateInformation);
}, },
onCredentialSelected( createNewCredential(
credentialType: string, credentialType: string,
credentialId: string | null | undefined, listenForAuthChange: boolean = false,
requiredCredentials = 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 // 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 will be handled in this component's watcher which will set subscribed credential accordingly
this.listeningForAuthChange = true; this.listeningForAuthChange = true;
this.subscribedToCredentialType = credentialType; this.subscribedToCredentialType = credentialType;
} }
if (!credentialId || credentialId === this.NEW_CREDENTIALS_TEXT) {
this.uiStore.openNewCredential(credentialType, requiredCredentials); this.uiStore.openNewCredential(credentialType, showAuthOptions);
this.$telemetry.track('User opened Credential modal', { this.$telemetry.track('User opened Credential modal', {
credential_type: credentialType, credential_type: credentialType,
source: 'node', source: 'node',
new_credential: true, new_credential: true,
workflow_id: this.workflowsStore.workflowId, 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; return;
} }

View File

@ -175,6 +175,7 @@ import ResourceLocatorDropdown from './ResourceLocatorDropdown.vue';
import { useDebounce } from '@/composables/useDebounce'; import { useDebounce } from '@/composables/useDebounce';
import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers'; import { useWorkflowHelpers } from '@/composables/useWorkflowHelpers';
import { useRouter } from 'vue-router'; import { useRouter } from 'vue-router';
import { ndvEventBus } from '@/event-bus';
interface IResourceLocatorQuery { interface IResourceLocatorQuery {
results: INodeListSearchItems[]; results: INodeListSearchItems[];
@ -566,12 +567,18 @@ export default defineComponent({
if (!nodeType) { if (!nodeType) {
return; return;
} }
const defaultCredentialType = nodeType.credentials?.[0].name ?? '';
const mainAuthType = getMainAuthField(nodeType); const mainAuthType = getMainAuthField(nodeType);
const showAuthSelector = const showAuthOptions =
mainAuthType !== null && mainAuthType !== null &&
Array.isArray(mainAuthType.options) && Array.isArray(mainAuthType.options) &&
mainAuthType.options?.length > 0; mainAuthType.options?.length > 0;
this.uiStore.openNewCredential('', showAuthSelector);
ndvEventBus.emit('credential.createNew', {
type: defaultCredentialType,
showAuthOptions,
});
}, },
findModeByName(name: string): INodePropertyMode | null { findModeByName(name: string): INodePropertyMode | null {
if (this.parameter.modes) { if (this.parameter.modes) {

View File

@ -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 = ( export const isRequiredCredential = (
nodeType: INodeTypeDescription | null, nodeType: INodeTypeDescription | null,
credential: INodeCredentialDescription, credential: INodeCredentialDescription,
@ -129,19 +131,24 @@ export const isRequiredCredential = (
if (!credential.displayOptions?.show) { if (!credential.displayOptions?.show) {
return true; return true;
} }
const mainAuthField = getMainAuthField(nodeType); const mainAuthField = getMainAuthField(nodeType);
if (mainAuthField) { if (mainAuthField) {
return mainAuthField.name in credential.displayOptions.show; return mainAuthField.name in credential.displayOptions.show;
} }
return false; 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 => { export const getMainAuthField = (nodeType: INodeTypeDescription | null): INodeProperties | null => {
if (!nodeType) { if (!nodeType) {
return null; return null;
} }
const credentialDependencies = getNodeAuthFields(nodeType); const credentialDependencies = getNodeAuthFields(nodeType);
const authenticationField = const authenticationField =
credentialDependencies.find( credentialDependencies.find(
@ -149,18 +156,21 @@ export const getMainAuthField = (nodeType: INodeTypeDescription | null): INodePr
prop.name === MAIN_AUTH_FIELD_NAME && prop.name === MAIN_AUTH_FIELD_NAME &&
!prop.options?.find((option) => 'value' in option && option.value === 'none'), !prop.options?.find((option) => 'value' in option && option.value === 'none'),
) ?? null; ) ?? null;
// If there is a field name `authentication`, use it // If there is a field name `authentication`, use it
// Otherwise, try to find alternative main auth field // Otherwise, try to find alternative main auth field
const mainAuthFiled = const mainAuthFiled =
authenticationField || findAlternativeAuthField(nodeType, credentialDependencies); authenticationField ?? findAlternativeAuthField(nodeType, credentialDependencies);
// Main authentication field has to be required // Main authentication field has to be required
const isFieldRequired = mainAuthFiled ? isNodeParameterRequired(nodeType, mainAuthFiled) : false; const isFieldRequired = mainAuthFiled ? isNodeParameterRequired(nodeType, mainAuthFiled) : false;
return mainAuthFiled && isFieldRequired ? mainAuthFiled : null; return mainAuthFiled && isFieldRequired ? mainAuthFiled : null;
}; };
// A field is considered main auth filed if: /**
// 1. It is a credential dependency * A field is considered main auth filed if:
// 2. If all of it's possible values are used in credential's display options * 1. It is a credential dependency
* 2. If all of it's possible values are used in credential's display options
*/
const findAlternativeAuthField = ( const findAlternativeAuthField = (
nodeType: INodeTypeDescription, nodeType: INodeTypeDescription,
fields: INodeProperties[], fields: INodeProperties[],
@ -191,7 +201,9 @@ const findAlternativeAuthField = (
return alternativeAuthField || null; 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 = ( export const getNodeAuthOptions = (
nodeType: INodeTypeDescription | null, nodeType: INodeTypeDescription | null,
nodeVersion?: number, nodeVersion?: number,
@ -257,9 +269,10 @@ export const getAllNodeCredentialForAuthType = (
return ( return (
nodeType.credentials?.filter( nodeType.credentials?.filter(
(cred) => cred.displayOptions?.show && authType in (cred.displayOptions.show || {}), (cred) => cred.displayOptions?.show && authType in (cred.displayOptions.show || {}),
) || [] ) ?? []
); );
} }
return []; return [];
}; };
@ -310,6 +323,9 @@ export const isAuthRelatedParameter = (
return isRelated; return isRelated;
}; };
/**
* Get all node type properties needed for determining whether to show authentication fields
*/
export const getNodeAuthFields = ( export const getNodeAuthFields = (
nodeType: INodeTypeDescription | null, nodeType: INodeTypeDescription | null,
nodeVersion?: number, nodeVersion?: number,