fix(editor): Fix selected credential being overwritten in NDV (#11496)

This commit is contained in:
Elias Meire 2024-11-06 11:01:45 +01:00 committed by GitHub
parent ccd2564cd4
commit a26c0e2c3c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 87 additions and 125 deletions

View file

@ -26,6 +26,22 @@ const nodeDetailsView = new NDV();
const NEW_CREDENTIAL_NAME = 'Something else';
const NEW_CREDENTIAL_NAME2 = 'Something else entirely';
function createNotionCredential() {
workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME);
workflowPage.actions.openNode(NOTION_NODE_NAME);
workflowPage.getters.nodeCredentialsSelect().click();
getVisibleSelect().find('li').last().click();
credentialsModal.actions.fillCredentialsForm();
cy.get('body').type('{esc}');
workflowPage.actions.deleteNode(NOTION_NODE_NAME);
}
function deleteSelectedCredential() {
workflowPage.getters.nodeCredentialsEditButton().click();
credentialsModal.getters.deleteButton().click();
cy.get('.el-message-box').find('button').contains('Yes').click();
}
describe('Credentials', () => {
beforeEach(() => {
cy.visit(credentialsPage.url);
@ -229,6 +245,40 @@ describe('Credentials', () => {
.should('have.value', NEW_CREDENTIAL_NAME);
});
it('should set a default credential when adding nodes', () => {
workflowPage.actions.visit();
createNotionCredential();
workflowPage.actions.addNodeToCanvas(NOTION_NODE_NAME, true, true);
workflowPage.getters
.nodeCredentialsSelect()
.find('input')
.should('have.value', NEW_NOTION_ACCOUNT_NAME);
deleteSelectedCredential();
});
it('should set a default credential when editing a node', () => {
workflowPage.actions.visit();
createNotionCredential();
workflowPage.actions.addNodeToCanvas(HTTP_REQUEST_NODE_NAME, true, true);
nodeDetailsView.getters.parameterInput('authentication').click();
getVisibleSelect().find('li').contains('Predefined').click();
nodeDetailsView.getters.parameterInput('nodeCredentialType').click();
getVisibleSelect().find('li').contains('Notion API').click();
workflowPage.getters
.nodeCredentialsSelect()
.find('input')
.should('have.value', NEW_NOTION_ACCOUNT_NAME);
deleteSelectedCredential();
});
it('should setup generic authentication for HTTP node', () => {
workflowPage.actions.visit();
workflowPage.actions.addNodeToCanvas(SCHEDULE_TRIGGER_NODE_NAME);

View file

@ -37,6 +37,7 @@ import {
N8nText,
N8nTooltip,
} from 'n8n-design-system';
import { isEmpty } from '@/utils/typesUtils';
interface CredentialDropdownOption extends ICredentialsResponse {
typeDisplayName: string;
@ -87,9 +88,9 @@ const credentialTypesNode = computed(() =>
);
const credentialTypesNodeDescriptionDisplayed = computed(() =>
credentialTypesNodeDescription.value.filter((credentialTypeDescription) =>
displayCredentials(credentialTypeDescription),
),
credentialTypesNodeDescription.value
.filter((credentialTypeDescription) => displayCredentials(credentialTypeDescription))
.map((type) => ({ type, options: getCredentialOptions(getAllRelatedCredentialTypes(type)) })),
);
const credentialTypesNodeDescription = computed(() => {
if (typeof props.overrideCredType !== 'string') return [];
@ -149,6 +150,27 @@ watch(
{ immediate: true, deep: true },
);
// Select most recent credential by default
watch(
credentialTypesNodeDescriptionDisplayed,
(types) => {
if (types.length === 0 || !isEmpty(selected.value)) return;
const allOptions = types.map((type) => type.options).flat();
if (allOptions.length === 0) return;
const mostRecentCredential = allOptions.reduce(
(mostRecent, current) =>
mostRecent && mostRecent.updatedAt > current.updatedAt ? mostRecent : current,
allOptions[0],
);
onCredentialSelected(mostRecentCredential.type, mostRecentCredential.id);
},
{ immediate: true },
);
onMounted(() => {
credentialsStore.$onAction(({ name, after, args }) => {
const listeningForActions = ['createNewCredential', 'updateCredential', 'deleteCredential'];
@ -481,12 +503,9 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s
v-if="credentialTypesNodeDescriptionDisplayed.length"
:class="['node-credentials', $style.container]"
>
<div
v-for="credentialTypeDescription in credentialTypesNodeDescriptionDisplayed"
:key="credentialTypeDescription.name"
>
<div v-for="{ type, options } in credentialTypesNodeDescriptionDisplayed" :key="type.name">
<N8nInputLabel
:label="getCredentialsFieldLabel(credentialTypeDescription)"
:label="getCredentialsFieldLabel(type)"
:bold="false"
size="small"
color="text-dark"
@ -494,7 +513,7 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s
>
<div v-if="readonly">
<N8nInput
:model-value="getSelectedName(credentialTypeDescription.name)"
:model-value="getSelectedName(type.name)"
disabled
size="small"
data-test-id="node-credentials-select"
@ -502,36 +521,20 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s
</div>
<div
v-else
:class="
getIssues(credentialTypeDescription.name).length && !hideIssues
? $style.hasIssues
: $style.input
"
:class="getIssues(type.name).length && !hideIssues ? $style.hasIssues : $style.input"
data-test-id="node-credentials-select"
>
<N8nSelect
:model-value="getSelectedId(credentialTypeDescription.name)"
:placeholder="
getSelectPlaceholder(
credentialTypeDescription.name,
getIssues(credentialTypeDescription.name),
)
"
:model-value="getSelectedId(type.name)"
:placeholder="getSelectPlaceholder(type.name, getIssues(type.name))"
size="small"
@update:model-value="
(value: string) =>
onCredentialSelected(
credentialTypeDescription.name,
value,
showMixedCredentials(credentialTypeDescription),
)
(value: string) => onCredentialSelected(type.name, value, showMixedCredentials(type))
"
@blur="emit('blur', 'credentials')"
>
<N8nOption
v-for="item in getCredentialOptions(
getAllRelatedCredentialTypes(credentialTypeDescription),
)"
v-for="item in options"
:key="item.id"
:data-test-id="`node-credentials-select-item-${item.id}`"
:label="item.name"
@ -551,15 +554,12 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s
</N8nOption>
</N8nSelect>
<div
v-if="getIssues(credentialTypeDescription.name).length && !hideIssues"
:class="$style.warning"
>
<div v-if="getIssues(type.name).length && !hideIssues" :class="$style.warning">
<N8nTooltip placement="top">
<template #content>
<TitledList
:title="`${$locale.baseText('nodeCredentials.issues')}:`"
:items="getIssues(credentialTypeDescription.name)"
:items="getIssues(type.name)"
/>
</template>
<font-awesome-icon icon="exclamation-triangle" />
@ -567,10 +567,7 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s
</div>
<div
v-if="
selected[credentialTypeDescription.name] &&
isCredentialExisting(credentialTypeDescription.name)
"
v-if="selected[type.name] && isCredentialExisting(type.name)"
:class="$style.edit"
data-test-id="credential-edit-button"
>
@ -578,7 +575,7 @@ function getCredentialsFieldLabel(credentialType: INodeCredentialDescription): s
icon="pen"
class="clickable"
:title="$locale.baseText('nodeCredentials.updateCredential')"
@click="editCredential(credentialTypeDescription.name)"
@click="editCredential(type.name)"
/>
</div>
</div>

View file

@ -191,36 +191,6 @@ describe('useCanvasOperations', () => {
expect(result.position).toEqual([20, 20]);
});
it('should create node with default credentials when only one credential is available', () => {
const credentialsStore = useCredentialsStore();
const credential = mock<ICredentialsResponse>({ id: '1', name: 'cred', type: 'cred' });
const nodeTypeName = 'type';
const nodeTypeDescription = mockNodeTypeDescription({
name: nodeTypeName,
credentials: [{ name: credential.name }],
});
credentialsStore.state.credentials = {
[credential.id]: credential,
};
// @ts-expect-error Known pinia issue when spying on store getters
vi.spyOn(credentialsStore, 'getUsableCredentialByType', 'get').mockReturnValue(() => [
credential,
]);
const { addNode } = useCanvasOperations({ router });
const result = addNode(
{
type: nodeTypeName,
typeVersion: 1,
},
nodeTypeDescription,
);
expect(result.credentials).toEqual({ [credential.name]: { id: '1', name: credential.name } });
});
it('should not assign credentials when multiple credentials are available', () => {
const credentialsStore = useCredentialsStore();
const credentialA = mock<ICredentialsResponse>({ id: '1', name: 'credA', type: 'cred' });

View file

@ -777,7 +777,6 @@ export function useCanvasOperations({ router }: { router: ReturnType<typeof useR
};
resolveNodeParameters(nodeData);
resolveNodeCredentials(nodeData, nodeTypeDescription);
resolveNodeName(nodeData);
resolveNodeWebhook(nodeData, nodeTypeDescription);
@ -840,60 +839,6 @@ export function useCanvasOperations({ router }: { router: ReturnType<typeof useR
node.parameters = nodeParameters ?? {};
}
function resolveNodeCredentials(node: INodeUi, nodeTypeDescription: INodeTypeDescription) {
const credentialPerType = nodeTypeDescription.credentials
?.map((type) => credentialsStore.getUsableCredentialByType(type.name))
.flat();
if (credentialPerType?.length === 1) {
const defaultCredential = credentialPerType[0];
const selectedCredentials = credentialsStore.getCredentialById(defaultCredential.id);
const selected = { id: selectedCredentials.id, name: selectedCredentials.name };
const credentials = {
[defaultCredential.type]: selected,
};
if (nodeTypeDescription.credentials) {
const authentication = nodeTypeDescription.credentials.find(
(type) => type.name === defaultCredential.type,
);
const authDisplayOptionsHide = authentication?.displayOptions?.hide;
const authDisplayOptionsShow = authentication?.displayOptions?.show;
if (!authDisplayOptionsHide) {
if (!authDisplayOptionsShow) {
node.credentials = credentials;
} else if (
Object.keys(authDisplayOptionsShow).length === 1 &&
authDisplayOptionsShow.authentication
) {
// ignore complex case when there's multiple dependencies
node.credentials = credentials;
let parameters: { [key: string]: string } = {};
for (const displayOption of Object.keys(authDisplayOptionsShow)) {
if (node.parameters && !node.parameters[displayOption]) {
parameters = {};
node.credentials = undefined;
break;
}
const optionValue = authDisplayOptionsShow[displayOption]?.[0];
if (optionValue && typeof optionValue === 'string') {
parameters[displayOption] = optionValue;
}
node.parameters = {
...node.parameters,
...parameters,
};
}
}
}
}
}
}
function resolveNodePosition(
node: Omit<INodeUi, 'position'> & { position?: INodeUi['position'] },
nodeTypeDescription: INodeTypeDescription,