From 47185d16f651496ff213ec74b926777730964abf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= <ivov.src@gmail.com>
Date: Thu, 5 May 2022 17:40:30 +0200
Subject: [PATCH] :zap: Refactor credentials dropdown for HTTP Request node
 (#3222)

---
 .../src/components/NodeCredentials.vue        |  40 +++++--
 .../editor-ui/src/components/NodeSettings.vue |  19 +++-
 .../src/components/ParameterInputList.vue     |  18 +++-
 .../src/components/mixins/nodeHelpers.ts      | 101 ++++++++++++++++++
 .../src/components/mixins/workflowHelpers.ts  |   5 +
 .../src/plugins/i18n/locales/en.json          |   1 +
 .../nodes/HttpRequest/HttpRequest.node.ts     |  26 ++---
 7 files changed, 183 insertions(+), 27 deletions(-)

diff --git a/packages/editor-ui/src/components/NodeCredentials.vue b/packages/editor-ui/src/components/NodeCredentials.vue
index f82d28e737ab6..07d8a04350b89 100644
--- a/packages/editor-ui/src/components/NodeCredentials.vue
+++ b/packages/editor-ui/src/components/NodeCredentials.vue
@@ -1,5 +1,5 @@
 <template>
-	<div v-if="credentialTypesNodeDescriptionDisplayed.length" :class="$style.container">
+	<div v-if="credentialTypesNodeDescriptionDisplayed.length" :class="['node-credentials', $style.container]">
 		<div v-for="credentialTypeDescription in credentialTypesNodeDescriptionDisplayed" :key="credentialTypeDescription.name">
 			<n8n-input-label
 				:label="$locale.baseText(
@@ -11,15 +11,20 @@
 					}
 				)"
 				:bold="false"
-				size="small"
-
 				:set="issues = getIssues(credentialTypeDescription.name)"
+				size="small"
 			>
 				<div v-if="isReadOnly">
-					<n8n-input disabled :value="selected && selected[credentialTypeDescription.name] && selected[credentialTypeDescription.name].name" size="small" />
+					<n8n-input
+						:value="selected && selected[credentialTypeDescription.name] && selected[credentialTypeDescription.name].name"
+						disabled
+						size="small"
+					/>
 				</div>
-
-				<div :class="issues.length ? $style.hasIssues : $style.input" v-else >
+				<div
+					v-else
+					:class="issues.length ? $style.hasIssues : $style.input"
+				>
 					<n8n-select :value="getSelectedId(credentialTypeDescription.name)" @change="(value) => onCredentialSelected(credentialTypeDescription.name, value)" :placeholder="$locale.baseText('nodeCredentials.selectCredential')" size="small">
 						<n8n-option
 							v-for="(item) in credentialOptions[credentialTypeDescription.name]"
