Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(core): Decouple workflow created, saved, deleted events from internal hooks (no-changelog) #10264

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we have Sentry's OnUnhandledRejection integration enabled, so having a dangling promise chain is not causing us any issues

});
}

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']) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved content but otherwise untouched.

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
Loading