Skip to content

Commit

Permalink
fix(editor): Fix new node credential creation via Resource Locator Co…
Browse files Browse the repository at this point in the history
…mponent (#9896)
  • Loading branch information
alexgrozav authored Jul 1, 2024
1 parent 59c8bf1 commit 55cbc90
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 24 deletions.
10 changes: 10 additions & 0 deletions cypress/e2e/26-resource-locator.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
1 change: 1 addition & 0 deletions cypress/pages/ndv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down
50 changes: 38 additions & 12 deletions packages/editor-ui/src/components/NodeCredentials.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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[] {
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
Expand Down Expand Up @@ -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) {
Expand Down
36 changes: 26 additions & 10 deletions packages/editor-ui/src/utils/nodeTypesUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,47 +120,57 @@ 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,
): boolean => {
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(
(prop) =>
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[],
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -257,9 +269,10 @@ export const getAllNodeCredentialForAuthType = (
return (
nodeType.credentials?.filter(
(cred) => cred.displayOptions?.show && authType in (cred.displayOptions.show || {}),
) || []
) ?? []
);
}

return [];
};

Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 55cbc90

Please sign in to comment.