apply feedback

Author: despairblue

Diff for: packages/@n8n/api-types/src/dto/common.ts

@@ -0,0 +1,3 @@
+import { z } from 'zod';
+
+export const booleanLiteral = z.union([z.literal('true'), z.literal('false')]);

Diff for: packages/@n8n/api-types/src/dto/credentials/credentials-get-many-request.dto.ts

@@ -1,14 +1,14 @@
-import { z } from 'zod';
 import { Z } from 'zod-class';
 
-// TODO: document what these flags do
-export class CredentialsGetManyRequest extends Z.class({
+import { booleanLiteral } from 'dto/common';
+
+export class CredentialsGetManyRequestQuery extends Z.class({
 	/**
 	 * Adds the `scopes` field to each credential which includes all scopes the
 	 * requesting user has in relation to the credential, e.g.
 	 *     ['credential:read', 'credential:update']
 	 */
-	includeScopes: z.union([z.literal('true'), z.literal('false')]).optional(),
+	includeScopes: booleanLiteral.optional(),
 
 	/**
 	 * Adds the decrypted `data` field to each credential.
@@ -18,5 +18,5 @@ export class CredentialsGetManyRequest extends Z.class({
 	 *
 	 * This switches `includeScopes` to true to be able to check for the scopes
 	 */
-	includeData: z.union([z.literal('true'), z.literal('false')]).optional(),
+	includeData: booleanLiteral.optional(),
 }) {}

Diff for: packages/@n8n/api-types/src/dto/credentials/credentials-get-one-request.dto.ts

@@ -1,6 +1,7 @@
-import { z } from 'zod';
 import { Z } from 'zod-class';
 
-export class CredentialsGetOneRequest extends Z.class({
-	includeData: z.union([z.literal('true'), z.literal('false')]).optional(),
+import { booleanLiteral } from 'dto/common';
+
+export class CredentialsGetOneRequestQuery extends Z.class({
+	includeData: booleanLiteral.optional(),
 }) {}

Diff for: packages/@n8n/api-types/src/dto/index.ts

@@ -4,5 +4,5 @@ export { SettingsUpdateRequestDto } from './user/settings-update-request.dto';
 export { UserUpdateRequestDto } from './user/user-update-request.dto';
 export { CommunityRegisteredRequestDto } from './license/community-registered-request.dto';
 export { VariableListRequestDto } from './variables/variables-list-request.dto';
-export { CredentialsGetOneRequest } from './credentials/credentials-get-one-request.dto';
-export { CredentialsGetManyRequest } from './credentials/credentials-get-many-request.dto';
+export { CredentialsGetOneRequestQuery } from './credentials/credentials-get-one-request.dto';
+export { CredentialsGetManyRequestQuery } from './credentials/credentials-get-many-request.dto';

Diff for: packages/cli/src/credentials/credentials.controller.ts

@@ -1,4 +1,4 @@
-import { CredentialsGetManyRequest, CredentialsGetOneRequest } from '@n8n/api-types';
+import { CredentialsGetManyRequestQuery, CredentialsGetOneRequestQuery } from '@n8n/api-types';
 import { GlobalConfig } from '@n8n/config';
 // eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
 import { In } from '@n8n/typeorm';
@@ -54,7 +54,7 @@ export class CredentialsController {
 	async getMany(
 		req: CredentialRequest.GetMany,
 		_res: unknown,
-		@Query query: CredentialsGetManyRequest,
+		@Query query: CredentialsGetManyRequestQuery,
 	) {
 		const credentials = await this.credentialsService.getMany(req.user, {
 			listQueryOptions: req.listQueryOptions,
@@ -93,7 +93,7 @@ export class CredentialsController {
 		req: CredentialRequest.Get,
 		_res: unknown,
 		@Param('credentialId') credentialId: string,
-		@Query query: CredentialsGetOneRequest,
+		@Query query: CredentialsGetOneRequestQuery,
 	) {
 		const { shared, ...credential } = this.license.isSharingEnabled()
 			? await this.enterpriseCredentialsService.getOne(

Diff for: packages/cli/src/credentials/credentials.service.ts

@@ -131,15 +131,9 @@ export class CredentialsService {
 
 			if (includeData) {
 				credentials = credentials.map((c: CredentialsEntity & ScopesField) => {
-					if (c.scopes.includes('credential:update')) {
-						return {
-							...c,
-							data: this.decrypt(c),
-						} as unknown as CredentialsEntity;
-					}
 					return {
 						...c,
-						data: undefined,
+						data: c.scopes.includes('credential:update') ? this.decrypt(c) : undefined,
 					} as unknown as CredentialsEntity;
 				});
 			}
@@ -191,15 +185,9 @@ export class CredentialsService {
 
 		if (includeData) {
 			credentials = credentials.map((c: CredentialsEntity & ScopesField) => {
-				if (c.scopes.includes('credential:update')) {
-					return {
-						...c,
-						data: this.decrypt(c),
-					} as unknown as CredentialsEntity;
-				}
 				return {
 					...c,
-					data: undefined,
+					data: c.scopes.includes('credential:update') ? this.decrypt(c) : undefined,
 				} as unknown as CredentialsEntity;
 			});
 		}

Diff for: packages/cli/src/databases/repositories/credentials.repository.ts

@@ -31,25 +31,14 @@ export class CredentialsRepository extends Repository<CredentialsEntity> {
 	) {
 		const findManyOptions = this.toFindManyOptions(listQueryOptions);
 
-		if (listQueryOptions?.includeData) {
-			if (Array.isArray(findManyOptions.select)) {
-				findManyOptions.select.push('data');
-			} else if (typeof findManyOptions.select === 'object') {
-				findManyOptions.select.data = true;
-			} else {
-				findManyOptions.select ??= [];
-				findManyOptions.select.push('data');
-			}
-		}
-
 		if (credentialIds) {
 			findManyOptions.where = { ...findManyOptions.where, id: In(credentialIds) };
 		}
 
 		return await this.find(findManyOptions);
 	}
 
-	private toFindManyOptions(listQueryOptions?: ListQuery.Options) {
+	private toFindManyOptions(listQueryOptions?: ListQuery.Options & { includeData?: boolean }) {
 		const findManyOptions: FindManyOptions<CredentialsEntity> = {};
 
 		type Select = Array<keyof CredentialsEntity>;
@@ -88,6 +77,17 @@ export class CredentialsRepository extends Repository<CredentialsEntity> {
 			findManyOptions.relations = defaultRelations;
 		}
 
+		if (listQueryOptions.includeData) {
+			if (Array.isArray(findManyOptions.select)) {
+				findManyOptions.select.push('data');
+			} else if (typeof findManyOptions.select === 'object') {
+				findManyOptions.select.data = true;
+			} else {
+				findManyOptions.select ??= [];
+				findManyOptions.select.push('data');
+			}
+		}
+
 		return findManyOptions;
 	}