fix(core): Remove unnecessary info from GET /workflows response (#5311)

* fix(core): Remove unnecessary info from `GET /workflows` response

* fix(core): Remove unnecessary info from `GET /workflows` response

* fix(core): Remove credentials from `GET /workflows` response

* fix(core): Update unit tests for `GET /workflows` response

* fix(core): Remove `usedCredentials` from `GET /workflows` response

* fix(core): Update unit tests for `GET /workflows` response

* fix(core): remove nodes from getMany

* fix(core): remove unnecessary owner props from workflow list items

* fix(core): fix lint error

* fix(core): remove unused function

* fix(core): simplifying ownerId usage

* fix(core): trim down the query for workflow listing
This commit is contained in:
Csaba Tuncsik 2023-02-16 10:36:24 +01:00 committed by GitHub
parent 1a20fd9f46
commit a2c6ea9e11
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 51 additions and 117 deletions

View file

@ -203,12 +203,16 @@ EEWorkflowController.post(
EEWorkflowController.get( EEWorkflowController.get(
'/', '/',
ResponseHelper.send(async (req: WorkflowRequest.GetAll) => { ResponseHelper.send(async (req: WorkflowRequest.GetAll) => {
const workflows = await EEWorkflows.getMany(req.user, req.query.filter); const [workflows, workflowOwnerRole] = await Promise.all([
await EEWorkflows.addCredentialsToWorkflows(workflows, req.user); EEWorkflows.getMany(req.user, req.query.filter),
Db.collections.Role.findOneOrFail({
select: ['id'],
where: { name: 'owner', scope: 'workflow' },
}),
]);
return workflows.map((workflow) => { return workflows.map((workflow) => {
EEWorkflows.addOwnerAndSharings(workflow); EEWorkflows.addOwnerId(workflow, workflowOwnerRole);
workflow.nodes = [];
return workflow; return workflow;
}); });
}), }),

View file

@ -5,6 +5,7 @@ import * as ResponseHelper from '@/ResponseHelper';
import * as WorkflowHelpers from '@/WorkflowHelpers'; import * as WorkflowHelpers from '@/WorkflowHelpers';
import type { ICredentialsDb } from '@/Interfaces'; import type { ICredentialsDb } from '@/Interfaces';
import { SharedWorkflow } from '@db/entities/SharedWorkflow'; import { SharedWorkflow } from '@db/entities/SharedWorkflow';
import type { Role } from '@db/entities/Role';
import type { User } from '@db/entities/User'; import type { User } from '@db/entities/User';
import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { WorkflowEntity } from '@db/entities/WorkflowEntity';
import { RoleService } from '@/role/role.service'; import { RoleService } from '@/role/role.service';
@ -13,6 +14,7 @@ import { WorkflowsService } from './workflows.services';
import type { import type {
CredentialUsedByWorkflow, CredentialUsedByWorkflow,
WorkflowWithSharingsAndCredentials, WorkflowWithSharingsAndCredentials,
WorkflowForList,
} from './workflows.types'; } from './workflows.types';
import { EECredentialsService as EECredentials } from '@/credentials/credentials.service.ee'; import { EECredentialsService as EECredentials } from '@/credentials/credentials.service.ee';
import { getSharedWorkflowIds } from '@/WorkflowHelpers'; import { getSharedWorkflowIds } from '@/WorkflowHelpers';
@ -87,6 +89,14 @@ export class EEWorkflowsService extends WorkflowsService {
return transaction.save(newSharedWorkflows); return transaction.save(newSharedWorkflows);
} }
static addOwnerId(workflow: WorkflowForList, workflowOwnerRole: Role): void {
const ownerId = workflow.shared?.find(
({ roleId }) => String(roleId) === workflowOwnerRole.id,
)?.userId;
workflow.ownedBy = ownerId ? { id: ownerId } : null;
delete workflow.shared;
}
static addOwnerAndSharings(workflow: WorkflowWithSharingsAndCredentials): void { static addOwnerAndSharings(workflow: WorkflowWithSharingsAndCredentials): void {
workflow.ownedBy = null; workflow.ownedBy = null;
workflow.sharedWith = []; workflow.sharedWith = [];
@ -156,72 +166,6 @@ export class EEWorkflowsService extends WorkflowsService {
}); });
} }
static async addCredentialsToWorkflows(
workflows: WorkflowWithSharingsAndCredentials[],
currentUser: User,
): Promise<void> {
// Create 2 maps: one with all the credential ids used by all workflows
// And another to match back workflow <> credentials
const allUsedCredentialIds = new Set<string>();
const mapsWorkflowsToUsedCredentials: string[][] = [];
workflows.forEach((workflow, idx) => {
workflow.nodes.forEach((node) => {
if (!node.credentials) {
return;
}
Object.keys(node.credentials).forEach((credentialType) => {
const credential = node.credentials?.[credentialType];
if (!credential?.id) {
return;
}
if (!mapsWorkflowsToUsedCredentials[idx]) {
mapsWorkflowsToUsedCredentials[idx] = [];
}
mapsWorkflowsToUsedCredentials[idx].push(credential.id);
allUsedCredentialIds.add(credential.id);
});
});
});
const usedWorkflowsCredentials = await EECredentials.getMany({
where: {
id: In(Array.from(allUsedCredentialIds)),
},
relations: ['shared', 'shared.user', 'shared.role'],
});
const userCredentials = await EECredentials.getAll(currentUser, { disableGlobalRole: true });
const userCredentialIds = userCredentials.map((credential) => credential.id);
const credentialsMap: Record<string, CredentialUsedByWorkflow> = {};
usedWorkflowsCredentials.forEach((credential) => {
const credentialId = credential.id;
credentialsMap[credentialId] = {
id: credentialId,
name: credential.name,
type: credential.type,
currentUserHasAccess: userCredentialIds.includes(credentialId),
sharedWith: [],
ownedBy: null,
};
credential.shared?.forEach(({ user, role }) => {
const { id, email, firstName, lastName } = user;
if (role.name === 'owner') {
credentialsMap[credentialId].ownedBy = { id, email, firstName, lastName };
} else {
credentialsMap[credentialId].sharedWith?.push({
id,
email,
firstName,
lastName,
});
}
});
});
mapsWorkflowsToUsedCredentials.forEach((usedCredentialIds, idx) => {
workflows[idx].usedCredentials = usedCredentialIds.map((id) => credentialsMap[id]);
});
}
static validateCredentialPermissionsToUser( static validateCredentialPermissionsToUser(
workflow: WorkflowEntity, workflow: WorkflowEntity,
allowedCredentials: ICredentialsDb[], allowedCredentials: ICredentialsDb[],

View file

@ -1,7 +1,7 @@
import { validate as jsonSchemaValidate } from 'jsonschema'; import { validate as jsonSchemaValidate } from 'jsonschema';
import type { INode, IPinData, JsonObject } from 'n8n-workflow'; import type { INode, IPinData, JsonObject } from 'n8n-workflow';
import { NodeApiError, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow'; import { NodeApiError, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow';
import type { FindOptionsWhere, UpdateResult } from 'typeorm'; import type { FindOptionsSelect, FindOptionsWhere, UpdateResult } from 'typeorm';
import { In } from 'typeorm'; import { In } from 'typeorm';
import pick from 'lodash.pick'; import pick from 'lodash.pick';
import { v4 as uuid } from 'uuid'; import { v4 as uuid } from 'uuid';
@ -25,12 +25,12 @@ import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'
import * as TestWebhooks from '@/TestWebhooks'; import * as TestWebhooks from '@/TestWebhooks';
import { getSharedWorkflowIds } from '@/WorkflowHelpers'; import { getSharedWorkflowIds } from '@/WorkflowHelpers';
import { isSharingEnabled, whereClause } from '@/UserManagement/UserManagementHelper'; import { isSharingEnabled, whereClause } from '@/UserManagement/UserManagementHelper';
import type { WorkflowForList } from '@/workflows/workflows.types';
export interface IGetWorkflowsQueryFilter { export type IGetWorkflowsQueryFilter = Pick<
id?: string; FindOptionsWhere<WorkflowEntity>,
name?: string; 'id' | 'name' | 'active'
active?: boolean; >;
}
const schemaGetWorkflowsQueryFilter = { const schemaGetWorkflowsQueryFilter = {
$id: '/IGetWorkflowsQueryFilter', $id: '/IGetWorkflowsQueryFilter',
@ -114,7 +114,7 @@ export class WorkflowsService {
return getSharedWorkflowIds(user, roles); return getSharedWorkflowIds(user, roles);
} }
static async getMany(user: User, rawFilter: string): Promise<WorkflowEntity[]> { static async getMany(user: User, rawFilter: string): Promise<WorkflowForList[]> {
const sharedWorkflowIds = await this.getWorkflowIdsForUser(user, ['owner']); const sharedWorkflowIds = await this.getWorkflowIdsForUser(user, ['owner']);
if (sharedWorkflowIds.length === 0) { if (sharedWorkflowIds.length === 0) {
// return early since without shared workflows there can be no hits // return early since without shared workflows there can be no hits
@ -122,7 +122,7 @@ export class WorkflowsService {
return []; return [];
} }
let filter: IGetWorkflowsQueryFilter | undefined = undefined; let filter: IGetWorkflowsQueryFilter = {};
if (rawFilter) { if (rawFilter) {
try { try {
const filterJson: JsonObject = jsonParse(rawFilter); const filterJson: JsonObject = jsonParse(rawFilter);
@ -152,31 +152,32 @@ export class WorkflowsService {
return []; return [];
} }
const fields: Array<keyof WorkflowEntity> = [ const select: FindOptionsSelect<WorkflowEntity> = {
'id', id: true,
'name', name: true,
'active', active: true,
'createdAt', createdAt: true,
'updatedAt', updatedAt: true,
'nodes', };
];
const relations: string[] = []; const relations: string[] = [];
if (!config.getEnv('workflowTagsDisabled')) { if (!config.getEnv('workflowTagsDisabled')) {
relations.push('tags'); relations.push('tags');
select.tags = { name: true };
} }
if (isSharingEnabled()) { if (isSharingEnabled()) {
relations.push('shared', 'shared.user', 'shared.role'); relations.push('shared');
select.shared = { userId: true, roleId: true };
select.versionId = true;
} }
filter.id = In(sharedWorkflowIds);
return Db.collections.Workflow.find({ return Db.collections.Workflow.find({
select: isSharingEnabled() ? [...fields, 'versionId'] : fields, select,
relations, relations,
where: { where: filter,
id: In(sharedWorkflowIds), order: { updatedAt: 'DESC' },
...filter,
},
}); });
} }

View file

@ -9,6 +9,12 @@ export interface WorkflowWithSharingsAndCredentials extends Omit<WorkflowEntity,
shared?: SharedWorkflow[]; shared?: SharedWorkflow[];
} }
export interface WorkflowForList
extends Omit<WorkflowEntity, 'ownedBy' | 'nodes' | 'connections' | 'shared' | 'settings'> {
ownedBy?: Pick<IUser, 'id'> | null;
shared?: SharedWorkflow[];
}
export interface CredentialUsedByWorkflow { export interface CredentialUsedByWorkflow {
id: string; id: string;
name: string; name: string;

View file

@ -151,7 +151,7 @@ describe('PUT /workflows/:id', () => {
}); });
describe('GET /workflows', () => { describe('GET /workflows', () => {
test('should return workflows with ownership, sharing and credential usage details', async () => { test('should return workflows without nodes, sharing and credential usage details', async () => {
const owner = await testDb.createUser({ globalRole: globalOwnerRole }); const owner = await testDb.createUser({ globalRole: globalOwnerRole });
const member = await testDb.createUser({ globalRole: globalMemberRole }); const member = await testDb.createUser({ globalRole: globalMemberRole });
@ -188,32 +188,11 @@ describe('GET /workflows', () => {
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);
expect(fetchedWorkflow.ownedBy).toMatchObject({ expect(fetchedWorkflow.ownedBy).toMatchObject({
id: owner.id, id: owner.id,
email: owner.email,
firstName: owner.firstName,
lastName: owner.lastName,
}); });
expect(fetchedWorkflow.sharedWith).toHaveLength(1); expect(fetchedWorkflow.sharedWith).not.toBeDefined()
expect(fetchedWorkflow.usedCredentials).not.toBeDefined()
const [sharee] = fetchedWorkflow.sharedWith; expect(fetchedWorkflow.nodes).not.toBeDefined()
expect(sharee).toMatchObject({
id: member.id,
email: member.email,
firstName: member.firstName,
lastName: member.lastName,
});
expect(fetchedWorkflow.usedCredentials).toHaveLength(1);
const [usedCredential] = fetchedWorkflow.usedCredentials;
expect(usedCredential).toMatchObject({
id: savedCredential.id,
name: savedCredential.name,
type: savedCredential.type,
currentUserHasAccess: true,
});
}); });
}); });