Skip to content

Commit

Permalink
refactor(core): Decouple workflow created, saved, deleted events from…
Browse files Browse the repository at this point in the history
… internal hooks (no-changelog)

Follow-up to #10221
  • Loading branch information
ivov committed Jul 31, 2024
1 parent cf73e29 commit 6d740a3
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 103 deletions.
82 changes: 1 addition & 81 deletions packages/cli/src/InternalHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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
Expand Down Expand Up @@ -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<void> {
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<MessageEventBus>();
Expand All @@ -29,6 +30,9 @@ describe('AuditEventRelay', () => {
id: 'wf123',
name: 'Test Workflow',
}),
publicApi: false,
projectId: 'proj123',
projectType: 'personal',
};

eventService.emit('workflow-created', event);
Expand Down Expand Up @@ -57,6 +61,7 @@ describe('AuditEventRelay', () => {
role: 'user',
},
workflowId: 'wf789',
publicApi: false,
};

eventService.emit('workflow-deleted', event);
Expand All @@ -83,8 +88,8 @@ describe('AuditEventRelay', () => {
lastName: 'Johnson',
role: 'editor',
},
workflowId: 'wf101',
workflowName: 'Updated Workflow',
workflow: mock<IWorkflowDb>({ id: 'wf101', name: 'Updated Workflow' }),
publicApi: false,
};

eventService.emit('workflow-saved', event);
Expand Down
8 changes: 4 additions & 4 deletions packages/cli/src/eventbus/audit-event-relay.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
});
}
Expand Down Expand Up @@ -272,7 +272,7 @@ export class AuditEventRelay {
}

/**
* API key
* Public API
*/

@Redactable()
Expand Down
10 changes: 7 additions & 3 deletions packages/cli/src/eventbus/event.types.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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': {
Expand Down
90 changes: 90 additions & 0 deletions packages/cli/src/telemetry/telemetry-event-relay.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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() {
Expand Down Expand Up @@ -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']) {
Expand Down Expand Up @@ -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');
Expand Down
11 changes: 4 additions & 7 deletions packages/cli/src/workflows/workflow.service.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 6d740a3

Please sign in to comment.