@@ -92,7 +97,16 @@ export default mixins(
 	computed: {
 		...mapGetters('credentials', {
 			credentialOptions: 'allCredentialsByType',
+			getCredentialTypeByName: 'getCredentialTypeByName',
 		}),
+		isProxyAuth(): boolean {
+			return this.isHttpRequestNodeV2(this.node) &&
+				this.node.parameters.authenticateWith === 'nodeCredential';
+		},
+		isGenericAuth(): boolean {
+			return this.isHttpRequestNodeV2(this.node) &&
+				this.node.parameters.authenticateWith === 'genericAuth';
+		},
 		credentialTypesNode (): string[] {
 			return this.credentialTypesNodeDescription
 				.map((credentialTypeDescription) => credentialTypeDescription.name);
@@ -106,6 +120,18 @@ export default mixins(
 		credentialTypesNodeDescription (): INodeCredentialDescription[] {
 			const node = this.node as INodeUi;
 
+			if (this.isGenericAuth) {
+				const { genericAuthType } = this.node.parameters as { genericAuthType: string };
+
+				return [this.getCredentialTypeByName(genericAuthType)];
+			}
+
+			if (this.isProxyAuth) {
+				const { nodeCredentialType } = this.node.parameters as { nodeCredentialType?: string };
+
+				if (nodeCredentialType) return [this.getCredentialTypeByName(nodeCredentialType)];
+			}
+
 			const activeNodeType = this.$store.getters.nodeType(node.type, node.typeVersion) as INodeTypeDescription | null;
 			if (activeNodeType && activeNodeType.credentials) {
 				return activeNodeType.credentials;
@@ -295,7 +321,7 @@ export default mixins(
 
 <style lang="scss" module>
 .container {
-	margin: var(--spacing-xs) 0;
+	margin: 0;
 
 	> * {
 		margin-bottom: var(--spacing-xs);
diff --git a/packages/editor-ui/src/components/NodeSettings.vue b/packages/editor-ui/src/components/NodeSettings.vue
index 58a7708e58f61..d45131acfcdcf 100644
--- a/packages/editor-ui/src/components/NodeSettings.vue
+++ b/packages/editor-ui/src/components/NodeSettings.vue
@@ -23,12 +23,23 @@
 		</div>
 		<div class="node-parameters-wrapper" v-if="node && nodeValid">
 			<div v-show="openPanel === 'params'">
-				<node-credentials :node="node" @credentialSelected="credentialSelected"></node-credentials>
-				<node-webhooks :node="node" :nodeType="nodeType" />
-				<parameter-input-list :parameters="parametersNoneSetting" :hideDelete="true" :nodeValues="nodeValues" path="parameters" @valueChanged="valueChanged" />
+				<node-webhooks
+					:node="node"
+					:nodeType="nodeType"
+				/>
+				<parameter-input-list
+					:parameters="parametersNoneSetting"
+					:hideDelete="true"
+					:nodeValues="nodeValues" path="parameters" @valueChanged="valueChanged"
+				>
+					<node-credentials
+						:node="node"
+						@credentialSelected="credentialSelected"
+					/>
+				</parameter-input-list>
 				<div v-if="parametersNoneSetting.length === 0" class="no-parameters">
 					<n8n-text>
-					{{ $locale.baseText('nodeSettings.thisNodeDoesNotHaveAnyParameters') }}
+						{{ $locale.baseText('nodeSettings.thisNodeDoesNotHaveAnyParameters') }}
 					</n8n-text>
 				</div>
 			</div>
diff --git a/packages/editor-ui/src/components/ParameterInputList.vue b/packages/editor-ui/src/components/ParameterInputList.vue
index 5591719ef253d..87ede15ea34ec 100644
--- a/packages/editor-ui/src/components/ParameterInputList.vue
+++ b/packages/editor-ui/src/components/ParameterInputList.vue
@@ -1,6 +1,8 @@
 <template>
-	<div class="paramter-input-list-wrapper">
-		<div v-for="parameter in filteredParameters" :key="parameter.name" :class="{indent}">
+	<div class="parameter-input-list-wrapper">
+		<div v-for="(parameter, index) in filteredParameters" :key="parameter.name">
+			<slot v-if="indexToShowSlotAt === index" />
+
 			<div
 				v-if="multipleValues(parameter) === true && parameter.type !== 'fixedCollection'"
 				class="parameter-item"
@@ -129,6 +131,11 @@ export default mixins(
 			node (): INodeUi {
 				return this.$store.getters.activeNode;
 			},
+			indexToShowSlotAt (): number {
+				if (this.isHttpRequestNodeV2(this.node)) return 2;
+
+				return 0;
+			},
 		},
 		methods: {
 			multipleValues (parameter: INodeProperties): boolean {
@@ -260,7 +267,12 @@ export default mixins(
 </script>
 
 <style lang="scss">
-.paramter-input-list-wrapper {
+.parameter-input-list-wrapper {
+
+	div:first-child > .node-credentials {
+		padding-top: var(--spacing-xs);
+	}
+
 	.delete-option {
 		display: none;
 		position: absolute;
diff --git a/packages/editor-ui/src/components/mixins/nodeHelpers.ts b/packages/editor-ui/src/components/mixins/nodeHelpers.ts
index 00379a2aa825d..9d09389a2ae81 100644
--- a/packages/editor-ui/src/components/mixins/nodeHelpers.ts
+++ b/packages/editor-ui/src/components/mixins/nodeHelpers.ts
@@ -1,4 +1,5 @@
 import {
+	HTTP_REQUEST_NODE_TYPE,
 	PLACEHOLDER_FILLED_AT_EXECUTION_TIME,
 } from '@/constants';
 
@@ -32,12 +33,19 @@ import { restApi } from '@/components/mixins/restApi';
 import { get } from 'lodash';
 
 import mixins from 'vue-typed-mixins';
+import { mapGetters } from 'vuex';
 
 export const nodeHelpers = mixins(
 	restApi,
 )
 	.extend({
+		computed: {
+			...mapGetters('credentials', [ 'getCredentialTypeByName', 'getCredentialsByType' ]),
+		},
 		methods: {
+			isHttpRequestNodeV2 (node: INodeUi): boolean {
+				return node.type === HTTP_REQUEST_NODE_TYPE && node.typeVersion === 2;
+			},
 
 			// Returns the parameter value
 			getParameterValue (nodeValues: INodeParameters, parameterName: string, path: string) {
@@ -116,6 +124,23 @@ export const nodeHelpers = mixins(
 				return false;
 			},
 
+			reportUnsetCredential(credentialType: ICredentialType) {
+				return {
+					credentials: {
+						[credentialType.name]: [
+							this.$locale.baseText(
+								'nodeHelpers.credentialsUnset',
+								{
+									interpolate: {
+										credentialType: credentialType.displayName,
+									},
+								},
+							),
+						],
+					},
+				};
+			},
+
 			// Updates the execution issues.
 			updateNodesExecutionIssues () {
 				const nodes = this.$store.getters.allNodes;
@@ -198,6 +223,46 @@ export const nodeHelpers = mixins(
 				let credentialType: ICredentialType | null;
 				let credentialDisplayName: string;
 				let selectedCredentials: INodeCredentialsDetails;
+
+				const {
+					authenticateWith,
+					genericAuthType,
+					nodeCredentialType,
+				} = node.parameters as HttpRequestNode.V2.AuthParams;
+
+				if (
+					this.isHttpRequestNodeV2(node) &&
+					authenticateWith === 'genericAuth' &&
+					selectedCredsAreUnusable(node, genericAuthType)
+				) {
+					const credential = this.getCredentialTypeByName(genericAuthType);
+					return this.reportUnsetCredential(credential);
+				}
+
+				if (
+					this.isHttpRequestNodeV2(node) &&
+					authenticateWith === 'nodeCredential' &&
+					nodeCredentialType !== '' &&
+					node.credentials !== undefined
+				) {
+					const stored = this.getCredentialsByType(nodeCredentialType);
+
+					if (selectedCredsDoNotExist(node, nodeCredentialType, stored)) {
+						const credential = this.getCredentialTypeByName(nodeCredentialType);
+						return this.reportUnsetCredential(credential);
+					}
+				}
+
+				if (
+					this.isHttpRequestNodeV2(node) &&
+					authenticateWith === 'nodeCredential' &&
+					nodeCredentialType !== '' &&
+					selectedCredsAreUnusable(node, nodeCredentialType)
+				) {
+					const credential = this.getCredentialTypeByName(nodeCredentialType);
+					return this.reportUnsetCredential(credential);
+				}
+
 				for (const credentialTypeDescription of nodeType!.credentials!) {
 					// Check if credentials should be displayed else ignore
 					if (this.displayParameter(node.parameters, credentialTypeDescription, '', node) !== true) {
@@ -394,3 +459,39 @@ export const nodeHelpers = mixins(
 			},
 		},
 	});
+
+/**
+ * Whether the node has no selected credentials, or none of the node's
+ * selected credentials are of the specified type.
+ */
+function selectedCredsAreUnusable(node: INodeUi, credentialType: string) {
+	return node.credentials === undefined || Object.keys(node.credentials).includes(credentialType) === false;
+}
+
+/**
+ * Whether the node's selected credentials of the specified type
+ * can no longer be found in the database.
+ */
+function selectedCredsDoNotExist(
+	node: INodeUi,
+	nodeCredentialType: string,
+	storedCredsByType: ICredentialsResponse[] | null,
+) {
+	if (!node.credentials || !storedCredsByType) return false;
+
+	const selectedCredsByType = node.credentials[nodeCredentialType];
+
+	if (!selectedCredsByType) return false;
+
+	return !storedCredsByType.find((c) => c.id === selectedCredsByType.id);
+}
+
+declare namespace HttpRequestNode {
+	namespace V2 {
+		type AuthParams = {
+			authenticateWith: 'none' | 'genericAuth' | 'nodeCredential';
+			genericAuthType: string;
+			nodeCredentialType: string;
+		};
+	}
+}
diff --git a/packages/editor-ui/src/components/mixins/workflowHelpers.ts b/packages/editor-ui/src/components/mixins/workflowHelpers.ts
index 8820b2ff59278..0e86843cc1da0 100644
--- a/packages/editor-ui/src/components/mixins/workflowHelpers.ts
+++ b/packages/editor-ui/src/components/mixins/workflowHelpers.ts
@@ -330,6 +330,11 @@ export const workflowHelpers = mixins(
 					if (node.credentials !== undefined && nodeType.credentials !== undefined) {
 						const saveCredenetials: INodeCredentials = {};
 						for (const nodeCredentialTypeName of Object.keys(node.credentials)) {
+							if (this.isHttpRequestNodeV2(node)) {
+								saveCredenetials[nodeCredentialTypeName] = node.credentials[nodeCredentialTypeName];
+								continue;
+							}
+
 							const credentialTypeDescription = nodeType.credentials
 								.find((credentialTypeDescription) => credentialTypeDescription.name === nodeCredentialTypeName);
 
diff --git a/packages/editor-ui/src/plugins/i18n/locales/en.json b/packages/editor-ui/src/plugins/i18n/locales/en.json
index 07d529d4c0baa..23f26d677ad2f 100644
--- a/packages/editor-ui/src/plugins/i18n/locales/en.json
+++ b/packages/editor-ui/src/plugins/i18n/locales/en.json
@@ -387,6 +387,7 @@
 	"nodeErrorView.stack": "Stack",
 	"nodeErrorView.theErrorCauseIsTooLargeToBeDisplayed": "The error cause is too large to be displayed",
 	"nodeErrorView.time": "Time",
+	"nodeHelpers.credentialsUnset": "Credentials for '{credentialType}' are not set.",
 	"nodeSettings.alwaysOutputData.description": "If active, will output a single, empty item when the output would have been empty. Use to prevent the workflow finishing on this node.",
 	"nodeSettings.alwaysOutputData.displayName": "Always Output Data",
 	"nodeSettings.clickOnTheQuestionMarkIcon": "Click the '?' icon to open this node on n8n.io",
diff --git a/packages/nodes-base/nodes/HttpRequest/HttpRequest.node.ts b/packages/nodes-base/nodes/HttpRequest/HttpRequest.node.ts
index 252ea31fd27b5..0475407326b86 100644
--- a/packages/nodes-base/nodes/HttpRequest/HttpRequest.node.ts
+++ b/packages/nodes-base/nodes/HttpRequest/HttpRequest.node.ts
@@ -52,7 +52,7 @@ export class HttpRequest implements INodeType {
 				displayOptions: {
 					show: {
 						authenticateWith: [
-							'basicAuth',
+							'httpBasicAuth',
 						],
 						'@version': [
 							2,
@@ -66,7 +66,7 @@ export class HttpRequest implements INodeType {
 				displayOptions: {
 					show: {
 						authenticateWith: [
-							'digestAuth',
+							'httpDigestAuth',
 						],
 						'@version': [
 							2,
@@ -80,7 +80,7 @@ export class HttpRequest implements INodeType {
 				displayOptions: {
 					show: {
 						authenticateWith: [
-							'headerAuth',
+							'httpHeaderAuth',
 						],
 						'@version': [
 							2,
@@ -94,7 +94,7 @@ export class HttpRequest implements INodeType {
 				displayOptions: {
 					show: {
 						authenticateWith: [
-							'queryAuth',
+							'httpQueryAuth',
 						],
 						'@version': [
 							2,
@@ -108,7 +108,7 @@ export class HttpRequest implements INodeType {
 				displayOptions: {
 					show: {
 						authenticateWith: [
-							'oAuth1',
+							'oAuth1Api',
 						],
 						'@version': [
 							2,
@@ -122,7 +122,7 @@ export class HttpRequest implements INodeType {
 				displayOptions: {
 					show: {
 						authenticateWith: [
-							'oAuth2',
+							'oAuth2Api',
 						],
 						'@version': [
 							2,
@@ -279,30 +279,30 @@ export class HttpRequest implements INodeType {
 				options: [
 					{
 						name: 'Basic Auth',
-						value: 'basicAuth',
+						value: 'httpBasicAuth',
 					},
 					{
 						name: 'Digest Auth',
-						value: 'digestAuth',
+						value: 'httpDigestAuth',
 					},
 					{
 						name: 'Header Auth',
-						value: 'headerAuth',
+						value: 'httpHeaderAuth',
 					},
 					{
 						name: 'Query Auth',
-						value: 'queryAuth',
+						value: 'httpQueryAuth',
 					},
 					{
 						name: 'OAuth1',
-						value: 'oAuth1',
+						value: 'oAuth1Api',
 					},
 					{
 						name: 'OAuth2',
-						value: 'oAuth2',
+						value: 'oAuth2Api',
 					},
 				],
-				default: 'basicAuth',
+				default: 'httpBasicAuth',
 				displayOptions: {
 					show: {
 						authenticateWith: [