From d8688bd463a22ba417ed99553f21742eedc4db87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Thu, 1 Aug 2024 13:44:23 +0200 Subject: [PATCH] refactor(core): Decouple workflow created, saved, deleted events from internal hooks (no-changelog) (#10264) --- packages/cli/src/InternalHooks.ts | 82 +---------------- .../handlers/workflows/workflows.handler.ts | 9 +- .../audit-event-relay.service.test.ts | 9 +- .../src/eventbus/audit-event-relay.service.ts | 8 +- packages/cli/src/eventbus/event.types.ts | 10 ++- .../telemetry-event-relay.service.ts | 90 +++++++++++++++++++ .../cli/src/workflows/workflow.service.ts | 11 +-- .../cli/src/workflows/workflows.controller.ts | 9 +- 8 files changed, 125 insertions(+), 103 deletions(-) diff --git a/packages/cli/src/InternalHooks.ts b/packages/cli/src/InternalHooks.ts index 7912054085..fda2c3f21d 100644 --- a/packages/cli/src/InternalHooks.ts +++ b/packages/cli/src/InternalHooks.ts @@ -10,22 +10,15 @@ import type { } from 'n8n-workflow'; import { TelemetryHelpers } from 'n8n-workflow'; -import config from '@/config'; import { N8N_VERSION } from '@/constants'; import type { AuthProviderType } from '@db/entities/AuthIdentity'; import type { User } from '@db/entities/User'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; import { determineFinalExecutionStatus } from '@/executionLifecycleHooks/shared/sharedHookFunctions'; -import type { - ITelemetryUserDeletionData, - IWorkflowDb, - IExecutionTrackProperties, -} from '@/Interfaces'; +import type { ITelemetryUserDeletionData, IExecutionTrackProperties } from '@/Interfaces'; import { WorkflowStatisticsService } from '@/services/workflow-statistics.service'; import { NodeTypes } from '@/NodeTypes'; import { Telemetry } from '@/telemetry'; -import type { Project } from '@db/entities/Project'; -import { ProjectRelationRepository } from './databases/repositories/projectRelation.repository'; import { MessageEventBus } from './eventbus/MessageEventBus/MessageEventBus'; /** @@ -40,7 +33,6 @@ export class InternalHooks { private readonly nodeTypes: NodeTypes, private readonly sharedWorkflowRepository: SharedWorkflowRepository, workflowStatisticsService: WorkflowStatisticsService, - private readonly projectRelationRepository: ProjectRelationRepository, // Can't use @ts-expect-error because only dev time tsconfig considers this as an error, but not build time // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - needed until we decouple telemetry @@ -72,78 +64,6 @@ export class InternalHooks { this.telemetry.track('User responded to personalization questions', personalizationSurveyData); } - onWorkflowCreated( - user: User, - workflow: IWorkflowBase, - project: Project, - publicApi: boolean, - ): void { - const { nodeGraph } = TelemetryHelpers.generateNodesGraph(workflow, this.nodeTypes); - - this.telemetry.track('User created workflow', { - user_id: user.id, - workflow_id: workflow.id, - node_graph_string: JSON.stringify(nodeGraph), - public_api: publicApi, - project_id: project.id, - project_type: project.type, - }); - } - - onWorkflowDeleted(user: User, workflowId: string, publicApi: boolean): void { - this.telemetry.track('User deleted workflow', { - user_id: user.id, - workflow_id: workflowId, - public_api: publicApi, - }); - } - - async onWorkflowSaved(user: User, workflow: IWorkflowDb, publicApi: boolean): Promise { - const isCloudDeployment = config.getEnv('deployment.type') === 'cloud'; - - const { nodeGraph } = TelemetryHelpers.generateNodesGraph(workflow, this.nodeTypes, { - isCloudDeployment, - }); - - let userRole: 'owner' | 'sharee' | 'member' | undefined = undefined; - const role = await this.sharedWorkflowRepository.findSharingRole(user.id, workflow.id); - if (role) { - userRole = role === 'workflow:owner' ? 'owner' : 'sharee'; - } else { - const workflowOwner = await this.sharedWorkflowRepository.getWorkflowOwningProject( - workflow.id, - ); - - if (workflowOwner) { - const projectRole = await this.projectRelationRepository.findProjectRole({ - userId: user.id, - projectId: workflowOwner.id, - }); - - if (projectRole && projectRole !== 'project:personalOwner') { - userRole = 'member'; - } - } - } - - const notesCount = Object.keys(nodeGraph.notes).length; - const overlappingCount = Object.values(nodeGraph.notes).filter( - (note) => note.overlapping, - ).length; - - this.telemetry.track('User saved workflow', { - user_id: user.id, - workflow_id: workflow.id, - node_graph_string: JSON.stringify(nodeGraph), - notes_count_overlapping: overlappingCount, - notes_count_non_overlapping: notesCount - overlappingCount, - version_cli: N8N_VERSION, - num_tags: workflow.tags?.length ?? 0, - public_api: publicApi, - sharing_role: userRole, - }); - } - // eslint-disable-next-line complexity async onWorkflowPostExecute( _executionId: string, diff --git a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts index ea5eebdfdb..4063d0b611 100644 --- a/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts +++ b/packages/cli/src/PublicApi/v1/handlers/workflows/workflows.handler.ts @@ -60,10 +60,12 @@ export = { ); await Container.get(ExternalHooks).run('workflow.afterCreate', [createdWorkflow]); - Container.get(InternalHooks).onWorkflowCreated(req.user, createdWorkflow, project, true); Container.get(EventService).emit('workflow-created', { workflow: createdWorkflow, user: req.user, + publicApi: true, + projectId: project.id, + projectType: project.type, }); return res.json(createdWorkflow); @@ -259,11 +261,10 @@ export = { } await Container.get(ExternalHooks).run('workflow.afterUpdate', [updateData]); - void Container.get(InternalHooks).onWorkflowSaved(req.user, updateData, true); Container.get(EventService).emit('workflow-saved', { user: req.user, - workflowId: updateData.id, - workflowName: updateData.name, + workflow: updateData, + publicApi: true, }); return res.json(updatedWorkflow); diff --git a/packages/cli/src/eventbus/__tests__/audit-event-relay.service.test.ts b/packages/cli/src/eventbus/__tests__/audit-event-relay.service.test.ts index 7196b9203c..52b86b58e1 100644 --- a/packages/cli/src/eventbus/__tests__/audit-event-relay.service.test.ts +++ b/packages/cli/src/eventbus/__tests__/audit-event-relay.service.test.ts @@ -4,6 +4,7 @@ import type { MessageEventBus } from '../MessageEventBus/MessageEventBus'; import type { Event } from '../event.types'; import { EventService } from '../event.service'; import type { INode, IRun, IWorkflowBase } from 'n8n-workflow'; +import type { IWorkflowDb } from '@/Interfaces'; describe('AuditEventRelay', () => { const eventBus = mock(); @@ -29,6 +30,9 @@ describe('AuditEventRelay', () => { id: 'wf123', name: 'Test Workflow', }), + publicApi: false, + projectId: 'proj123', + projectType: 'personal', }; eventService.emit('workflow-created', event); @@ -57,6 +61,7 @@ describe('AuditEventRelay', () => { role: 'user', }, workflowId: 'wf789', + publicApi: false, }; eventService.emit('workflow-deleted', event); @@ -83,8 +88,8 @@ describe('AuditEventRelay', () => { lastName: 'Johnson', role: 'editor', }, - workflowId: 'wf101', - workflowName: 'Updated Workflow', + workflow: mock({ id: 'wf101', name: 'Updated Workflow' }), + publicApi: false, }; eventService.emit('workflow-saved', event); diff --git a/packages/cli/src/eventbus/audit-event-relay.service.ts b/packages/cli/src/eventbus/audit-event-relay.service.ts index 56f4dd95cd..f8a95a3ebf 100644 --- a/packages/cli/src/eventbus/audit-event-relay.service.ts +++ b/packages/cli/src/eventbus/audit-event-relay.service.ts @@ -86,13 +86,13 @@ export class AuditEventRelay { } @Redactable() - private workflowSaved({ user, workflowId, workflowName }: Event['workflow-saved']) { + private workflowSaved({ user, workflow }: Event['workflow-saved']) { void this.eventBus.sendAuditEvent({ eventName: 'n8n.audit.workflow.updated', payload: { ...user, - workflowId, - workflowName, + workflowId: workflow.id, + workflowName: workflow.name, }, }); } @@ -272,7 +272,7 @@ export class AuditEventRelay { } /** - * API key + * Public API */ @Redactable() diff --git a/packages/cli/src/eventbus/event.types.ts b/packages/cli/src/eventbus/event.types.ts index 513c659d3e..b62d3bc141 100644 --- a/packages/cli/src/eventbus/event.types.ts +++ b/packages/cli/src/eventbus/event.types.ts @@ -1,5 +1,5 @@ import type { AuthenticationMethod, IRun, IWorkflowBase } from 'n8n-workflow'; -import type { IWorkflowExecutionDataProcess } from '@/Interfaces'; +import type { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces'; import type { ProjectRole } from '@/databases/entities/ProjectRelation'; import type { GlobalRole } from '@/databases/entities/User'; @@ -20,17 +20,21 @@ export type Event = { 'workflow-created': { user: UserLike; workflow: IWorkflowBase; + publicApi: boolean; + projectId: string; + projectType: string; }; 'workflow-deleted': { user: UserLike; workflowId: string; + publicApi: boolean; }; 'workflow-saved': { user: UserLike; - workflowId: string; - workflowName: string; + workflow: IWorkflowDb; + publicApi: boolean; }; 'workflow-pre-execute': { diff --git a/packages/cli/src/telemetry/telemetry-event-relay.service.ts b/packages/cli/src/telemetry/telemetry-event-relay.service.ts index 97c30cae40..9077abdf9c 100644 --- a/packages/cli/src/telemetry/telemetry-event-relay.service.ts +++ b/packages/cli/src/telemetry/telemetry-event-relay.service.ts @@ -8,6 +8,10 @@ import { License } from '@/License'; import { GlobalConfig } from '@n8n/config'; import { N8N_VERSION } from '@/constants'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { TelemetryHelpers } from 'n8n-workflow'; +import { NodeTypes } from '@/NodeTypes'; +import { SharedWorkflowRepository } from '@/databases/repositories/sharedWorkflow.repository'; +import { ProjectRelationRepository } from '@/databases/repositories/projectRelation.repository'; @Service() export class TelemetryEventRelay { @@ -17,6 +21,9 @@ export class TelemetryEventRelay { private readonly license: License, private readonly globalConfig: GlobalConfig, private readonly workflowRepository: WorkflowRepository, + private readonly nodeTypes: NodeTypes, + private readonly sharedWorkflowRepository: SharedWorkflowRepository, + private readonly projectRelationRepository: ProjectRelationRepository, ) {} async init() { @@ -101,6 +108,16 @@ export class TelemetryEventRelay { this.eventService.on('login-failed-due-to-ldap-disabled', (event) => { this.loginFailedDueToLdapDisabled(event); }); + + this.eventService.on('workflow-created', (event) => { + this.workflowCreated(event); + }); + this.eventService.on('workflow-deleted', (event) => { + this.workflowDeleted(event); + }); + this.eventService.on('workflow-saved', async (event) => { + await this.workflowSaved(event); + }); } private teamProjectUpdated({ userId, role, members, projectId }: Event['team-project-updated']) { @@ -431,6 +448,79 @@ export class TelemetryEventRelay { this.telemetry.track('User login failed since ldap disabled', { user_ud: userId }); } + private workflowCreated({ + user, + workflow, + publicApi, + projectId, + projectType, + }: Event['workflow-created']) { + const { nodeGraph } = TelemetryHelpers.generateNodesGraph(workflow, this.nodeTypes); + + this.telemetry.track('User created workflow', { + user_id: user.id, + workflow_id: workflow.id, + node_graph_string: JSON.stringify(nodeGraph), + public_api: publicApi, + project_id: projectId, + project_type: projectType, + }); + } + + private workflowDeleted({ user, workflowId, publicApi }: Event['workflow-deleted']) { + this.telemetry.track('User deleted workflow', { + user_id: user.id, + workflow_id: workflowId, + public_api: publicApi, + }); + } + + private async workflowSaved({ user, workflow, publicApi }: Event['workflow-saved']) { + const isCloudDeployment = config.getEnv('deployment.type') === 'cloud'; + + const { nodeGraph } = TelemetryHelpers.generateNodesGraph(workflow, this.nodeTypes, { + isCloudDeployment, + }); + + let userRole: 'owner' | 'sharee' | 'member' | undefined = undefined; + const role = await this.sharedWorkflowRepository.findSharingRole(user.id, workflow.id); + if (role) { + userRole = role === 'workflow:owner' ? 'owner' : 'sharee'; + } else { + const workflowOwner = await this.sharedWorkflowRepository.getWorkflowOwningProject( + workflow.id, + ); + + if (workflowOwner) { + const projectRole = await this.projectRelationRepository.findProjectRole({ + userId: user.id, + projectId: workflowOwner.id, + }); + + if (projectRole && projectRole !== 'project:personalOwner') { + userRole = 'member'; + } + } + } + + const notesCount = Object.keys(nodeGraph.notes).length; + const overlappingCount = Object.values(nodeGraph.notes).filter( + (note) => note.overlapping, + ).length; + + this.telemetry.track('User saved workflow', { + user_id: user.id, + workflow_id: workflow.id, + node_graph_string: JSON.stringify(nodeGraph), + notes_count_overlapping: overlappingCount, + notes_count_non_overlapping: notesCount - overlappingCount, + version_cli: N8N_VERSION, + num_tags: workflow.tags?.length ?? 0, + public_api: publicApi, + sharing_role: userRole, + }); + } + private async serverStarted() { const cpus = os.cpus(); const binaryDataConfig = config.getEnv('binaryDataManager'); diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index 7c0cbfc242..4f59f8238f 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -1,4 +1,4 @@ -import Container, { Service } from 'typedi'; +import { Service } from 'typedi'; import { NodeApiError } from 'n8n-workflow'; import pick from 'lodash/pick'; import omit from 'lodash/omit'; @@ -17,7 +17,6 @@ import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import { hasSharing, type ListQuery } from '@/requests'; import { TagService } from '@/services/tag.service'; -import { InternalHooks } from '@/InternalHooks'; import { OwnershipService } from '@/services/ownership.service'; import { WorkflowHistoryService } from './workflowHistory/workflowHistory.service.ee'; import { Logger } from '@/Logger'; @@ -219,11 +218,10 @@ export class WorkflowService { } await this.externalHooks.run('workflow.afterUpdate', [updatedWorkflow]); - void Container.get(InternalHooks).onWorkflowSaved(user, updatedWorkflow, false); this.eventService.emit('workflow-saved', { user, - workflowId: updatedWorkflow.id, - workflowName: updatedWorkflow.name, + workflow: updatedWorkflow, + publicApi: false, }); if (updatedWorkflow.active) { @@ -282,8 +280,7 @@ export class WorkflowService { await this.workflowRepository.delete(workflowId); await this.binaryDataService.deleteMany(idsForDeletion); - Container.get(InternalHooks).onWorkflowDeleted(user, workflowId, false); - this.eventService.emit('workflow-deleted', { user, workflowId }); + this.eventService.emit('workflow-deleted', { user, workflowId, publicApi: false }); await this.externalHooks.run('workflow.afterDelete', [workflowId]); return workflow; diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index 05863edb84..c774b21917 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -179,8 +179,13 @@ export class WorkflowsController { delete savedWorkflowWithMetaData.shared; await this.externalHooks.run('workflow.afterCreate', [savedWorkflow]); - this.internalHooks.onWorkflowCreated(req.user, newWorkflow, project!, false); - this.eventService.emit('workflow-created', { user: req.user, workflow: newWorkflow }); + this.eventService.emit('workflow-created', { + user: req.user, + workflow: newWorkflow, + publicApi: false, + projectId: project!.id, + projectType: project!.type, + }); const scopes = await this.workflowService.getWorkflowScopes(req.user, savedWorkflow.id);